Merge pull request #391 from chali/ImproveLinkedBuildFilesDiscovery

Discover build files for linting if they are linked via apply from with paths containing projectDir or rootDir

src/main/groovy/com/netflix/nebula/lint/plugin/AppliedFilesAstVisitor.groovy

@@ -5,22 +5,37 @@ import org.codehaus.groovy.ast.ClassCodeVisitorSupport
 import org.codehaus.groovy.ast.expr.MapExpression
 import org.codehaus.groovy.ast.expr.MethodCallExpression
 import org.codehaus.groovy.control.SourceUnit
+import org.gradle.api.Project
 
 /**
  * Groovy AST visitor which searches for `apply from: 'another.gradle'` It takes the file and process it recursively
  */
 class AppliedFilesAstVisitor extends ClassCodeVisitorSupport {
 
-    File projectDir
+    Project project
     List<File> appliedFiles = new ArrayList()
+    Map<String, String> projectVariablesMapping
 
-    AppliedFilesAstVisitor(File projectDir) {
-        this.projectDir = projectDir
+    AppliedFilesAstVisitor(Project project) {
+        this.project = project
+        projectVariablesMapping = [
+                "\$projectDir" : project.projectDir.toString(),
+                "\$project.projectDir" : project.projectDir.toString(),
+                "\$rootDir" : project.rootDir.toString(),
+                "\$project.rootDir" : project.rootDir.toString(),
+        ]
     }
 
     void visitApplyFrom(String from) {
         if (! isHttpLink(from)) {
-            appliedFiles.addAll(SourceCollector.getAllFiles(new File(projectDir, from), projectDir))
+            //handle if path contains ${rootDir} ${project.rootDir} ${projectDir} ${project.projectDir}
+            def projectVariable = projectVariablesMapping.find {from.contains(it.key) }
+            if (projectVariable) {
+                def absolutePath = from.replaceAll("\\$projectVariable.key", projectVariable.value)
+                appliedFiles.addAll(SourceCollector.getAllFiles(new File(absolutePath), project))
+            } else {
+                appliedFiles.addAll(SourceCollector.getAllFiles(new File(project.projectDir, from), project))
+            }
         }
     }
 

src/main/groovy/com/netflix/nebula/lint/plugin/LintService.groovy

@@ -116,7 +116,7 @@ class LintService {
         def analyzer = new ReportableAnalyzer(project)
 
         ([project] + project.subprojects).each { p ->
-            def files = SourceCollector.getAllFiles(p.buildFile, p.projectDir)
+            def files = SourceCollector.getAllFiles(p.buildFile, p)
             def buildFiles = new BuildFiles(files)
             def ruleSet = ruleSetForProject(p, onlyCriticalRules)
             if (!ruleSet.rules.isEmpty()) {

src/main/groovy/com/netflix/nebula/lint/plugin/SourceCollector.groovy

@@ -4,22 +4,23 @@ import org.codehaus.groovy.ast.ClassNode
 import org.codehaus.groovy.ast.ModuleNode
 import org.codenarc.source.SourceCode
 import org.codenarc.source.SourceString
+import org.gradle.api.Project
 
 class SourceCollector {
 
     /**
      * It scans given build file for possible `apply from: 'another.gradle'` and recursively
      * collect all build files which are present.
      */
-    static List<File> getAllFiles(File buildFile, File projectDir) {
+    static List<File> getAllFiles(File buildFile, Project project) {
         if (buildFile.exists()) {
             List<File> result = new ArrayList<>()
             result.add(buildFile)
             SourceCode sourceCode = new SourceString(buildFile.text)
             ModuleNode ast = sourceCode.getAst()
             if (ast != null && ast.getClasses() != null) {
                 for (ClassNode classNode : ast.getClasses()) {
-                    AppliedFilesAstVisitor visitor = new AppliedFilesAstVisitor(projectDir)
+                    AppliedFilesAstVisitor visitor = new AppliedFilesAstVisitor(project)
                     visitor.visitClass(classNode)
                     result.addAll(visitor.appliedFiles)
                 }

src/test/groovy/com/netflix/nebula/lint/plugin/SourceCollectorTest.groovy

@@ -1,20 +1,17 @@
 package com.netflix.nebula.lint.plugin
 
-import org.junit.Rule
-import org.junit.rules.TemporaryFolder
-import spock.lang.Specification
+import nebula.test.ProjectSpec
 
-class SourceCollectorTest extends Specification {
+class SourceCollectorTest extends ProjectSpec {
 
-    @Rule
-    TemporaryFolder temporaryFolder
 
     def 'all build files are collected'() {
         given:
-        def rootFile = temporaryFolder.newFile('build.gradle')
-        def level1Sibling1 = temporaryFolder.newFile('level1Sibling1.gradle')
-        def level1Sibling2 = temporaryFolder.newFile('level1Sibling2.gradle')
-        def level2 = temporaryFolder.newFile('level2.gradle')
+        def projectDir = project.projectDir
+        def rootFile = new File(projectDir, 'build.gradle')
+        def level1Sibling1 = new File(projectDir, 'level1Sibling1.gradle')
+        def level1Sibling2 = new File(projectDir, 'level1Sibling2.gradle')
+        def level2 = new File(projectDir, 'level2.gradle')
         rootFile.text = """
             apply from: 'level1Sibling1.gradle'
             apply from: 'level1Sibling2.gradle'
@@ -23,11 +20,40 @@ class SourceCollectorTest extends Specification {
         level1Sibling1.text = """
             apply from: 'level2.gradle'
         """
+        level1Sibling2.text = " "
+        level2.text = " "
 
         when:
-        def files = SourceCollector.getAllFiles(rootFile, temporaryFolder.root)
+        def files = SourceCollector.getAllFiles(rootFile, project)
 
         then:
         files.containsAll([rootFile, level1Sibling1, level1Sibling2, level2])
     }
+
+    def 'all build files are collected when absolute paths with project variables are used'() {
+        given:
+        def projectDir = project.projectDir
+        def rootFile = new File(projectDir, 'build.gradle')
+        def level1Sibling1 = new File(projectDir, 'level1Sibling1.gradle')
+        def level1Sibling2 = new File(projectDir, 'level1Sibling2.gradle')
+        def level1Sibling3 = new File(projectDir, 'level1Sibling2.gradle')
+        def level2 = new File(projectDir, 'level2.gradle')
+        rootFile.text = """
+            apply from: "\${project.projectDir}/level1Sibling1.gradle"
+            apply from: "\${projectDir}/level1Sibling2.gradle"
+            apply from: "\${rootDir}/level1Sibling3.gradle"
+        """
+        level1Sibling1.text = """
+            apply from: "\${project.rootDir}/level2.gradle"
+        """
+        level1Sibling2.text = " "
+        level1Sibling3.text = " "
+        level2.text = " "
+
+        when:
+        def files = SourceCollector.getAllFiles(rootFile, project)
+
+        then:
+        files.containsAll([rootFile, level1Sibling1, level1Sibling2, level1Sibling3, level2])
+    }
 }

src/test/groovy/com/netflix/nebula/lint/rule/dependency/UnusedDependencyRuleSpec.groovy

@@ -300,14 +300,14 @@ class UnusedDependencyRuleSpec extends IntegrationTestKitSpec {
             buildscript {
                 repositories { maven { url "https://plugins.gradle.org/m2/" } }
                 dependencies {
-                    classpath "com.netflix.nebula:nebula-project-plugin:7.0.7"
+                    classpath "com.netflix.nebula:nebula-project-plugin:10.1.4"
                 }
             }
             plugins {
                 id 'nebula.lint'
                 id 'java'
             }
-            apply plugin: "nebula.integtest"
+            apply plugin: "com.netflix.nebula.integtest"
             gradleLint.rules = ['unused-dependency']
             repositories { mavenCentral() }
             dependencies {
@@ -339,14 +339,14 @@ class UnusedDependencyRuleSpec extends IntegrationTestKitSpec {
             buildscript {
                 repositories { maven { url "https://plugins.gradle.org/m2/" } }
                 dependencies {
-                    classpath "com.netflix.nebula:nebula-project-plugin:7.0.7"
+                    classpath "com.netflix.nebula:nebula-project-plugin:10.1.4"
                 }
             }
             plugins {
                 id 'nebula.lint'
                 id 'java'
             }
-            apply plugin: "nebula.integtest"
+            apply plugin: "com.netflix.nebula.integtest"
             gradleLint.rules = ['unused-dependency']
             repositories { mavenCentral() }
             dependencies {
@@ -368,9 +368,8 @@ class UnusedDependencyRuleSpec extends IntegrationTestKitSpec {
         def result = runTasks('compileTestJava', 'fixGradleLint')
 
         then:
-        result.output.contains('fixed          unused-dependency                  this dependency should be moved to configuration testImplementation')
-        !result.output.contains('unfixed        unused-dependency')
-        !result.output.contains('this dependency is unused and can be removed')
+        result.output.contains('unused-dependency                  this dependency should be moved to configuration testImplementation')
+        result.output.contains('unused-dependency                  this dependency should be moved to configuration integTestImplementation')
     }
 
     def 'webjars should be moved to runtime'() {
@@ -547,7 +546,7 @@ class UnusedDependencyRuleSpec extends IntegrationTestKitSpec {
             plugins {
                 id 'nebula.lint'
                 id 'java'
-                id 'nebula.integtest' version '7.0.7'
+                id 'com.netflix.nebula.integtest' version '10.1.4'
             }
 
             repositories { mavenCentral() }