Android Lint Support (#228)

- Adds support for generating `lint_*` `genrule`s for `android_library` targets - Hooks are in place to let lint rules be generated for any jvm library targets. Theoretically lint can be run on java, kotlin or scala etc with some small tweaks. Lint can also be run on test sources with a few small tweaks as well. These changes will be part of a subsequent PR - Supports custom lint jars embedded in aars - Supports custom lint project/external dependencies. For example, a java library that defines custom lint rules can be added to another library as a linter by making it a dependency of the `buckLint` configuration - Fully incremental, parallel and buck cache/network cache friendly - Supports many android dsl options specified in `lintOptions` - Ability to pin lint api version and not be surprised when android gradle plugin pulls in new checks on updates - Requires an updated `buckw` by running the `buckWrapper` to watch for changes to `lint.xml` files and run `okbuck` again as needed - Lint on all android rules can be run like `buck query "filter('lint_*', kind('genrule', '//...'))" | xargs buck build`
uber · Nov 6, 2016 · c0a3df1 · c0a3df1
1 parent 2b0435b
commit c0a3df1
Show file tree

Hide file tree

Showing 37 changed files with 567 additions and 45 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -26,6 +26,7 @@ script:
   - mv buildSrc notBuildSrc
   - buck query "kind('android_binary', '//...')" | xargs buck build
   - buck test --include unit
+  - buck query "filter('lint_*', kind('genrule', '//...'))" | xargs buck build
   - android-wait-for-emulator
   - adb shell input keyevent 82
   - buck test //app:instrumentation_demoDebug_test

diff --git a/another-app/build.gradle b/another-app/build.gradle
@@ -24,6 +24,10 @@ android {
 
         test.setRoot('test')
     }
+
+    lintOptions {
+        disable 'HardcodedDebugMode'
+    }
 }
 
 dependencies {

diff --git a/buckw b/buckw
@@ -4,7 +4,7 @@
 ##
 ##  Buck wrapper script to invoke okbuck when needed, before running buck
 ##
-##  Created by OkBuck Gradle Plugin on : Mon Oct 10 12:58:03 PDT 2016
+##  Created by OkBuck Gradle Plugin on : Sun Nov 06 00:35:21 PDT 2016
 ##
 #########################################################################
 
@@ -93,7 +93,8 @@ getChanges ( ) {
         ["anyof",
             ["imatch", "**/*.gradle", "wholename"],
             ["imatch", "**/src/**/AndroidManifest.xml", "wholename"],
-            ["imatch", "**/gradle-wrapper.properties", "wholename"]
+            ["imatch", "**/gradle-wrapper.properties", "wholename"],
+            ["imatch", "**/lint.xml", "wholename"]
         ]
     ],
     "fields": ["name"]
@@ -128,11 +129,12 @@ runOkBuck ( ) {
     if [[ ! -z "$INSTALLED_WATCHMAN" ]]; then
         getToClean | jsonq '"\n".join(obj["files"])' | xargs rm
         info "DELETED OLD BUCK FILES"
-        EXTRA_ARGS="-xokbuckClean"
+        EXTRA_OKBUCK_ARGS="-xokbuckClean $EXTRA_OKBUCK_ARGS"
     fi
 
     rm -f $OKBUCK_SUCCESS
-    ( $WORKING_DIR/gradlew -p $WORKING_DIR okbuck -Dokbuck.wrapper=true $EXTRA_ARGS --stacktrace && updateOkBuckSuccess && success "PROCEEDING WITH BUCK" ) || die "OKBUCK FAILED"
+    ( $WORKING_DIR/gradlew -p $WORKING_DIR okbuck -Dokbuck.wrapper=true $EXTRA_OKBUCK_ARGS --stacktrace &&
+    updateOkBuckSuccess && success "PROCEEDING WITH BUCK" ) || die "OKBUCK FAILED"
 }
 
 watchmanWorkflow ( ) {

diff --git a/build.gradle b/build.gradle
@@ -62,6 +62,10 @@ def addCommonConfigurationForAndroidModules(Project project) {
                 targetCompatibility JavaVersion.VERSION_1_7
             }
         }
+
+        lintOptions {
+            lintConfig project.rootProject.file('config/lint/lint.xml')
+        }
     }
     if (project.plugins.hasPlugin('com.android.application')) {
         project.android {
@@ -139,6 +143,10 @@ if (project.file("buildSrc").exists()) {
         gradleGen {
             options = ["--offline", "-Dorg.gradle.configureondemand=true", "-Dorg.gradle.parallel=true"]
         }
+
+        experimental {
+            lint = true
+        }
     }
 
     afterEvaluate {

diff --git a/buildSrc/src/main/groovy/com/uber/okbuck/OkBuckGradlePlugin.groovy b/buildSrc/src/main/groovy/com/uber/okbuck/OkBuckGradlePlugin.groovy
@@ -3,16 +3,18 @@ package com.uber.okbuck
 import com.uber.okbuck.config.BUCKFile
 import com.uber.okbuck.core.dependency.DependencyCache
 import com.uber.okbuck.core.task.OkBuckCleanTask
+import com.uber.okbuck.core.util.LintUtil
 import com.uber.okbuck.core.util.RobolectricUtil
 import com.uber.okbuck.extension.ExperimentalExtension
 import com.uber.okbuck.extension.GradleGenExtension
 import com.uber.okbuck.extension.IntellijExtension
+import com.uber.okbuck.extension.LintExtension
 import com.uber.okbuck.extension.OkBuckExtension
 import com.uber.okbuck.extension.TestExtension
+import com.uber.okbuck.extension.WrapperExtension
 import com.uber.okbuck.generator.BuckFileGenerator
 import com.uber.okbuck.generator.DotBuckConfigLocalGenerator
 import com.uber.okbuck.wrapper.BuckWrapperTask
-import com.uber.okbuck.extension.WrapperExtension
 import org.apache.commons.io.IOUtils
 import org.gradle.api.Plugin
 import org.gradle.api.Project
@@ -32,8 +34,9 @@ class OkBuckGradlePlugin implements Plugin<Project> {
     static final String BUCK_WRAPPER = "buckWrapper"
     static final String DEFAULT_CACHE_PATH = ".okbuck/cache"
     static final String CONFIGURATION_POST_PROCESS = "postProcess"
-
     static final String GROUP = "okbuck"
+    static final String BUCK_LINT = "buckLint"
+    static final String LINT = "lint"
 
     static DependencyCache depCache
     static Logger LOGGER
@@ -45,6 +48,7 @@ class OkBuckGradlePlugin implements Plugin<Project> {
         ExperimentalExtension experimental = okbuck.extensions.create(EXPERIMENTAL, ExperimentalExtension)
         TestExtension test = okbuck.extensions.create(TEST, TestExtension)
         IntellijExtension intellij = okbuck.extensions.create(INTELLIJ, IntellijExtension)
+        LintExtension lint = okbuck.extensions.create(LINT, LintExtension, project)
         okbuck.extensions.create(GRADLE_GEN, GradleGenExtension, project)
 
         Task okBuck = project.task(OKBUCK)
@@ -68,7 +72,8 @@ class OkBuckGradlePlugin implements Plugin<Project> {
                 generate(project)
             }
 
-            depCache = new DependencyCache(project, DEFAULT_CACHE_PATH, true, true, intellij.sources)
+            depCache = new DependencyCache(project, DEFAULT_CACHE_PATH, true, DependencyCache.THIRD_PARTY_BUCK_FILE,
+                    intellij.sources, experimental.lint)
 
             if (test.robolectric) {
                 Task fetchRobolectricRuntimeDeps = project.task('fetchRobolectricRuntimeDeps')
@@ -90,6 +95,19 @@ class OkBuckGradlePlugin implements Plugin<Project> {
             })
             buckWrapper.setGroup(GROUP)
             buckWrapper.setDescription("Create buck wrapper")
+
+            if (experimental.lint) {
+                okbuck.buckProjects.each { Project buckProject ->
+                    buckProject.configurations.maybeCreate(BUCK_LINT)
+                }
+
+                Task fetchLintDeps = project.task('fetchLintDeps')
+                okBuck.dependsOn(fetchLintDeps)
+                fetchLintDeps.mustRunAfter(okBuckClean)
+                fetchLintDeps << {
+                    LintUtil.fetchLintDeps(project, lint.version)
+                }
+            }
         }
     }
 

diff --git a/buildSrc/src/main/groovy/com/uber/okbuck/composer/AndroidLibraryRuleComposer.groovy b/buildSrc/src/main/groovy/com/uber/okbuck/composer/AndroidLibraryRuleComposer.groovy
@@ -1,6 +1,5 @@
 package com.uber.okbuck.composer
 
-import com.uber.okbuck.constant.BuckConstants
 import com.uber.okbuck.core.model.AndroidLibTarget
 import com.uber.okbuck.core.model.AndroidTarget
 import com.uber.okbuck.core.model.Target

diff --git a/buildSrc/src/main/groovy/com/uber/okbuck/composer/AndroidTestRuleComposer.groovy b/buildSrc/src/main/groovy/com/uber/okbuck/composer/AndroidTestRuleComposer.groovy
@@ -1,6 +1,5 @@
 package com.uber.okbuck.composer
 
-import com.uber.okbuck.constant.BuckConstants
 import com.uber.okbuck.core.model.AndroidLibTarget
 import com.uber.okbuck.core.model.AndroidTarget
 import com.uber.okbuck.core.model.Target

diff --git a/...dSrc/src/main/groovy/com/uber/okbuck/composer/ExopackageAndroidLibraryRuleComposer.groovy b/...dSrc/src/main/groovy/com/uber/okbuck/composer/ExopackageAndroidLibraryRuleComposer.groovy
@@ -1,6 +1,5 @@
 package com.uber.okbuck.composer
 
-import com.uber.okbuck.constant.BuckConstants
 import com.uber.okbuck.core.model.AndroidAppTarget
 import com.uber.okbuck.generator.RetroLambdaGenerator
 import com.uber.okbuck.printable.PostProcessClassessCommands

diff --git a/buildSrc/src/main/groovy/com/uber/okbuck/composer/JavaBuckRuleComposer.groovy b/buildSrc/src/main/groovy/com/uber/okbuck/composer/JavaBuckRuleComposer.groovy
@@ -19,4 +19,8 @@ abstract class JavaBuckRuleComposer extends BuckRuleComposer {
     static String aptJar(JavaTarget target) {
         return "apt_jar_${target.name}"
     }
+
+    static String lint(JavaTarget target) {
+        return "lint_${target.name}"
+    }
 }
diff --git a/buildSrc/src/main/groovy/com/uber/okbuck/composer/JavaLibraryRuleComposer.groovy b/buildSrc/src/main/groovy/com/uber/okbuck/composer/JavaLibraryRuleComposer.groovy
@@ -1,6 +1,5 @@
 package com.uber.okbuck.composer
 
-import com.uber.okbuck.constant.BuckConstants
 import com.uber.okbuck.core.model.JavaLibTarget
 import com.uber.okbuck.generator.RetroLambdaGenerator
 import com.uber.okbuck.printable.PostProcessClassessCommands

diff --git a/buildSrc/src/main/groovy/com/uber/okbuck/composer/JavaTestRuleComposer.groovy b/buildSrc/src/main/groovy/com/uber/okbuck/composer/JavaTestRuleComposer.groovy
@@ -1,6 +1,5 @@
 package com.uber.okbuck.composer
 
-import com.uber.okbuck.constant.BuckConstants
 import com.uber.okbuck.core.model.JavaLibTarget
 import com.uber.okbuck.generator.RetroLambdaGenerator
 import com.uber.okbuck.printable.PostProcessClassessCommands

diff --git a/buildSrc/src/main/groovy/com/uber/okbuck/composer/LintRuleComposer.groovy b/buildSrc/src/main/groovy/com/uber/okbuck/composer/LintRuleComposer.groovy
@@ -0,0 +1,45 @@
+package com.uber.okbuck.composer
+
+import com.uber.okbuck.core.model.JavaTarget
+import com.uber.okbuck.core.model.Target
+import com.uber.okbuck.core.util.LintUtil
+import com.uber.okbuck.extension.LintExtension
+import com.uber.okbuck.generator.LintWrapperGenerator
+import com.uber.okbuck.rule.LintRule
+
+final class LintRuleComposer extends JavaBuckRuleComposer {
+
+    private LintRuleComposer() {
+        // no instance
+    }
+
+    static LintRule compose(JavaTarget target) {
+        Set<String> inputs = []
+        if (target.lintOptions.lintConfig != null && target.lintOptions.lintConfig.exists()) {
+            inputs.add(LintUtil.getLintwConfigRule(target.project, target.lintOptions.lintConfig))
+        }
+
+        LintExtension lintExtension = target.rootProject.okbuck.lint
+        LintWrapperGenerator.generate(target, lintExtension.jvmArgs)
+
+        Set<String> customLintTargets = [] as Set
+        Set<Target> lintJarTargets = target.lint.targetDeps.findAll {
+            (it instanceof JavaTarget) && (it.hasLintRegistry())
+        }
+
+        customLintTargets.addAll(targets(lintJarTargets))
+        customLintTargets.addAll(external(target.main.packagedLintJars))
+
+        Set<String> lintDeps = [] as Set
+        lintDeps.addAll(LintUtil.LINT_DEPS_RULE)
+        lintDeps.addAll(targets(lintJarTargets))
+
+        return new LintRule(
+                lint(target),
+                inputs,
+                LintUtil.getLintwRule(target),
+                customLintTargets,
+                lintDeps,
+                target.main.sources.empty ? null : ":${src(target)}")
+    }
+}
diff --git a/buildSrc/src/main/groovy/com/uber/okbuck/core/dependency/DependencyCache.groovy b/buildSrc/src/main/groovy/com/uber/okbuck/core/dependency/DependencyCache.groovy
@@ -4,34 +4,44 @@ import com.uber.okbuck.core.util.FileUtil
 import org.apache.commons.io.FileUtils
 import org.gradle.api.Project
 
+import java.nio.file.FileSystem
+import java.nio.file.FileSystems
+import java.nio.file.Files
+import java.nio.file.Path
+
 class DependencyCache {
 
+    static final String THIRD_PARTY_BUCK_FILE = "thirdparty/BUCK_FILE"
     final Project rootProject
     final File cacheDir
     final boolean useFullDepName
     final boolean fetchSources
+    final boolean extractLintJars
 
     private Map<VersionlessDependency, String> finalDepFiles = [:]
+    private Map<VersionlessDependency, String> lintJars = [:]
     private Map<VersionlessDependency, ExternalDependency> greatestVersions = [:]
 
     DependencyCache(Project rootProject,
                     String cacheDirPath,
-                    useFullDepName = false,
-                    createBuckFile = true,
-                    fetchSources = false) {
+                    boolean useFullDepName = false,
+                    String buckFile = THIRD_PARTY_BUCK_FILE,
+                    boolean fetchSources = false,
+                    boolean extractLintJars = false) {
         this.rootProject = rootProject
         this.useFullDepName = useFullDepName
         this.fetchSources = fetchSources
+        this.extractLintJars = extractLintJars
         cacheDir = new File(rootProject.projectDir, cacheDirPath)
         cacheDir.mkdirs()
-        if (createBuckFile) {
-            FileUtil.copyResourceToProject("thirdparty/BUCK_FILE", new File(cacheDir, "BUCK"))
+        if (buckFile) {
+            FileUtil.copyResourceToProject(buckFile, new File(cacheDir, "BUCK"))
         }
     }
 
     void put(ExternalDependency dependency) {
         if (!isValid(dependency.depFile)) {
-            throw new IllegalArgumentException("${dependency.depFile.absolutePath} is not a valid jar/aar file")
+            throw new InValidDependencyException("${dependency.depFile.absolutePath} is not a valid dependency")
         }
 
         ExternalDependency externalDependency = greatestVersions.get(dependency)
@@ -46,7 +56,14 @@ class DependencyCache {
             File depFile = greatestVersion.depFile
             File cachedCopy = new File(cacheDir, useFullDepName ? dependency.cacheName : dependency.depFile.name)
             if (!cachedCopy.exists()) {
-                FileUtils.copyFile(depFile, cachedCopy)
+                Files.copy(depFile.toPath(), cachedCopy.toPath())
+            }
+            if (extractLintJars && cachedCopy.name.endsWith(".aar")) {
+                File lintJar = getPackagedLintJar(cachedCopy)
+                if (lintJar != null) {
+                    String lintJarPath = FileUtil.getRelativePath(rootProject.projectDir, lintJar)
+                    lintJars.put(greatestVersion, lintJarPath)
+                }
             }
             if (fetchSources) {
                 fetchSourcesFor(dependency)
@@ -58,6 +75,10 @@ class DependencyCache {
         return finalDepFiles.get(greatestVersion)
     }
 
+    String getLintJar(ExternalDependency dependency) {
+        return lintJars.get(dependency)
+    }
+
     private void fetchSourcesFor(ExternalDependency dependency) {
         File depFile = dependency.depFile
         String sourcesJarName = depFile.name.replaceFirst(/\.(jar|aar)$/, '-sources.jar')
@@ -79,7 +100,22 @@ class DependencyCache {
         }
     }
 
-    private static boolean isValid(File dep) {
+    boolean isValid(File dep) {
         return (dep.name.endsWith(".jar") || dep.name.endsWith(".aar"))
     }
+
+    static File getPackagedLintJar(File aar) {
+        File lintJar = new File(aar.parentFile, aar.name.replaceFirst(/\.aar$/, '-lint.jar'))
+        if (lintJar.exists()) {
+            return lintJar
+        }
+        FileSystem zipFile = FileSystems.newFileSystem(aar.toPath(), null)
+        Path packagedLintJar = zipFile.getPath("lint.jar")
+        if (Files.exists(packagedLintJar)) {
+            Files.copy(packagedLintJar, lintJar.toPath())
+            return lintJar
+        } else {
+            return null
+        }
+    }
 }
diff --git a/buildSrc/src/main/groovy/com/uber/okbuck/core/dependency/InValidDependencyException.groovy b/buildSrc/src/main/groovy/com/uber/okbuck/core/dependency/InValidDependencyException.groovy
@@ -0,0 +1,8 @@
+package com.uber.okbuck.core.dependency
+
+class InValidDependencyException extends IllegalStateException {
+
+    InValidDependencyException(String msg) {
+        super(msg)
+    }
+}
diff --git a/buildSrc/src/main/groovy/com/uber/okbuck/core/model/AndroidTarget.groovy b/buildSrc/src/main/groovy/com/uber/okbuck/core/model/AndroidTarget.groovy
@@ -6,6 +6,7 @@ import com.android.build.gradle.api.UnitTestVariant
 import com.android.build.gradle.internal.api.TestedVariant
 import com.android.build.gradle.internal.variant.BaseVariantData
 import com.android.builder.model.ClassField
+import com.android.builder.model.LintOptions
 import com.android.builder.model.SourceProvider
 import com.android.manifmerger.ManifestMerger2
 import com.android.manifmerger.MergingReport
@@ -126,6 +127,11 @@ abstract class AndroidTarget extends JavaLibTarget {
         return tasks
     }
 
+    @Override
+    LintOptions getLintOptions() {
+        return project.android.lintOptions
+    }
+
     Set<String> getSrcDirNames() {
         return baseVariant.sourceSets.collect { SourceProvider provider ->
             provider.name