Skip to content

Commit

Permalink
Check possible active plugin or plugin point naming clashes (#4596)
Browse files Browse the repository at this point in the history
  • Loading branch information
@aitorvs
aitorvs authored May 29, 2024
1 parent 313b651 commit 0115768
Show file tree
Hide file tree
Showing 4 changed files with 139 additions and 81 deletions.
23 changes: 23 additions & 0 deletions .../src/main/java/com/duckduckgo/anvil/compiler/ContributesActivePluginPointCodeGenerator.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import com.squareup.kotlinpoet.TypeSpec
import com.squareup.kotlinpoet.asClassName
import dagger.Binds
import java.io.File
import java.util.concurrent.ConcurrentHashMap
import javax.inject.Inject
import org.jetbrains.kotlin.descriptors.ModuleDescriptor
import org.jetbrains.kotlin.name.FqName
Expand Down Expand Up @@ -123,6 +124,16 @@ class ContributesActivePluginPointCodeGenerator : CodeGenerator {
val pluginClassType = vmClass.pluginClassName(ContributesActivePluginPoint::class.fqName) ?: vmClass.asClassName()
val featureName = "pluginPoint${pluginClassType.simpleName}"

// Check if there's another plugin point class that has the same class simplename
// we can't allow that because the backing remote feature would be the same
val existingFeature = featureBackedClassNames.putIfAbsent(featureName, vmClass.fqName)
if (existingFeature != null) {
throw AnvilCompilationException(
"${vmClass.fqName} plugin point naming is duplicated, previous found in $existingFeature",
element = vmClass.clazz.identifyingElement,
)
}

val content = FileSpec.buildFile(generatedPackage, pluginPointClassFileName) {
// This is the normal plugin point
addType(
Expand Down Expand Up @@ -304,6 +315,16 @@ class ContributesActivePluginPointCodeGenerator : CodeGenerator {
val pluginRemoteFeatureStoreClassName = "${vmClass.shortName}_ActivePlugin_RemoteFeature_MultiProcessStore"
val pluginPriority = vmClass.annotations.firstOrNull { it.fqName == ContributesActivePlugin::class.fqName }?.priorityOrNull()

// Check if there's another plugin class, in the same plugin point, that has the same class simplename
// we can't allow that because the backing remote feature would be the same
val existingFeature = featureBackedClassNames.putIfAbsent("${featureName}_$parentFeatureName", vmClass.fqName)
if (existingFeature != null) {
throw AnvilCompilationException(
"${vmClass.fqName} plugin name is duplicated, previous found in $existingFeature",
element = vmClass.clazz.identifyingElement,
)
}

val content = FileSpec.buildFile(generatedPackage, pluginClassName) {
// First create the class that will contribute the active plugin.
// We do expect that the plugins are define using the "ContributesActivePlugin" annotation but are also injected
Expand Down Expand Up @@ -570,6 +591,8 @@ class ContributesActivePluginPointCodeGenerator : CodeGenerator {
}

companion object {
internal val featureBackedClassNames = ConcurrentHashMap<String, FqName>()

private val pluginPointFqName = FqName("com.duckduckgo.common.utils.plugins.PluginPoint")
private val dispatcherProviderFqName = FqName("com.duckduckgo.common.utils.DispatcherProvider")
private val activePluginPointFqName = FqName("com.duckduckgo.common.utils.plugins.InternalActivePluginPoint")
Expand Down
4 changes: 2 additions & 2 deletions lint-rules/src/main/java/com/duckduckgo/lint/WrongPluginPointCollectorDetector.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class WrongPluginPointCollectorDetector : Detector(), SourceCodeScanner {
}

private fun PsiClass.isActivePlugin(): Boolean {
return this.isSubtypeOf("com.duckduckgo.common.utils.plugins.ActivePluginPoint.ActivePlugin")
return this.isSubtypeOf("com.duckduckgo.common.utils.plugins.ActivePlugin")
}
private fun handleField(node: UField) {
node.type.let { psiType ->
Expand All @@ -95,7 +95,7 @@ class WrongPluginPointCollectorDetector : Detector(), SourceCodeScanner {
for (typeArgument in typeArguments) {
val typeArgumentClass = (typeArgument as? PsiClassType)?.resolve()
if (typeArgumentClass?.isSubtypeOf(
"com.duckduckgo.common.utils.plugins.ActivePluginPoint.ActivePlugin"
"com.duckduckgo.common.utils.plugins.ActivePlugin"
) == true) {
context.reportError(node, WRONG_PLUGIN_POINT_ISSUE)
}
Expand Down
146 changes: 67 additions & 79 deletions lint-rules/src/test/java/com/duckduckgo/lint/WrongPluginPointCollectorDetectorTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,40 +19,40 @@ package com.duckduckgo.lint
import com.android.tools.lint.checks.infrastructure.TestFiles.kt
import com.android.tools.lint.checks.infrastructure.TestLintTask.lint
import com.duckduckgo.lint.WrongPluginPointCollectorDetector.Companion.WRONG_PLUGIN_POINT_ISSUE
import com.duckduckgo.lint.utils.PLUGIN_POINT_ANNOTATIONS_API
import com.duckduckgo.lint.utils.PLUGIN_POINT_API
import org.junit.Test

class WrongPluginPointCollectorDetectorTest {
@Test
fun `test normal plugin point constructor parameter collecting active plugins`() {
lint()
.files(kt("""
package com.duckduckgo.common.utils.plugins
interface PluginPoint<T> {
fun getPlugins(): Collection<T>
}
interface ActivePluginPoint<out T : ActivePluginPoint.ActivePlugin> {
interface ActivePlugin {
suspend fun isActive(): Boolean = true
}
}
interface MyPlugin
interface MyPluginActivePlugin : ActivePluginPoint.ActivePlugin
.files(
PLUGIN_POINT_API,
PLUGIN_POINT_ANNOTATIONS_API,
kt("""
package com.test.plugins
class Duck(private val pp: PluginPoint<MyPluginActivePlugin>) {
fun quack() {
import com.duckduckgo.common.utils.plugins.ActivePlugin
import com.duckduckgo.common.utils.plugins.PluginPoint
import com.duckduckgo.anvil.annotations.ContributesActivePlugin
interface MyPlugin
interface MyPluginActivePlugin : ActivePlugin
class Duck(private val pp: PluginPoint<MyPluginActivePlugin>) {
fun quack() {
}
}
}
""").indented())
""").indented()
)
.issues(WRONG_PLUGIN_POINT_ISSUE)
.run()
.expect("""
src/com/duckduckgo/common/utils/plugins/PluginPoint.kt:16: Error: PluginPoint cannot be collector of ActivePlugin(s) [WrongPluginPointCollectorDetector]
src/com/test/plugins/MyPlugin.kt:10: Error: PluginPoint cannot be collector of ActivePlugin(s) [WrongPluginPointCollectorDetector]
class Duck(private val pp: PluginPoint<MyPluginActivePlugin>) {
~~~~
src/com/duckduckgo/common/utils/plugins/PluginPoint.kt:16: Error: PluginPoint cannot be collector of ActivePlugin(s) [WrongPluginPointCollectorDetector]
src/com/test/plugins/MyPlugin.kt:10: Error: PluginPoint cannot be collector of ActivePlugin(s) [WrongPluginPointCollectorDetector]
class Duck(private val pp: PluginPoint<MyPluginActivePlugin>) {
~~
2 errors, 0 warnings
Expand All @@ -62,26 +62,23 @@ class WrongPluginPointCollectorDetectorTest {
@Test
fun `test active plugin point constructor parameter collecting active plugins`() {
lint()
.files(kt("""
package com.duckduckgo.common.utils.plugins
interface PluginPoint<T> {
fun getPlugins(): Collection<T>
}
interface ActivePluginPoint<out T : ActivePluginPoint.ActivePlugin> {
interface ActivePlugin {
suspend fun isActive(): Boolean = true
}
}
interface MyPlugin
interface MyPluginActivePlugin : ActivePluginPoint.ActivePlugin
.files(
PLUGIN_POINT_API,
PLUGIN_POINT_ANNOTATIONS_API,
kt("""
package com.test.plugins
import com.duckduckgo.common.utils.plugins.ActivePlugin
import com.duckduckgo.common.utils.plugins.PluginPoint
import com.duckduckgo.anvil.annotations.ContributesActivePlugin
class Duck(private val pp: PluginPoint<MyPlugin>) {
fun quack() {
interface MyPlugin
interface MyPluginActivePlugin : ActivePlugin
class Duck(private val pp: PluginPoint<MyPlugin>) {
fun quack() {
}
}
}
""").indented())
.issues(WRONG_PLUGIN_POINT_ISSUE)
.run()
Expand All @@ -91,33 +88,30 @@ class WrongPluginPointCollectorDetectorTest {
@Test
fun `test normal plugin point field collecting active plugins`() {
lint()
.files(kt("""
package com.duckduckgo.common.utils.plugins
interface PluginPoint<T> {
fun getPlugins(): Collection<T>
}
interface ActivePluginPoint<out T : ActivePluginPoint.ActivePlugin> {
interface ActivePlugin {
suspend fun isActive(): Boolean = true
}
}
.files(
PLUGIN_POINT_API,
PLUGIN_POINT_ANNOTATIONS_API,
kt("""
package com.test.plugins
interface MyPlugin
interface MyPluginActivePlugin : ActivePluginPoint.ActivePlugin
import com.duckduckgo.common.utils.plugins.ActivePlugin
import com.duckduckgo.common.utils.plugins.PluginPoint
import com.duckduckgo.anvil.annotations.ContributesActivePlugin
class Duck {
private val pp: PluginPoint<MyPluginActivePlugin>
fun quack() {
interface MyPlugin
interface MyPluginActivePlugin : ActivePlugin
class Duck {
private val pp: PluginPoint<MyPluginActivePlugin>
fun quack() {
}
}
}
""").indented())
.issues(WRONG_PLUGIN_POINT_ISSUE)
.run()
.expect("""
src/com/duckduckgo/common/utils/plugins/PluginPoint.kt:17: Error: PluginPoint cannot be collector of ActivePlugin(s) [WrongPluginPointCollectorDetector]
src/com/test/plugins/MyPlugin.kt:11: Error: PluginPoint cannot be collector of ActivePlugin(s) [WrongPluginPointCollectorDetector]
private val pp: PluginPoint<MyPluginActivePlugin>
~~
1 errors, 0 warnings
Expand All @@ -127,28 +121,22 @@ class WrongPluginPointCollectorDetectorTest {
@Test
fun `test active plugin point field collecting active plugins`() {
lint()
.files(kt("""
package com.duckduckgo.common.utils.plugins
interface PluginPoint<T> {
fun getPlugins(): Collection<T>
}
interface ActivePluginPoint<out T : ActivePluginPoint.ActivePlugin> {
interface ActivePlugin {
suspend fun isActive(): Boolean = true
}
}
interface MyPlugin
interface MyPluginActivePlugin : ActivePluginPoint.ActivePlugin
.files(
PLUGIN_POINT_API,
PLUGIN_POINT_ANNOTATIONS_API,
kt("""
package com.test.plugins
class Duck {
private val pp: PluginPoint<MyPlugin>
fun quack() {
import com.duckduckgo.common.utils.plugins.ActivePlugin
import com.duckduckgo.common.utils.plugins.PluginPoint
import com.duckduckgo.anvil.annotations.ContributesActivePlugin
class Duck {
private val pp: PluginPoint<MyPlugin>
fun quack() {
}
}
}
""").indented())
.issues(WRONG_PLUGIN_POINT_ISSUE)
.run()
Expand Down
47 changes: 47 additions & 0 deletions lint-rules/src/test/java/com/duckduckgo/lint/utils/PluginUtils.kt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* Copyright (c) 2024 DuckDuckGo
*
* 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 com.duckduckgo.lint.utils

import com.android.tools.lint.checks.infrastructure.TestFiles

internal val PLUGIN_POINT_API = TestFiles.kt(
"""
package com.duckduckgo.common.utils.plugins
interface PluginPoint<T> {
fun getPlugins(): Collection<T>
}
interface InternalActivePluginPoint<out T : ActivePluginPoint.ActivePlugin> {
suspend fun getPlugins(): Collection<T>
}
interface ActivePlugin {
suspend fun isActive(): Boolean = true
}
typealias ActivePluginPoint<T> = InternalActivePluginPoint<@JvmSuppressWildcards T>
"""
).indented()
internal val PLUGIN_POINT_ANNOTATIONS_API = TestFiles.kt(
"""
package com.duckduckgo.anvil.annotations
annotation class ContributesActivePlugin
annotation class ContributesActivePluginPoint(
val boundType: KClass<*> = Unit::class,
)
"""
).indented()

0 comments on commit 0115768

Please sign in to comment.