Skip to content

Commit

Permalink
Lint rule to fix the space assignment syntax (#415)
Browse files Browse the repository at this point in the history
  • Loading branch information
ov7a authored Jan 6, 2025
1 parent 9d60ce5 commit 0f04b24
Show file tree
Hide file tree
Showing 7 changed files with 402 additions and 16 deletions.
2 changes: 1 addition & 1 deletion gradle/wrapper/gradle-wrapper.properties
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionUrl=https://services.gradle.org/distributions/gradle-8.11.1-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-8.12-bin.zip
networkTimeout=10000
validateDistributionUrl=true
zipStoreBase=GRADLE_USER_HOME
Expand Down
3 changes: 1 addition & 2 deletions gradlew
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,7 @@ done
# shellcheck disable=SC2034
APP_BASE_NAME=${0##*/}
# Discard cd standard output in case $CDPATH is set (https://github.com/gradle/gradle/issues/25036)
APP_HOME=$( cd -P "${APP_HOME:-./}" > /dev/null && printf '%s
' "$PWD" ) || exit
APP_HOME=$( cd -P "${APP_HOME:-./}" > /dev/null && printf '%s\n' "$PWD" ) || exit

# Use the maximum available, or set MAX_FD != -1 to use that value.
MAX_FD=maximum
Expand Down
27 changes: 16 additions & 11 deletions src/main/groovy/com/netflix/nebula/lint/rule/GradleLintRule.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,7 @@ import com.netflix.nebula.lint.GradleViolation
import com.netflix.nebula.lint.plugin.LintRuleRegistry
import org.codehaus.groovy.ast.ASTNode
import org.codehaus.groovy.ast.ClassNode
import org.codehaus.groovy.ast.expr.ArgumentListExpression
import org.codehaus.groovy.ast.expr.BinaryExpression
import org.codehaus.groovy.ast.expr.ClosureExpression
import org.codehaus.groovy.ast.expr.ConstantExpression
import org.codehaus.groovy.ast.expr.Expression
import org.codehaus.groovy.ast.expr.GStringExpression
import org.codehaus.groovy.ast.expr.MapExpression
import org.codehaus.groovy.ast.expr.MethodCallExpression
import org.codehaus.groovy.ast.expr.PropertyExpression
import org.codehaus.groovy.ast.expr.VariableExpression
import org.codehaus.groovy.ast.expr.*
import org.codehaus.groovy.ast.stmt.ExpressionStatement
import org.codenarc.rule.AbstractAstVisitorRule
import org.codenarc.rule.AstVisitor
Expand All @@ -43,7 +34,6 @@ import org.slf4j.LoggerFactory
import java.text.ParseException

abstract class GradleLintRule extends GroovyAstVisitor implements Rule {
Project project // will be non-null if type is GradleModelAware, otherwise null
BuildFiles buildFiles
SourceCode sourceCode
List<GradleViolation> gradleViolations = []
Expand Down Expand Up @@ -152,6 +142,21 @@ abstract class GradleLintRule extends GroovyAstVisitor implements Rule {
calls.collect { call -> _dslStack(call) }.flatten() as List<String>
}

final List<Expression> typedDslStack(List<MethodCallExpression> calls) {
def _dslStack
_dslStack = { Expression expr ->
if (expr instanceof PropertyExpression)
_dslStack(expr.objectExpression) + expr.property
else if (expr instanceof MethodCallExpression)
_dslStack(expr.objectExpression) + expr
else if (expr instanceof VariableExpression)
expr.text == 'this' ? [] : [expr]
else []
}

calls.collect { call -> _dslStack(call) }.flatten() as List<String>
}

private final String containingConfiguration(MethodCallExpression call) {
def stackStartingWithConfName = dslStack(callStack + call).dropWhile { it != 'configurations' }.drop(1)
stackStartingWithConfName.isEmpty() ? null : stackStartingWithConfName[0]
Expand Down
186 changes: 184 additions & 2 deletions src/main/groovy/com/netflix/nebula/lint/rule/GradleModelAware.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,194 @@

package com.netflix.nebula.lint.rule

import org.codehaus.groovy.ast.expr.*
import org.gradle.api.Action
import org.gradle.api.Project
import org.gradle.api.Task
import org.gradle.api.internal.DefaultDomainObjectCollection
import org.gradle.api.plugins.ExtensionAware
import org.gradle.configuration.ImportsReader

import javax.annotation.Nullable

/**
* Decorate lint rule visitors with this interface in order to use the
* evaluated Gradle project model in the rule
*/
interface GradleModelAware {
void setProject(Project project)
trait GradleModelAware {
Project project
Map<String, List<String>> projectDefaultImports = null

TypeInformation receiver(MethodCallExpression call) {
List<Expression> fullCallStack = typedDslStack(callStack + call)
List<TypeInformation> typedStack = []
for (Expression currentMethod in fullCallStack) {
if (typedStack.empty) {
typedStack.add(new TypeInformation(project))
}
while (!typedStack.empty) {
def current = typedStack.last()
def candidate = findDirectCandidate(current, currentMethod)
if (candidate != null) {
typedStack.add(candidate)
break
}
typedStack.removeLast()
}
}
if (typedStack.size() >= 2) { //there should be the method return type and the receiver at least
return typedStack[-2]
} else {
return null
}
}

private findDirectCandidate(TypeInformation current, Expression currentExpression) {
String methodName
switch (currentExpression) {
case MethodCallExpression:
methodName = currentExpression.methodAsString
break
case PropertyExpression:
methodName = currentExpression.propertyAsString
break
case VariableExpression:
methodName = currentExpression.text
break
case ConstantExpression:
methodName = currentExpression.text
break
default:
return null
}
def getter = current.clazz.getMethods().find { it.name == "get${methodName.capitalize()}" }
if (getter != null) {
if (current.object != null) {
try {
return new TypeInformation(getter.invoke(current.object))
} catch (ignored) {
// ignore and fallback to the return type
}
}
return new TypeInformation(null, getter.returnType)
}

// there is no public API for DomainObjectCollection.type
if (current.object != null && DefaultDomainObjectCollection.class.isAssignableFrom(current.clazz)) {
def collectionItemType = ((DefaultDomainObjectCollection) current.object).type

if (methodName == "withType" && currentExpression instanceof MethodCallExpression && currentExpression.arguments.size() >= 1) {
def className = currentExpression.arguments[0]
def candidate = findSuitableClass(className.text, collectionItemType)
if (candidate != null) {
collectionItemType = candidate
}
}

if ((methodName == "create" || methodName == "register") && currentExpression instanceof MethodCallExpression && currentExpression.arguments.size() >= 2 && currentExpression.arguments[1] !instanceof ClosureExpression) {
def className = currentExpression.arguments[1]
def candidate = findSuitableClass(className.text, collectionItemType)
if (candidate != null) {
collectionItemType = candidate
}
}

def transformationOrFactoryMethod = current.clazz.getMethods().find { it.name == methodName && it.parameterTypes[-1] == Action.class }
if (transformationOrFactoryMethod != null) {
if (collectionItemType.isAssignableFrom(transformationOrFactoryMethod.returnType)) {
return new TypeInformation(null, transformationOrFactoryMethod.returnType)
} else {
// assume that all actions are done on the collection type
return new TypeInformation(null, collectionItemType)
}
}
}

// note that we can't use tasks.findByName because it may lead to unwanted side effects because of potential task creation
if (Project.class.isAssignableFrom(current.clazz) && methodName == "task" && currentExpression instanceof MethodCallExpression) {
def taskType = extractTaskType(currentExpression)
if (taskType != null) {
return new TypeInformation(null, taskType)
}
return new TypeInformation(null, Task.class)
}

def factoryMethod = current.clazz.getMethods().find { it.name == methodName && it.parameterTypes[-1] == Action.class }
if (factoryMethod != null) {
// assume that this is a factory method that returns the created type
return new TypeInformation(null, factoryMethod.returnType)
}

if (current.object != null && current.object instanceof ExtensionAware) {
def extension = current.object.extensions.findByName(methodName)
if (extension != null) {
return new TypeInformation(extension)
}
}
return null;
}

private List<Class> findClassInScope(String name) {
if (this.projectDefaultImports == null) {
this.projectDefaultImports = project.services.get(ImportsReader.class).getSimpleNameToFullClassNamesMapping()
}
return this.projectDefaultImports.get(name);
}

@Nullable
private Class findSuitableClass(String className, Class parentClass) {
def candidates = (findClassInScope(className) ?: []) + [className]
for (String candidate in candidates) {
try {
def candidateClass = Class.forName(candidate)
if (parentClass.isAssignableFrom(candidateClass)) {
return candidateClass
}
} catch (ignored) {
// ignore and try the next candidate
}
}
return null
}

@Nullable
private Class extractTaskType(MethodCallExpression currentExpression) {
for (Expression arg in currentExpression.arguments) {
if (arg instanceof VariableExpression || arg instanceof ConstantExpression) {
def candidate = findSuitableClass(arg.text, Task.class)
if (candidate != null) {
return candidate
}
} else if (arg instanceof MapExpression) {
def type = arg
.mapEntryExpressions
.find { it.keyExpression.text == "type" }
?.valueExpression?.text
if (type != null) {
def candidate = findSuitableClass(type, Task.class)
if (candidate != null) {
return candidate
}
}
}
}
return null
}
}

class TypeInformation {
@Nullable
Class clazz
@Nullable
Object object

TypeInformation(Object object) {
this.object = object
this.clazz = object.class
}

TypeInformation(Object object, Class clazz) {
this.object = object
this.clazz = clazz
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package com.netflix.nebula.lint.rule.dsl

import com.netflix.nebula.lint.rule.GradleLintRule
import com.netflix.nebula.lint.rule.GradleModelAware
import org.codehaus.groovy.ast.expr.ClosureExpression
import org.codehaus.groovy.ast.expr.MethodCallExpression

class SpaceAssignmentRule extends GradleLintRule implements GradleModelAware {

String description = "space-assignment syntax is deprecated"

@Override
void visitMethodCallExpression(MethodCallExpression call) {
if (call.arguments.size() != 1 || call.arguments[-1] instanceof ClosureExpression) {
return
}

def receiverClass = receiver(call)?.clazz
if (receiverClass == null) {
return // no enough data to analyze
}

def invokedMethodName = call.method.value

// check if the method has a matching property
def setter = receiverClass.getMethods().find { it.name == "set${invokedMethodName.capitalize()}" }
if (setter == null) {
return // no matching property
}

// check if it's a generated method for space assignment
def exactMethod = receiverClass.getMethods().find { it.name == invokedMethodName }
if (exactMethod != null) {
def deprecatedAnnotation = exactMethod.getAnnotation(Deprecated)
if (deprecatedAnnotation != null) {
// may be false positive when the explicit method is deprecated
addBuildLintViolation(description, call)
.replaceWith(call, getReplacement(call))
}
} else {
addBuildLintViolation(description, call)
.replaceWith(call, getReplacement(call))
}
}

def getReplacement(MethodCallExpression call){
def originalLine = getSourceCode().line(call.lineNumber-1)
return originalLine.replaceFirst(call.methodAsString, call.methodAsString + " =")
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
implementation-class=com.netflix.nebula.lint.rule.dsl.SpaceAssignmentRule
Loading

0 comments on commit 0f04b24

Please sign in to comment.