Repository: RBusarow/ModuleCheck (kotlin)

Issue:
"unused" check doesn't consider resource references in AndroidManifest.xml
Given a simple manifest like this:

```xml
<manifest
  xmlns:android="http://schemas.android.com/apk/res/android"
  package="com.example.app"
  >

  <application
    android:name=".App"
    android:allowBackup="true"
    android:icon="@mipmap/ic_launcher"
    android:label="@string/app_name"
    android:roundIcon="@mipmap/ic_launcher_round"
    android:supportsRtl="true"
    android:theme="@style/AppTheme"
    />
</manifest>
```
If the launcher icons and/or `app_name` string are in a different module, with no other references to that module, that module will be reported as unused.

Pull request:
fixes #202

Patch:
diff --git a/modulecheck-api/src/main/kotlin/modulecheck/api/AndroidProject2Impl.kt b/modulecheck-api/src/main/kotlin/modulecheck/api/AndroidProject2Impl.kt
index 2ce75589cd..d265ecb94a 100644
--- a/modulecheck-api/src/main/kotlin/modulecheck/api/AndroidProject2Impl.kt
+++ b/modulecheck-api/src/main/kotlin/modulecheck/api/AndroidProject2Impl.kt
@@ -34,7 +34,8 @@ data class AndroidProject2Impl(
   override val androidResourcesEnabled: Boolean,
   override val viewBindingEnabled: Boolean,
   override val resourceFiles: Set<File>,
-  override val androidPackageOrNull: String?
+  override val androidPackageOrNull: String?,
+  override val manifests: Map<SourceSetName, File>
 ) : AndroidProject2 {
 
   private val context = ProjectContextImpl(this)
diff --git a/modulecheck-api/src/main/kotlin/modulecheck/api/Project2.kt b/modulecheck-api/src/main/kotlin/modulecheck/api/Project2.kt
index 24d46dffcf..eb019d4561 100644
--- a/modulecheck-api/src/main/kotlin/modulecheck/api/Project2.kt
+++ b/modulecheck-api/src/main/kotlin/modulecheck/api/Project2.kt
@@ -80,4 +80,5 @@ interface AndroidProject2 : Project2 {
   val viewBindingEnabled: Boolean
   val resourceFiles: Set<File>
   val androidPackageOrNull: String?
+  val manifests: Map<SourceSetName, File>
 }
diff --git a/modulecheck-api/src/main/kotlin/modulecheck/api/context/Imports.kt b/modulecheck-api/src/main/kotlin/modulecheck/api/context/Imports.kt
index 8e2c40af56..0c6899e7da 100644
--- a/modulecheck-api/src/main/kotlin/modulecheck/api/context/Imports.kt
+++ b/modulecheck-api/src/main/kotlin/modulecheck/api/context/Imports.kt
@@ -46,9 +46,7 @@ data class Imports(
             .flatMap { it.customViews }
             .toSet()
 
-          val combined = jvm + layout
-
-          combined
+          jvm + layout
         }
 
       return Imports(ConcurrentHashMap(map))
diff --git a/modulecheck-api/src/main/kotlin/modulecheck/api/context/ManifestFiles.kt b/modulecheck-api/src/main/kotlin/modulecheck/api/context/ManifestFiles.kt
new file mode 100644
index 0000000000..ba13e3cee9
--- /dev/null
+++ b/modulecheck-api/src/main/kotlin/modulecheck/api/context/ManifestFiles.kt
@@ -0,0 +1,57 @@
+/*
+ * Copyright (C) 2021 Rick Busarow
+ * 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
+ *
+ *      http://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.
+ */
+
+package modulecheck.api.context
+
+import modulecheck.api.AndroidProject2
+import modulecheck.api.Project2
+import modulecheck.api.SourceSetName
+import modulecheck.parsing.xml.XmlFile
+import java.util.concurrent.ConcurrentHashMap
+import java.util.concurrent.ConcurrentMap
+
+data class ManifestFiles(
+  internal val delegate: ConcurrentMap<SourceSetName, XmlFile.ManifestFile>
+) : ConcurrentMap<SourceSetName, XmlFile.ManifestFile> by delegate,
+  ProjectContext.Element {
+
+  override val key: ProjectContext.Key<ManifestFiles>
+    get() = Key
+
+  companion object Key : ProjectContext.Key<ManifestFiles> {
+    override operator fun invoke(project: Project2): ManifestFiles {
+
+      if (project !is AndroidProject2) return ManifestFiles(ConcurrentHashMap())
+
+      val map = project
+        .manifests
+        .mapValues { (_, file) ->
+          XmlFile.ManifestFile(file)
+        }
+
+      return ManifestFiles(ConcurrentHashMap(map))
+    }
+  }
+}
+
+val ProjectContext.manifestFiles: ManifestFiles
+  get() = get(ManifestFiles)
+
+fun ProjectContext.manifestFilesForSourceSetName(
+  sourceSetName: SourceSetName
+): XmlFile.ManifestFile? = manifestFiles[sourceSetName]
+  ?.takeIf { it.file.exists() }
+  ?: manifestFiles[SourceSetName.MAIN]
+    ?.takeIf { it.file.exists() }
diff --git a/modulecheck-api/src/main/kotlin/modulecheck/api/context/PossibleReferences.kt b/modulecheck-api/src/main/kotlin/modulecheck/api/context/PossibleReferences.kt
index 82ba3a2ba0..4fbf2e5fbe 100644
--- a/modulecheck-api/src/main/kotlin/modulecheck/api/context/PossibleReferences.kt
+++ b/modulecheck-api/src/main/kotlin/modulecheck/api/context/PossibleReferences.kt
@@ -34,11 +34,22 @@ data class PossibleReferences(
     override operator fun invoke(project: Project2): PossibleReferences {
       val map = project
         .sourceSets
-        .mapValues { (_, sourceSet) ->
-          project[JvmFiles][sourceSet.name]
+        .mapValues { (name, sourceSet) ->
+          val jvm = project[JvmFiles][sourceSet.name]
             .orEmpty()
             .flatMap { jvmFile -> jvmFile.maybeExtraReferences }
             .toSet()
+
+          val layout = project[LayoutFiles][name]
+            .orEmpty()
+            .flatMap { it.resourceReferencesAsRReferences }
+            .toSet()
+
+          val manifest = project.manifestFilesForSourceSetName(name)
+            ?.resourceReferencesAsRReferences
+            .orEmpty()
+
+          jvm + layout + manifest
         }
 
       return PossibleReferences(ConcurrentHashMap(map))
diff --git a/modulecheck-parsing/api/src/main/kotlin/modulecheck/parsing/AndroidResource.kt b/modulecheck-parsing/api/src/main/kotlin/modulecheck/parsing/AndroidResource.kt
index 9db562045d..5a4908d2be 100644
--- a/modulecheck-parsing/api/src/main/kotlin/modulecheck/parsing/AndroidResource.kt
+++ b/modulecheck-parsing/api/src/main/kotlin/modulecheck/parsing/AndroidResource.kt
@@ -37,7 +37,7 @@ sealed class AndroidResource(val prefix: kotlin.String) {
 
   companion object {
 
-    private val REGEX = """"@(.*)/(.*)"""".toRegex()
+    private val REGEX = """"?@(.*)/(.*)"?""".toRegex()
 
     @Suppress("ComplexMethod")
     fun fromFile(file: File): AndroidResource? {
diff --git a/modulecheck-parsing/xml/src/main/kotlin/modulecheck/parsing/xml/AndroidManifestParser.kt b/modulecheck-parsing/xml/src/main/kotlin/modulecheck/parsing/xml/AndroidManifestParser.kt
index 86cc0627df..b33db5c280 100644
--- a/modulecheck-parsing/xml/src/main/kotlin/modulecheck/parsing/xml/AndroidManifestParser.kt
+++ b/modulecheck-parsing/xml/src/main/kotlin/modulecheck/parsing/xml/AndroidManifestParser.kt
@@ -31,4 +31,14 @@ object AndroidManifestParser {
     .filterNotNull()
     .filterIsInstance<MutableEntry<String, String>>()
     .associate { it.key to it.value }
+
+  fun parseResources(file: File): Set<String> {
+    return parser.parse(file)
+      .breadthFirst()
+      .filterIsInstance<Node>()
+      .mapNotNull { it.attributes() }
+      .flatMap { it.values.mapNotNull { value -> value } }
+      .filterIsInstance<String>()
+      .toSet()
+  }
 }
diff --git a/modulecheck-parsing/xml/src/main/kotlin/modulecheck/parsing/xml/XmlFile.kt b/modulecheck-parsing/xml/src/main/kotlin/modulecheck/parsing/xml/XmlFile.kt
index 6719985a7a..cc9a819fb7 100644
--- a/modulecheck-parsing/xml/src/main/kotlin/modulecheck/parsing/xml/XmlFile.kt
+++ b/modulecheck-parsing/xml/src/main/kotlin/modulecheck/parsing/xml/XmlFile.kt
@@ -15,11 +15,12 @@
 
 package modulecheck.parsing.xml
 
+import modulecheck.parsing.AndroidResource
 import java.io.File
 
-sealed class XmlFile {
+sealed interface XmlFile {
 
-  data class LayoutFile(val file: File) : XmlFile() {
+  data class LayoutFile(val file: File) : XmlFile {
 
     val customViews: Set<String> by lazy {
       AndroidLayoutParser.parseViews(file)
@@ -30,6 +31,29 @@ sealed class XmlFile {
         .filter { attribute -> PREFIXES.any { attribute.startsWith(it) } }
         .toSet()
     }
+
+    val resourceReferencesAsRReferences: Set<String> by lazy {
+      resourceReferences
+        .mapNotNull { AndroidResource.fromString(it) }
+        .map { "R.${it.prefix}.${it.name}" }
+        .toSet()
+    }
+  }
+
+  data class ManifestFile(val file: File) : XmlFile {
+
+    val resourceReferences: Set<String> by lazy {
+      AndroidManifestParser.parseResources(file)
+        .filter { attribute -> PREFIXES.any { attribute.startsWith(it) } }
+        .toSet()
+    }
+
+    val resourceReferencesAsRReferences: Set<String> by lazy {
+      resourceReferences
+        .mapNotNull { AndroidResource.fromString(it) }
+        .map { "R.${it.prefix}.${it.name}" }
+        .toSet()
+    }
   }
 
   companion object {
diff --git a/modulecheck-plugin/src/main/kotlin/modulecheck/gradle/GradleProjectProvider.kt b/modulecheck-plugin/src/main/kotlin/modulecheck/gradle/GradleProjectProvider.kt
index 4e7a6e911c..967f99c1a9 100644
--- a/modulecheck-plugin/src/main/kotlin/modulecheck/gradle/GradleProjectProvider.kt
+++ b/modulecheck-plugin/src/main/kotlin/modulecheck/gradle/GradleProjectProvider.kt
@@ -104,7 +104,8 @@ class GradleProjectProvider(
         androidResourcesEnabled = libraryExtension?.buildFeatures?.androidResources != false,
         viewBindingEnabled = testedExtension?.buildFeatures?.viewBinding == true,
         resourceFiles = gradleProject.androidResourceFiles(),
-        androidPackageOrNull = gradleProject.androidPackageOrNull()
+        androidPackageOrNull = gradleProject.androidPackageOrNull(),
+        manifests = gradleProject.androidManifests().orEmpty()
       )
     } else {
       Project2Impl(
@@ -246,8 +247,8 @@ class GradleProjectProvider(
   private fun GradleProject.androidPackageOrNull(): String? {
 
     return androidManifests()
-      ?.filter { it.exists() }
-      ?.map { AndroidManifestParser.parse(it)["package"] }
+      ?.filter { it.value.exists() }
+      ?.map { AndroidManifestParser.parse(it.value)["package"] }
       ?.distinct()
       ?.also {
         require(it.size == 1) {
diff --git a/modulecheck-plugin/src/main/kotlin/modulecheck/gradle/internal/project.kt b/modulecheck-plugin/src/main/kotlin/modulecheck/gradle/internal/project.kt
index b03445998a..6b4590d13f 100644
--- a/modulecheck-plugin/src/main/kotlin/modulecheck/gradle/internal/project.kt
+++ b/modulecheck-plugin/src/main/kotlin/modulecheck/gradle/internal/project.kt
@@ -16,6 +16,8 @@
 package modulecheck.gradle.internal
 
 import com.android.build.gradle.TestedExtension
+import modulecheck.api.SourceSetName
+import modulecheck.api.toSourceSetName
 import org.gradle.api.Project
 import org.gradle.kotlin.dsl.findByType
 import java.io.File
@@ -43,9 +45,9 @@ fun Project.isAndroid(): Boolean = extensions.findByType(TestedExtension::class)
 fun Project.testedExtensionOrNull(): TestedExtension? = extensions
   .findByType(TestedExtension::class)
 
-fun Project.androidManifests() = testedExtensionOrNull()
+fun Project.androidManifests(): Map<SourceSetName, File>? = testedExtensionOrNull()
   ?.sourceSets
-  ?.map { it.manifest.srcFile }
+  ?.associate { it.name.toSourceSetName() to it.manifest.srcFile }
 
 /**
  * @return the main src `AndroidManifest.xml` file if it exists. This will typically be


Test patch:
diff --git a/modulecheck-plugin/src/test/kotlin/modulecheck/gradle/UnusedDependenciesTest.kt b/modulecheck-plugin/src/test/kotlin/modulecheck/gradle/UnusedDependenciesTest.kt
index e92e94a2a6..f3ce7d80a6 100644
--- a/modulecheck-plugin/src/test/kotlin/modulecheck/gradle/UnusedDependenciesTest.kt
+++ b/modulecheck-plugin/src/test/kotlin/modulecheck/gradle/UnusedDependenciesTest.kt
@@ -885,6 +885,98 @@ class UnusedDependenciesTest : BasePluginTest() {
     build("moduleCheckUnusedDependency").shouldSucceed()
   }
 
+  @Test
+  fun `module with a string resource used in subject module's AndroidManifest should not be unused`() {
+
+    val appProject = ProjectSpec("app") {
+      addBuildSpec(
+        ProjectBuildSpec {
+          addPlugin("""id("com.android.library")""")
+          addPlugin("kotlin(\"android\")")
+          android = true
+          addProjectDependency("api", jvmSub1)
+        }
+      )
+      addSrcSpec(
+        ProjectSrcSpec(Path.of("src/main")) {
+          addRawFile(
+            RawFile(
+              "AndroidManifest.xml",
+              """<manifest
+                |  xmlns:android="http://schemas.android.com/apk/res/android"
+                |  package="com.example.app"
+                |  >
+                |
+                |  <application
+                |    android:name=".App"
+                |    android:allowBackup="true"
+                |    android:icon="@mipmap/ic_launcher"
+                |    android:label="@string/app_name"
+                |    android:roundIcon="@mipmap/ic_launcher_round"
+                |    android:supportsRtl="true"
+                |    android:theme="@style/AppTheme"
+                |    />
+                |</manifest>
+                """.trimMargin()
+            )
+          )
+        }
+      )
+    }
+
+    val androidSub1 = ProjectSpec("lib-1") {
+      addSrcSpec(
+        ProjectSrcSpec(Path.of("src/main/res/values")) {
+          addRawFile(
+            RawFile(
+              "strings.xml",
+              """<resources>
+                |  <string name="app_name" translatable="false">MyApp</string>
+                |</resources>
+                """.trimMargin()
+            )
+          )
+        }
+      )
+      addSrcSpec(
+        ProjectSrcSpec(Path.of("src/main")) {
+          addRawFile(
+            RawFile(
+              "AndroidManifest.xml",
+              """<manifest package="com.example.lib1" />
+                """.trimMargin()
+            )
+          )
+        }
+      )
+      addBuildSpec(
+        ProjectBuildSpec {
+          addPlugin("""id("com.android.library")""")
+          addPlugin("kotlin(\"android\")")
+          android = true
+        }
+      )
+    }
+
+    ProjectSpec("project") {
+      addSubproject(appProject)
+      addSubproject(androidSub1)
+      addSettingsSpec(projectSettings.build())
+      addBuildSpec(
+        projectBuild
+          .addBlock(
+            """moduleCheck {
+            |  autoCorrect = false
+            |}
+          """.trimMargin()
+          ).build()
+      )
+    }
+      .writeIn(testProjectDir.toPath())
+
+    build("moduleCheckUnusedDependency").shouldSucceed()
+  }
+
   @Test
   fun `module with an auto-generated manifest and a string resource used in subject module should not be unused`() {