Skip to content

Commit 809db88

Browse files
ClankerGuruClankerGuru
authored
Add forbidden name detection, DI violations, and severity icons (#7)
* Add forbidden name detection, DI violations, and severity icons - Add FORBIDDEN severity level with 🚫 icon to AntiPattern.Severity and FindingSeverity - Detect forbidden package names (utils, helpers, managers, common, misc, base, shared) - Detect dependency inversion violations: flag concrete class usage when interface exists - Update all renderers (Dashboard, ProjectReport, IncludedBuild) to use emoji severity icons - Smell classes in forbidden-named packages elevated to FORBIDDEN severity - Tests for all new detection rules * Add cross-build hub class detection ContextTask now collects source dirs from root project AND all included builds, runs the full analysis pipeline on the merged set. Hub classes referenced across build boundaries now appear in a Hot Classes (cross-build) section with a table and per-hub dependent lists. Uses model-layer AnalysisSummary to respect package boundary rules (report does not import from analysis). * Make forbidden patterns configurable via DSL Add forbiddenPackages and forbiddenClassSuffixes to the srcx extension DSL. Both are additive on top of defaults. Also adds detectForbiddenClassNames for suffix-based class name detection. Usage: srcx { forbiddenPackages.add("legacy") forbiddenClassSuffixes.add("BaseActivity") } * Use varargs DSL and contains matching for forbidden patterns - forbiddenPackages("legacy", "internal") vararg syntax - forbiddenClassPatterns("Base", "Impl") vararg syntax - Class name matching uses contains instead of endsWith so BaseActivity, DataHelperImpl, AbstractManager all match - Rename DEFAULT_FORBIDDEN_CLASS_SUFFIXES to DEFAULT_FORBIDDEN_CLASS_PATTERNS - Remove class doc comments from constants * Replace string concatenation with templates, fix line length Extract long string literals into vals. Use triple-quoted templates where needed. Remove @Suppress("MaxLineLength") annotations. All messages stay as single strings, no + concatenation. * Split output into focused files, add layer detection New split output files under .srcx/: - hot-classes.md, entry-points.md, anti-patterns.md - interfaces.md, cross-build.md, flows/{EntryPoint}.md New analysis: ArchitecturalLayer enum, EntryPointKind classification. context.md slimmed to overview + links to detail files. 6 new renderers with tests. Coverage at 91.8%. * Rename Hot Classes to Hub Classes across all output files * Fix false positive cycles, add test icons to hub classes - Strip KDoc and comments before same-package reference matching to eliminate false circular deps from @see cross-references - Skip self-edges in cycle detection (Case -> Case) - Add isTest field to HubClass model - Hub classes renderer separates production and test classes with 🧪 icon prefix for test classes * Improve output formats, remove flows, fix false positives - Hub classes: tree format with ├── └── connectors, test paths marked 🧪 - Entry points: removed useless "First Call" column, added description - Cross-build: tree showing dependents per hub with file paths - Interfaces: fixed false detection of enum values, grouped by source set - Flows: removed entirely (import-chain tracing produced identical diagrams) - Dashboard: slimmed to overview + tables + links only, removed class diagram - Fixed false positive cycles from KDoc @see references (strip comments) - Fixed self-cycles in cycle detection (skip self-edges) * Fix kotlin-compiler-embeddable version conflict Remove pinned 2.1.20 version — use Gradle's embedded Kotlin version instead. The pinned version conflicts with Gradle 9.4.1's embedded Kotlin 2.3.0, causing ClasspathEntrySnapshotTransform failures in composite builds. Also remove self-applied srcx plugin from settings (causes bootstrap conflict) and rename build-logic for composite build compatibility. * Fix entry point classification: use classifyEntryPoints for proper TEST/APP/MOCK split Test classes (ending in Test/Spec, in /test/ paths) were incorrectly listed as App Entry Points because buildEntryPoints() used findEntryPoints() which returns graph roots without filtering tests. Replaced with classifyEntryPoints() which already handles TEST/MOCK/APP classification. Simplified EntryPointsRenderer to accept pre-classified entries instead of doing its own broken source-set-based detection. * Fix dashboard view links resolving to wrong directory context.md lives inside .srcx/, so relative links to included build reports need an extra ../ to escape the .srcx directory first. * Address CodeRabbit review feedback - Fix DI violation message: "Dependency on concrete" instead of misleading "Constructor takes concrete" (detection is import-based) - Add Windows path separator check for test file exclusion consistency - Include included-build warnings in dashboard totalWarnings count - Add empty-state message in CrossBuildRenderer for present but empty analysis - Exclude mock/fake/stub classes from interface implementation counts
1 parent 1d0f29a commit 809db88

29 files changed

Lines changed: 2770 additions & 221 deletions

‎build.gradle.kts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ plugins {
33
}
44

55
dependencies {
6-
implementation("org.jetbrains.kotlin:kotlin-compiler-embeddable:2.1.20")
6+
implementation("org.jetbrains.kotlin:kotlin-compiler-embeddable")
77
}
88

99
gradlePlugin {

‎settings.gradle.kts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
pluginManagement {
2-
includeBuild("build-logic")
2+
includeBuild("build-logic") { name = "srcx-build-logic" }
33
}
44

55
plugins {

‎src/main/kotlin/zone/clanker/gradle/srcx/Srcx.kt‎

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import org.gradle.api.initialization.Settings
77
import org.gradle.api.logging.Logging
88
import org.gradle.api.provider.Property
99
import org.gradle.api.provider.SetProperty
10+
import org.gradle.api.tasks.Internal
1011
import zone.clanker.gradle.srcx.scan.ProjectScanner
1112
import zone.clanker.gradle.srcx.scan.SymbolExtractor
1213
import zone.clanker.gradle.srcx.task.CleanTask
@@ -71,15 +72,21 @@ data object Srcx {
7172
}
7273
}
7374

75+
val DEFAULT_FORBIDDEN_PACKAGES: Set<String> =
76+
setOf("util", "utils", "helper", "helpers", "manager", "managers", "misc", "base")
77+
78+
val DEFAULT_FORBIDDEN_CLASS_PATTERNS: Set<String> =
79+
setOf("Helper", "Manager", "Utils", "Util")
80+
7481
/**
7582
* DSL extension registered as `srcx { }` on the Settings object.
7683
*
77-
* Controls the output directory and auto-generation behavior.
78-
*
7984
* ```kotlin
8085
* srcx {
8186
* outputDir.set(".srcx")
8287
* autoGenerate.set(true)
88+
* forbiddenPackages("legacy", "internal", "compat")
89+
* forbiddenClassPatterns("Base", "Impl", "Abstract")
8390
* }
8491
* ```
8592
*
@@ -89,14 +96,23 @@ data object Srcx {
8996
abstract class SettingsExtension
9097
@Inject
9198
constructor() {
92-
/** Output directory relative to the root project. */
9399
abstract val outputDir: Property<String>
94-
95-
/** When true, compileKotlin/compileJava tasks will finalize with srcx-context. */
96100
abstract val autoGenerate: Property<Boolean>
97-
98-
/** Dependency scopes to exclude from scanning. All others are discovered automatically. */
99101
abstract val excludeDepScopes: SetProperty<String>
102+
103+
@get:Internal
104+
abstract val forbiddenPackageNames: SetProperty<String>
105+
106+
@get:Internal
107+
abstract val forbiddenClassNamePatterns: SetProperty<String>
108+
109+
fun forbiddenPackages(vararg names: String) {
110+
forbiddenPackageNames.addAll(names.toList())
111+
}
112+
113+
fun forbiddenClassPatterns(vararg patterns: String) {
114+
forbiddenClassNamePatterns.addAll(patterns.toList())
115+
}
100116
}
101117

102118
/**
@@ -120,6 +136,8 @@ data object Srcx {
120136
extension.outputDir.convention(OUTPUT_DIR)
121137
extension.autoGenerate.convention(false)
122138
extension.excludeDepScopes.convention(DEFAULT_EXCLUDED_DEP_SCOPES)
139+
extension.forbiddenPackageNames.convention(DEFAULT_FORBIDDEN_PACKAGES)
140+
extension.forbiddenClassNamePatterns.convention(DEFAULT_FORBIDDEN_CLASS_PATTERNS)
123141

124142
settings.gradle.rootProject(
125143
Action { rootProject ->
@@ -163,6 +181,8 @@ data object Srcx {
163181
task.includedBuildInfos.set(
164182
rootProject.provider { collectIncludedBuildInfos(rootProject) },
165183
)
184+
task.forbiddenPackages.convention(extension.forbiddenPackageNames)
185+
task.forbiddenClassSuffixes.convention(extension.forbiddenClassNamePatterns)
166186
}
167187
}
168188
val cleanTask =

‎src/main/kotlin/zone/clanker/gradle/srcx/analysis/AntiPatternDetector.kt‎

Lines changed: 160 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
@file:Suppress("ktlint:standard:filename")
1+
@file:Suppress("ktlint:standard:filename", "TooManyFunctions")
22

33
package zone.clanker.gradle.srcx.analysis
44

@@ -30,8 +30,9 @@ data class AntiPattern(
3030
enum class Severity(
3131
val icon: String,
3232
) {
33-
WARNING("WARNING"),
34-
INFO("INFO"),
33+
FORBIDDEN("\uD83D\uDEAB"),
34+
WARNING("⚠\uFE0F"),
35+
INFO("ℹ\uFE0F"),
3536
}
3637
}
3738

@@ -40,15 +41,20 @@ fun detectAntiPatterns(
4041
components: List<ClassifiedComponent>,
4142
edges: List<ClassDependency>,
4243
rootDir: File,
44+
forbiddenPackages: Set<String> = zone.clanker.gradle.srcx.Srcx.DEFAULT_FORBIDDEN_PACKAGES,
45+
forbiddenClassPatterns: Set<String> = zone.clanker.gradle.srcx.Srcx.DEFAULT_FORBIDDEN_CLASS_PATTERNS,
4346
): List<AntiPattern> {
4447
val resolver = SupertypeResolver(components)
4548
val patterns = mutableListOf<AntiPattern>()
4649

47-
patterns.addAll(detectSmellClasses(components, rootDir))
50+
patterns.addAll(detectSmellClasses(components, rootDir, forbiddenPackages))
51+
patterns.addAll(detectForbiddenNames(components, rootDir, forbiddenPackages))
52+
patterns.addAll(detectForbiddenClassNames(components, rootDir, forbiddenClassPatterns))
4853
patterns.addAll(detectSingleImplInterfaces(components, resolver, rootDir))
4954
patterns.addAll(detectGodClasses(components, rootDir))
5055
patterns.addAll(detectDeepInheritance(components, resolver, rootDir))
5156
patterns.addAll(detectCircularDeps(edges))
57+
patterns.addAll(detectDependencyInversionViolations(components, resolver, rootDir))
5258
patterns.addAll(detectMissingTests(components, rootDir))
5359

5460
return patterns.sortedWith(compareBy({ it.severity }, { it.file.path }))
@@ -80,21 +86,151 @@ private class SupertypeResolver(
8086
private fun detectSmellClasses(
8187
components: List<ClassifiedComponent>,
8288
rootDir: File,
89+
forbiddenPackages: Set<String>,
8390
): List<AntiPattern> =
8491
components
8592
.filter { it.role in setOf(ComponentRole.MANAGER, ComponentRole.HELPER, ComponentRole.UTIL) }
8693
.map { c ->
8794
val roleLabel = c.role.name.lowercase()
95+
val lastSegment = c.source.packageName.substringAfterLast(".")
96+
val severity =
97+
if (lastSegment in forbiddenPackages) {
98+
AntiPattern.Severity.FORBIDDEN
99+
} else {
100+
AntiPattern.Severity.WARNING
101+
}
102+
val suggestion =
103+
"Behavior in $roleLabel classes belongs closer to where it's used."
88104
AntiPattern(
89-
severity = AntiPattern.Severity.WARNING,
105+
severity = severity,
90106
message = "`${c.source.simpleName}` is a $roleLabel class",
91107
file = c.source.file.relativeTo(rootDir),
92-
suggestion =
93-
"Behavior in $roleLabel classes usually belongs in a specific class " +
94-
"closer to where it's used. Consider moving methods to the classes that actually need them.",
108+
suggestion = suggestion,
95109
)
96110
}
97111

112+
@Suppress("UnusedParameter")
113+
private fun detectForbiddenNames(
114+
components: List<ClassifiedComponent>,
115+
rootDir: File,
116+
forbiddenPackages: Set<String>,
117+
): List<AntiPattern> {
118+
val patterns = mutableListOf<AntiPattern>()
119+
120+
val inForbiddenPackages =
121+
components.filter { c ->
122+
val lastSegment = c.source.packageName.substringAfterLast(".")
123+
lastSegment in forbiddenPackages
124+
}
125+
val packageGroups = inForbiddenPackages.groupBy { it.source.packageName }
126+
for ((pkg, _) in packageGroups) {
127+
val lastSegment = pkg.substringAfterLast(".")
128+
val suggestion =
129+
"Rename the package to describe what it does instead of a generic name."
130+
patterns.add(
131+
AntiPattern(
132+
severity = AntiPattern.Severity.FORBIDDEN,
133+
message = "Package `$pkg` uses forbidden name `$lastSegment`",
134+
file = File("."),
135+
suggestion = suggestion,
136+
),
137+
)
138+
}
139+
140+
return patterns
141+
}
142+
143+
private fun detectForbiddenClassNames(
144+
components: List<ClassifiedComponent>,
145+
rootDir: File,
146+
forbiddenPatterns: Set<String>,
147+
): List<AntiPattern> =
148+
components
149+
.filter { c -> forbiddenPatterns.any { pattern -> c.source.simpleName.contains(pattern) } }
150+
.filter {
151+
!it.source.file.path
152+
.contains("/test/") &&
153+
!it.source.file.path
154+
.contains("\\test\\")
155+
}.map { c ->
156+
val matched = forbiddenPatterns.first { c.source.simpleName.contains(it) }
157+
AntiPattern(
158+
severity = AntiPattern.Severity.WARNING,
159+
message = "`${c.source.simpleName}` contains forbidden pattern `$matched`",
160+
file = c.source.file.relativeTo(rootDir),
161+
suggestion = "Rename to describe what the class does instead of using a generic name.",
162+
)
163+
}
164+
165+
private fun detectDependencyInversionViolations(
166+
components: List<ClassifiedComponent>,
167+
resolver: SupertypeResolver,
168+
rootDir: File,
169+
): List<AntiPattern> {
170+
val nonTestComponents =
171+
components.filter { c ->
172+
!c.source.file.path
173+
.contains("/test/") &&
174+
!c.source.file.path
175+
.contains("\\test\\") &&
176+
!c.source.isInterface
177+
}
178+
179+
val patterns = mutableListOf<AntiPattern>()
180+
181+
nonTestComponents.forEach { c ->
182+
patterns.addAll(checkImportsForDiViolations(c, resolver, rootDir))
183+
}
184+
185+
return patterns.distinctBy { it.message }
186+
}
187+
188+
private fun checkImportsForDiViolations(
189+
c: ClassifiedComponent,
190+
resolver: SupertypeResolver,
191+
rootDir: File,
192+
): List<AntiPattern> =
193+
c.source.imports.mapNotNull { importedFqn ->
194+
val importedSimpleName = importedFqn.substringAfterLast(".")
195+
val resolved = resolver.resolve(c, importedSimpleName) ?: return@mapNotNull null
196+
val isAbstraction = resolved.source.isInterface || resolved.source.isAbstract || resolved.source.isDataClass
197+
if (isAbstraction) return@mapNotNull null
198+
buildDiViolationPattern(c, resolved, resolver, rootDir)
199+
}
200+
201+
private fun buildDiViolationPattern(
202+
c: ClassifiedComponent,
203+
resolved: ClassifiedComponent,
204+
resolver: SupertypeResolver,
205+
rootDir: File,
206+
): AntiPattern {
207+
val implementedInterfaces =
208+
resolved.source.supertypes
209+
.mapNotNull { supertype ->
210+
resolver.resolve(resolved, supertype)
211+
}.filter { it.source.isInterface }
212+
213+
return if (implementedInterfaces.isNotEmpty()) {
214+
val ifaceName = implementedInterfaces.first().source.simpleName
215+
val concreteName = resolved.source.simpleName
216+
val msg =
217+
"Dependency on concrete `$concreteName` instead of interface `$ifaceName`"
218+
AntiPattern(
219+
severity = AntiPattern.Severity.WARNING,
220+
message = msg,
221+
file = c.source.file.relativeTo(rootDir),
222+
suggestion = "Depend on `$ifaceName` instead of the concrete class.",
223+
)
224+
} else {
225+
AntiPattern(
226+
severity = AntiPattern.Severity.INFO,
227+
message = "Dependency on concrete class `${resolved.source.simpleName}` in `${c.source.simpleName}`",
228+
file = c.source.file.relativeTo(rootDir),
229+
suggestion = "Consider extracting an interface for `${resolved.source.simpleName}`.",
230+
)
231+
}
232+
}
233+
98234
private fun detectSingleImplInterfaces(
99235
components: List<ClassifiedComponent>,
100236
resolver: SupertypeResolver,
@@ -104,15 +240,15 @@ private fun detectSingleImplInterfaces(
104240
val impls = resolver.findImplementors(iface)
105241
if (impls.size == 1) {
106242
val impl = impls[0]
243+
val ifaceName = iface.source.simpleName
244+
val implName = impl.source.simpleName
245+
val msg =
246+
"Interface `$ifaceName` has only one implementation: `$implName`"
107247
AntiPattern(
108248
severity = AntiPattern.Severity.INFO,
109-
message =
110-
"Interface `${iface.source.simpleName}` has only one implementation: " +
111-
"`${impl.source.simpleName}`",
249+
message = msg,
112250
file = iface.source.file.relativeTo(rootDir),
113-
suggestion =
114-
"If this interface isn't meant for testing or future extension, " +
115-
"consider using `${impl.source.simpleName}` directly.",
251+
suggestion = "Consider using `$implName` directly unless needed for testing.",
116252
)
117253
} else {
118254
null
@@ -131,13 +267,13 @@ private fun detectGodClasses(
131267
}.filter { it.role != ComponentRole.CONFIGURATION }
132268
.map { c ->
133269
val reasons = buildGodClassReasons(c)
270+
val suggestion =
271+
"Split into smaller, focused classes with a single responsibility."
134272
AntiPattern(
135273
severity = AntiPattern.Severity.WARNING,
136274
message = "`${c.source.simpleName}` may be doing too much (${reasons.joinToString(", ")})",
137275
file = c.source.file.relativeTo(rootDir),
138-
suggestion =
139-
"Consider splitting into smaller, focused classes. " +
140-
"Each class should have a single responsibility.",
276+
suggestion = suggestion,
141277
)
142278
}
143279

@@ -207,9 +343,7 @@ private fun detectCircularDeps(edges: List<ClassDependency>): List<AntiPattern>
207343
severity = AntiPattern.Severity.WARNING,
208344
message = "Circular dependency: ${cycle.joinToString(" -> ")}",
209345
file = File("."),
210-
suggestion =
211-
"Break the cycle by extracting a shared interface or " +
212-
"moving shared logic to a separate class.",
346+
suggestion = "Break the cycle by extracting a shared interface or moving shared logic to a separate class.",
213347
)
214348
}
215349
}
@@ -242,14 +376,17 @@ private fun detectMissingTests(
242376
.filter { it.source.simpleName !in testNames }
243377

244378
return if (untested.size > MAX_UNTESTED_BEFORE_SUMMARY) {
379+
val preview =
380+
untested
381+
.take(UNTESTED_PREVIEW_COUNT)
382+
.joinToString(", ") { "`${it.source.simpleName}`" }
383+
val suggestion = "Consider adding tests for key components, especially: $preview"
245384
listOf(
246385
AntiPattern(
247386
severity = AntiPattern.Severity.INFO,
248387
message = "${untested.size} classes have no corresponding test file",
249388
file = File("."),
250-
suggestion =
251-
"Consider adding tests for key components, especially: " +
252-
untested.take(UNTESTED_PREVIEW_COUNT).joinToString(", ") { "`${it.source.simpleName}`" },
389+
suggestion = suggestion,
253390
),
254391
)
255392
} else {

0 commit comments

Comments
 (0)