Skip to content

Commit 9d47145

Browse files
nmarsollierviaductbot
authored andcommitted
Create a rule in detekt to avoid println in...
To avoid using the println command in gradle configuration files. Create a rule in detekt to avoid println in config files. Github-Change-Id: 940397 GitOrigin-RevId: f5311c2a5a93b57d3f42c7affb1b752ca863ba55
1 parent 12e67a9 commit 9d47145

File tree

11 files changed

+302
-7
lines changed

11 files changed

+302
-7
lines changed

build-logic/build.gradle.kts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ dependencies {
1111
implementation(plugin(libs.plugins.detekt))
1212
implementation(plugin(libs.plugins.ktlintPlugin))
1313
implementation(plugin(libs.plugins.dokka))
14+
compileOnly(libs.detekt.api)
1415

1516
// buildroot dependencies
1617
implementation(plugin(libs.plugins.dependency.analysis))
@@ -25,4 +26,4 @@ dependencies {
2526
* Helper function that transforms a Gradle Plugin alias from a Version Catalog into a valid dependency notation
2627
*/
2728
fun plugin(plugin: Provider<PluginDependency>) =
28-
plugin.map { "${it.pluginId}:${it.pluginId}.gradle.plugin:${it.version}" }
29+
plugin.map { "${it.pluginId}:${it.pluginId}.gradle.plugin:${it.version}" }

build-logic/src/main/kotlin/conventions/kotlin-static-analysis.gradle.kts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import viaduct.gradle.internal.repoRoot
55
plugins {
66
id("io.gitlab.arturbosch.detekt")
77
id("org.jlleitschuh.gradle.ktlint")
8+
id("detekt.viaduct-detekt-rules")
89
}
910

1011
val detektConfigFile = providers.provider { repoRoot().file("detekt.yml") }
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
package detekt
2+
3+
import io.gitlab.arturbosch.detekt.api.CodeSmell
4+
import io.gitlab.arturbosch.detekt.api.Config
5+
import io.gitlab.arturbosch.detekt.api.Debt
6+
import io.gitlab.arturbosch.detekt.api.Entity
7+
import io.gitlab.arturbosch.detekt.api.Issue
8+
import io.gitlab.arturbosch.detekt.api.Rule
9+
import io.gitlab.arturbosch.detekt.api.Severity
10+
import org.jetbrains.kotlin.psi.KtCallExpression
11+
import org.jetbrains.kotlin.psi.KtFile
12+
13+
/**
14+
* A custom Detekt rule that flags the use of `println(...)` in Gradle scripts.
15+
*/
16+
class NoPrintlnInGradleRule(config: Config) : Rule(config) {
17+
override val issue = Issue(
18+
id = "NoPrintlnInGradle",
19+
severity = Severity.CodeSmell,
20+
description = "Avoid println(...) in Gradle scripts; use Gradle logger.",
21+
debt = Debt.FIVE_MINS
22+
)
23+
24+
private var isGradleScript = false
25+
26+
override fun visitKtFile(file: KtFile) {
27+
isGradleScript = file.name.endsWith(".gradle.kts")
28+
if (!isGradleScript) return
29+
super.visitKtFile(file)
30+
}
31+
32+
override fun visitCallExpression(expression: KtCallExpression) {
33+
if (!isGradleScript) return
34+
if (expression.calleeExpression?.text == "println") {
35+
report(
36+
CodeSmell(
37+
issue,
38+
Entity.from(expression),
39+
"Use logger.lifecycle/info/debug instead of println in Gradle scripts."
40+
)
41+
)
42+
}
43+
super.visitCallExpression(expression)
44+
}
45+
}
Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,187 @@
1+
package detekt
2+
3+
import io.gitlab.arturbosch.detekt.api.CodeSmell
4+
import io.gitlab.arturbosch.detekt.api.Config
5+
import io.gitlab.arturbosch.detekt.api.Debt
6+
import io.gitlab.arturbosch.detekt.api.Entity
7+
import io.gitlab.arturbosch.detekt.api.Issue
8+
import io.gitlab.arturbosch.detekt.api.Rule
9+
import io.gitlab.arturbosch.detekt.api.Severity
10+
import org.jetbrains.kotlin.com.intellij.psi.PsiElement
11+
import org.jetbrains.kotlin.psi.KtCallExpression
12+
import org.jetbrains.kotlin.psi.KtExpression
13+
import org.jetbrains.kotlin.psi.KtFile
14+
import org.jetbrains.kotlin.psi.KtLambdaExpression
15+
import org.jetbrains.kotlin.psi.KtStringTemplateExpression
16+
import org.jetbrains.kotlin.psi.psiUtil.parents
17+
18+
/**
19+
* Flags Gradle dependency declarations using raw String coordinates
20+
* (e.g., implementation("group:artifact:version") or group/name/version named params),
21+
* enforcing the use of Version Catalog (libs.*), project(), platform(libs.*), etc.
22+
*/
23+
class NoStringDependenciesInGradleRule(config: Config = Config.empty) : Rule(config) {
24+
override val issue: Issue = Issue(
25+
id = "NoStringDependenciesInGradle",
26+
severity = Severity.CodeSmell,
27+
description = "Dependencies must use version catalog (libs.*), project(), or platform(libs.*). " +
28+
"Raw String coordinates and group/name/version named params are forbidden.",
29+
debt = Debt.TEN_MINS
30+
)
31+
32+
// Common Gradle dependency configurations (add/remove as needed).
33+
private val knownConfigs = setOf(
34+
"api",
35+
"compileOnly",
36+
"debugImplementation",
37+
"implementation",
38+
"kapt",
39+
"ksp",
40+
"runtimeOnly",
41+
"testFixturesApi",
42+
"testFixturesCompileOnly",
43+
"testFixturesImplementation",
44+
"testFixturesRuntimeOnly",
45+
"testImplementation",
46+
"jacocoAggregation"
47+
)
48+
49+
// Accept both exact matches above and any custom configuration ending with these suffixes
50+
// (e.g., "myFlavorImplementation", "stagingApi").
51+
private val configSuffixes = listOf(
52+
"Implementation",
53+
"Api",
54+
"CompileOnly",
55+
"RuntimeOnly"
56+
)
57+
58+
private var isGradleScript = false
59+
60+
override fun visitKtFile(file: KtFile) {
61+
isGradleScript = file.name.endsWith(".gradle.kts")
62+
if (!isGradleScript) return
63+
super.visitKtFile(file)
64+
}
65+
66+
override fun visitCallExpression(expression: KtCallExpression) {
67+
if (!isGradleScript) return
68+
69+
val callee = expression.calleeExpression?.text ?: return
70+
if (!isDependencyConfigCall(callee)) {
71+
// Not a dependency configuration call, nothing to do.
72+
super.visitCallExpression(expression)
73+
return
74+
}
75+
76+
// Sanity check: optionally ensure we’re inside a `dependencies {}` scope, to reduce false positives.
77+
// If you want to be stricter, comment this out.
78+
val insideDependenciesBlock = expression.parents.any { isDependenciesBlock(it) }
79+
if (!insideDependenciesBlock) {
80+
super.visitCallExpression(expression)
81+
return
82+
}
83+
84+
// 1) If first argument is a string literal like "g:a:v", flag it unless explicitly allowed.
85+
val firstArg = expression.valueArguments.firstOrNull()
86+
val argExpr = firstArg?.getArgumentExpression()
87+
88+
if (argExpr != null) {
89+
// Allowed forms: libs.*, platform(libs.*), enforcedPlatform(libs.*), project(":"), testFixtures(project(":")), libs.bundles.*
90+
if (isAllowedExpression(argExpr)) {
91+
super.visitCallExpression(expression)
92+
return
93+
}
94+
95+
// Literal "group:artifact:version" without interpolation → forbidden
96+
if (argExpr is KtStringTemplateExpression && !argExpr.hasInterpolation()) {
97+
reportForbidden(expression, callee, argExpr.text)
98+
super.visitCallExpression(expression)
99+
return
100+
}
101+
}
102+
103+
// 2) Also forbid named params style: implementation(group = "...", name = "...", version = "...")
104+
val hasNamedStringParams = expression.valueArguments.any { va ->
105+
val name = va.getArgumentName()?.asName?.asString()
106+
val isNamed = name in setOf("group", "name", "version")
107+
val v = va.getArgumentExpression()
108+
val isString = v is KtStringTemplateExpression && !v.hasInterpolation()
109+
isNamed && isString
110+
}
111+
112+
if (hasNamedStringParams) {
113+
report(
114+
CodeSmell(
115+
issue,
116+
Entity.from(expression),
117+
"Use version catalog (libs.*) or project()/platform(libs.*). " +
118+
"Do not use group/name/version string parameters."
119+
)
120+
)
121+
}
122+
123+
super.visitCallExpression(expression)
124+
}
125+
126+
private fun isDependencyConfigCall(name: String): Boolean {
127+
if (name in knownConfigs) return true
128+
return configSuffixes.any { name.endsWith(it) }
129+
}
130+
131+
private fun reportForbidden(
132+
expression: KtCallExpression,
133+
callee: String,
134+
literal: String
135+
) {
136+
report(
137+
CodeSmell(
138+
issue,
139+
Entity.from(expression),
140+
"Dependency coordinates must not be raw string literals. " +
141+
"Use $callee(libs.some.alias) or project(\":module\") or platform(libs.some.bom). Found: $literal"
142+
)
143+
)
144+
}
145+
146+
/**
147+
* True if the expression is one of the allowed forms:
148+
* - libs.something / libs.bundles.something
149+
* - project(":module")
150+
* - platform(libs.something) / enforcedPlatform(libs.something)
151+
* - testFixtures(project(":module"))
152+
*/
153+
private fun isAllowedExpression(expr: KtExpression): Boolean {
154+
val text = expr.text.trim()
155+
156+
// plain catalog alias or bundle
157+
if (text.matches(Regex("""^libs(\.bundles)?\.[A-Za-z0-9_.]+$"""))) return true
158+
159+
// project(":...")
160+
if (text.startsWith("project(")) return true
161+
162+
// platform(libs...) or enforcedPlatform(libs...)
163+
if (text.matches(Regex("""^(enforcedPlatform|platform)\s*\(\s*libs(\.bundles)?\.[A-Za-z0-9_.]+\s*\)$"""))) return true
164+
165+
// testFixtures(project(":..."))
166+
if (text.matches(Regex("""^testFixtures\s*\(\s*project\s*\(.*\)\s*\)$"""))) return true
167+
168+
return false
169+
}
170+
171+
/**
172+
* Try to detect a dependencies { ... } block. We check if the PSI element is
173+
* a call 'dependencies' or a lambda passed to it.
174+
*/
175+
private fun isDependenciesBlock(el: PsiElement): Boolean {
176+
// Example shapes:
177+
// dependencies { ... }
178+
// dependencies(…) { ... }
179+
return when (el) {
180+
is KtCallExpression -> el.calleeExpression?.text == "dependencies"
181+
is KtLambdaExpression -> (el.parent?.parent as? KtCallExpression)
182+
?.calleeExpression?.text == "dependencies"
183+
184+
else -> false
185+
}
186+
}
187+
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
package detekt
2+
3+
import io.gitlab.arturbosch.detekt.api.Config
4+
import io.gitlab.arturbosch.detekt.api.RuleSet
5+
import io.gitlab.arturbosch.detekt.api.RuleSetProvider
6+
7+
/**
8+
* Provides the custom Viaduct Detekt rules to the Detekt framework.
9+
*/
10+
class ViaductRuleSetProvider : RuleSetProvider {
11+
override val ruleSetId: String = "viaduct-detekt-rules"
12+
13+
override fun instance(config: Config) =
14+
RuleSet(
15+
ruleSetId,
16+
listOf(NoPrintlnInGradleRule(config), NoStringDependenciesInGradleRule(config))
17+
)
18+
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
package detekt
2+
3+
import io.gitlab.arturbosch.detekt.Detekt
4+
import viaduct.gradle.internal.repoRoot
5+
6+
val detektPluginsCfg = configurations.maybeCreate("detektPlugins")
7+
val selfJar = files(javaClass.protectionDomain.codeSource.location)
8+
dependencies { add(detektPluginsCfg.name, selfJar) }
9+
10+
tasks.register<Detekt>("detektCustomRules") {
11+
description = "Detekt for Custom Rules"
12+
group = "verification"
13+
val detektConfigFile = providers.provider { repoRoot().file("detekt.yml") }
14+
val detektViaductConfigFile = providers.provider { repoRoot().file("detekt-viaduct.yml") }
15+
16+
setSource(files(project.projectDir))
17+
include("build.gradle.kts", "**/*.gradle.kts", "**/*.kt")
18+
exclude("**/build/**", "**/.gradle/**")
19+
20+
config.setFrom(detektConfigFile, detektViaductConfigFile)
21+
pluginClasspath.setFrom(detektPluginsCfg)
22+
23+
ignoreFailures = false
24+
25+
reports {
26+
html.required.set(true)
27+
html.outputLocation.set(layout.buildDirectory.file("reports/detekt/detekt-custom-rules.html"))
28+
txt.required.set(true)
29+
txt.outputLocation.set(layout.buildDirectory.file("reports/detekt/detekt-custom-rules.txt"))
30+
}
31+
}
32+
33+
plugins.withId("base") {
34+
tasks.named("check").configure { dependsOn("detektCustomRules") }
35+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
detekt.ViaductRuleSetProvider

detekt-viaduct.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
viaduct-detekt-rules:
2+
NoPrintlnInGradle:
3+
active: true
4+
NoStringDependenciesInGradle:
5+
active: true

gradle-plugins/application-plugin/build.gradle.kts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,11 @@ dependencies {
1515
implementation(project(":common"))
1616

1717
// Your runtime helpers used by the plugin implementation (keep as needed)
18-
implementation("com.airbnb.viaduct:tenant-codegen")
19-
implementation("com.airbnb.viaduct:shared-graphql")
18+
implementation(libs.viaduct.tenant.codegen)
19+
implementation(libs.viaduct.shared.graphql)
2020

2121
// Do NOT leak the Kotlin Gradle Plugin at runtime
22-
compileOnly("org.jetbrains.kotlin:kotlin-gradle-plugin:1.9.24")
22+
compileOnly(libs.kotlin.gradle.plugin)
2323
}
2424

2525
// Manifest with Implementation-Version for runtime access if you need it

gradle-plugins/module-plugin/build.gradle.kts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,11 @@ dependencies {
1515
implementation(project(":common"))
1616

1717
// Your runtime helpers used by the plugin implementation (keep as needed)
18-
implementation("com.airbnb.viaduct:tenant-codegen")
19-
implementation("com.airbnb.viaduct:shared-graphql")
18+
implementation(libs.viaduct.tenant.codegen)
19+
implementation(libs.viaduct.shared.graphql)
2020

2121
// Do NOT leak the Kotlin Gradle Plugin at runtime
22-
compileOnly("org.jetbrains.kotlin:kotlin-gradle-plugin:1.9.24")
22+
compileOnly(libs.kotlin.gradle.plugin)
2323
}
2424

2525
// Manifest with Implementation-Version for runtime access if you need it

0 commit comments

Comments
 (0)