From ab7f507b038f82bacb299e0741306e13b68a7003 Mon Sep 17 00:00:00 2001 From: Alex Hart Date: Thu, 8 Apr 2021 14:51:27 -0300 Subject: [PATCH] Add custom lint for AlertDialog.Builder usage. --- app/lint.xml | 2 + .../lint/AlertDialogBuilderDetector.java | 86 +++++++++ .../main/java/org/signal/lint/Registry.java | 3 +- .../lint/AlertDialogBuilderDetectorTest.java | 173 ++++++++++++++++++ .../resources/AppCompatAlertDialogStub.java | 13 ++ 5 files changed, 276 insertions(+), 1 deletion(-) create mode 100644 lintchecks/src/main/java/org/signal/lint/AlertDialogBuilderDetector.java create mode 100644 lintchecks/src/test/java/org/signal/lint/AlertDialogBuilderDetectorTest.java create mode 100644 lintchecks/src/test/resources/AppCompatAlertDialogStub.java diff --git a/app/lint.xml b/app/lint.xml index 805f69b92..09635764a 100644 --- a/app/lint.xml +++ b/app/lint.xml @@ -31,6 +31,8 @@ + + diff --git a/lintchecks/src/main/java/org/signal/lint/AlertDialogBuilderDetector.java b/lintchecks/src/main/java/org/signal/lint/AlertDialogBuilderDetector.java new file mode 100644 index 000000000..ceefa5b2b --- /dev/null +++ b/lintchecks/src/main/java/org/signal/lint/AlertDialogBuilderDetector.java @@ -0,0 +1,86 @@ +package org.signal.lint; + +import com.android.tools.lint.client.api.JavaEvaluator; +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.LintFix; +import com.android.tools.lint.detector.api.Scope; +import com.android.tools.lint.detector.api.Severity; +import com.intellij.psi.PsiMethod; + +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.jetbrains.uast.UCallExpression; +import org.jetbrains.uast.UExpression; + +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +@SuppressWarnings("UnstableApiUsage") +public final class AlertDialogBuilderDetector extends Detector implements Detector.UastScanner { + + static final Issue ALERT_DIALOG_BUILDER_USAGE = Issue.create("AlertDialogBuilderUsage", + "Creating dialog with AlertDialog.Builder instead of MaterialAlertDialogBuilder", + "Signal utilizes MaterialAlertDialogBuilder for more consistent and pleasant AlertDialogs.", + Category.MESSAGES, + 5, + Severity.WARNING, + new Implementation(AlertDialogBuilderDetector.class, Scope.JAVA_FILE_SCOPE)); + + @Override + public @Nullable List getApplicableConstructorTypes() { + return Arrays.asList("android.app.AlertDialog.Builder", "androidx.appcompat.app.AlertDialog.Builder"); + } + + @Override + public void visitConstructor(JavaContext context, @NotNull UCallExpression call, @NotNull PsiMethod method) { + JavaEvaluator evaluator = context.getEvaluator(); + + if (evaluator.isMemberInClass(method, "android.app.AlertDialog.Builder")) { + LintFix fix = quickFixIssueAlertDialogBuilder(call); + context.report(ALERT_DIALOG_BUILDER_USAGE, + call, + context.getLocation(call), + "Using 'android.app.AlertDialog.Builder' instead of com.google.android.material.dialog.MaterialAlertDialogBuilder", + fix); + } + + if (evaluator.isMemberInClass(method, "androidx.appcompat.app.AlertDialog.Builder")) { + LintFix fix = quickFixIssueAlertDialogBuilder(call); + context.report(ALERT_DIALOG_BUILDER_USAGE, + call, + context.getLocation(call), + "Using 'androidx.appcompat.app.AlertDialog.Builder' instead of com.google.android.material.dialog.MaterialAlertDialogBuilder", + fix); + } + } + + private LintFix quickFixIssueAlertDialogBuilder(@NotNull UCallExpression alertBuilderCall) { + List arguments = alertBuilderCall.getValueArguments(); + UExpression context = arguments.get(0); + + String fixSource = "new com.google.android.material.dialog.MaterialAlertDialogBuilder"; + + switch (arguments.size()) { + case 1: + fixSource += String.format("(%s)", context); + break; + case 2: + UExpression themeOverride = arguments.get(1); + fixSource += String.format("(%s, %s)", context, themeOverride); + break; + + default: + throw new IllegalStateException("MaterialAlertDialogBuilder overloads should have 1 or 2 arguments"); + } + + String builderCallSource = alertBuilderCall.asSourceString(); + LintFix.GroupBuilder fixGrouper = fix().group(); + fixGrouper.add(fix().replace().text(builderCallSource).shortenNames().reformat(true).with(fixSource).build()); + return fixGrouper.build(); + } +} \ No newline at end of file diff --git a/lintchecks/src/main/java/org/signal/lint/Registry.java b/lintchecks/src/main/java/org/signal/lint/Registry.java index cf9ecd31e..9831d848e 100644 --- a/lintchecks/src/main/java/org/signal/lint/Registry.java +++ b/lintchecks/src/main/java/org/signal/lint/Registry.java @@ -15,7 +15,8 @@ public final class Registry extends IssueRegistry { return Arrays.asList(SignalLogDetector.LOG_NOT_SIGNAL, SignalLogDetector.LOG_NOT_APP, SignalLogDetector.INLINE_TAG, - VersionCodeDetector.VERSION_CODE_USAGE); + VersionCodeDetector.VERSION_CODE_USAGE, + AlertDialogBuilderDetector.ALERT_DIALOG_BUILDER_USAGE); } @Override diff --git a/lintchecks/src/test/java/org/signal/lint/AlertDialogBuilderDetectorTest.java b/lintchecks/src/test/java/org/signal/lint/AlertDialogBuilderDetectorTest.java new file mode 100644 index 000000000..dc0e01015 --- /dev/null +++ b/lintchecks/src/test/java/org/signal/lint/AlertDialogBuilderDetectorTest.java @@ -0,0 +1,173 @@ +package org.signal.lint; + +import com.android.tools.lint.checks.infrastructure.TestFile; + +import org.junit.Test; + +import java.io.InputStream; +import java.util.Scanner; + +import static com.android.tools.lint.checks.infrastructure.TestFiles.java; +import static com.android.tools.lint.checks.infrastructure.TestLintTask.lint; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +@SuppressWarnings("UnstableApiUsage") +public final class AlertDialogBuilderDetectorTest { + + private static final TestFile appCompatAlertDialogStub = java(readResourceAsString("AppCompatAlertDialogStub.java")); + + @Test + public void androidAlertDialogBuilderUsed_LogAlertDialogBuilderUsage_1_arg() { + lint() + .files( + java("package foo;\n" + + "import android.app.AlertDialog;\n" + + "public class Example {\n" + + " public void buildDialog() {\n" + + " new AlertDialog.Builder(context).show();\n" + + " }\n" + + "}") + ) + .issues(AlertDialogBuilderDetector.ALERT_DIALOG_BUILDER_USAGE) + .run() + .expect("src/foo/Example.java:5: Warning: Using 'android.app.AlertDialog.Builder' instead of com.google.android.material.dialog.MaterialAlertDialogBuilder [AlertDialogBuilderUsage]\n" + + " new AlertDialog.Builder(context).show();\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" + + "0 errors, 1 warnings") + .expectFixDiffs("Fix for src/foo/Example.java line 5: Replace with new com.google.android.material.dialog.MaterialAlertDialogBuilder(context):\n" + + "@@ -5 +5\n" + + "- new AlertDialog.Builder(context).show();\n" + + "+ new com.google.android.material.dialog.MaterialAlertDialogBuilder(context).show();"); + } + + @Test + public void androidAlertDialogBuilderUsed_LogAlertDialogBuilderUsage_2_arg() { + lint() + .files( + java("package foo;\n" + + "import android.app.AlertDialog;\n" + + "public class Example {\n" + + " public void buildDialog() {\n" + + " new AlertDialog.Builder(context, themeOverride).show();\n" + + " }\n" + + "}") + ) + .issues(AlertDialogBuilderDetector.ALERT_DIALOG_BUILDER_USAGE) + .run() + .expect("src/foo/Example.java:5: Warning: Using 'android.app.AlertDialog.Builder' instead of com.google.android.material.dialog.MaterialAlertDialogBuilder [AlertDialogBuilderUsage]\n" + + " new AlertDialog.Builder(context, themeOverride).show();\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" + + "0 errors, 1 warnings") + .expectFixDiffs("Fix for src/foo/Example.java line 5: Replace with new com.google.android.material.dialog.MaterialAlertDialogBuilder(context, themeOverride):\n" + + "@@ -5 +5\n" + + "- new AlertDialog.Builder(context, themeOverride).show();\n" + + "+ new com.google.android.material.dialog.MaterialAlertDialogBuilder(context, themeOverride).show();"); + } + + @Test + public void androidAlertDialogBuilderUsed_withAssignment_LogAlertDialogBuilderUsage_1_arg() { + lint() + .files( + java("package foo;\n" + + "import android.app.AlertDialog;\n" + + "public class Example {\n" + + " public void buildDialog() {\n" + + " AlertDialog.Builder builder = new AlertDialog.Builder(context)\n" + + " .show();\n" + + " }\n" + + "}") + ) + .issues(AlertDialogBuilderDetector.ALERT_DIALOG_BUILDER_USAGE) + .run() + .expect("src/foo/Example.java:5: Warning: Using 'android.app.AlertDialog.Builder' instead of com.google.android.material.dialog.MaterialAlertDialogBuilder [AlertDialogBuilderUsage]\n" + + " AlertDialog.Builder builder = new AlertDialog.Builder(context)\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" + + "0 errors, 1 warnings") + .expectFixDiffs("Fix for src/foo/Example.java line 5: Replace with new com.google.android.material.dialog.MaterialAlertDialogBuilder(context):\n" + + "@@ -5 +5\n" + + "- AlertDialog.Builder builder = new AlertDialog.Builder(context)\n" + + "+ AlertDialog.Builder builder = new com.google.android.material.dialog.MaterialAlertDialogBuilder(context)"); + } + + @Test + public void appcompatAlertDialogBuilderUsed_LogAlertDialogBuilderUsage_1_arg() { + lint() + .files(appCompatAlertDialogStub, + java("package foo;\n" + + "import androidx.appcompat.app.AlertDialog;\n" + + "public class Example {\n" + + " public void buildDialog() {\n" + + " new AlertDialog.Builder(context).show();\n" + + " }\n" + + "}") + ) + .issues(AlertDialogBuilderDetector.ALERT_DIALOG_BUILDER_USAGE) + .run() + .expect("src/foo/Example.java:5: Warning: Using 'androidx.appcompat.app.AlertDialog.Builder' instead of com.google.android.material.dialog.MaterialAlertDialogBuilder [AlertDialogBuilderUsage]\n" + + " new AlertDialog.Builder(context).show();\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" + + "0 errors, 1 warnings") + .expectFixDiffs("Fix for src/foo/Example.java line 5: Replace with new com.google.android.material.dialog.MaterialAlertDialogBuilder(context):\n" + + "@@ -5 +5\n" + + "- new AlertDialog.Builder(context).show();\n" + + "+ new com.google.android.material.dialog.MaterialAlertDialogBuilder(context).show();"); + } + + @Test + public void appcompatAlertDialogBuilderUsed_LogAlertDialogBuilderUsage_2_arg() { + lint() + .files(appCompatAlertDialogStub, + java("package foo;\n" + + "import androidx.appcompat.app.AlertDialog;\n" + + "public class Example {\n" + + " public void buildDialog() {\n" + + " new AlertDialog.Builder(context, themeOverride).show();\n" + + " }\n" + + "}") + ) + .issues(AlertDialogBuilderDetector.ALERT_DIALOG_BUILDER_USAGE) + .run() + .expect("src/foo/Example.java:5: Warning: Using 'androidx.appcompat.app.AlertDialog.Builder' instead of com.google.android.material.dialog.MaterialAlertDialogBuilder [AlertDialogBuilderUsage]\n" + + " new AlertDialog.Builder(context, themeOverride).show();\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" + + "0 errors, 1 warnings") + .expectFixDiffs("Fix for src/foo/Example.java line 5: Replace with new com.google.android.material.dialog.MaterialAlertDialogBuilder(context, themeOverride):\n" + + "@@ -5 +5\n" + + "- new AlertDialog.Builder(context, themeOverride).show();\n" + + "+ new com.google.android.material.dialog.MaterialAlertDialogBuilder(context, themeOverride).show();"); + } + + @Test + public void appcompatAlertDialogBuilderUsed_withAssignment_LogAlertDialogBuilderUsage_1_arg() { + lint() + .files(appCompatAlertDialogStub, + java("package foo;\n" + + "import androidx.appcompat.app.AlertDialog;\n" + + "public class Example {\n" + + " public void buildDialog() {\n" + + " AlertDialog.Builder builder = new AlertDialog.Builder(context)\n" + + " .show();\n" + + " }\n" + + "}") + ) + .issues(AlertDialogBuilderDetector.ALERT_DIALOG_BUILDER_USAGE) + .run() + .expect("src/foo/Example.java:5: Warning: Using 'androidx.appcompat.app.AlertDialog.Builder' instead of com.google.android.material.dialog.MaterialAlertDialogBuilder [AlertDialogBuilderUsage]\n" + + " AlertDialog.Builder builder = new AlertDialog.Builder(context)\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" + + "0 errors, 1 warnings") + .expectFixDiffs("Fix for src/foo/Example.java line 5: Replace with new com.google.android.material.dialog.MaterialAlertDialogBuilder(context):\n" + + "@@ -5 +5\n" + + "- AlertDialog.Builder builder = new AlertDialog.Builder(context)\n" + + "+ AlertDialog.Builder builder = new com.google.android.material.dialog.MaterialAlertDialogBuilder(context)"); + } + + private static String readResourceAsString(String resourceName) { + InputStream inputStream = ClassLoader.getSystemClassLoader().getResourceAsStream(resourceName); + assertNotNull(inputStream); + Scanner scanner = new Scanner(inputStream).useDelimiter("\\A"); + assertTrue(scanner.hasNext()); + return scanner.next(); + } +} diff --git a/lintchecks/src/test/resources/AppCompatAlertDialogStub.java b/lintchecks/src/test/resources/AppCompatAlertDialogStub.java new file mode 100644 index 000000000..2df340fe0 --- /dev/null +++ b/lintchecks/src/test/resources/AppCompatAlertDialogStub.java @@ -0,0 +1,13 @@ +package androidx.appcompat.app; + +public class AlertDialog { + + public static class Builder { + + public Builder(Context context) { + } + + public Builder(Context context, int themeOverrideId) { + } + } +}