Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lint rule to fix the space assignment syntax #415

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading