From f3dbe4416fb1dec0780311a4ea325ac2d22e6548 Mon Sep 17 00:00:00 2001 From: Alan Evans Date: Fri, 10 Jul 2020 11:18:12 -0300 Subject: [PATCH] Add lint to detect non-numeric version code checks. --- .../main/java/org/signal/lint/Registry.java | 3 +- .../org/signal/lint/VersionCodeDetector.java | 91 +++++++++++++ .../java/org/signal/lint/LogDetectorTest.java | 1 + .../signal/lint/VersionCodeDetectorTest.java | 122 ++++++++++++++++++ 4 files changed, 216 insertions(+), 1 deletion(-) create mode 100644 lintchecks/src/main/java/org/signal/lint/VersionCodeDetector.java create mode 100644 lintchecks/src/test/java/org/signal/lint/VersionCodeDetectorTest.java diff --git a/lintchecks/src/main/java/org/signal/lint/Registry.java b/lintchecks/src/main/java/org/signal/lint/Registry.java index 56446cadc..cf9ecd31e 100644 --- a/lintchecks/src/main/java/org/signal/lint/Registry.java +++ b/lintchecks/src/main/java/org/signal/lint/Registry.java @@ -14,7 +14,8 @@ public final class Registry extends IssueRegistry { public List getIssues() { return Arrays.asList(SignalLogDetector.LOG_NOT_SIGNAL, SignalLogDetector.LOG_NOT_APP, - SignalLogDetector.INLINE_TAG); + SignalLogDetector.INLINE_TAG, + VersionCodeDetector.VERSION_CODE_USAGE); } @Override diff --git a/lintchecks/src/main/java/org/signal/lint/VersionCodeDetector.java b/lintchecks/src/main/java/org/signal/lint/VersionCodeDetector.java new file mode 100644 index 000000000..a06bec4e6 --- /dev/null +++ b/lintchecks/src/main/java/org/signal/lint/VersionCodeDetector.java @@ -0,0 +1,91 @@ +package org.signal.lint; + +import com.android.annotations.NonNull; +import com.android.tools.lint.client.api.JavaEvaluator; +import com.android.tools.lint.client.api.UElementHandler; +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.PsiClass; +import com.intellij.psi.PsiElement; +import com.intellij.psi.PsiType; + +import org.jetbrains.annotations.NotNull; +import org.jetbrains.uast.UElement; +import org.jetbrains.uast.UExpression; + +import java.util.Collections; +import java.util.List; + +@SuppressWarnings("UnstableApiUsage") +public final class VersionCodeDetector extends Detector implements Detector.UastScanner { + + static final Issue VERSION_CODE_USAGE = Issue.create("VersionCodeUsage", + "Using 'VERSION_CODES' reference instead of the numeric value", + "Signal style is to use the numeric value.", + Category.CORRECTNESS, + 5, + Severity.WARNING, + new Implementation(VersionCodeDetector.class, Scope.JAVA_FILE_SCOPE)); + + @Override + public List> getApplicableUastTypes() { + return Collections.singletonList(UExpression.class); + } + + @Override + public UElementHandler createUastHandler(@NonNull JavaContext context) { + return new ExpressionChecker(context); + } + + private class ExpressionChecker extends UElementHandler { + private final JavaContext context; + private final JavaEvaluator evaluator; + private final PsiClass versionCodeClass; + + public ExpressionChecker(JavaContext context) { + this.context = context; + this.evaluator = context.getEvaluator(); + this.versionCodeClass = evaluator.findClass("android.os.Build.VERSION_CODES"); + } + + @Override + public void visitExpression(@NotNull UExpression node) { + if (versionCodeClass != null && node.getExpressionType() == PsiType.INT) { + PsiElement javaPsi = node.getJavaPsi(); + + if (javaPsi != null) { + PsiElement resolved = evaluator.resolve(javaPsi); + + if (resolved != null && resolved.getParent().equals(versionCodeClass)) { + Object evaluated = node.evaluate(); + + if (evaluated != null) { + context.report(VERSION_CODE_USAGE, node, context.getLocation(node), "Using 'VERSION_CODES' reference instead of the numeric value " + evaluated, quickFixIssueInlineValue(node, evaluated.toString())); + } else { + context.report(VERSION_CODE_USAGE, node, context.getLocation(node), "Using 'VERSION_CODES' reference instead of the numeric value", null); + } + } + } + } + } + } + + private LintFix quickFixIssueInlineValue(@NotNull UExpression node, @NotNull String fixSource) { + String expressionSource = node.asSourceString(); + LintFix.GroupBuilder fixGrouper = fix().group(); + + fixGrouper.add(fix().replace() + .text(expressionSource) + .reformat(true) + .with(fixSource) + .build()); + + return fixGrouper.build(); + } +} \ No newline at end of file diff --git a/lintchecks/src/test/java/org/signal/lint/LogDetectorTest.java b/lintchecks/src/test/java/org/signal/lint/LogDetectorTest.java index 08f693ffd..3d6e26ac4 100644 --- a/lintchecks/src/test/java/org/signal/lint/LogDetectorTest.java +++ b/lintchecks/src/test/java/org/signal/lint/LogDetectorTest.java @@ -12,6 +12,7 @@ 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 LogDetectorTest { private static final TestFile serviceLogStub = java(readResourceAsString("ServiceLogStub.java")); diff --git a/lintchecks/src/test/java/org/signal/lint/VersionCodeDetectorTest.java b/lintchecks/src/test/java/org/signal/lint/VersionCodeDetectorTest.java new file mode 100644 index 000000000..0d9695244 --- /dev/null +++ b/lintchecks/src/test/java/org/signal/lint/VersionCodeDetectorTest.java @@ -0,0 +1,122 @@ +package org.signal.lint; + +import org.junit.Test; + +import static com.android.tools.lint.checks.infrastructure.TestFiles.java; +import static com.android.tools.lint.checks.infrastructure.TestLintTask.lint; + +@SuppressWarnings("UnstableApiUsage") +public final class VersionCodeDetectorTest { + + @Test + public void version_code_constant_referenced_in_code() { + lint() + .files( + java("package foo;\n" + + "import android.os.Build;\n" + + "public class Example {\n" + + " public void versionCodeMention() {\n" + + " if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {\n" + + " }\n" + + " }\n" + + "}") + ) + .issues(VersionCodeDetector.VERSION_CODE_USAGE) + .run() + .expect("src/foo/Example.java:5: Warning: Using 'VERSION_CODES' reference instead of the numeric value 21 [VersionCodeUsage]\n" + + " if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" + + "0 errors, 1 warnings") + .expectFixDiffs("Fix for src/foo/Example.java line 5: Replace with 21:\n" + + "@@ -5 +5\n" + + "- if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {\n" + + "+ if (Build.VERSION.SDK_INT >= 21) {"); + } + + @Test + public void numeric_value_referenced_in_code() { + lint() + .files( + java("package foo;\n" + + "import android.os.Build;\n" + + "public class Example {\n" + + " public void versionCodeMention() {\n" + + " if (Build.VERSION.SDK_INT >= 22) {\n" + + " }\n" + + " }\n" + + "}") + ) + .issues(VersionCodeDetector.VERSION_CODE_USAGE) + .run() + .expectClean(); + } + + @Test + public void non_version_code_constant_referenced_in_code() { + lint() + .files( + java("package foo;\n" + + "import android.os.Build;\n" + + "public class Example {\n" + + " private final static int LOLLIPOP = 21;\n" + + " public void versionCodeMention() {\n" + + " if (Build.VERSION.SDK_INT >= LOLLIPOP) {\n" + + " }\n" + + " }\n" + + "}") + ) + .issues(VersionCodeDetector.VERSION_CODE_USAGE) + .run() + .expectClean(); + } + + @Test + public void version_code_constant_referenced_in_TargetApi_attribute_and_inner_class_import() { + lint() + .files( + java("package foo;\n" + + "import android.os.Build.VERSION_CODES;\n" + + "import android.annotation.TargetApi;\n" + + "public class Example {\n" + + " @TargetApi(VERSION_CODES.N)\n" + + " public void versionCodeMention() {\n" + + " }\n" + + "}") + ) + .issues(VersionCodeDetector.VERSION_CODE_USAGE) + .run() + .expect("src/foo/Example.java:5: Warning: Using 'VERSION_CODES' reference instead of the numeric value 24 [VersionCodeUsage]\n" + + " @TargetApi(VERSION_CODES.N)\n" + + " ~~~~~~~~~~~~~~~\n" + + "0 errors, 1 warnings") + .expectFixDiffs("Fix for src/foo/Example.java line 5: Replace with 24:\n" + + "@@ -5 +5\n" + + "- @TargetApi(VERSION_CODES.N)\n" + + "+ @TargetApi(24)"); + } + + @Test + public void version_code_constant_referenced_in_RequiresApi_attribute_with_named_parameter() { + lint() + .files( + java("package foo;\n" + + "import android.os.Build;\n" + + "import android.annotation.RequiresApi;\n" + + "public class Example {\n" + + " @RequiresApi(app = Build.VERSION_CODES.M)\n" + + " public void versionCodeMention() {\n" + + " }\n" + + "}") + ) + .issues(VersionCodeDetector.VERSION_CODE_USAGE) + .run() + .expect("src/foo/Example.java:5: Warning: Using 'VERSION_CODES' reference instead of the numeric value 23 [VersionCodeUsage]\n" + + " @RequiresApi(app = Build.VERSION_CODES.M)\n" + + " ~~~~~~~~~~~~~~~~~~~~~\n" + + "0 errors, 1 warnings") + .expectFixDiffs("Fix for src/foo/Example.java line 5: Replace with 23:\n" + + "@@ -5 +5\n" + + "- @RequiresApi(app = Build.VERSION_CODES.M)\n" + + "+ @RequiresApi(app = 23)"); + } +}