New logging lint checks.
[LogNotAppSignal] tells you about using signal service logger in the app. [LogTagInlined] tells you about not using a constant tag.
This commit is contained in:
parent
26e582d806
commit
c63e42715e
6 changed files with 220 additions and 3 deletions
|
@ -19,6 +19,8 @@
|
|||
|
||||
<!-- Custom lints -->
|
||||
<issue id="LogNotSignal" severity="error" />
|
||||
<issue id="LogNotAppSignal" severity="warning" />
|
||||
<issue id="LogTagInlined" severity="warning" />
|
||||
|
||||
<issue id="RestrictedApi" severity="error">
|
||||
<ignore path="*/org/thoughtcrime/securesms/mediasend/camerax/VideoCapture.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<Issue> getIssues() {
|
||||
return Collections.singletonList(SignalLogDetector.LOG_NOT_SIGNAL);
|
||||
return Arrays.asList(SignalLogDetector.LOG_NOT_SIGNAL,
|
||||
SignalLogDetector.LOG_NOT_APP,
|
||||
SignalLogDetector.INLINE_TAG);
|
||||
}
|
||||
|
||||
@Override
|
||||
|
|
|
@ -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<String> 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<UExpression> 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();
|
||||
|
|
|
@ -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();
|
||||
}
|
||||
}
|
||||
|
|
41
lintchecks/src/test/resources/AppLogStub.java
Normal file
41
lintchecks/src/test/resources/AppLogStub.java
Normal file
|
@ -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) {
|
||||
}
|
||||
}
|
37
lintchecks/src/test/resources/ServiceLogStub.java
Normal file
37
lintchecks/src/test/resources/ServiceLogStub.java
Normal file
|
@ -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) {
|
||||
}
|
||||
}
|
Loading…
Add table
Reference in a new issue