diff --git a/app/lint.xml b/app/lint.xml
index 520c72361..78c972738 100644
--- a/app/lint.xml
+++ b/app/lint.xml
@@ -19,6 +19,8 @@
+
+
diff --git a/lintchecks/src/main/java/org/signal/lint/Registry.java b/lintchecks/src/main/java/org/signal/lint/Registry.java
index c03c3d672..56446cadc 100644
--- a/lintchecks/src/main/java/org/signal/lint/Registry.java
+++ b/lintchecks/src/main/java/org/signal/lint/Registry.java
@@ -4,7 +4,7 @@ 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.Arrays;
import java.util.List;
@SuppressWarnings("UnstableApiUsage")
@@ -12,7 +12,9 @@ public final class Registry extends IssueRegistry {
@Override
public List getIssues() {
- return Collections.singletonList(SignalLogDetector.LOG_NOT_SIGNAL);
+ return Arrays.asList(SignalLogDetector.LOG_NOT_SIGNAL,
+ SignalLogDetector.LOG_NOT_APP,
+ SignalLogDetector.INLINE_TAG);
}
@Override
diff --git a/lintchecks/src/main/java/org/signal/lint/SignalLogDetector.java b/lintchecks/src/main/java/org/signal/lint/SignalLogDetector.java
index dad320ffe..42e6e914b 100644
--- a/lintchecks/src/main/java/org/signal/lint/SignalLogDetector.java
+++ b/lintchecks/src/main/java/org/signal/lint/SignalLogDetector.java
@@ -14,6 +14,7 @@ import com.intellij.psi.PsiMethod;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.uast.UCallExpression;
import org.jetbrains.uast.UExpression;
+import org.jetbrains.uast.java.JavaUSimpleNameReferenceExpression;
import java.util.Arrays;
import java.util.List;
@@ -29,6 +30,22 @@ public final class SignalLogDetector extends Detector implements Detector.UastSc
Severity.ERROR,
new Implementation(SignalLogDetector.class, Scope.JAVA_FILE_SCOPE));
+ static final Issue LOG_NOT_APP = Issue.create("LogNotAppSignal",
+ "Logging call to Signal Service Log instead of App level Logger",
+ "Signal app layer has its own logger which must be used.",
+ Category.MESSAGES,
+ 5,
+ Severity.ERROR,
+ new Implementation(SignalLogDetector.class, Scope.JAVA_FILE_SCOPE));
+
+ static final Issue INLINE_TAG = Issue.create("LogTagInlined",
+ "Use of an inline string in a TAG",
+ "Often a sign of left in temporary log statements, always use a tag constant.",
+ 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");
@@ -42,6 +59,19 @@ public final class SignalLogDetector extends Detector implements Detector.UastSc
LintFix fix = quickFixIssueLog(call);
context.report(LOG_NOT_SIGNAL, call, context.getLocation(call), "Using 'android.util.Log' instead of a Signal Logger", fix);
}
+
+ if (evaluator.isMemberInClass(method, "org.whispersystems.libsignal.logging.Log")) {
+ LintFix fix = quickFixIssueLog(call);
+ context.report(LOG_NOT_APP, call, context.getLocation(call), "Using Signal server logger instead of app level Logger", fix);
+ }
+
+ if (evaluator.isMemberInClass(method, "org.thoughtcrime.securesms.logging.Log")) {
+ List arguments = call.getValueArguments();
+ UExpression tag = arguments.get(0);
+ if (!(tag instanceof JavaUSimpleNameReferenceExpression)) {
+ context.report(INLINE_TAG, call, context.getLocation(call), "Not using a tag constant");
+ }
+ }
}
private LintFix quickFixIssueLog(@NotNull UCallExpression logCall) {
@@ -64,7 +94,7 @@ public final class SignalLogDetector extends Detector implements Detector.UastSc
break;
default:
- throw new IllegalStateException("android.util.Log overloads should have 2 or 3 arguments");
+ throw new IllegalStateException("Log overloads should have 2 or 3 arguments");
}
String logCallSource = logCall.asSourceString();
diff --git a/lintchecks/src/test/java/org/signal/lint/LogDetectorTest.java b/lintchecks/src/test/java/org/signal/lint/LogDetectorTest.java
index 77b8404b3..08f693ffd 100644
--- a/lintchecks/src/test/java/org/signal/lint/LogDetectorTest.java
+++ b/lintchecks/src/test/java/org/signal/lint/LogDetectorTest.java
@@ -1,12 +1,22 @@
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;
public final class LogDetectorTest {
+ private static final TestFile serviceLogStub = java(readResourceAsString("ServiceLogStub.java"));
+ private static final TestFile appLogStub = java(readResourceAsString("AppLogStub.java"));
+
@Test
public void androidLogUsed_LogNotSignal_2_args() {
lint()
@@ -54,4 +64,99 @@ public final class LogDetectorTest {
"- Log.w(\"TAG\", \"msg\", new Exception());\n" +
"+ org.thoughtcrime.securesms.logging.Log.w(\"TAG\", \"msg\", new Exception());");
}
+
+ @Test
+ public void signalServiceLogUsed_LogNotApp_2_args() {
+ lint()
+ .files(serviceLogStub,
+ java("package foo;\n" +
+ "import org.whispersystems.libsignal.logging.Log;\n" +
+ "public class Example {\n" +
+ " public void log() {\n" +
+ " Log.d(\"TAG\", \"msg\");\n" +
+ " }\n" +
+ "}")
+ )
+ .issues(SignalLogDetector.LOG_NOT_APP)
+ .run()
+ .expect("src/foo/Example.java:5: Error: Using Signal server logger instead of app level Logger [LogNotAppSignal]\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 signalServiceLogUsed_LogNotApp_3_args() {
+ lint()
+ .files(serviceLogStub,
+ java("package foo;\n" +
+ "import org.whispersystems.libsignal.logging.Log;\n" +
+ "public class Example {\n" +
+ " public void log() {\n" +
+ " Log.w(\"TAG\", \"msg\", new Exception());\n" +
+ " }\n" +
+ "}")
+ )
+ .issues(SignalLogDetector.LOG_NOT_APP)
+ .run()
+ .expect("src/foo/Example.java:5: Error: Using Signal server logger instead of app level Logger [LogNotAppSignal]\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());");
+ }
+
+ @Test
+ public void log_uses_tag_constant() {
+ lint()
+ .files(appLogStub,
+ java("package foo;\n" +
+ "import org.thoughtcrime.securesms.logging.Log;\n" +
+ "public class Example {\n" +
+ " private static final String TAG = Log.tag(Example.class);\n" +
+ " public void log() {\n" +
+ " Log.d(TAG, \"msg\");\n" +
+ " }\n" +
+ "}")
+ )
+ .issues(SignalLogDetector.INLINE_TAG)
+ .run()
+ .expectClean();
+ }
+
+ @Test
+ public void log_uses_inline_tag() {
+ lint()
+ .files(appLogStub,
+ java("package foo;\n" +
+ "import org.thoughtcrime.securesms.logging.Log;\n" +
+ "public class Example {\n" +
+ " public void log() {\n" +
+ " Log.d(\"TAG\", \"msg\");\n" +
+ " }\n" +
+ "}")
+ )
+ .issues(SignalLogDetector.INLINE_TAG)
+ .run()
+ .expect("src/foo/Example.java:5: Error: Not using a tag constant [LogTagInlined]\n" +
+ " Log.d(\"TAG\", \"msg\");\n" +
+ " ~~~~~~~~~~~~~~~~~~~\n" +
+ "1 errors, 0 warnings")
+ .expectFixDiffs("");
+ }
+
+ 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/AppLogStub.java b/lintchecks/src/test/resources/AppLogStub.java
new file mode 100644
index 000000000..94f3113d3
--- /dev/null
+++ b/lintchecks/src/test/resources/AppLogStub.java
@@ -0,0 +1,41 @@
+package org.thoughtcrime.securesms.logging;
+
+public class Log {
+
+ public static String tag(Class clazz) {
+ return "";
+ }
+
+ public static void v(String tag, String msg) {
+ }
+
+ public static void v(String tag, String msg, Throwable tr) {
+ }
+
+ public static void d(String tag, String msg) {
+ }
+
+ public static void d(String tag, String msg, Throwable tr) {
+ }
+
+ public static void i(String tag, String msg) {
+ }
+
+ public static void i(String tag, String msg, Throwable tr) {
+ }
+
+ public static void w(String tag, String msg) {
+ }
+
+ public static void w(String tag, String msg, Throwable tr) {
+ }
+
+ public static void w(String tag, Throwable tr) {
+ }
+
+ public static void e(String tag, String msg) {
+ }
+
+ public static void e(String tag, String msg, Throwable tr) {
+ }
+}
diff --git a/lintchecks/src/test/resources/ServiceLogStub.java b/lintchecks/src/test/resources/ServiceLogStub.java
new file mode 100644
index 000000000..d68b230d7
--- /dev/null
+++ b/lintchecks/src/test/resources/ServiceLogStub.java
@@ -0,0 +1,37 @@
+package org.whispersystems.libsignal.logging;
+
+public class Log {
+
+ public static void v(String tag, String msg) {
+ }
+
+ public static void v(String tag, String msg, Throwable tr) {
+ }
+
+ public static void d(String tag, String msg) {
+ }
+
+ public static void d(String tag, String msg, Throwable tr) {
+ }
+
+ public static void i(String tag, String msg) {
+ }
+
+ public static void i(String tag, String msg, Throwable tr) {
+ }
+
+ public static void w(String tag, String msg) {
+ }
+
+ public static void w(String tag, String msg, Throwable tr) {
+ }
+
+ public static void w(String tag, Throwable tr) {
+ }
+
+ public static void e(String tag, String msg) {
+ }
+
+ public static void e(String tag, String msg, Throwable tr) {
+ }
+}