From 456857bbbd05cdb0f81d2f279dbd1948f4dc2d51 Mon Sep 17 00:00:00 2001 From: Alan Evans Date: Fri, 1 May 2020 16:19:24 -0300 Subject: [PATCH] Add custom lint rule project. --- app/build.gradle | 2 + app/lint.xml | 3 + .../securesms/logging/AndroidLogger.java | 5 +- .../thoughtcrime/securesms/logging/Log.java | 5 +- .../securesms/logging/PersistentLogger.java | 4 +- lintchecks/build.gradle | 20 +++++ .../main/java/org/signal/lint/Registry.java | 22 ++++++ .../org/signal/lint/SignalLogDetector.java | 77 +++++++++++++++++++ .../java/org/signal/lint/LogDetectorTest.java | 57 ++++++++++++++ settings.gradle | 4 +- 10 files changed, 195 insertions(+), 4 deletions(-) create mode 100644 lintchecks/build.gradle create mode 100644 lintchecks/src/main/java/org/signal/lint/Registry.java create mode 100644 lintchecks/src/main/java/org/signal/lint/SignalLogDetector.java create mode 100644 lintchecks/src/test/java/org/signal/lint/LogDetectorTest.java diff --git a/app/build.gradle b/app/build.gradle index a45d2e4ef9..32ccdb9c88 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -255,6 +255,8 @@ android { } dependencies { + lintChecks project(':lintchecks') + implementation 'androidx.appcompat:appcompat:1.1.0' implementation 'androidx.recyclerview:recyclerview:1.0.0' implementation 'com.google.android.material:material:1.1.0' diff --git a/app/lint.xml b/app/lint.xml index 2c80352535..99fff898e3 100644 --- a/app/lint.xml +++ b/app/lint.xml @@ -17,6 +17,9 @@ + + + diff --git a/app/src/main/java/org/thoughtcrime/securesms/logging/AndroidLogger.java b/app/src/main/java/org/thoughtcrime/securesms/logging/AndroidLogger.java index 570fda623a..aa4743de6b 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/logging/AndroidLogger.java +++ b/app/src/main/java/org/thoughtcrime/securesms/logging/AndroidLogger.java @@ -1,6 +1,9 @@ package org.thoughtcrime.securesms.logging; -public class AndroidLogger extends Log.Logger { +import android.annotation.SuppressLint; + +@SuppressLint("LogNotSignal") +public final class AndroidLogger extends Log.Logger { @Override public void v(String tag, String message, Throwable t) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/logging/Log.java b/app/src/main/java/org/thoughtcrime/securesms/logging/Log.java index ea4e106861..2bd7adc607 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/logging/Log.java +++ b/app/src/main/java/org/thoughtcrime/securesms/logging/Log.java @@ -1,8 +1,11 @@ package org.thoughtcrime.securesms.logging; +import android.annotation.SuppressLint; + import androidx.annotation.MainThread; -public class Log { +@SuppressLint("LogNotSignal") +public final class Log { private static Logger[] loggers; diff --git a/app/src/main/java/org/thoughtcrime/securesms/logging/PersistentLogger.java b/app/src/main/java/org/thoughtcrime/securesms/logging/PersistentLogger.java index 8e35ca122d..63bb723d32 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/logging/PersistentLogger.java +++ b/app/src/main/java/org/thoughtcrime/securesms/logging/PersistentLogger.java @@ -1,5 +1,6 @@ package org.thoughtcrime.securesms.logging; +import android.annotation.SuppressLint; import android.content.Context; import androidx.annotation.AnyThread; import androidx.annotation.WorkerThread; @@ -21,7 +22,8 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.Executor; import java.util.concurrent.Executors; -public class PersistentLogger extends Log.Logger { +@SuppressLint("LogNotSignal") +public final class PersistentLogger extends Log.Logger { private static final String TAG = PersistentLogger.class.getSimpleName(); diff --git a/lintchecks/build.gradle b/lintchecks/build.gradle new file mode 100644 index 0000000000..08d9342abe --- /dev/null +++ b/lintchecks/build.gradle @@ -0,0 +1,20 @@ +apply plugin: 'java-library' + +repositories { + google() + jcenter() +} + +dependencies { + compileOnly 'com.android.tools.lint:lint-api:26.6.3' + compileOnly 'com.android.tools.lint:lint-checks:26.6.3' + + testImplementation 'com.android.tools.lint:lint-tests:26.6.3' + testImplementation 'junit:junit:4.12' +} + +jar { + manifest { + attributes('Lint-Registry-v2': 'org.signal.lint.Registry') + } +} diff --git a/lintchecks/src/main/java/org/signal/lint/Registry.java b/lintchecks/src/main/java/org/signal/lint/Registry.java new file mode 100644 index 0000000000..c03c3d6720 --- /dev/null +++ b/lintchecks/src/main/java/org/signal/lint/Registry.java @@ -0,0 +1,22 @@ +package org.signal.lint; + +import com.android.tools.lint.client.api.IssueRegistry; +import com.android.tools.lint.detector.api.ApiKt; +import com.android.tools.lint.detector.api.Issue; + +import java.util.Collections; +import java.util.List; + +@SuppressWarnings("UnstableApiUsage") +public final class Registry extends IssueRegistry { + + @Override + public List getIssues() { + return Collections.singletonList(SignalLogDetector.LOG_NOT_SIGNAL); + } + + @Override + public int getApi() { + return ApiKt.CURRENT_API; + } +} diff --git a/lintchecks/src/main/java/org/signal/lint/SignalLogDetector.java b/lintchecks/src/main/java/org/signal/lint/SignalLogDetector.java new file mode 100644 index 0000000000..dad320ffe9 --- /dev/null +++ b/lintchecks/src/main/java/org/signal/lint/SignalLogDetector.java @@ -0,0 +1,77 @@ +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.uast.UCallExpression; +import org.jetbrains.uast.UExpression; + +import java.util.Arrays; +import java.util.List; + +@SuppressWarnings("UnstableApiUsage") +public final class SignalLogDetector extends Detector implements Detector.UastScanner { + + static final Issue LOG_NOT_SIGNAL = Issue.create("LogNotSignal", + "Logging call to Android Log instead of Signal's Logger", + "Signal has its own logger which must be used.", + Category.MESSAGES, + 5, + Severity.ERROR, + new Implementation(SignalLogDetector.class, Scope.JAVA_FILE_SCOPE)); + + @Override + public List getApplicableMethodNames() { + return Arrays.asList("v", "d", "i", "w", "e", "wtf"); + } + + @Override + public void visitMethodCall(JavaContext context, @NotNull UCallExpression call, @NotNull PsiMethod method) { + JavaEvaluator evaluator = context.getEvaluator(); + + if (evaluator.isMemberInClass(method, "android.util.Log")) { + LintFix fix = quickFixIssueLog(call); + context.report(LOG_NOT_SIGNAL, call, context.getLocation(call), "Using 'android.util.Log' instead of a Signal Logger", fix); + } + } + + private LintFix quickFixIssueLog(@NotNull UCallExpression logCall) { + List arguments = logCall.getValueArguments(); + String methodName = logCall.getMethodName(); + UExpression tag = arguments.get(0); + + String fixSource = "org.thoughtcrime.securesms.logging.Log."; + + switch (arguments.size()) { + case 2: + UExpression msgOrThrowable = arguments.get(1); + fixSource += String.format("%s(%s, %s)", methodName, tag, msgOrThrowable.asSourceString()); + break; + + case 3: + UExpression msg = arguments.get(1); + UExpression throwable = arguments.get(2); + fixSource += String.format("%s(%s, %s, %s)", methodName, tag, msg.asSourceString(), throwable.asSourceString()); + break; + + default: + throw new IllegalStateException("android.util.Log overloads should have 2 or 3 arguments"); + } + + String logCallSource = logCall.asSourceString(); + LintFix.GroupBuilder fixGrouper = fix().group(); + fixGrouper.add(fix().replace().text(logCallSource).shortenNames().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 new file mode 100644 index 0000000000..77b8404b35 --- /dev/null +++ b/lintchecks/src/test/java/org/signal/lint/LogDetectorTest.java @@ -0,0 +1,57 @@ +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; + +public final class LogDetectorTest { + + @Test + public void androidLogUsed_LogNotSignal_2_args() { + lint() + .files( + java("package foo;\n" + + "import android.util.Log;\n" + + "public class Example {\n" + + " public void log() {\n" + + " Log.d(\"TAG\", \"msg\");\n" + + " }\n" + + "}") + ) + .issues(SignalLogDetector.LOG_NOT_SIGNAL) + .run() + .expect("src/foo/Example.java:5: Error: Using 'android.util.Log' instead of a Signal Logger [LogNotSignal]\n" + + " Log.d(\"TAG\", \"msg\");\n" + + " ~~~~~~~~~~~~~~~~~~~\n" + + "1 errors, 0 warnings") + .expectFixDiffs("Fix for src/foo/Example.java line 5: Replace with org.thoughtcrime.securesms.logging.Log.d(\"TAG\", \"msg\"):\n" + + "@@ -5 +5\n" + + "- Log.d(\"TAG\", \"msg\");\n" + + "+ org.thoughtcrime.securesms.logging.Log.d(\"TAG\", \"msg\");"); + } + + @Test + public void androidLogUsed_LogNotSignal_3_args() { + lint() + .files( + java("package foo;\n" + + "import android.util.Log;\n" + + "public class Example {\n" + + " public void log() {\n" + + " Log.w(\"TAG\", \"msg\", new Exception());\n" + + " }\n" + + "}") + ) + .issues(SignalLogDetector.LOG_NOT_SIGNAL) + .run() + .expect("src/foo/Example.java:5: Error: Using 'android.util.Log' instead of a Signal Logger [LogNotSignal]\n" + + " Log.w(\"TAG\", \"msg\", new Exception());\n" + + " ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" + + "1 errors, 0 warnings") + .expectFixDiffs("Fix for src/foo/Example.java line 5: Replace with org.thoughtcrime.securesms.logging.Log.w(\"TAG\", \"msg\", new Exception()):\n" + + "@@ -5 +5\n" + + "- Log.w(\"TAG\", \"msg\", new Exception());\n" + + "+ org.thoughtcrime.securesms.logging.Log.w(\"TAG\", \"msg\", new Exception());"); + } +} diff --git a/settings.gradle b/settings.gradle index 20d98cbcf6..32b7bc2875 100644 --- a/settings.gradle +++ b/settings.gradle @@ -1,4 +1,6 @@ -include ':app', ':libsignal-service' +include ':app' +include ':libsignal-service' +include ':lintchecks' project(':app').name = 'Signal-Android'