Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 02998af7 authored by Dave Mankoff's avatar Dave Mankoff Committed by Android (Google) Code Review
Browse files

Merge "Error Linter for Marking Activities and Services and Singleton" into main

parents 07e07a44 e1492243
Loading
Loading
Loading
Loading
+2 −1
Original line number Diff line number Diff line
@@ -38,8 +38,9 @@ java_test_host {
    defaults: ["AndroidLintCheckerTestDefaults"],
    srcs: ["tests/**/*.kt"],
    data: [
        ":framework",
        ":androidx.annotation_annotation",
        ":dagger2",
        ":framework",
        ":kotlinx-coroutines-core",
    ],
    static_libs: [
+160 −0
Original line number Diff line number Diff line
/*
 * Copyright (C) 2024 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
 *
 *      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.android.internal.systemui.lint

import com.android.tools.lint.detector.api.AnnotationInfo
import com.android.tools.lint.detector.api.AnnotationUsageInfo
import com.android.tools.lint.detector.api.AnnotationUsageType
import com.android.tools.lint.detector.api.Category
import com.android.tools.lint.detector.api.Detector
import com.android.tools.lint.detector.api.Implementation
import com.android.tools.lint.detector.api.Issue
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 com.android.tools.lint.detector.api.SourceCodeScanner
import org.jetbrains.uast.UAnnotation
import org.jetbrains.uast.UClass
import org.jetbrains.uast.UElement
import org.jetbrains.uast.UMethod

/**
 * Prevents binding Activities, Services, and BroadcastReceivers as Singletons in the Dagger graph.
 *
 * It is OK to mark a BroadcastReceiver as singleton as long as it is being constructed/injected and
 * registered directly in the code. If instead it is declared in the manifest, and we let Android
 * construct it for us, we also need to let Android destroy it for us, so don't allow marking it as
 * singleton.
 */
class SingletonAndroidComponentDetector : Detector(), SourceCodeScanner {
    override fun applicableAnnotations(): List<String> {
        return listOf(
            "com.android.systemui.dagger.SysUISingleton",
        )
    }

    override fun isApplicableAnnotationUsage(type: AnnotationUsageType): Boolean =
        type == AnnotationUsageType.DEFINITION

    override fun visitAnnotationUsage(
        context: JavaContext,
        element: UElement,
        annotationInfo: AnnotationInfo,
        usageInfo: AnnotationUsageInfo
    ) {
        if (element !is UAnnotation) {
            return
        }

        val parent = element.uastParent ?: return

        if (isInvalidBindingMethod(parent)) {
            context.report(
                ISSUE,
                element,
                context.getLocation(element),
                "Do not bind Activities, Services, or BroadcastReceivers as Singleton."
            )
        } else if (isInvalidClassDeclaration(parent)) {
            context.report(
                ISSUE,
                element,
                context.getLocation(element),
                "Do not mark Activities or Services as Singleton."
            )
        }
    }

    private fun isInvalidBindingMethod(parent: UElement): Boolean {
        if (parent !is UMethod) {
            return false
        }

        if (
            parent.returnType?.canonicalText !in
                listOf(
                    "android.app.Activity",
                    "android.app.Service",
                    "android.content.BroadcastReceiver",
                )
        ) {
            return false
        }

        if (
            !MULTIBIND_ANNOTATIONS.all { it in parent.annotations.map { it.qualifiedName } } &&
                !MULTIPROVIDE_ANNOTATIONS.all { it in parent.annotations.map { it.qualifiedName } }
        ) {
            return false
        }
        return true
    }

    private fun isInvalidClassDeclaration(parent: UElement): Boolean {
        if (parent !is UClass) {
            return false
        }

        if (
            parent.javaPsi.superClass?.qualifiedName !in
                listOf(
                    "android.app.Activity",
                    "android.app.Service",
                    // Fine to mark BroadcastReceiver as singleton in this scenario
                )
        ) {
            return false
        }

        return true
    }

    companion object {
        @JvmField
        val ISSUE: Issue =
            Issue.create(
                id = "SingletonAndroidComponent",
                briefDescription = "Activity, Service, or BroadcastReceiver marked as Singleton",
                explanation =
                    """Activities, Services, and BroadcastReceivers are created and destroyed by
                        the Android System Server. Marking them with a Dagger scope
                        results in them being cached and reused by Dagger. Trying to reuse a
                        component like this will make for a very bad time.""",
                category = Category.CORRECTNESS,
                priority = 10,
                severity = Severity.ERROR,
                moreInfo =
                    "https://developer.android.com/guide/components/activities/process-lifecycle",
                // Note that JAVA_FILE_SCOPE also includes Kotlin source files.
                implementation =
                    Implementation(
                        SingletonAndroidComponentDetector::class.java,
                        Scope.JAVA_FILE_SCOPE
                    )
            )

        private val MULTIBIND_ANNOTATIONS =
            listOf("dagger.Binds", "dagger.multibindings.IntoMap", "dagger.multibindings.ClassKey")

        val MULTIPROVIDE_ANNOTATIONS =
            listOf(
                "dagger.Provides",
                "dagger.multibindings.IntoMap",
                "dagger.multibindings.ClassKey"
            )
    }
}
+1 −0
Original line number Diff line number Diff line
@@ -40,6 +40,7 @@ class SystemUIIssueRegistry : IssueRegistry() {
                RegisterReceiverViaContextDetector.ISSUE,
                SoftwareBitmapDetector.ISSUE,
                NonInjectedServiceDetector.ISSUE,
                SingletonAndroidComponentDetector.ISSUE,
                StaticSettingsProviderDetector.ISSUE,
                DemotingTestWithoutBugDetector.ISSUE,
                TestFunctionNameViolationDetector.ISSUE,
+2 −1
Original line number Diff line number Diff line
@@ -21,8 +21,9 @@ import java.io.File

internal val libraryNames =
    arrayOf(
        "framework.jar",
        "androidx.annotation_annotation.jar",
        "dagger2.jar",
        "framework.jar",
        "kotlinx-coroutines-core.jar",
    )

+182 −0
Original line number Diff line number Diff line
/*
 * Copyright (C) 2024 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
 *
 *      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.android.internal.systemui.lint

import com.android.tools.lint.checks.infrastructure.TestFile
import com.android.tools.lint.checks.infrastructure.TestFiles
import com.android.tools.lint.detector.api.Detector
import com.android.tools.lint.detector.api.Issue
import org.junit.Test

class SingletonAndroidComponentDetectorTest : SystemUILintDetectorTest() {
    override fun getDetector(): Detector = SingletonAndroidComponentDetector()

    override fun getIssues(): List<Issue> = listOf(SingletonAndroidComponentDetector.ISSUE)

    @Test
    fun testBindsServiceAsSingleton() {
        lint()
            .files(
                TestFiles.kotlin(
                    """
                    package test.pkg

                    import android.app.Service
                    import com.android.systemui.dagger.SysUISingleton
                    import dagger.Binds
                    import dagger.Module
                    import dagger.multibindings.ClassKey
                    import dagger.multibindings.IntoMap

                    @Module
                    interface BadModule {
                       @SysUISingleton
                       @Binds
                       @IntoMap
                       @ClassKey(SingletonService::class)
                       fun bindSingletonService(service: SingletonService): Service
                    }
                """
                        .trimIndent()
                ),
                *stubs
            )
            .issues(SingletonAndroidComponentDetector.ISSUE)
            .run()
            .expect(
                """
                src/test/pkg/BadModule.kt:12: Error: Do not bind Activities, Services, or BroadcastReceivers as Singleton. [SingletonAndroidComponent]
                   @SysUISingleton
                   ~~~~~~~~~~~~~~~
                1 errors, 0 warnings
                """
                    .trimIndent()
            )
    }

    @Test
    fun testProvidesBroadcastReceiverAsSingleton() {
        lint()
            .files(
                TestFiles.kotlin(
                    """
                    package test.pkg

                    import android.content.BroadcastReceiver
                    import com.android.systemui.dagger.SysUISingleton
                    import dagger.Provides
                    import dagger.Module
                    import dagger.multibindings.ClassKey
                    import dagger.multibindings.IntoMap

                    @Module
                    abstract class BadModule {
                       @SysUISingleton
                       @Provides
                       @IntoMap
                       @ClassKey(SingletonBroadcastReceiver::class)
                       fun providesSingletonBroadcastReceiver(br: SingletonBroadcastReceiver): BroadcastReceiver {
                          return br
                       }
                    }
                """
                        .trimIndent()
                ),
                *stubs
            )
            .issues(SingletonAndroidComponentDetector.ISSUE)
            .run()
            .expect(
                """
                src/test/pkg/BadModule.kt:12: Error: Do not bind Activities, Services, or BroadcastReceivers as Singleton. [SingletonAndroidComponent]
                   @SysUISingleton
                   ~~~~~~~~~~~~~~~
                1 errors, 0 warnings
                """
                    .trimIndent()
            )
    }
    @Test
    fun testMarksActivityAsSingleton() {
        lint()
            .files(
                TestFiles.kotlin(
                    """
                    package test.pkg

                    import android.app.Activity
                    import com.android.systemui.dagger.SysUISingleton

                    @SysUISingleton
                    class BadActivity : Activity() {
                    }
                """
                        .trimIndent()
                ),
                *stubs
            )
            .issues(SingletonAndroidComponentDetector.ISSUE)
            .run()
            .expect(
                """
                src/test/pkg/BadActivity.kt:6: Error: Do not mark Activities or Services as Singleton. [SingletonAndroidComponent]
                @SysUISingleton
                ~~~~~~~~~~~~~~~
                1 errors, 0 warnings
                """
                    .trimIndent()
            )
    }
    @Test
    fun testMarksBroadcastReceiverAsSingleton() {
        lint()
            .files(
                TestFiles.kotlin(
                    """
                    package test.pkg

                    import android.content.BroadcastReceiver
                    import com.android.systemui.dagger.SysUISingleton

                    @SysUISingleton
                    class SingletonReceveiver : BroadcastReceiver() {
                    }
                """
                        .trimIndent()
                ),
                *stubs
            )
            .issues(SingletonAndroidComponentDetector.ISSUE)
            .run()
            .expectClean()
    }

    // Define stubs for Android imports. The tests don't run on Android so
    // they don't "see" any of Android specific classes. We need to define
    // the method parameters for proper resolution.
    private val singletonStub: TestFile =
        java(
            """
        package com.android.systemui.dagger;

        public @interface SysUISingleton {
        }
        """
        )

    private val stubs = arrayOf(singletonStub) + androidStubs
}