Merge pull request #319 from chali/HandleGStringExpressions

Handle better GString expressions within dependency declaration
nebula-plugins · Dec 14, 2020 · 22e6c0a · 22e6c0a
2 parents 6eda9cd + 09919fd
commit 22e6c0a
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 4 deletions.
diff --git a/src/main/groovy/com/netflix/nebula/lint/rule/GradleAstUtil.groovy b/src/main/groovy/com/netflix/nebula/lint/rule/GradleAstUtil.groovy
@@ -16,15 +16,26 @@
 
 package com.netflix.nebula.lint.rule
 
+import org.codehaus.groovy.ast.expr.MapEntryExpression
 import org.codehaus.groovy.ast.expr.MapExpression
 import org.codehaus.groovy.ast.expr.MethodCallExpression
+import org.codenarc.source.SourceCode
 
 class GradleAstUtil {
-    static Map<String, String> collectEntryExpressions(MethodCallExpression call) {
+    static Map<String, String> collectEntryExpressions(MethodCallExpression call, SourceCode originalSource = null) {
         call.arguments.expressions
                 .findAll { it instanceof MapExpression }
                 .collect { it.mapEntryExpressions }
                 .flatten()
-                .collectEntries { [it.keyExpression.text, it.valueExpression.text] } as Map<String, String>
+                .collectEntries { MapEntryExpression entry -> [entry.keyExpression.text, extractValue(entry, originalSource)] } as Map<String, String>
+    }
+
+    private static String extractValue(MapEntryExpression entry, SourceCode originalSource) {
+        def value = entry.valueExpression
+        //for one line declaration we try to be more precise and extract original source code since `.text` can be lossy for GString expressions
+        if (originalSource != null && value.lineNumber == value.lastLineNumber)
+            originalSource.lines.get(value.lineNumber - 1).substring(value.columnNumber, value.lastColumnNumber - 2)
+        else
+            entry.valueExpression.text
     }
 }
diff --git a/src/main/groovy/com/netflix/nebula/lint/rule/GradleLintRule.groovy b/src/main/groovy/com/netflix/nebula/lint/rule/GradleLintRule.groovy
@@ -403,7 +403,7 @@ abstract class GradleLintRule extends GroovyAstVisitor implements Rule {
                     def dependency = null
 
                     if (call.arguments.expressions.any { it instanceof MapExpression }) {
-                        def entries = GradleAstUtil.collectEntryExpressions(call)
+                        def entries = GradleAstUtil.collectEntryExpressions(call, sourceCode)
                         dependency = new GradleDependency(
                                 entries.group,
                                 entries.name,
@@ -419,7 +419,10 @@ abstract class GradleLintRule extends GroovyAstVisitor implements Rule {
                             if (it instanceof ConstantExpression)
                                 return it.value
                             if (it instanceof GStringExpression)
-                                return it.text
+                                if (it.lineNumber == it.lastLineNumber)
+                                    return sourceCode.lines.get(it.lineNumber - 1).substring(it.columnNumber, it.lastColumnNumber - 2)
+                                else
+                                    return it.text
                             return null
                         }
                         dependency = GradleDependency.fromConstant(expr)

diff --git a/src/test/groovy/com/netflix/nebula/lint/rule/GradleLintRuleSpec.groovy b/src/test/groovy/com/netflix/nebula/lint/rule/GradleLintRuleSpec.groovy
@@ -391,6 +391,25 @@ class GradleLintRuleSpec extends AbstractRuleSpec {
         a.syntax == GradleDependency.Syntax.MapNotation
     }
 
+    def 'visit dependencies that are defined with GString and braces are preserved'() {
+        given:
+        project.buildFile << """
+            def v = 1
+            dependencies {
+                compile group: 'a', name: 'a', version: "\${v}"
+                compile "b:b:\${v}"
+            }
+        """
+
+        when:
+        def a = new DependencyVisitingRule().run().deps.find { it.name == 'a' }
+        def b = new DependencyVisitingRule().run().deps.find { it.name == 'b' }
+
+        then:
+        a.version == '${v}'
+        b.version == '${v}'
+    }
+
     def 'visit dependency with no version'() {
         when:
         project.buildFile << """