Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a test case for false positive #419

Merged
merged 2 commits into from
Apr 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,16 @@
package modulecheck.api.context

import modulecheck.parsing.android.AndroidResourceParser
import modulecheck.parsing.gradle.AndroidPlatformPlugin
import modulecheck.parsing.gradle.SourceSetName
import modulecheck.parsing.source.UnqualifiedAndroidResourceDeclaredName
import modulecheck.project.McProject
import modulecheck.project.ProjectContext
import modulecheck.project.isAndroid
import modulecheck.utils.LazySet
import modulecheck.utils.SafeCache
import modulecheck.utils.dataSource
import modulecheck.utils.emptyLazySet
import modulecheck.utils.flatMapToSet
import modulecheck.utils.lazySet
import modulecheck.utils.toLazySet

Expand All @@ -37,7 +38,8 @@ data class AndroidUnqualifiedDeclarationNames(
get() = Key

suspend fun get(sourceSetName: SourceSetName): LazySet<UnqualifiedAndroidResourceDeclaredName> {
if (!project.isAndroid()) return emptyLazySet()
val platformPlugin = project.platformPlugin as? AndroidPlatformPlugin
?: return emptyLazySet()

return delegate.getOrPut(sourceSetName) {

Expand All @@ -57,6 +59,13 @@ data class AndroidUnqualifiedDeclarationNames(
}
}

val resValues = dataSource {
sourceSetName.withUpstream(project)
.flatMapToSet { sourceSetOrUpstream ->
platformPlugin.resValues[sourceSetOrUpstream].orEmpty()
}
}

val declarations = project
.resourceFilesForSourceSetName(sourceSetOrUpstream)
.map { file ->
Expand All @@ -66,6 +75,7 @@ data class AndroidUnqualifiedDeclarationNames(
}
}
.plus(layoutsAndIds)
.plus(resValues)

lazySet(declarations)
}.toLazySet()
Expand Down
1 change: 1 addition & 0 deletions modulecheck-parsing/gradle/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ dependencies {
api(libs.rickBusarow.dispatch.core)
api(libs.semVer)

api(project(path = ":modulecheck-parsing:source"))
api(project(path = ":modulecheck-utils"))

compileOnly(gradleApi())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

package modulecheck.parsing.gradle

import modulecheck.parsing.source.UnqualifiedAndroidResourceDeclaredName
import java.io.File
import kotlin.contracts.contract

Expand Down Expand Up @@ -61,6 +62,7 @@ sealed interface AndroidPlatformPlugin : PlatformPlugin {
val viewBindingEnabled: Boolean
val kotlinAndroidExtensionEnabled: Boolean
val manifests: Map<SourceSetName, File>
val resValues: Map<SourceSetName, Set<UnqualifiedAndroidResourceDeclaredName>>

interface CanDisableAndroidResources {
val androidResourcesEnabled: Boolean
Expand All @@ -76,7 +78,8 @@ sealed interface AndroidPlatformPlugin : PlatformPlugin {
override val nonTransientRClass: Boolean,
override val viewBindingEnabled: Boolean,
override val kotlinAndroidExtensionEnabled: Boolean,
override val manifests: Map<SourceSetName, File>
override val manifests: Map<SourceSetName, File>,
override val resValues: Map<SourceSetName, Set<UnqualifiedAndroidResourceDeclaredName>>
) : PlatformPlugin, AndroidPlatformPlugin

data class AndroidLibraryPlugin(
Expand All @@ -87,7 +90,8 @@ sealed interface AndroidPlatformPlugin : PlatformPlugin {
override val kotlinAndroidExtensionEnabled: Boolean,
override val manifests: Map<SourceSetName, File>,
override val androidResourcesEnabled: Boolean,
override val buildConfigEnabled: Boolean
override val buildConfigEnabled: Boolean,
override val resValues: Map<SourceSetName, Set<UnqualifiedAndroidResourceDeclaredName>>
) : PlatformPlugin,
AndroidPlatformPlugin,
CanDisableAndroidResources,
Expand All @@ -100,7 +104,8 @@ sealed interface AndroidPlatformPlugin : PlatformPlugin {
override val viewBindingEnabled: Boolean,
override val kotlinAndroidExtensionEnabled: Boolean,
override val manifests: Map<SourceSetName, File>,
override val buildConfigEnabled: Boolean
override val buildConfigEnabled: Boolean,
override val resValues: Map<SourceSetName, Set<UnqualifiedAndroidResourceDeclaredName>>
) : PlatformPlugin,
AndroidPlatformPlugin,
CanDisableAndroidBuildConfig
Expand All @@ -112,7 +117,8 @@ sealed interface AndroidPlatformPlugin : PlatformPlugin {
override val viewBindingEnabled: Boolean,
override val kotlinAndroidExtensionEnabled: Boolean,
override val manifests: Map<SourceSetName, File>,
override val buildConfigEnabled: Boolean
override val buildConfigEnabled: Boolean,
override val resValues: Map<SourceSetName, Set<UnqualifiedAndroidResourceDeclaredName>>
) : PlatformPlugin,
AndroidPlatformPlugin,
CanDisableAndroidBuildConfig
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@ import com.android.build.api.dsl.ApplicationExtension
import com.android.build.api.dsl.CommonExtension
import com.android.build.api.dsl.DynamicFeatureExtension
import com.android.build.api.dsl.TestExtension
import com.android.build.gradle.AppExtension
import com.android.build.gradle.LibraryExtension
import com.android.build.gradle.internal.api.ApplicationVariantImpl
import com.android.build.gradle.internal.api.LibraryVariantImpl
import com.android.build.gradle.internal.core.InternalBaseVariant.MergedFlavor
import modulecheck.core.rule.KOTLIN_ANDROID_EXTENSIONS_PLUGIN_ID
import modulecheck.gradle.AndroidPlatformPluginFactory.Type.Application
import modulecheck.gradle.AndroidPlatformPluginFactory.Type.DynamicFeature
Expand All @@ -32,6 +36,10 @@ import modulecheck.parsing.gradle.AndroidPlatformPlugin.AndroidApplicationPlugin
import modulecheck.parsing.gradle.AndroidPlatformPlugin.AndroidDynamicFeaturePlugin
import modulecheck.parsing.gradle.AndroidPlatformPlugin.AndroidLibraryPlugin
import modulecheck.parsing.gradle.AndroidPlatformPlugin.AndroidTestPlugin
import modulecheck.parsing.gradle.SourceSetName
import modulecheck.parsing.gradle.asSourceSetName
import modulecheck.parsing.source.UnqualifiedAndroidResourceDeclaredName
import modulecheck.utils.cast
import javax.inject.Inject

typealias AndroidCommonExtension = CommonExtension<*, *, *, *>
Expand Down Expand Up @@ -59,6 +67,8 @@ class AndroidPlatformPluginFactory @Inject constructor(

val manifests = gradleProject.androidManifests().orEmpty()

val resValues = parseResValues(type)

val hasKotlinAndroidExtensions = gradleProject
.pluginManager
.hasPlugin(KOTLIN_ANDROID_EXTENSIONS_PLUGIN_ID)
Expand Down Expand Up @@ -90,7 +100,7 @@ class AndroidPlatformPluginFactory @Inject constructor(
nonTransientRClass = nonTransientRClass,
viewBindingEnabled = viewBindingEnabled,
kotlinAndroidExtensionEnabled = hasKotlinAndroidExtensions,
manifests = manifests
manifests = manifests, resValues = resValues
)
is DynamicFeature -> AndroidDynamicFeaturePlugin(
sourceSets = sourceSets,
Expand All @@ -99,7 +109,7 @@ class AndroidPlatformPluginFactory @Inject constructor(
viewBindingEnabled = viewBindingEnabled,
kotlinAndroidExtensionEnabled = hasKotlinAndroidExtensions,
manifests = manifests,
buildConfigEnabled = buildConfigEnabled
buildConfigEnabled = buildConfigEnabled, resValues = resValues
)
is Library -> AndroidLibraryPlugin(
sourceSets = sourceSets,
Expand All @@ -109,7 +119,7 @@ class AndroidPlatformPluginFactory @Inject constructor(
kotlinAndroidExtensionEnabled = hasKotlinAndroidExtensions,
manifests = manifests,
androidResourcesEnabled = androidResourcesEnabled,
buildConfigEnabled = buildConfigEnabled
buildConfigEnabled = buildConfigEnabled, resValues = resValues
)
is Test -> AndroidTestPlugin(
sourceSets = sourceSets,
Expand All @@ -118,11 +128,53 @@ class AndroidPlatformPluginFactory @Inject constructor(
viewBindingEnabled = viewBindingEnabled,
kotlinAndroidExtensionEnabled = hasKotlinAndroidExtensions,
manifests = manifests,
buildConfigEnabled = buildConfigEnabled
buildConfigEnabled = buildConfigEnabled, resValues = resValues
)
}
}

private fun parseResValues(
type: Type<*>
): MutableMap<SourceSetName, Set<UnqualifiedAndroidResourceDeclaredName>> {
fun AndroidCommonExtension.mergedFlavors(): List<MergedFlavor> {
return when (this) {
is AppExtension -> applicationVariants.map { it.cast<ApplicationVariantImpl>().mergedFlavor }
is LibraryExtension -> libraryVariants.map { it.cast<LibraryVariantImpl>().mergedFlavor }
else -> emptyList()
}
}

fun AndroidCommonExtension.buildTypes(): List<com.android.builder.model.BuildType> {
return when (this) {
is AppExtension -> applicationVariants.mapNotNull { it.buildType }
is LibraryExtension -> libraryVariants.mapNotNull { it.buildType }
else -> emptyList()
}
}

val mfs = type.extension.mergedFlavors()
.associate { mf ->
val sourceSetName = mf.name.asSourceSetName()

sourceSetName to mf.resValues.values
.mapNotNull { classField ->
UnqualifiedAndroidResourceDeclaredName.fromValuePair(classField.type, classField.name)
}.toSet()
}.toMutableMap()

type.extension.buildTypes()
.forEach { buildType ->
val sourceSetName = buildType.name.asSourceSetName()

mfs[sourceSetName] = buildType.resValues.values
.mapNotNull { classField ->
UnqualifiedAndroidResourceDeclaredName.fromValuePair(classField.type, classField.name)
}.toSet()
}

return mfs
}

sealed interface Type<T : AndroidCommonExtension> {
val extension: T

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import modulecheck.specs.ProjectBuildSpecBuilder
import modulecheck.specs.ProjectSettingsSpecBuilder
import modulecheck.specs.ProjectSpec
import modulecheck.specs.ProjectSrcSpec
import modulecheck.specs.ProjectSrcSpecBuilder.RawFile
import modulecheck.utils.applyEach
import org.junit.jupiter.api.Test
import java.io.File
Expand Down Expand Up @@ -125,24 +126,7 @@ class UnusedDependenciesPluginTest : BasePluginTest() {
@Test
fun `module with an auto-generated manifest and a string resource used in subject module should not be unused`() {

val appFile = FileSpec.builder("com.example.app", "MyApp")
.addType(
TypeSpec.classBuilder("MyApp")
.addProperty(
PropertySpec.builder("appNameRes", Int::class.asTypeName())
.getter(
FunSpec.getterBuilder()
.addCode(
"""return %T.string.app_name""",
ClassName.bestGuess("com.example.app.R")
)
.build()
)
.build()
)
.build()
)
.build()
val appFile = createAppFile()

val appProject = ProjectSpec("app") {
addBuildSpec(
Expand Down Expand Up @@ -321,4 +305,88 @@ class UnusedDependenciesPluginTest : BasePluginTest() {

shouldSucceed("moduleCheck")
}

@Test
fun `module with generated string resource used in subject module 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/java")) {
addFileSpec(createAppFile())
}
)
addSrcSpec(
ProjectSrcSpec(Path.of("src/main")) {
addRawFile(
RawFile(
"AndroidManifest.xml",
"""<manifest package="com.example.app" />
""".trimMargin()
)
)
}
)
}

val androidSub1 = ProjectSpec("lib-1") {

addBuildSpec(
ProjectBuildSpec {
addPlugin("""id("com.android.library")""")
addPlugin("kotlin(\"android\")")
android = true
addBlock(
"""
android {
defaultConfig {
resValue("string", "app_name", "AppName")
}
buildTypes {
getByName("debug") {
resValue("string", "debug_thing", "debug!")
}
}
}
"""
)
}
)
}

ProjectSpec("project") {
addSubproject(appProject)
addSubproject(androidSub1)
addSettingsSpec(projectSettings.build())
addBuildSpec(projectBuild.build())
}
.writeIn(testProjectDir.toPath())

shouldSucceed("moduleCheck")
}

fun createAppFile() = FileSpec.builder("com.example.app", "MyApp")
.addType(
TypeSpec.classBuilder("MyApp")
.addProperty(
PropertySpec.builder("appNameRes", Int::class.asTypeName())
.getter(
FunSpec.getterBuilder()
.addCode(
"""return %T.string.app_name""",
ClassName.bestGuess("com.example.app.R")
)
.build()
)
.build()
)
.build()
)
.build()
}
Loading