Add checks to ensure atomic library groups are read in properly
The checks ensure that the:
1) filename property is properly set in lint.xml
2) file exists
3) file should not be empty
This change also:
- Updates the atomic library group text file location from scoped (generated once per library and stored under each library's subdirectory) to unscoped (single location not nested under a library)
- Changes the export task to only run in the root plugin, rather than per lint project
- Sets the system OUT_DIR environment variable if it isn't set.
Bug: 162069062
Bug: 204371669
Test: BanInappropriateExperimentalUsageTest
Test: bOS
Change-Id: I7c6c00eb4492bc63714c341a1944c58da36b7e5a
diff --git a/buildSrc/lint.xml b/buildSrc/lint.xml
index 5ec6632..08fefd5 100644
--- a/buildSrc/lint.xml
+++ b/buildSrc/lint.xml
@@ -32,6 +32,19 @@
<!-- Required for AppSearch icing tests. -->
<ignore path="**/java/tests/**" />
</issue>
+
+ <issue id="IllegalExperimentalApiUsage">
+
+ <!--
+ The placeholder string "USE_SYSTEM_OUT_DIR" must match the string
+ ATOMIC_LIBRARY_GROUP_FILE_PLACEHOLDER defined in BanInappropriateExperimentalUsage.
+ The suffix must match `fileLocation` defined in AndroidXRootImplPlugin.
+ -->
+ <option
+ name="atomicLibraryGroupFilename"
+ value="USE_SYSTEM_OUT_DIR/lint/atomic-library-groups.txt"/>
+ </issue>
+
<!-- Re-enable high-priority checks for tests (and everything else). -->
<issue id="NewApi" severity="fatal" />
<issue id="WrongThread" severity="fatal" />
diff --git a/buildSrc/out.gradle b/buildSrc/out.gradle
index be42e80..10c8ad8 100644
--- a/buildSrc/out.gradle
+++ b/buildSrc/out.gradle
@@ -29,6 +29,7 @@
}
outDir = new File("${checkoutRoot}/out${subdir}")
project.ext.buildSrcOut = new File("${checkoutRoot}/out/buildSrc")
+ environment "OUT_DIR", "${checkoutRoot}/out${subdir}"
} else {
outDir = new File(outDir)
project.ext.buildSrcOut = new File("${outDir}/buildSrc")
diff --git a/buildSrc/private/src/main/kotlin/androidx/build/AndroidXImplPlugin.kt b/buildSrc/private/src/main/kotlin/androidx/build/AndroidXImplPlugin.kt
index 291728eb..33f88d0 100644
--- a/buildSrc/private/src/main/kotlin/androidx/build/AndroidXImplPlugin.kt
+++ b/buildSrc/private/src/main/kotlin/androidx/build/AndroidXImplPlugin.kt
@@ -113,7 +113,6 @@
project.configureTaskTimeouts()
project.configureMavenArtifactUpload(extension)
- project.configureExportAtomicLibraryGroupsToText()
project.configureExternalDependencyLicenseCheck()
project.configureProjectStructureValidation(extension)
project.configureProjectVersionValidation(extension)
@@ -393,17 +392,6 @@
project.addToProjectMap(extension)
}
- private fun Project.configureExportAtomicLibraryGroupsToText() {
- project.tasks.register(
- "exportAtomicLibraryGroupsToText",
- ExportAtomicLibraryGroupsToTextTask::class.java
- ) { task ->
- task.libraryGroupFile = project.file("${project.getSupportRootFolder()}" +
- "/buildSrc/public/src/main/kotlin/androidx/build/LibraryGroups.kt")
- task.textOutputFile = project.file("${project.buildDir}/lint/atomic-library-groups.txt")
- }
- }
-
private fun Project.configureProjectStructureValidation(
extension: AndroidXExtension
) {
diff --git a/buildSrc/private/src/main/kotlin/androidx/build/AndroidXRootImplPlugin.kt b/buildSrc/private/src/main/kotlin/androidx/build/AndroidXRootImplPlugin.kt
index 07922a9..00a1892 100644
--- a/buildSrc/private/src/main/kotlin/androidx/build/AndroidXRootImplPlugin.kt
+++ b/buildSrc/private/src/main/kotlin/androidx/build/AndroidXRootImplPlugin.kt
@@ -92,6 +92,20 @@
tasks.register(AndroidXImplPlugin.CREATE_LIBRARY_BUILD_INFO_FILES_TASK)
)
+ buildOnServerTask.dependsOn(
+ tasks.register(
+ "exportAtomicLibraryGroupsToTextRoot",
+ ExportAtomicLibraryGroupsToTextTask::class.java
+ ) { task ->
+ task.libraryGroupFile = project.file("${project.getSupportRootFolder()}" +
+ "/buildSrc/public/src/main/kotlin/androidx/build/LibraryGroups.kt")
+
+ // This suffix must match the suffix in buildSrc/lint.xml
+ val fileLocation = "${getRootOutDirectory()}/lint/atomic-library-groups.txt"
+ task.textOutputFile = project.file(fileLocation)
+ }
+ )
+
VerifyPlaygroundGradleConfigurationTask.createIfNecessary(project)?.let {
buildOnServerTask.dependsOn(it)
}
diff --git a/buildSrc/private/src/main/kotlin/androidx/build/LintConfiguration.kt b/buildSrc/private/src/main/kotlin/androidx/build/LintConfiguration.kt
index 6345639..53d7c42 100644
--- a/buildSrc/private/src/main/kotlin/androidx/build/LintConfiguration.kt
+++ b/buildSrc/private/src/main/kotlin/androidx/build/LintConfiguration.kt
@@ -72,7 +72,6 @@
// Create fake variant tasks since that is what is invoked by developers.
val lintTask = tasks.named("lint")
lintTask.configure { task ->
- task.dependsOn(tasks.named("exportAtomicLibraryGroupsToText"))
AffectedModuleDetector.configureTaskGuard(task)
}
afterEvaluate {
@@ -123,7 +122,6 @@
}}"
).configure { task ->
AffectedModuleDetector.configureTaskGuard(task)
- task.dependsOn(tasks.named("exportAtomicLibraryGroupsToText"))
}
tasks.named(
"lintAnalyze${variant.name.replaceFirstChar {
@@ -131,7 +129,6 @@
}}"
).configure { task ->
AffectedModuleDetector.configureTaskGuard(task)
- task.dependsOn(tasks.named("exportAtomicLibraryGroupsToText"))
}
/* TODO: uncomment when we upgrade to AGP 7.1.0-alpha04
tasks.named("lintReport${variant.name.capitalize(Locale.US)}").configure { task ->
diff --git a/lint-checks/src/main/java/androidx/build/lint/BanInappropriateExperimentalUsage.kt b/lint-checks/src/main/java/androidx/build/lint/BanInappropriateExperimentalUsage.kt
index c5ac47b..37e2930 100644
--- a/lint-checks/src/main/java/androidx/build/lint/BanInappropriateExperimentalUsage.kt
+++ b/lint-checks/src/main/java/androidx/build/lint/BanInappropriateExperimentalUsage.kt
@@ -18,6 +18,7 @@
package androidx.build.lint
+import com.android.tools.lint.client.api.Configuration
import com.android.tools.lint.client.api.UElementHandler
import com.android.tools.lint.detector.api.Category
import com.android.tools.lint.detector.api.Detector
@@ -27,6 +28,8 @@
import com.android.tools.lint.detector.api.JavaContext
import com.android.tools.lint.detector.api.Scope
import com.android.tools.lint.detector.api.Severity
+import java.io.File
+import java.io.FileNotFoundException
import org.jetbrains.uast.UAnnotated
import org.jetbrains.uast.UAnnotation
import org.jetbrains.uast.UClass
@@ -45,6 +48,10 @@
}
private inner class AnnotationChecker(val context: JavaContext) : UElementHandler() {
+ val atomicGroupList: List<String> by lazy {
+ loadAtomicLibraryGroupData(context.configuration, ISSUE)
+ }
+
override fun visitAnnotation(node: UAnnotation) {
if (DEBUG) {
if (APPLICABLE_ANNOTATIONS.contains(node.qualifiedName) && node.sourcePsi != null) {
@@ -68,17 +75,55 @@
"${context.project}"
)
}
- verifyUsageOfElementIsWithinSameGroup(context, node, annotation, ISSUE)
+
+ verifyUsageOfElementIsWithinSameGroup(
+ context,
+ node,
+ annotation,
+ ISSUE,
+ atomicGroupList
+ )
}
}
}
+
+ private fun loadAtomicLibraryGroupData(
+ configuration: Configuration,
+ issue: Issue
+ ): List<String> {
+ val filename = configuration.getOption(issue, ATOMIC_LIBGROUP_FILE_PROPERTY, null)
+ ?: throw RuntimeException(
+ "Property $ATOMIC_LIBGROUP_FILE_PROPERTY is not set in lint.xml.")
+
+ val libGroupFile = if (filename.contains(OUT_DIR_PLACEHOLDER)) {
+ val fileLocation = filename.replace(OUT_DIR_PLACEHOLDER, System.getenv("OUT_DIR"))
+ val file = File(fileLocation)
+ if (!file.exists()) {
+ throw FileNotFoundException("Couldn't find atomic library group file $filename")
+ }
+ file
+ } else {
+ configuration.getOptionAsFile(issue, ATOMIC_LIBGROUP_FILE_PROPERTY, null)
+ ?: throw FileNotFoundException(
+ "Couldn't find atomic library group file $filename")
+ }
+
+ val atomicLibraryGroups = libGroupFile.readLines()
+ if (atomicLibraryGroups.isEmpty()) {
+ throw RuntimeException("Atomic library group file should not be empty")
+ }
+
+ return atomicLibraryGroups
+ }
}
+ @Suppress("UNUSED_PARAMETER") // TODO: write logic + tests in future CL that uses groupList
fun verifyUsageOfElementIsWithinSameGroup(
context: JavaContext,
usage: UElement,
annotation: UElement,
issue: Issue,
+ atomicGroupList: List<String>,
) {
val evaluator = context.evaluator
val usageCoordinates = evaluator.getLibrary(usage) ?: context.project.mavenCoordinate
@@ -105,6 +150,17 @@
private const val DEBUG = false
/**
+ * This string must match the value defined in buildSrc/lint.xml
+ *
+ * This is needed as we need to define the location of the atomic library group file for
+ * non-test usage. For tests, we can directly use the value defined in test lint.xml files.
+ */
+ private const val OUT_DIR_PLACEHOLDER = "USE_SYSTEM_OUT_DIR"
+
+ // This must match the setting in buildSrc/lint.xml
+ private const val ATOMIC_LIBGROUP_FILE_PROPERTY = "atomicLibraryGroupFilename"
+
+ /**
* Even though Kotlin's [Experimental] annotation is deprecated in favor of [RequiresOptIn],
* we still want to check for its use in Lint.
*/
diff --git a/lint-checks/src/test/java/androidx/build/lint/BanInappropriateExperimentalUsageTest.kt b/lint-checks/src/test/java/androidx/build/lint/BanInappropriateExperimentalUsageTest.kt
index 3003f41..a0c0e6f 100644
--- a/lint-checks/src/test/java/androidx/build/lint/BanInappropriateExperimentalUsageTest.kt
+++ b/lint-checks/src/test/java/androidx/build/lint/BanInappropriateExperimentalUsageTest.kt
@@ -19,10 +19,13 @@
package androidx.build.lint
import com.android.tools.lint.checks.infrastructure.ProjectDescription
+import com.android.tools.lint.checks.infrastructure.TestFile
+import com.android.tools.lint.checks.infrastructure.TestFiles
import com.android.tools.lint.checks.infrastructure.TestMode
import org.junit.Test
import org.junit.runner.RunWith
import org.junit.runners.JUnit4
+import java.io.FileNotFoundException
@RunWith(JUnit4::class)
class BanInappropriateExperimentalUsageTest : AbstractLintDetectorTest(
@@ -30,6 +33,27 @@
useIssues = listOf(BanInappropriateExperimentalUsage.ISSUE),
stubs = arrayOf(Stubs.OptIn),
) {
+ // This must match the setting in buildSrc/lint.xml
+ private val validLintConfig: TestFile = TestFiles.xml(
+ "lint.xml",
+ """
+<lint>
+ <issue id="IllegalExperimentalApiUsage">
+ <option name="atomicLibraryGroupFilename" value="atomic-library-groups.txt"/>
+ </issue>
+</lint>
+ """.trimIndent()
+ )
+
+ private val libraryGroups = TestFiles.source(
+ "atomic-library-groups.txt",
+ """
+androidx.a
+androidx.b
+ """.trimIndent()
+ )
+
+ private val UNUSED_PLACEHOLDER = "unused"
@Test
fun `Test within-module Experimental usage via Gradle model`() {
@@ -44,6 +68,8 @@
group=sample.annotation.provider
"""
).indented(),
+ validLintConfig,
+ libraryGroups,
)
/* ktlint-disable max-line-length */
@@ -85,6 +111,8 @@
group=androidx.sample.consumer
"""
).indented(),
+ validLintConfig,
+ libraryGroups,
)
/* ktlint-disable max-line-length */
@@ -102,4 +130,128 @@
// TODO: Using TestMode.DEFAULT due to b/188814760; remove testModes once bug is resolved
check(provider, consumer, testModes = listOf(TestMode.DEFAULT)).expect(expected)
}
+
+ @Test(expected = RuntimeException::class)
+ fun `Missing atomicLibraryGroupFilename property should throw an exception`() {
+ val invalidLintConfig: TestFile = TestFiles.xml(
+ "lint.xml",
+ """
+<lint>
+ <issue id="IllegalExperimentalApiUsage">
+ <option name="foo" value="bar"/>
+ </issue>
+</lint>
+ """.trimIndent()
+ )
+ val provider = project()
+ .name("provider")
+ .type(ProjectDescription.Type.LIBRARY)
+ .report(false)
+ .files(
+ ktSample("sample.annotation.provider.ExperimentalSampleAnnotation"),
+ javaSample("sample.annotation.provider.ExperimentalSampleAnnotationJava"),
+ javaSample("sample.annotation.provider.RequiresOptInSampleAnnotationJava"),
+ gradle(
+ """
+ apply plugin: 'com.android.library'
+ group=sample.annotation.provider
+ """
+ ).indented(),
+ )
+
+ val consumer = project()
+ .name("consumer")
+ .type(ProjectDescription.Type.LIBRARY)
+ .dependsOn(provider)
+ .files(
+ ktSample("androidx.sample.consumer.OutsideGroupExperimentalAnnotatedClass"),
+ gradle(
+ """
+ apply plugin: 'com.android.library'
+ group=androidx.sample.consumer
+ """
+ ).indented(),
+ invalidLintConfig,
+ )
+
+ // TODO: Using TestMode.DEFAULT due to b/188814760; remove testModes once bug is resolved
+ check(provider, consumer, testModes = listOf(TestMode.DEFAULT)).expect(UNUSED_PLACEHOLDER)
+ }
+
+ @Test(expected = FileNotFoundException::class)
+ fun `Missing atomic library group file should throw an exception`() {
+ val provider = project()
+ .name("provider")
+ .type(ProjectDescription.Type.LIBRARY)
+ .report(false)
+ .files(
+ ktSample("sample.annotation.provider.ExperimentalSampleAnnotation"),
+ javaSample("sample.annotation.provider.ExperimentalSampleAnnotationJava"),
+ javaSample("sample.annotation.provider.RequiresOptInSampleAnnotationJava"),
+ gradle(
+ """
+ apply plugin: 'com.android.library'
+ group=sample.annotation.provider
+ """
+ ).indented(),
+ )
+
+ val consumer = project()
+ .name("consumer")
+ .type(ProjectDescription.Type.LIBRARY)
+ .dependsOn(provider)
+ .files(
+ ktSample("androidx.sample.consumer.OutsideGroupExperimentalAnnotatedClass"),
+ gradle(
+ """
+ apply plugin: 'com.android.library'
+ group=androidx.sample.consumer
+ """
+ ).indented(),
+ validLintConfig,
+ )
+
+ // TODO: Using TestMode.DEFAULT due to b/188814760; remove testModes once bug is resolved
+ check(provider, consumer, testModes = listOf(TestMode.DEFAULT)).expect(UNUSED_PLACEHOLDER)
+ }
+
+ @Test(expected = RuntimeException::class)
+ fun `Empty atomic library group file should throw an exception`() {
+ val emptyLibraryGroups = TestFiles.source("atomic-library-groups.txt", "")
+
+ val provider = project()
+ .name("provider")
+ .type(ProjectDescription.Type.LIBRARY)
+ .report(false)
+ .files(
+ ktSample("sample.annotation.provider.ExperimentalSampleAnnotation"),
+ javaSample("sample.annotation.provider.ExperimentalSampleAnnotationJava"),
+ javaSample("sample.annotation.provider.RequiresOptInSampleAnnotationJava"),
+ gradle(
+ """
+ apply plugin: 'com.android.library'
+ group=sample.annotation.provider
+ """
+ ).indented(),
+ )
+
+ val consumer = project()
+ .name("consumer")
+ .type(ProjectDescription.Type.LIBRARY)
+ .dependsOn(provider)
+ .files(
+ ktSample("androidx.sample.consumer.OutsideGroupExperimentalAnnotatedClass"),
+ gradle(
+ """
+ apply plugin: 'com.android.library'
+ group=androidx.sample.consumer
+ """
+ ).indented(),
+ validLintConfig,
+ emptyLibraryGroups,
+ )
+
+ // TODO: Using TestMode.DEFAULT due to b/188814760; remove testModes once bug is resolved
+ check(provider, consumer, testModes = listOf(TestMode.DEFAULT)).expect(UNUSED_PLACEHOLDER)
+ }
}
