From d562d566a8eab632cdfb5ee67ed7dd62b3bbc20c Mon Sep 17 00:00:00 2001 From: Simon Marquis Date: Sat, 27 May 2023 14:44:11 +0200 Subject: [PATCH 1/7] Android Lint improvements - Create `LintConventionPlugin` to configure Lint on all compatible modules: Android applications, Android libraries and JVM modules. - Run `lintProdRelease` in CI instead of the default `lintDemoDebug` which is less important compared to production code. - Rearrange CI steps to make it more clear that Lint should is an additional step after build (build -> test -> lint). - Enable SARIF support and upload results to GitHub's CodeQL to get inline feedback on PRs. If we really need better perfs, we could restore `lint.checkDependencies = true` on the `:app` module and only execute `:app:lintProdRelease`. But in practice, this does not change the total build time on this project. --- .github/workflows/Build.yaml | 26 +++++++---- build-logic/convention/build.gradle.kts | 4 ++ .../AndroidApplicationConventionPlugin.kt | 1 + .../kotlin/AndroidLibraryConventionPlugin.kt | 1 + .../kotlin/AndroidLintConventionPlugin.kt | 46 +++++++++++++++++++ .../main/kotlin/JvmLibraryConventionPlugin.kt | 1 + core/designsystem/build.gradle.kts | 3 -- 7 files changed, 69 insertions(+), 13 deletions(-) create mode 100644 build-logic/convention/src/main/kotlin/AndroidLintConventionPlugin.kt diff --git a/.github/workflows/Build.yaml b/.github/workflows/Build.yaml index a3b328dcb..a297a3ca3 100644 --- a/.github/workflows/Build.yaml +++ b/.github/workflows/Build.yaml @@ -36,21 +36,28 @@ jobs: - name: Check spotless run: ./gradlew spotlessCheck --init-script gradle/init.gradle.kts --no-configuration-cache - - name: Check lint - run: ./gradlew lintDemoDebug - - name: Build all build type and flavor permutations run: ./gradlew assemble - - name: Run local tests - run: ./gradlew testDemoDebug testProdDebug - - name: Upload build outputs (APKs) uses: actions/upload-artifact@v3 with: name: APKs path: '**/build/outputs/apk/**/*.apk' + - name: Run local tests + run: ./gradlew testDemoDebug testProdDebug + + - name: Upload test results (XML) + if: always() + uses: actions/upload-artifact@v3 + with: + name: test-results + path: '**/build/test-results/test*UnitTest/**.xml' + + - name: Check lint + run: ./gradlew lintProdRelease + - name: Upload lint reports (HTML) if: always() uses: actions/upload-artifact@v3 @@ -58,12 +65,11 @@ jobs: name: lint-reports path: '**/build/reports/lint-results-*.html' - - name: Upload test results (XML) + - name: Upload lint reports (SARIF) if: always() - uses: actions/upload-artifact@v3 + uses: github/codeql-action/upload-sarif@v2 with: - name: test-results - path: '**/build/test-results/test*UnitTest/**.xml' + sarif_file: '**/build/reports/lint-results-*.sarif' androidTest: needs: build diff --git a/build-logic/convention/build.gradle.kts b/build-logic/convention/build.gradle.kts index c8e25e17c..9230dd6b7 100644 --- a/build-logic/convention/build.gradle.kts +++ b/build-logic/convention/build.gradle.kts @@ -92,6 +92,10 @@ gradlePlugin { id = "nowinandroid.android.application.flavors" implementationClass = "AndroidApplicationFlavorsConventionPlugin" } + register("androidLint") { + id = "nowinandroid.android.lint" + implementationClass = "AndroidLintConventionPlugin" + } register("jvmLibrary") { id = "nowinandroid.jvm.library" implementationClass = "JvmLibraryConventionPlugin" diff --git a/build-logic/convention/src/main/kotlin/AndroidApplicationConventionPlugin.kt b/build-logic/convention/src/main/kotlin/AndroidApplicationConventionPlugin.kt index 26b6951d3..a8bed4bec 100644 --- a/build-logic/convention/src/main/kotlin/AndroidApplicationConventionPlugin.kt +++ b/build-logic/convention/src/main/kotlin/AndroidApplicationConventionPlugin.kt @@ -29,6 +29,7 @@ class AndroidApplicationConventionPlugin : Plugin { with(pluginManager) { apply("com.android.application") apply("org.jetbrains.kotlin.android") + apply("nowinandroid.android.lint") } extensions.configure { diff --git a/build-logic/convention/src/main/kotlin/AndroidLibraryConventionPlugin.kt b/build-logic/convention/src/main/kotlin/AndroidLibraryConventionPlugin.kt index 275a26620..83ce09324 100644 --- a/build-logic/convention/src/main/kotlin/AndroidLibraryConventionPlugin.kt +++ b/build-logic/convention/src/main/kotlin/AndroidLibraryConventionPlugin.kt @@ -35,6 +35,7 @@ class AndroidLibraryConventionPlugin : Plugin { with(pluginManager) { apply("com.android.library") apply("org.jetbrains.kotlin.android") + apply("nowinandroid.android.lint") } extensions.configure { diff --git a/build-logic/convention/src/main/kotlin/AndroidLintConventionPlugin.kt b/build-logic/convention/src/main/kotlin/AndroidLintConventionPlugin.kt new file mode 100644 index 000000000..23d8e234c --- /dev/null +++ b/build-logic/convention/src/main/kotlin/AndroidLintConventionPlugin.kt @@ -0,0 +1,46 @@ +/* + * Copyright 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import com.android.build.api.dsl.ApplicationExtension +import com.android.build.api.dsl.Lint +import com.android.build.gradle.LibraryExtension +import org.gradle.api.Plugin +import org.gradle.api.Project +import org.gradle.kotlin.dsl.configure + +class AndroidLintConventionPlugin : Plugin { + override fun apply(target: Project) { + with(target) { + when { + pluginManager.hasPlugin("com.android.application") -> + configure { lint(Lint::configure) } + + pluginManager.hasPlugin("com.android.library") -> + configure { lint(Lint::configure) } + + else -> { + pluginManager.apply("com.android.lint") + configure(Lint::configure) + } + } + } + } +} + +private fun Lint.configure() { + xmlReport = true + sarifReport = true +} diff --git a/build-logic/convention/src/main/kotlin/JvmLibraryConventionPlugin.kt b/build-logic/convention/src/main/kotlin/JvmLibraryConventionPlugin.kt index 4067e289b..35932c835 100644 --- a/build-logic/convention/src/main/kotlin/JvmLibraryConventionPlugin.kt +++ b/build-logic/convention/src/main/kotlin/JvmLibraryConventionPlugin.kt @@ -23,6 +23,7 @@ class JvmLibraryConventionPlugin : Plugin { with(target) { with(pluginManager) { apply("org.jetbrains.kotlin.jvm") + apply("nowinandroid.android.lint") } configureKotlinJvm() } diff --git a/core/designsystem/build.gradle.kts b/core/designsystem/build.gradle.kts index a40926383..cf9873e2c 100644 --- a/core/designsystem/build.gradle.kts +++ b/core/designsystem/build.gradle.kts @@ -23,9 +23,6 @@ android { defaultConfig { testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner" } - lint { - checkDependencies = true - } namespace = "com.google.samples.apps.nowinandroid.core.designsystem" } From c754e78387911ad1bce41f134cd8aea3b80ea50c Mon Sep 17 00:00:00 2001 From: Simon Marquis Date: Sat, 27 May 2023 19:03:19 +0200 Subject: [PATCH 2/7] Call `github/codeql-action/upload-sarif` with the root folder as argument It does not support path globbing. --- .github/workflows/Build.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/Build.yaml b/.github/workflows/Build.yaml index a297a3ca3..7caa7359f 100644 --- a/.github/workflows/Build.yaml +++ b/.github/workflows/Build.yaml @@ -69,7 +69,7 @@ jobs: if: always() uses: github/codeql-action/upload-sarif@v2 with: - sarif_file: '**/build/reports/lint-results-*.sarif' + sarif_file: './' androidTest: needs: build From a8e22badf79cfe7adb8f9db4f094fd4eac539250 Mon Sep 17 00:00:00 2001 From: Simon Marquis Date: Sat, 27 May 2023 19:56:15 +0200 Subject: [PATCH 3/7] Add lint plugin to the `:lint` module --- lint/build.gradle.kts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lint/build.gradle.kts b/lint/build.gradle.kts index 4ae719aa6..35b6ec1e8 100644 --- a/lint/build.gradle.kts +++ b/lint/build.gradle.kts @@ -19,7 +19,7 @@ import org.jetbrains.kotlin.gradle.tasks.KotlinCompile plugins { `java-library` kotlin("jvm") - id("com.android.lint") + id("nowinandroid.android.lint") } java { From 7268e36f60a5a3d3c0c29d64f692e37a2177c4d3 Mon Sep 17 00:00:00 2001 From: Simon Marquis Date: Sat, 27 May 2023 19:56:45 +0200 Subject: [PATCH 4/7] Enable `checkDependencies` and run lint tasks on project roots --- .github/workflows/Build.yaml | 2 +- .../convention/src/main/kotlin/AndroidLintConventionPlugin.kt | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/Build.yaml b/.github/workflows/Build.yaml index 7caa7359f..d4485cf6a 100644 --- a/.github/workflows/Build.yaml +++ b/.github/workflows/Build.yaml @@ -56,7 +56,7 @@ jobs: path: '**/build/test-results/test*UnitTest/**.xml' - name: Check lint - run: ./gradlew lintProdRelease + run: ./gradlew :app:lintProdRelease :app-nia-catalog:lintRelease :lint:lint - name: Upload lint reports (HTML) if: always() diff --git a/build-logic/convention/src/main/kotlin/AndroidLintConventionPlugin.kt b/build-logic/convention/src/main/kotlin/AndroidLintConventionPlugin.kt index 23d8e234c..68ca58b2a 100644 --- a/build-logic/convention/src/main/kotlin/AndroidLintConventionPlugin.kt +++ b/build-logic/convention/src/main/kotlin/AndroidLintConventionPlugin.kt @@ -15,8 +15,8 @@ */ import com.android.build.api.dsl.ApplicationExtension +import com.android.build.api.dsl.LibraryExtension import com.android.build.api.dsl.Lint -import com.android.build.gradle.LibraryExtension import org.gradle.api.Plugin import org.gradle.api.Project import org.gradle.kotlin.dsl.configure @@ -43,4 +43,5 @@ class AndroidLintConventionPlugin : Plugin { private fun Lint.configure() { xmlReport = true sarifReport = true + checkDependencies = true } From 5c2dc8f4d1b097c80196cbfcdc020a46028db9da Mon Sep 17 00:00:00 2001 From: Simon Marquis Date: Sat, 27 May 2023 20:20:23 +0200 Subject: [PATCH 5/7] Testing... To be removed! --- app/src/main/AndroidManifest.xml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index 0b0482c13..1dadf7413 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -19,6 +19,12 @@ + + + + + +