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

Commit e0afa566 authored by Peter Kalauskas's avatar Peter Kalauskas
Browse files

New lint checks for static Settings.* usages 

New lint checks for static put/get methods in Settings.Global,
Settings.Secure, and Settings.System.

Also, create a common super class where we confiure the SDK settings.

Test: SystemUILintCheckerTest
Bug: 238923086
Change-Id: I42b16510c9ec631dcc339d61ffe3ce53b99ee9d0
parent 305c452b

packages/SystemUI/checks/src/com/android/internal/systemui/lint/StaticSettingsProviderDetector.kt

0 → 100644
+100 −0
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2022 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.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 com.intellij.psi.PsiMethod

import org.jetbrains.uast.UCallExpression



private const val CLASS_SETTINGS = "android.provider.Settings"



/**

 * Detects usage of static methods in android.provider.Settings and suggests to use an injected

 * settings provider instance instead.

 */

@Suppress("UnstableApiUsage")

class StaticSettingsProviderDetector : Detector(), SourceCodeScanner {

    override fun getApplicableMethodNames(): List<String> {

        return listOf(

            "getFloat",

            "getInt",

            "getLong",

            "getString",

            "getUriFor",

            "putFloat",

            "putInt",

            "putLong",

            "putString"

        )

    }



    override fun visitMethodCall(context: JavaContext, node: UCallExpression, method: PsiMethod) {

        val evaluator = context.evaluator

        val className = method.containingClass?.qualifiedName

        if (

            className != "$CLASS_SETTINGS.Global" &&

                className != "$CLASS_SETTINGS.Secure" &&

                className != "$CLASS_SETTINGS.System"

        ) {

            return

        }

        if (!evaluator.isStatic(method)) {

            return

        }



        val subclassName = className.substring(CLASS_SETTINGS.length + 1)



        context.report(

            ISSUE,

            method,

            context.getNameLocation(node),

            "`@Inject` a ${subclassName}Settings instead"

        )

    }



    companion object {

        @JvmField

        val ISSUE: Issue =

            Issue.create(

                id = "StaticSettingsProvider",

                briefDescription = "Static settings provider usage",

                explanation =

                    """

                    Static settings provider methods, such as `Settings.Global.putInt()`, should \

                    not be used because they make testing difficult. Instead, use an injected \

                    settings provider. For example, instead of calling `Settings.Secure.getInt()`, \

                    annotate the class constructor with `@Inject` and add `SecureSettings` to the \

                    parameters.

                    """,

                category = Category.CORRECTNESS,

                priority = 8,

                severity = Severity.WARNING,

                implementation =

                    Implementation(

                        StaticSettingsProviderDetector::class.java,

                        Scope.JAVA_FILE_SCOPE

                    )

            )

    }

}

packages/SystemUI/checks/src/com/android/internal/systemui/lint/SystemUIIssueRegistry.kt

+1 −0
Original line number Diff line number Diff line
@@ -36,6 +36,7 @@ class SystemUIIssueRegistry : IssueRegistry() { 
                RegisterReceiverViaContextDetector.ISSUE,

                SoftwareBitmapDetector.ISSUE,

                NonInjectedServiceDetector.ISSUE,

                StaticSettingsProviderDetector.ISSUE

        )



    override val api: Int

packages/SystemUI/checks/tests/com/android/internal/systemui/lint/AndroidStubs.kt

+65 −0
Original line number Diff line number Diff line
@@ -24,6 +24,47 @@ import org.intellij.lang.annotations.Language 
@NonNull

private fun indentedJava(@NonNull @Language("JAVA") source: String) = java(source).indented()



internal val commonSettingsCode =

    """

public static float getFloat(ContentResolver cr, String name) { return 0.0f; }

public static long getLong(ContentResolver cr, String name) {

    return 0L;

}

public static int getInt(ContentResolver cr, String name) {

    return 0;

}

public static String getString(ContentResolver cr, String name) {

    return "";

}

public static float getFloat(ContentResolver cr, String name, float def) {

    return 0.0f;

}

public static long getLong(ContentResolver cr, String name, long def) {

    return 0L;

}

public static int getInt(ContentResolver cr, String name, int def) {

    return 0;

}

public static String getString(ContentResolver cr, String name, String def) {

    return "";

}

public static boolean putFloat(ContentResolver cr, String name, float value) {

    return true;

}

public static boolean putLong(ContentResolver cr, String name, long value) {

    return true;

}

public static boolean putInt(ContentResolver cr, String name, int value) {

    return true;

}

public static boolean putFloat(ContentResolver cr, String name) {

    return true;

}

public static boolean putString(ContentResolver cr, String name, String value) {

    return true;

}

"""



/*

 * This file contains stubs of framework APIs and System UI classes for testing purposes only. The

 * stubs are not used in the lint detectors themselves.
@@ -184,6 +225,30 @@ import static java.lang.annotation.RetentionPolicy.SOURCE; 
@Target({METHOD,CONSTRUCTOR,TYPE,PARAMETER})

public @interface WorkerThread {

}

"""

        ),

        indentedJava(

            """

package android.provider;



public class Settings {

    public static final class Global {

        public static final String UNLOCK_SOUND = "unlock_sound";

        """ +

                commonSettingsCode +

                """

    }

    public static final class Secure {

    """ +

                commonSettingsCode +

                """

    }

    public static final class System {

    """ +

                commonSettingsCode +

                """

    }

}

"""

        ),

    )

packages/SystemUI/checks/tests/com/android/internal/systemui/lint/BindServiceOnMainThreadDetectorTest.kt

+1 −4
Original line number Diff line number Diff line
@@ -16,18 +16,15 @@ 


package com.android.internal.systemui.lint



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

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

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

import com.android.tools.lint.detector.api.Detector

import com.android.tools.lint.detector.api.Issue

import org.junit.Test



@Suppress("UnstableApiUsage")

class BindServiceOnMainThreadDetectorTest : LintDetectorTest() {

class BindServiceOnMainThreadDetectorTest : SystemUILintDetectorTest() {



    override fun getDetector(): Detector = BindServiceOnMainThreadDetector()

    override fun lint(): TestLintTask = super.lint().allowMissingSdk(true)



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

packages/SystemUI/checks/tests/com/android/internal/systemui/lint/BroadcastSentViaContextDetectorTest.kt

+1 −4
Original line number Diff line number Diff line
@@ -16,18 +16,15 @@ 


package com.android.internal.systemui.lint



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

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

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

import com.android.tools.lint.detector.api.Detector

import com.android.tools.lint.detector.api.Issue

import org.junit.Test



@Suppress("UnstableApiUsage")

class BroadcastSentViaContextDetectorTest : LintDetectorTest() {

class BroadcastSentViaContextDetectorTest : SystemUILintDetectorTest() {



    override fun getDetector(): Detector = BroadcastSentViaContextDetector()

    override fun lint(): TestLintTask = super.lint().allowMissingSdk(true)



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