Skip to content

Commit a7a091b

Browse files
wakingrufuswakingrufus
authored
Merge pull request #435 from nebula-plugins/fix-constructor-declaration
skip visiting method and constructor declarations if they are declared within a class
2 parents eeec94a + d774637 commit a7a091b

File tree

6 files changed

+87
-30
lines changed

6 files changed

+87
-30
lines changed

‎build.gradle‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,11 +87,17 @@ dependencies {
8787
}
8888
plugin 'org.eclipse.jdt:core:3.3.0-v_771'
8989

90+
// These are because the AbstractRuleSpec test harness is included in src/main
9091
compileOnly "com.netflix.nebula:nebula-test:11.+"
92+
compileOnly("org.spockframework:spock-core:2.3-groovy-4.0")
93+
compileOnly("junit:junit:4.13.2")
94+
compileOnly(gradleTestKit())
9195

9296
testImplementation 'org.ow2.asm:asm-util:9.8'
9397
testImplementation 'joda-time:joda-time:latest.release'
9498
testImplementation 'com.netflix.nebula:gradle-info-plugin:latest.release'
99+
testImplementation("org.spockframework:spock-core:2.3-groovy-4.0")
100+
testImplementation("org.spockframework:spock-junit4:2.3-groovy-4.0")
95101
}
96102

