diff --git a/tools/lint/checks/src/main/java/com/google/android/lint/AndroidFrameworkIssueRegistry.kt b/tools/lint/checks/src/main/java/com/google/android/lint/AndroidFrameworkIssueRegistry.kt index 5736a8082ca3d..900c2145975a1 100644 --- a/tools/lint/checks/src/main/java/com/google/android/lint/AndroidFrameworkIssueRegistry.kt +++ b/tools/lint/checks/src/main/java/com/google/android/lint/AndroidFrameworkIssueRegistry.kt @@ -30,7 +30,8 @@ class AndroidFrameworkIssueRegistry : IssueRegistry() { CallingIdentityTokenDetector.ISSUE_NESTED_CLEAR_IDENTITY_CALLS, CallingIdentityTokenDetector.ISSUE_RESTORE_IDENTITY_CALL_NOT_IN_FINALLY_BLOCK, CallingIdentityTokenDetector.ISSUE_USE_OF_CALLER_AWARE_METHODS_WITH_CLEARED_IDENTITY, - CallingIdentityTokenDetector.ISSUE_CLEAR_IDENTITY_CALL_NOT_FOLLOWED_BY_TRY_FINALLY + CallingIdentityTokenDetector.ISSUE_CLEAR_IDENTITY_CALL_NOT_FOLLOWED_BY_TRY_FINALLY, + CallingSettingsNonUserGetterMethodsDetector.ISSUE_NON_USER_GETTER_CALLED ) override val api: Int diff --git a/tools/lint/checks/src/main/java/com/google/android/lint/CallingSettingsNonUserGetterMethodsDetector.kt b/tools/lint/checks/src/main/java/com/google/android/lint/CallingSettingsNonUserGetterMethodsDetector.kt new file mode 100644 index 0000000000000..641f337ab9870 --- /dev/null +++ b/tools/lint/checks/src/main/java/com/google/android/lint/CallingSettingsNonUserGetterMethodsDetector.kt @@ -0,0 +1,82 @@ +/* + * Copyright (C) 2021 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.google.android.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 + +/** + * Lint Detector that finds issues with improper usages of the non-user getter methods of Settings + */ +@Suppress("UnstableApiUsage") +class CallingSettingsNonUserGetterMethodsDetector : Detector(), SourceCodeScanner { + override fun getApplicableMethodNames(): List = listOf( + "getString", + "getInt", + "getLong", + "getFloat" + ) + + override fun visitMethodCall(context: JavaContext, node: UCallExpression, method: PsiMethod) { + val evaluator = context.evaluator + if (evaluator.isMemberInClass(method, "android.provider.Settings.Secure") || + evaluator.isMemberInClass(method, "android.provider.Settings.System") + ) { + val message = getIncidentMessageNonUserGetterMethods(getMethodSignature(method)) + context.report(ISSUE_NON_USER_GETTER_CALLED, node, context.getLocation(node), message) + } + } + + private fun getMethodSignature(method: PsiMethod) = + method.containingClass + ?.qualifiedName + ?.let { "$it#${method.name}" } + ?: method.name + + companion object { + @JvmField + val ISSUE_NON_USER_GETTER_CALLED: Issue = Issue.create( + id = "NonUserGetterCalled", + briefDescription = "Non-ForUser Getter Method called to Settings", + explanation = """ + System process should not call the non-ForUser getter methods of \ + `Settings.Secure` or `Settings.System`. For example, instead of \ + `Settings.Secure.getInt()`, use `Settings.Secure.getIntForUser()` instead. \ + This will make sure that the correct Settings value is retrieved. + """, + category = Category.CORRECTNESS, + priority = 6, + severity = Severity.WARNING, + implementation = Implementation( + CallingSettingsNonUserGetterMethodsDetector::class.java, + Scope.JAVA_FILE_SCOPE + ) + ) + + fun getIncidentMessageNonUserGetterMethods(methodSignature: String) = + "`$methodSignature()` called from system process. " + + "Please call `${methodSignature}ForUser()` instead. " + } +} diff --git a/tools/lint/checks/src/test/java/com/google/android/lint/CallingSettingsNonUserGetterMethodsIssueDetectorTest.kt b/tools/lint/checks/src/test/java/com/google/android/lint/CallingSettingsNonUserGetterMethodsIssueDetectorTest.kt new file mode 100644 index 0000000000000..1034029f6e9d8 --- /dev/null +++ b/tools/lint/checks/src/test/java/com/google/android/lint/CallingSettingsNonUserGetterMethodsIssueDetectorTest.kt @@ -0,0 +1,217 @@ +/* + * Copyright (C) 2021 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.google.android.lint + +import com.android.tools.lint.checks.infrastructure.LintDetectorTest +import com.android.tools.lint.checks.infrastructure.TestFile +import com.android.tools.lint.checks.infrastructure.TestLintTask +import com.android.tools.lint.detector.api.Detector +import com.android.tools.lint.detector.api.Issue + +@Suppress("UnstableApiUsage") +class CallingSettingsNonUserGetterMethodsIssueDetectorTest : LintDetectorTest() { + override fun getDetector(): Detector = CallingSettingsNonUserGetterMethodsDetector() + + override fun getIssues(): List = listOf( + CallingSettingsNonUserGetterMethodsDetector.ISSUE_NON_USER_GETTER_CALLED + ) + + override fun lint(): TestLintTask = super.lint().allowMissingSdk(true) + + fun testDoesNotDetectIssues() { + lint().files( + java( + """ + package test.pkg; + import android.provider.Settings.Secure; + public class TestClass1 { + private void testMethod(Context context) { + final int value = Secure.getIntForUser(context.getContentResolver(), + Settings.Secure.KEY1, 0, 0); + } + } + """ + ).indented(), + *stubs + ) + .run() + .expectClean() + } + + fun testDetectsNonUserGetterCalledFromSecure() { + lint().files( + java( + """ + package test.pkg; + import android.provider.Settings.Secure; + public class TestClass1 { + private void testMethod(Context context) { + final int value = Secure.getInt(context.getContentResolver(), + Settings.Secure.KEY1); + } + } + """ + ).indented(), + *stubs + ) + .run() + .expect( + """ + src/test/pkg/TestClass1.java:5: Warning: \ + android.provider.Settings.Secure#getInt() called from system process. \ + Please call android.provider.Settings.Secure#getIntForUser() instead. \ + [NonUserGetterCalled] + final int value = Secure.getInt(context.getContentResolver(), + ^ + 0 errors, 1 warnings + """.addLineContinuation() + ) + } + fun testDetectsNonUserGetterCalledFromSystem() { + lint().files( + java( + """ + package test.pkg; + import android.provider.Settings.System; + public class TestClass1 { + private void testMethod(Context context) { + final float value = System.getFloat(context.getContentResolver(), + Settings.System.KEY1); + } + } + """ + ).indented(), + *stubs + ) + .run() + .expect( + """ + src/test/pkg/TestClass1.java:5: Warning: \ + android.provider.Settings.System#getFloat() called from system process. \ + Please call android.provider.Settings.System#getFloatForUser() instead. \ + [NonUserGetterCalled] + final float value = System.getFloat(context.getContentResolver(), + ^ + 0 errors, 1 warnings + """.addLineContinuation() + ) + } + + fun testDetectsNonUserGetterCalledFromSettings() { + lint().files( + java( + """ + package test.pkg; + import android.provider.Settings; + public class TestClass1 { + private void testMethod(Context context) { + float value = Settings.System.getFloat(context.getContentResolver(), + Settings.System.KEY1); + } + } + """ + ).indented(), + *stubs + ) + .run() + .expect( + """ + src/test/pkg/TestClass1.java:5: Warning: \ + android.provider.Settings.System#getFloat() called from system process. \ + Please call android.provider.Settings.System#getFloatForUser() instead. \ + [NonUserGetterCalled] + float value = Settings.System.getFloat(context.getContentResolver(), + ^ + 0 errors, 1 warnings + """.addLineContinuation() + ) + } + + fun testDetectsNonUserGettersCalledFromSystemAndSecure() { + lint().files( + java( + """ + package test.pkg; + import android.provider.Settings.Secure; + import android.provider.Settings.System; + public class TestClass1 { + private void testMethod(Context context) { + final long value1 = Secure.getLong(context.getContentResolver(), + Settings.Secure.KEY1, 0); + final String value2 = System.getString(context.getContentResolver(), + Settings.System.KEY2); + } + } + """ + ).indented(), + *stubs + ) + .run() + .expect( + """ + src/test/pkg/TestClass1.java:6: Warning: \ + android.provider.Settings.Secure#getLong() called from system process. \ + Please call android.provider.Settings.Secure#getLongForUser() instead. \ + [NonUserGetterCalled] + final long value1 = Secure.getLong(context.getContentResolver(), + ^ + src/test/pkg/TestClass1.java:8: Warning: \ + android.provider.Settings.System#getString() called from system process. \ + Please call android.provider.Settings.System#getStringForUser() instead. \ + [NonUserGetterCalled] + final String value2 = System.getString(context.getContentResolver(), + ^ + 0 errors, 2 warnings + """.addLineContinuation() + ) + } + + private val SettingsStub: TestFile = java( + """ + package android.provider; + public class Settings { + public class Secure { + float getFloat(ContentResolver cr, String key) { + return 0.0f; + } + long getLong(ContentResolver cr, String key) { + return 0l; + } + int getInt(ContentResolver cr, String key) { + return 0; + } + } + public class System { + float getFloat(ContentResolver cr, String key) { + return 0.0f; + } + long getLong(ContentResolver cr, String key) { + return 0l; + } + String getString(ContentResolver cr, String key) { + return null; + } + } + } + """ + ).indented() + + private val stubs = arrayOf(SettingsStub) + + // Substitutes "backslash + new line" with an empty string to imitate line continuation + private fun String.addLineContinuation(): String = this.trimIndent().replace("\\\n", "") +}