From eb0712d9b4b8a6f04c0586f7e88316ec98795de5 Mon Sep 17 00:00:00 2001
From: "Juan C. Goncalves" <juan.goncalves498@gmail.com>
Date: Tue, 20 Oct 2020 16:23:43 -0400
Subject: [PATCH] For #11397 - Add lint rule for AppCompatResources instead of
 ContextCompat (#16011)

---
 .../mozilla/fenix/browser/BrowserFragment.kt  |   3 +-
 .../readermode/ReaderModeController.kt        |   6 +-
 .../providers/MasterPasswordTipProvider.kt    |   4 +-
 .../RecentlyClosedFragmentView.kt             |   4 +-
 .../fenix/search/SearchDialogFragment.kt      |   4 +-
 mozilla-lint-rules/build.gradle               |   1 +
 .../fenix/lintrules/ContextCompatDetector.kt  | 103 +++++++++++
 .../fenix/lintrules/LintIssueRegistry.kt      |   2 +-
 .../lintrules/ContextCompatDetectorTest.kt    | 163 ++++++++++++++++++
 9 files changed, 278 insertions(+), 12 deletions(-)
 create mode 100644 mozilla-lint-rules/src/main/java/org/mozilla/fenix/lintrules/ContextCompatDetector.kt
 create mode 100644 mozilla-lint-rules/src/test/java/org/mozilla/fenix/lintrules/ContextCompatDetectorTest.kt

diff --git a/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt b/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt
index 1642afe09d..ebe364c3fe 100644
--- a/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt
+++ b/app/src/main/java/org/mozilla/fenix/browser/BrowserFragment.kt
@@ -11,7 +11,6 @@ import android.view.LayoutInflater
 import android.view.View
 import android.view.ViewGroup
 import androidx.appcompat.content.res.AppCompatResources
-import androidx.core.content.ContextCompat
 import androidx.lifecycle.Observer
 import androidx.navigation.fragment.findNavController
 import com.google.android.material.snackbar.Snackbar
@@ -86,7 +85,7 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler {
 
             val readerModeAction =
                 BrowserToolbar.ToggleButton(
-                    image = ContextCompat.getDrawable(requireContext(), R.drawable.ic_readermode)!!,
+                    image = AppCompatResources.getDrawable(requireContext(), R.drawable.ic_readermode)!!,
                     imageSelected =
                         AppCompatResources.getDrawable(requireContext(), R.drawable.ic_readermode_selected)!!,
                     contentDescription = requireContext().getString(R.string.browser_menu_read),
diff --git a/app/src/main/java/org/mozilla/fenix/browser/readermode/ReaderModeController.kt b/app/src/main/java/org/mozilla/fenix/browser/readermode/ReaderModeController.kt
index 4f1de82d04..478f5dc89a 100644
--- a/app/src/main/java/org/mozilla/fenix/browser/readermode/ReaderModeController.kt
+++ b/app/src/main/java/org/mozilla/fenix/browser/readermode/ReaderModeController.kt
@@ -7,7 +7,7 @@ package org.mozilla.fenix.browser.readermode
 import android.view.View
 import android.widget.Button
 import android.widget.RadioButton
-import androidx.core.content.ContextCompat
+import androidx.appcompat.content.res.AppCompatResources
 import mozilla.components.feature.readerview.ReaderViewFeature
 import mozilla.components.support.base.feature.ViewBoundFeatureWrapper
 import org.mozilla.fenix.R
@@ -54,7 +54,7 @@ class DefaultReaderModeController(
             findViewById<Button>(it)
         }.forEach {
             it.setTextColor(
-                ContextCompat.getColorStateList(
+                AppCompatResources.getColorStateList(
                     context,
                     R.color.readerview_private_button_color
                 )
@@ -68,7 +68,7 @@ class DefaultReaderModeController(
             findViewById<RadioButton>(it)
         }.forEach {
             it.setTextColor(
-                ContextCompat.getColorStateList(
+                AppCompatResources.getColorStateList(
                     context,
                     R.color.readerview_private_radio_color
                 )
diff --git a/app/src/main/java/org/mozilla/fenix/components/tips/providers/MasterPasswordTipProvider.kt b/app/src/main/java/org/mozilla/fenix/components/tips/providers/MasterPasswordTipProvider.kt
index 269e0ca39a..0c28b2c187 100644
--- a/app/src/main/java/org/mozilla/fenix/components/tips/providers/MasterPasswordTipProvider.kt
+++ b/app/src/main/java/org/mozilla/fenix/components/tips/providers/MasterPasswordTipProvider.kt
@@ -9,7 +9,7 @@ import android.text.Editable
 import android.text.TextWatcher
 import android.view.LayoutInflater
 import androidx.appcompat.app.AlertDialog
-import androidx.core.content.ContextCompat
+import androidx.appcompat.content.res.AppCompatResources
 import androidx.core.view.isVisible
 import com.google.android.material.button.MaterialButton
 import com.google.android.material.textfield.TextInputEditText
@@ -73,7 +73,7 @@ class MasterPasswordTipProvider(
             title = context.getString(R.string.mp_homescreen_tip_title),
             description = context.getString(R.string.mp_homescreen_tip_message),
             learnMoreURL = null,
-            titleDrawable = ContextCompat.getDrawable(context, R.drawable.ic_login)
+            titleDrawable = AppCompatResources.getDrawable(context, R.drawable.ic_login)
         )
 
     private fun showMasterPasswordMigration() {
diff --git a/app/src/main/java/org/mozilla/fenix/library/recentlyclosed/RecentlyClosedFragmentView.kt b/app/src/main/java/org/mozilla/fenix/library/recentlyclosed/RecentlyClosedFragmentView.kt
index 0ec06e48e8..d6297c13bd 100644
--- a/app/src/main/java/org/mozilla/fenix/library/recentlyclosed/RecentlyClosedFragmentView.kt
+++ b/app/src/main/java/org/mozilla/fenix/library/recentlyclosed/RecentlyClosedFragmentView.kt
@@ -6,8 +6,8 @@ package org.mozilla.fenix.library.recentlyclosed
 
 import android.view.LayoutInflater
 import android.view.ViewGroup
+import androidx.appcompat.content.res.AppCompatResources
 import androidx.constraintlayout.widget.ConstraintLayout
-import androidx.core.content.ContextCompat
 import androidx.core.view.isVisible
 import androidx.recyclerview.widget.LinearLayoutManager
 import kotlinx.android.extensions.LayoutContainer
@@ -91,7 +91,7 @@ class RecentlyClosedFragmentView(
             overflowView.isVisible = false
             iconView.background = null
             iconView.setImageDrawable(
-                ContextCompat.getDrawable(
+                AppCompatResources.getDrawable(
                     containerView.context,
                     R.drawable.ic_history
                 )
diff --git a/app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt b/app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt
index 41910ff5dd..3540c697ee 100644
--- a/app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt
+++ b/app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt
@@ -21,11 +21,11 @@ import android.view.ViewStub
 import android.view.WindowManager
 import androidx.appcompat.app.AlertDialog
 import androidx.appcompat.app.AppCompatDialogFragment
+import androidx.appcompat.content.res.AppCompatResources
 import androidx.constraintlayout.widget.ConstraintProperties.BOTTOM
 import androidx.constraintlayout.widget.ConstraintProperties.PARENT_ID
 import androidx.constraintlayout.widget.ConstraintProperties.TOP
 import androidx.constraintlayout.widget.ConstraintSet
-import androidx.core.content.ContextCompat
 import androidx.core.view.isVisible
 import androidx.navigation.fragment.findNavController
 import androidx.navigation.fragment.navArgs
@@ -431,7 +431,7 @@ class SearchDialogFragment : AppCompatDialogFragment(), UserInteractionHandler {
     private fun addSearchButton(toolbarView: ToolbarView) {
         toolbarView.view.addEditAction(
             BrowserToolbar.Button(
-                ContextCompat.getDrawable(requireContext(), R.drawable.ic_microphone)!!,
+                AppCompatResources.getDrawable(requireContext(), R.drawable.ic_microphone)!!,
                 requireContext().getString(R.string.voice_search_content_description),
                 visible = {
                     store.state.searchEngineSource.searchEngine.identifier.contains("google") &&
diff --git a/mozilla-lint-rules/build.gradle b/mozilla-lint-rules/build.gradle
index 30c4e87a70..29587123e4 100644
--- a/mozilla-lint-rules/build.gradle
+++ b/mozilla-lint-rules/build.gradle
@@ -21,6 +21,7 @@ dependencies {
     testImplementation Deps.kotlin_reflect
     testImplementation "com.android.tools.lint:lint:${Versions.android_lint_api}"
     testImplementation "com.android.tools.lint:lint-tests:${Versions.android_lint_api}"
+    testImplementation Deps.junitApi
 }
 
 jar {
diff --git a/mozilla-lint-rules/src/main/java/org/mozilla/fenix/lintrules/ContextCompatDetector.kt b/mozilla-lint-rules/src/main/java/org/mozilla/fenix/lintrules/ContextCompatDetector.kt
new file mode 100644
index 0000000000..481276c0d8
--- /dev/null
+++ b/mozilla-lint-rules/src/main/java/org/mozilla/fenix/lintrules/ContextCompatDetector.kt
@@ -0,0 +1,103 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package org.mozilla.fenix.lintrules
+
+import com.android.tools.lint.detector.api.*
+import com.intellij.psi.PsiMethod
+import org.jetbrains.uast.UCallExpression
+
+class ContextCompatDetector : Detector(), SourceCodeScanner {
+
+    companion object {
+
+        const val FULLY_QUALIFIED_CONTEXT_COMPAT = "androidx.core.content.ContextCompat"
+
+        private val Implementation = Implementation(
+            ContextCompatDetector::class.java,
+            Scope.JAVA_FILE_SCOPE
+        )
+
+        val ISSUE_GET_DRAWABLE_CALL = Issue.create(
+            id = "UnsafeCompatGetDrawable",
+            briefDescription = "Prohibits using the ContextCompat.getDrawable method",
+            explanation = "Using this method can lead to crashes in older Android versions as newer features might not be available",
+            category = Category.CORRECTNESS,
+            severity = Severity.ERROR,
+            implementation = Implementation
+        )
+
+        val ISSUE_GET_COLOR_STATE_LIST_CALL = Issue.create(
+            id = "UnsafeCompatGetColorStateList",
+            briefDescription = "Prohibits using the ContextCompat.getColorStateList method",
+            explanation = "Using this method can lead to crashes in older Android versions as newer features might not be available",
+            category = Category.CORRECTNESS,
+            severity = Severity.ERROR,
+            implementation = Implementation
+        )
+
+        val ISSUES = listOf(
+            ISSUE_GET_DRAWABLE_CALL,
+            ISSUE_GET_COLOR_STATE_LIST_CALL
+        )
+    }
+
+    override fun getApplicableMethodNames(): List<String>? = listOf(
+        "getDrawable",
+        "getColorStateList"
+    )
+
+    override fun visitMethodCall(context: JavaContext, node: UCallExpression, method: PsiMethod) {
+        val evaluator = context.evaluator
+
+        if (!evaluator.isMemberInClass(method, FULLY_QUALIFIED_CONTEXT_COMPAT)) {
+            return
+        }
+
+        when (node.methodName) {
+            "getDrawable" -> reportGetDrawableCall(context, node)
+            "getColorStateList" -> reportGetColorStateListCall(context, node)
+        }
+    }
+
+    private fun reportGetDrawableCall(context: JavaContext, node: UCallExpression) = context.report(
+        ISSUE_GET_DRAWABLE_CALL,
+        context.getLocation(node),
+        "This call can lead to crashes, replace with AppCompatResources.getDrawable",
+        replaceUnsafeGetDrawableQuickFix(node)
+    )
+
+    private fun reportGetColorStateListCall(context: JavaContext, node: UCallExpression) =
+        context.report(
+            ISSUE_GET_COLOR_STATE_LIST_CALL,
+            context.getLocation(node),
+            "This call can lead to crashes, replace with AppCompatResources.getColorStateList",
+            replaceUnsafeGetColorStateListCallQuickFix(node)
+        )
+
+    private fun replaceUnsafeGetDrawableQuickFix(node: UCallExpression): LintFix {
+        val arguments = node.valueArguments.joinToString { it.asSourceString() }
+        val newText = "AppCompatResources.getDrawable($arguments)"
+
+        return LintFix.create()
+            .name("Replace with AppCompatResources.getDrawable")
+            .replace()
+            .all()
+            .with(newText)
+            .build()
+    }
+
+    private fun replaceUnsafeGetColorStateListCallQuickFix(node: UCallExpression): LintFix {
+        val arguments = node.valueArguments.joinToString { it.asSourceString() }
+        val newText = "AppCompatResources.getColorStateList($arguments)"
+
+        return LintFix.create()
+            .name("Replace with AppCompatResources.getColorStateList")
+            .replace()
+            .all()
+            .with(newText)
+            .build()
+    }
+
+}
\ No newline at end of file
diff --git a/mozilla-lint-rules/src/main/java/org/mozilla/fenix/lintrules/LintIssueRegistry.kt b/mozilla-lint-rules/src/main/java/org/mozilla/fenix/lintrules/LintIssueRegistry.kt
index fa2ed2f659..ebbc63e273 100644
--- a/mozilla-lint-rules/src/main/java/org/mozilla/fenix/lintrules/LintIssueRegistry.kt
+++ b/mozilla-lint-rules/src/main/java/org/mozilla/fenix/lintrules/LintIssueRegistry.kt
@@ -20,5 +20,5 @@ class LintIssueRegistry : IssueRegistry() {
         ImageViewAndroidTintXmlDetector.ISSUE_XML_SRC_USAGE,
         LicenseDetector.ISSUE_MISSING_LICENSE,
         LicenseDetector.ISSUE_INVALID_LICENSE_FORMAT
-    ) + ConstraintLayoutPerfDetector.ISSUES
+    ) + ConstraintLayoutPerfDetector.ISSUES + ContextCompatDetector.ISSUES
 }
diff --git a/mozilla-lint-rules/src/test/java/org/mozilla/fenix/lintrules/ContextCompatDetectorTest.kt b/mozilla-lint-rules/src/test/java/org/mozilla/fenix/lintrules/ContextCompatDetectorTest.kt
new file mode 100644
index 0000000000..658845be71
--- /dev/null
+++ b/mozilla-lint-rules/src/test/java/org/mozilla/fenix/lintrules/ContextCompatDetectorTest.kt
@@ -0,0 +1,163 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package org.mozilla.fenix.lintrules
+
+import com.android.tools.lint.checks.infrastructure.LintDetectorTest
+import com.android.tools.lint.detector.api.Detector
+import com.android.tools.lint.detector.api.Issue
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.JUnit4
+
+@RunWith(JUnit4::class)
+class ContextCompatDetectorTest : LintDetectorTest() {
+
+    override fun getDetector(): Detector = ContextCompatDetector()
+
+    override fun getIssues(): MutableList<Issue> = ContextCompatDetector.ISSUES.toMutableList()
+
+    @Test
+    fun `report error when the ContextCompat getDrawable method is called`() {
+        val code = """
+            |package example
+            |
+            |import androidx.core.content.ContextCompat
+            |import android.content.Context
+            |
+            |class Fake {
+            |
+            |   fun trigger(context: Context, id: Int) {
+            |       ContextCompat.getDrawable(context, id)
+            |   }
+            |
+            |}""".trimMargin()
+
+        val expectedReport = """
+            |src/example/Fake.kt:9: Error: This call can lead to crashes, replace with AppCompatResources.getDrawable [UnsafeCompatGetDrawable]
+            |       ContextCompat.getDrawable(context, id)
+            |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+            |1 errors, 0 warnings""".trimMargin()
+
+        val expectedFixOutput = """
+            |Fix for src/example/Fake.kt line 9: Replace with AppCompatResources.getDrawable:
+            |@@ -9 +9
+            |-        ContextCompat.getDrawable(context, id)
+            |+        AppCompatResources.getDrawable(context, id)""".trimMargin()
+
+        lint()
+            .files(Context, ContextCompat, kotlin(code))
+            .allowMissingSdk(true)
+            .run()
+            .expect(expectedReport)
+            .expectFixDiffs(expectedFixOutput)
+    }
+
+    @Test
+    fun `report error when the ContextCompat getColorStateList method is called`() {
+        val code = """
+            |package example
+            |
+            |import androidx.core.content.ContextCompat
+            |import android.content.Context
+            |
+            |class Fake {
+            |
+            |   fun trigger(context: Context, id: Int) {
+            |       ContextCompat.getColorStateList(context, id)
+            |   }
+            |
+            |}""".trimMargin()
+
+        val expectedReport = """
+            |src/example/Fake.kt:9: Error: This call can lead to crashes, replace with AppCompatResources.getColorStateList [UnsafeCompatGetColorStateList]
+            |       ContextCompat.getColorStateList(context, id)
+            |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+            |1 errors, 0 warnings""".trimMargin()
+
+        val expectedFixOutput = """
+            |Fix for src/example/Fake.kt line 9: Replace with AppCompatResources.getColorStateList:
+            |@@ -9 +9
+            |-        ContextCompat.getColorStateList(context, id)
+            |+        AppCompatResources.getColorStateList(context, id)""".trimMargin()
+
+        lint()
+            .files(Context, ContextCompat, kotlin(code))
+            .allowMissingSdk(true)
+            .run()
+            .expect(expectedReport)
+            .expectFixDiffs(expectedFixOutput)
+    }
+
+    @Test
+    fun `does not report any errors if the getDrawable method being called is not from the ContextCompat class`() {
+        val code = """
+            |package example
+            |
+            |import android.content.Context
+            |
+            |class Fake {
+            |
+            |   fun trigger(context: Context, id: Int) {
+            |       getDrawable(context, id)
+            |   }
+            |   
+            |   fun getDrawable(context: Context, id: Int) {}
+            |
+            |}""".trimMargin()
+
+        lint()
+            .files(Context, kotlin(code))
+            .allowMissingSdk(true)
+            .run()
+            .expectClean()
+    }
+
+    @Test
+    fun `does not report any errors if the getColorStateList method being called is not from the ContextCompat class`() {
+        val code = """
+            |package example
+            |
+            |import android.content.Context
+            |
+            |class Fake {
+            |
+            |   fun trigger(context: Context, id: Int) {
+            |       getColorStateList(context, id)
+            |   }
+            |   
+            |   fun getColorStateList(context: Context, id: Int) {}
+            |
+            |}""".trimMargin()
+
+        lint()
+            .files(Context, kotlin(code))
+            .allowMissingSdk(true)
+            .run()
+            .expectClean()
+    }
+
+    // As Lint doesn't have access to the real corresponding classes, we have to provide stubs
+    companion object Stubs {
+
+        private val Context = java(
+            """
+            |package android.content;
+            |
+            |public class Context {}
+            |""".trimMargin()
+        )
+
+        private val ContextCompat = java(
+            """
+            |package androidx.core.content;
+            |
+            |public class ContextCompat {
+            |   public static void getDrawable(Context context, int resId) {}
+            |   public static void getColorStateList(Context context, int resId) {}
+            |}
+            |""".trimMargin()
+        )
+    }
+}
\ No newline at end of file