97103
afterEvaluate {
@@ -204,3 +210,9 @@ validatePlugins {
204210
javaCrossCompile {
205211
disableKotlinSupport = true
206212
}
213+
214+
tasks.wrapper {
215+
distributionType = Wrapper.DistributionType.BIN
216+
gradleVersion = "9.0.0"
217+
distributionSha256Sum = "8fad3d78296ca518113f3d29016617c7f9367dc005f932bd9d93bf45ba46072b"
218+
}

‎gradle.lockfile‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,14 @@ com.netflix.nebula:gradle-contacts-plugin:7.0.1=integTestRuntimeClasspath,testRu
1212
com.netflix.nebula:gradle-info-plugin:14.0.0=integTestCompileClasspath,integTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath
1313
com.netflix.nebula:nebula-gradle-interop:2.3.0=integTestRuntimeClasspath
1414
com.netflix.nebula:nebula-gradle-interop:3.0.0=compileClasspath,runtimeClasspath,shadow,testCompileClasspath,testRuntimeClasspath
15-
com.netflix.nebula:nebula-test:11.0.0=compileClasspath,integTestCompileClasspath,integTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath
15+
com.netflix.nebula:nebula-test:11.0.0=integTestCompileClasspath,integTestRuntimeClasspath
16+
com.netflix.nebula:nebula-test:11.1.5=compileClasspath,testCompileClasspath,testRuntimeClasspath
1617
com.perforce:p4java:2015.2.1365273=integTestRuntimeClasspath,testRuntimeClasspath
1718
commons-lang:commons-lang:2.6=compileClasspath,runtimeClasspath,testCompileClasspath,testRuntimeClasspath
1819
javax.inject:javax.inject:1=compileClasspath,runtimeClasspath,shadow,testCompileClasspath,testRuntimeClasspath
1920
joda-time:joda-time:2.14.0=integTestCompileClasspath,integTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath
2021
junit:junit:4.13.2=compileClasspath,integTestCompileClasspath,integTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath
22+
net.bytebuddy:byte-buddy:1.15.11=compileClasspath,testCompileClasspath,testRuntimeClasspath
2123
net.java.dev.jna:jna-platform:5.16.0=integTestRuntimeClasspath,testRuntimeClasspath
2224
net.java.dev.jna:jna:5.16.0=integTestRuntimeClasspath,testRuntimeClasspath
2325
org.apache.commons:commons-lang3:3.8.1=compileClasspath,runtimeClasspath,shadow,testCompileClasspath,testRuntimeClasspath
@@ -27,6 +29,7 @@ org.apache.maven:maven-builder-support:3.8.3=compileClasspath,runtimeClasspath,s
2729
org.apache.maven:maven-model-builder:3.8.3=compileClasspath,runtimeClasspath,shadow,testCompileClasspath,testRuntimeClasspath
2830
org.apache.maven:maven-model:3.8.3=compileClasspath,runtimeClasspath,shadow,testCompileClasspath,testRuntimeClasspath
2931
org.apiguardian:apiguardian-api:1.1.2=compileClasspath,integTestCompileClasspath,integTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath
32+
org.assertj:assertj-core:3.27.3=compileClasspath,testCompileClasspath,testRuntimeClasspath
3033
org.codehaus.gpars:gpars:1.2.1=compileClasspath,runtimeClasspath,shadow,testCompileClasspath,testRuntimeClasspath
3134
org.codehaus.jsr166-mirror:jsr166y:1.7.0=compileClasspath,runtimeClasspath,shadow,testCompileClasspath,testRuntimeClasspath
3235
org.codehaus.plexus:plexus-interpolation:1.26=compileClasspath,runtimeClasspath,shadow,testCompileClasspath,testRuntimeClasspath
@@ -53,9 +56,12 @@ org.hamcrest:hamcrest:2.2=compileClasspath,integTestCompileClasspath,integTestRu
5356
org.jetbrains.kotlin:kotlin-stdlib:2.2.0=compileClasspath,integTestCompileClasspath,integTestRuntimeClasspath,runtimeClasspath,shadow,testCompileClasspath,testRuntimeClasspath
5457
org.jetbrains:annotations:13.0=compileClasspath,integTestCompileClasspath,integTestRuntimeClasspath,runtimeClasspath,shadow,testCompileClasspath,testRuntimeClasspath
5558
org.jspecify:jspecify:1.0.0=compileClasspath,runtimeClasspath,testCompileClasspath,testRuntimeClasspath
56-
org.junit.platform:junit-platform-commons:1.13.1=compileClasspath,integTestCompileClasspath,integTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath
57-
org.junit.platform:junit-platform-engine:1.13.1=compileClasspath,integTestCompileClasspath,integTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath
58-
org.junit.platform:junit-platform-launcher:1.13.1=compileClasspath,integTestCompileClasspath,integTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath
59+
org.junit.platform:junit-platform-commons:1.13.1=integTestCompileClasspath,integTestRuntimeClasspath
60+
org.junit.platform:junit-platform-commons:1.13.4=compileClasspath,testCompileClasspath,testRuntimeClasspath
61+
org.junit.platform:junit-platform-engine:1.13.1=integTestCompileClasspath,integTestRuntimeClasspath
62+
org.junit.platform:junit-platform-engine:1.13.4=compileClasspath,testCompileClasspath,testRuntimeClasspath
63+
org.junit.platform:junit-platform-launcher:1.13.1=integTestCompileClasspath,integTestRuntimeClasspath
64+
org.junit.platform:junit-platform-launcher:1.13.4=compileClasspath,testCompileClasspath,testRuntimeClasspath
5965
org.multiverse:multiverse-core:0.7.0=compileClasspath,runtimeClasspath,shadow,testCompileClasspath,testRuntimeClasspath
6066
org.objenesis:objenesis:2.4=integTestRuntimeClasspath,testRuntimeClasspath
6167
org.opentest4j:opentest4j:1.3.0=compileClasspath,integTestCompileClasspath,integTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath
@@ -65,5 +71,5 @@ org.ow2.asm:asm-tree:9.8=compileClasspath,integTestCompileClasspath,integTestRun
6571
org.ow2.asm:asm-util:9.8=integTestCompileClasspath,integTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath
6672
org.ow2.asm:asm:9.8=compileClasspath,integTestCompileClasspath,integTestRuntimeClasspath,runtimeClasspath,testCompileClasspath,testRuntimeClasspath
6773
org.spockframework:spock-core:2.3-groovy-4.0=compileClasspath,integTestCompileClasspath,integTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath
68-
org.spockframework:spock-junit4:2.3-groovy-4.0=compileClasspath,integTestCompileClasspath,integTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath
74+
org.spockframework:spock-junit4:2.3-groovy-4.0=integTestCompileClasspath,integTestRuntimeClasspath,testCompileClasspath,testRuntimeClasspath
6975
empty=annotationProcessor,compile,integTestAnnotationProcessor,integTestCompile,integTestCompileOnly,integTestRuntime,runtime,testAnnotationProcessor,testCompile,testCompileOnly,testRuntime

‎gradle/wrapper/gradle-wrapper.properties‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
distributionBase=GRADLE_USER_HOME
22
distributionPath=wrapper/dists
3-
distributionSha256Sum=f09991ce63e889bf8a5f579a467a82bdf7621bc93d59cd2f8fe5982f39f28e2a
4-
distributionUrl=https\://services.gradle.org/distributions/gradle-9.0.0-rc-4-bin.zip
3+
distributionSha256Sum=8fad3d78296ca518113f3d29016617c7f9367dc005f932bd9d93bf45ba46072b
4+
distributionUrl=https\://services.gradle.org/distributions/gradle-9.0.0-bin.zip
55
networkTimeout=10000
66
validateDistributionUrl=true
77
zipStoreBase=GRADLE_USER_HOME

‎src/main/groovy/com/netflix/nebula/lint/rule/CompositeGroovyAstVisitor.groovy‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -85,19 +85,28 @@ class CompositeGroovyAstVisitor extends ClassCodeVisitorSupport implements AstVi
8585

8686
@Override
8787
protected void visitConstructorOrMethod(MethodNode node, boolean isConstructor) {
88-
super.visitConstructorOrMethod(node, isConstructor)
88+
// within a method or constructor declaration, this code is not actually being invoked
89+
if (node.declaringClass.name == "None") {
90+
super.visitConstructorOrMethod(node, isConstructor)
91+
}
8992
}
9093

9194
@Override
9295
void visitConstructor(ConstructorNode node) {
93-
visitors.each { it.visitConstructor(node) }
94-
super.visitConstructor(node)
96+
// within a method or constructor declaration, this code is not actually being invoked
97+
if (node.declaringClass.name == "None") {
98+
visitors.each { it.visitConstructor(node) }
99+
super.visitConstructor(node)
100+
}
95101
}
96102

97103
@Override
98104
void visitMethod(MethodNode node) {
99-
visitors.each { it.visitMethod(node) }
100-
super.visitMethod(node)
105+
// within a method or constructor declaration, this code is not actually being invoked
106+
if (node.declaringClass.name == "None") {
107+
visitors.each { it.visitMethod(node) }
108+
super.visitMethod(node)
109+
}
101110
}
102111

103112
@Override

‎src/main/groovy/com/netflix/nebula/lint/rule/GradleLintRule.groovy‎

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,9 @@ import com.netflix.nebula.lint.GradleViolation
2020
import com.netflix.nebula.lint.plugin.LintRuleRegistry
2121
import com.netflix.nebula.lint.plugin.UnexpectedLintRuleFailureException
2222
import org.codehaus.groovy.ast.ASTNode
23+
import org.codehaus.groovy.ast.AnnotatedNode
2324
import org.codehaus.groovy.ast.ClassNode
24-
import org.codehaus.groovy.ast.expr.ArgumentListExpression
25-
import org.codehaus.groovy.ast.expr.BinaryExpression
26-
import org.codehaus.groovy.ast.expr.ClosureExpression
27-
import org.codehaus.groovy.ast.expr.ConstantExpression
28-
import org.codehaus.groovy.ast.expr.Expression
29-
import org.codehaus.groovy.ast.expr.GStringExpression
30-
import org.codehaus.groovy.ast.expr.MapExpression
31-
import org.codehaus.groovy.ast.expr.MethodCallExpression
32-
import org.codehaus.groovy.ast.expr.PropertyExpression
33-
import org.codehaus.groovy.ast.expr.VariableExpression
25+
import org.codehaus.groovy.ast.expr.*
3426
import org.codehaus.groovy.ast.stmt.ExpressionStatement
3527
import org.codenarc.rule.AbstractAstVisitorRule
3628
import org.codenarc.rule.AstVisitor
@@ -465,12 +457,12 @@ abstract class GradleLintRule extends GroovyAstVisitor implements Rule {
465457
return null
466458
}
467459
dependency = GradleDependency.fromConstant(expr)
468-
} else if (call.arguments.expressions.any { it instanceof MethodCallExpression && it.methodAsString == 'project'}) {
460+
} else if (call.arguments.expressions.any { it instanceof MethodCallExpression && it.methodAsString == 'project' }) {
469461
MethodCallExpression projectMethodCall = call.arguments.expressions
470-
.find { it instanceof MethodCallExpression && it.methodAsString == 'project'} as MethodCallExpression
462+
.find { it instanceof MethodCallExpression && it.methodAsString == 'project' } as MethodCallExpression
471463
ConstantExpression projectName =
472464
projectMethodCall.arguments.expressions.
473-
find { it instanceof ConstantExpression} as ConstantExpression
465+
find { it instanceof ConstantExpression } as ConstantExpression
474466
if (projectName != null)
475467
visitAnySubmoduleDependency(call, methodName, projectName.value.toString())
476468
else if (projectMethodCall.arguments.expressions.any { it instanceof MapExpression }) {

‎src/test/groovy/com/netflix/nebula/lint/rule/GradleLintRuleSpec.groovy‎

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2015-2019 Netflix, Inc.
2+
* Copyright 2015-2025 Netflix, Inc.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -21,6 +21,7 @@ import com.netflix.nebula.lint.plugin.GradleLintPlugin
2121
import com.netflix.nebula.lint.plugin.LintRuleRegistry
2222
import com.netflix.nebula.lint.rule.test.AbstractRuleSpec
2323
import org.codehaus.groovy.ast.ClassNode
24+
import org.codehaus.groovy.ast.expr.BinaryExpression
2425
import org.codehaus.groovy.ast.expr.Expression
2526
import org.codehaus.groovy.ast.expr.MethodCallExpression
2627
import org.codehaus.groovy.ast.expr.VariableExpression
@@ -102,7 +103,7 @@ class GradleLintRuleSpec extends AbstractRuleSpec {
102103

103104
@Override
104105
void visitGradlePlugin(MethodCallExpression call, String conf, GradlePlugin gradlePluginGradleLintRuleSpec) {
105-
if(call.methodAsString == 'apply') {
106+
if (call.methodAsString == 'apply') {
106107
pluginCount++
107108
}
108109
}
@@ -115,6 +116,44 @@ class GradleLintRuleSpec extends AbstractRuleSpec {
115116
plugin << ['java', 'org.gradle.java']
116117
}
117118

119+
def 'dont visit constructor declaration'() {
120+
when:
121+
project.buildFile << """
122+
b = 2
123+
a = 1
124+
class A {
125+
int a
126+
A() {
127+
a = 1
128+
}
129+
}
130+
"""
131+
132+
int assignA = 0
133+
int assignARoot = 0
134+
135+
runRulesAgainst(
136+
new AbstractExampleGradleLintRule() {
137+
String description = 'test'
138+
139+
@Override
140+
void visitBinaryExpression(BinaryExpression binaryExpression) {
141+
if (binaryExpression.leftExpression.variable == 'a') {
142+
if (callStack.isEmpty()) {
143+
assignARoot++
144+
} else {
145+
assignA++
146+
}
147+
}
148+
}
149+
}
150+
)
151+
152+
then:
153+
assignA == 0
154+
assignARoot == 1
155+
}
156+
118157
def 'skip nested `plugins`'() {
119158
when:
120159
project.buildFile << """
@@ -240,7 +279,6 @@ class GradleLintRuleSpec extends AbstractRuleSpec {
240279
}
241280

242281

243-
244282
def 'visit dependencies in subprojects block'() {
245283
when:
246284
def subproject = addSubproject('test')
@@ -711,7 +749,7 @@ class GradleLintRuleSpec extends AbstractRuleSpec {
711749
void visitMethodCallExpression(MethodCallExpression call) {
712750
if (call.methodAsString == 'block')
713751
addBuildLintViolation('inserting into block', call)
714-
.insertIntoClosure(call, 'inserted true')
752+
.insertIntoClosure(call, 'inserted true')
715753
}
716754
})
717755

@@ -738,7 +776,7 @@ class GradleLintRuleSpec extends AbstractRuleSpec {
738776
void visitMethodCallExpression(MethodCallExpression call) {
739777
if (call.methodAsString == 'block')
740778
addBuildLintViolation('inserting into block', call)
741-
.insertIntoClosure(call, 'inserted true')
779+
.insertIntoClosure(call, 'inserted true')
742780
}
743781
})
744782

0 commit comments

Comments
 (0)