diff --git a/timber-lint/src/main/java/timber/lint/WrongTimberUsageDetector.kt b/timber-lint/src/main/java/timber/lint/WrongTimberUsageDetector.kt index a146b11f9..bcb94d1de 100644 --- a/timber-lint/src/main/java/timber/lint/WrongTimberUsageDetector.kt +++ b/timber-lint/src/main/java/timber/lint/WrongTimberUsageDetector.kt @@ -74,7 +74,10 @@ class WrongTimberUsageDetector : Detector(), UastScanner { val methodName = node.methodName val evaluator = context.evaluator - if ("format" == methodName && evaluator.isMemberInClass(method, "java.lang.String")) { + if ("format" == methodName && + (evaluator.isMemberInClass(method, "java.lang.String") || + evaluator.isMemberInClass(method, "kotlin.text.StringsKt__StringsJVMKt")) + ) { checkNestedStringFormat(context, node) return } @@ -122,8 +125,9 @@ class WrongTimberUsageDetector : Detector(), UastScanner { } if (current.isMethodCall()) { val psiMethod = (current as UCallExpression).resolve() - if (Pattern.matches(TIMBER_TREE_LOG_METHOD_REGEXP, psiMethod!!.name) - && context.evaluator.isMemberInClass(psiMethod, "timber.log.Timber") + if (psiMethod != null && + Pattern.matches(TIMBER_TREE_LOG_METHOD_REGEXP, psiMethod.name) + && isTimberLogMethod(psiMethod, context.evaluator) ) { context.report( Incident( @@ -615,15 +619,15 @@ class WrongTimberUsageDetector : Detector(), UastScanner { 3 -> { val msg = arguments[1] val throwable = arguments[2] - fixSource1 += "$methodName(${throwable.asSourceString()}, ${msg.asSourceString()})" - fixSource2 += "$methodName(${throwable.asSourceString()}, ${msg.asSourceString()})" + fixSource1 += "$methodName(${throwable.sourcePsi?.text}, ${msg.asSourceString()})" + fixSource2 += "$methodName(${throwable.sourcePsi?.text}, ${msg.asSourceString()})" } else -> { throw IllegalStateException("android.util.Log overloads should have 2 or 3 arguments") } } - val logCallSource = logCall.asSourceString() + val logCallSource = logCall.uastParent!!.sourcePsi?.text return fix().group() .add( fix().replace().text(logCallSource).shortenNames().reformat(true).with(fixSource1).build() diff --git a/timber-lint/src/test/java/timber/lint/WrongTimberUsageDetectorTest.kt b/timber-lint/src/test/java/timber/lint/WrongTimberUsageDetectorTest.kt index 6acca9417..cb8643b59 100644 --- a/timber-lint/src/test/java/timber/lint/WrongTimberUsageDetectorTest.kt +++ b/timber-lint/src/test/java/timber/lint/WrongTimberUsageDetectorTest.kt @@ -2,7 +2,6 @@ package timber.lint import com.android.tools.lint.checks.infrastructure.TestFiles.java import com.android.tools.lint.checks.infrastructure.TestFiles.kotlin -import com.android.tools.lint.checks.infrastructure.TestFiles.kt import com.android.tools.lint.checks.infrastructure.TestFiles.manifest import com.android.tools.lint.checks.infrastructure.TestLintTask.lint import org.junit.Test @@ -32,6 +31,14 @@ class WrongTimberUsageDetectorTest { | public void log() { | Log.d("TAG", "msg"); | } + |}""".trimMargin()), + kotlin(""" + |package foo + |import android.util.Log + |class Example { + | fun log() { + | Log.d("TAG", "msg") + | } |}""".trimMargin()) ) .issues(WrongTimberUsageDetector.ISSUE_LOG) @@ -40,16 +47,27 @@ class WrongTimberUsageDetectorTest { |src/foo/Example.java:5: Warning: Using 'Log' instead of 'Timber' [LogNotTimber] | Log.d("TAG", "msg"); | ~~~~~~~~~~~~~~~~~~~ - |0 errors, 1 warnings""".trimMargin()) + |src/foo/Example.kt:5: Warning: Using 'Log' instead of 'Timber' [LogNotTimber] + | Log.d("TAG", "msg") + | ~~~~~~~~~~~~~~~~~~~ + |0 errors, 2 warnings""".trimMargin()) .expectFixDiffs(""" - |Fix for src/foo/Example.java line 4: Replace with Timber.tag("TAG").d("msg"): + |Fix for src/foo/Example.java line 5: Replace with Timber.tag("TAG").d("msg"): |@@ -5 +5 |- Log.d("TAG", "msg"); |+ Timber.tag("TAG").d("msg"); - |Fix for src/foo/Example.java line 4: Replace with Timber.d("msg"): + |Fix for src/foo/Example.java line 5: Replace with Timber.d("msg"): |@@ -5 +5 |- Log.d("TAG", "msg"); |+ Timber.d("msg"); + |Fix for src/foo/Example.kt line 5: Replace with Timber.tag("TAG").d("msg"): + |@@ -5 +5 + |- Log.d("TAG", "msg") + |+ Timber.tag("TAG").d("msg") + |Fix for src/foo/Example.kt line 5: Replace with Timber.d("msg"): + |@@ -5 +5 + |- Log.d("TAG", "msg") + |+ Timber.d("msg") |""".trimMargin()) } @@ -63,6 +81,14 @@ class WrongTimberUsageDetectorTest { | public void log() { | Log.d("TAG", "msg", new Exception()); | } + |}""".trimMargin()), + kotlin(""" + |package foo + |import android.util.Log + |class Example { + | fun log() { + | Log.d("TAG", "msg", Exception()) + | } |}""".trimMargin()) ) .issues(WrongTimberUsageDetector.ISSUE_LOG) @@ -71,16 +97,27 @@ class WrongTimberUsageDetectorTest { |src/foo/Example.java:5: Warning: Using 'Log' instead of 'Timber' [LogNotTimber] | Log.d("TAG", "msg", new Exception()); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - |0 errors, 1 warnings""".trimMargin()) + |src/foo/Example.kt:5: Warning: Using 'Log' instead of 'Timber' [LogNotTimber] + | Log.d("TAG", "msg", Exception()) + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + |0 errors, 2 warnings""".trimMargin()) .expectFixDiffs(""" - |Fix for src/foo/Example.java line 4: Replace with Timber.tag("TAG").d(new Exception(), "msg"): + |Fix for src/foo/Example.java line 5: Replace with Timber.tag("TAG").d(new Exception(), "msg"): |@@ -5 +5 |- Log.d("TAG", "msg", new Exception()); |+ Timber.tag("TAG").d(new Exception(), "msg"); - |Fix for src/foo/Example.java line 4: Replace with Timber.d(new Exception(), "msg"): + |Fix for src/foo/Example.java line 5: Replace with Timber.d(new Exception(), "msg"): |@@ -5 +5 |- Log.d("TAG", "msg", new Exception()); |+ Timber.d(new Exception(), "msg"); + |Fix for src/foo/Example.kt line 5: Replace with Timber.tag("TAG").d(Exception(), "msg"): + |@@ -5 +5 + |- Log.d("TAG", "msg", Exception()) + |+ Timber.tag("TAG").d(Exception(), "msg") + |Fix for src/foo/Example.kt line 5: Replace with Timber.d(Exception(), "msg"): + |@@ -5 +5 + |- Log.d("TAG", "msg", Exception()) + |+ Timber.d(Exception(), "msg") |""".trimMargin()) } @@ -93,6 +130,13 @@ class WrongTimberUsageDetectorTest { | public void log() { | android.util.Log.d("TAG", "msg"); | } + |}""".trimMargin()), + kotlin(""" + |package foo + |class Example { + | fun log() { + | android.util.Log.d("TAG", "msg") + | } |}""".trimMargin()) ) .issues(WrongTimberUsageDetector.ISSUE_LOG) @@ -101,16 +145,27 @@ class WrongTimberUsageDetectorTest { |src/foo/Example.java:4: Warning: Using 'Log' instead of 'Timber' [LogNotTimber] | android.util.Log.d("TAG", "msg"); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - |0 errors, 1 warnings""".trimMargin()) + |src/foo/Example.kt:4: Warning: Using 'Log' instead of 'Timber' [LogNotTimber] + | android.util.Log.d("TAG", "msg") + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + |0 errors, 2 warnings""".trimMargin()) .expectFixDiffs(""" - |Fix for src/foo/Example.java line 3: Replace with Timber.tag("TAG").d("msg"): + |Fix for src/foo/Example.java line 4: Replace with Timber.tag("TAG").d("msg"): |@@ -4 +4 |- android.util.Log.d("TAG", "msg"); |+ Timber.tag("TAG").d("msg"); - |Fix for src/foo/Example.java line 3: Replace with Timber.d("msg"): + |Fix for src/foo/Example.java line 4: Replace with Timber.d("msg"): |@@ -4 +4 |- android.util.Log.d("TAG", "msg"); |+ Timber.d("msg"); + |Fix for src/foo/Example.kt line 4: Replace with Timber.tag("TAG").d("msg"): + |@@ -4 +4 + |- android.util.Log.d("TAG", "msg") + |+ Timber.tag("TAG").d("msg") + |Fix for src/foo/Example.kt line 4: Replace with Timber.d("msg"): + |@@ -4 +4 + |- android.util.Log.d("TAG", "msg") + |+ Timber.d("msg") |""".trimMargin()) } @@ -123,6 +178,13 @@ class WrongTimberUsageDetectorTest { | public void log() { | android.util.Log.d("TAG", "msg", new Exception()); | } + |}""".trimMargin()), + kotlin(""" + |package foo + |class Example { + | fun log() { + | android.util.Log.d("TAG", "msg", Exception()) + | } |}""".trimMargin()) ) .issues(WrongTimberUsageDetector.ISSUE_LOG) @@ -131,16 +193,27 @@ class WrongTimberUsageDetectorTest { |src/foo/Example.java:4: Warning: Using 'Log' instead of 'Timber' [LogNotTimber] | android.util.Log.d("TAG", "msg", new Exception()); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - |0 errors, 1 warnings""".trimMargin()) + |src/foo/Example.kt:4: Warning: Using 'Log' instead of 'Timber' [LogNotTimber] + | android.util.Log.d("TAG", "msg", Exception()) + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + |0 errors, 2 warnings""".trimMargin()) .expectFixDiffs(""" - |Fix for src/foo/Example.java line 3: Replace with Timber.tag("TAG").d(new Exception(), "msg"): + |Fix for src/foo/Example.java line 4: Replace with Timber.tag("TAG").d(new Exception(), "msg"): |@@ -4 +4 |- android.util.Log.d("TAG", "msg", new Exception()); |+ Timber.tag("TAG").d(new Exception(), "msg"); - |Fix for src/foo/Example.java line 3: Replace with Timber.d(new Exception(), "msg"): + |Fix for src/foo/Example.java line 4: Replace with Timber.d(new Exception(), "msg"): |@@ -4 +4 |- android.util.Log.d("TAG", "msg", new Exception()); |+ Timber.d(new Exception(), "msg"); + |Fix for src/foo/Example.kt line 4: Replace with Timber.tag("TAG").d(Exception(), "msg"): + |@@ -4 +4 + |- android.util.Log.d("TAG", "msg", Exception()) + |+ Timber.tag("TAG").d(Exception(), "msg") + |Fix for src/foo/Example.kt line 4: Replace with Timber.d(Exception(), "msg"): + |@@ -4 +4 + |- android.util.Log.d("TAG", "msg", Exception()) + |+ Timber.d(Exception(), "msg") |""".trimMargin()) } @@ -154,6 +227,14 @@ class WrongTimberUsageDetectorTest { | public void log() { | Timber.d(String.format("%s", "arg1")); | } + |}""".trimMargin()), + kotlin(""" + |package foo + |import timber.log.Timber + |class Example { + | fun log() { + | Timber.d(String.format("%s", "arg1")) + | } |}""".trimMargin()) ) .issues(WrongTimberUsageDetector.ISSUE_FORMAT) @@ -162,12 +243,19 @@ class WrongTimberUsageDetectorTest { |src/foo/Example.java:5: Warning: Using 'String#format' inside of 'Timber' [StringFormatInTimber] | Timber.d(String.format("%s", "arg1")); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ - |0 errors, 1 warnings""".trimMargin()) + |src/foo/Example.kt:5: Warning: Using 'String#format' inside of 'Timber' [StringFormatInTimber] + | Timber.d(String.format("%s", "arg1")) + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ + |0 errors, 2 warnings""".trimMargin()) .expectFixDiffs(""" - |Fix for src/foo/Example.java line 4: Remove String.format(...): + |Fix for src/foo/Example.java line 5: Remove String.format(...): |@@ -5 +5 |- Timber.d(String.format("%s", "arg1")); |+ Timber.d("%s", "arg1"); + |Fix for src/foo/Example.kt line 5: Remove String.format(...): + |@@ -5 +5 + |- Timber.d(String.format("%s", "arg1")) + |+ Timber.d("%s", "arg1") |""".trimMargin()) } @@ -182,6 +270,15 @@ class WrongTimberUsageDetectorTest { | public void log() { | Timber.d(format("%s", "arg1")); | } + |}""".trimMargin()), + kotlin(""" + |package foo + |import timber.log.Timber + |import java.lang.String.format + |class Example { + | fun log() { + | Timber.d(format("%s", "arg1")) + | } |}""".trimMargin()) ) // Remove when AGP 7.1.0-alpha07 is out @@ -193,12 +290,19 @@ class WrongTimberUsageDetectorTest { |src/foo/Example.java:6: Warning: Using 'String#format' inside of 'Timber' [StringFormatInTimber] | Timber.d(format("%s", "arg1")); | ~~~~~~~~~~~~~~~~~~~~ - |0 errors, 1 warnings""".trimMargin()) + |src/foo/Example.kt:6: Warning: Using 'String#format' inside of 'Timber' [StringFormatInTimber] + | Timber.d(format("%s", "arg1")) + | ~~~~~~~~~~~~~~~~~~~~ + |0 errors, 2 warnings""".trimMargin()) .expectFixDiffs(""" - |Fix for src/foo/Example.java line 5: Remove String.format(...): + |Fix for src/foo/Example.java line 6: Remove String.format(...): |@@ -6 +6 |- Timber.d(format("%s", "arg1")); |+ Timber.d("%s", "arg1"); + |Fix for src/foo/Example.kt line 6: Remove String.format(...): + |@@ -6 +6 + |- Timber.d(format("%s", "arg1")) + |+ Timber.d("%s", "arg1") |""".trimMargin()) } @@ -213,6 +317,15 @@ class WrongTimberUsageDetectorTest { | Timber.d(id(String.format("%s", "arg1"))); | } | private String id(String s) { return s; } + |}""".trimMargin()), + kotlin(""" + |package foo + |import timber.log.Timber + |class Example { + | fun log() { + | Timber.d(id(String.format("%s", "arg1"))) + | } + | private fun id(s: String): String { return s } |}""".trimMargin()) ) .issues(WrongTimberUsageDetector.ISSUE_FORMAT) @@ -221,7 +334,10 @@ class WrongTimberUsageDetectorTest { |src/foo/Example.java:5: Warning: Using 'String#format' inside of 'Timber' [StringFormatInTimber] | Timber.d(id(String.format("%s", "arg1"))); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ - |0 errors, 1 warnings""".trimMargin()) + |src/foo/Example.kt:5: Warning: Using 'String#format' inside of 'Timber' [StringFormatInTimber] + | Timber.d(id(String.format("%s", "arg1"))) + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ + |0 errors, 2 warnings""".trimMargin()) } @Test fun innerStringFormatInNestedAssignment() { @@ -236,6 +352,7 @@ class WrongTimberUsageDetectorTest { | Timber.d(msg = String.format("msg")); | } |}""".trimMargin()) + // no kotlin equivalent, since nested assignments do not exist ) .issues(WrongTimberUsageDetector.ISSUE_FORMAT) .run() @@ -251,13 +368,21 @@ class WrongTimberUsageDetectorTest { .files(TIMBER_STUB, java(""" |package foo; - |import timber.log.Timber; |public class Example { | public void log() { | for(;;) { | String name = String.format("msg"); | } | } + |}""".trimMargin()), + kotlin(""" + |package foo + |class Example { + | fun log() { + | while(true) { + | val name = String.format("msg") + | } + | } |}""".trimMargin()) ) .issues(WrongTimberUsageDetector.ISSUE_FORMAT) @@ -274,6 +399,13 @@ class WrongTimberUsageDetectorTest { | public void log() { | new Exception(String.format("msg")); | } + |}""".trimMargin()), + kotlin(""" + |package foo + |class Example { + | fun log() { + | Exception(String.format("msg")) + | } |}""".trimMargin()) ) .issues(WrongTimberUsageDetector.ISSUE_FORMAT) @@ -288,6 +420,13 @@ class WrongTimberUsageDetectorTest { |package foo; |public class Example { | static String[] X = { String.format("%s", 100) }; + |}""".trimMargin()), + kotlin(""" + |package foo + |class Example { + | companion object { + | val X = arrayOf(String.format("%s", 100)) + | } |}""".trimMargin()) ) .issues(WrongTimberUsageDetector.ISSUE_FORMAT) @@ -306,6 +445,15 @@ class WrongTimberUsageDetectorTest { | Exception e = new Exception(); | Timber.d("%s", e); | } + |}""".trimMargin()), + kotlin(""" + |package foo + |import timber.log.Timber + |class Example { + | fun log() { + | val e = Exception() + | Timber.d("%s", e) + | } |}""".trimMargin()) ) .issues(WrongTimberUsageDetector.ISSUE_THROWABLE) @@ -314,12 +462,19 @@ class WrongTimberUsageDetectorTest { |src/foo/Example.java:6: Warning: Throwable should be first argument [ThrowableNotAtBeginning] | Timber.d("%s", e); | ~~~~~~~~~~~~~~~~~ - |0 errors, 1 warnings""".trimMargin()) + |src/foo/Example.kt:6: Warning: Throwable should be first argument [ThrowableNotAtBeginning] + | Timber.d("%s", e) + | ~~~~~~~~~~~~~~~~~ + |0 errors, 2 warnings""".trimMargin()) .expectFixDiffs(""" - |Fix for src/foo/Example.java line 5: Replace with e, "%s": + |Fix for src/foo/Example.java line 6: Replace with e, "%s": |@@ -6 +6 |- Timber.d("%s", e); |+ Timber.d(e, "%s"); + |Fix for src/foo/Example.kt line 6: Replace with e, "%s": + |@@ -6 +6 + |- Timber.d("%s", e) + |+ Timber.d(e, "%s") |""".trimMargin()) } @@ -333,6 +488,14 @@ class WrongTimberUsageDetectorTest { | public void log() { | Timber.d("foo" + "bar"); | } + |}""".trimMargin()), + kotlin(""" + |package foo + |import timber.log.Timber + |class Example { + | fun log() { + | Timber.d("foo" + "bar") + | } |}""".trimMargin()) ) .issues(WrongTimberUsageDetector.ISSUE_BINARY) @@ -351,6 +514,15 @@ class WrongTimberUsageDetectorTest { | String foo = "foo"; | Timber.d(foo + "bar"); | } + |}""".trimMargin()), + kotlin(""" + |package foo + |import timber.log.Timber + |class Example { + | fun log() { + | val foo = "foo" + | Timber.d("${"$"}{foo}bar") + | } |}""".trimMargin()) ) .issues(WrongTimberUsageDetector.ISSUE_BINARY) @@ -379,6 +551,15 @@ class WrongTimberUsageDetectorTest { | String bar = "bar"; | Timber.d("foo" + bar); | } + |}""".trimMargin()), + kotlin(""" + |package foo + |import timber.log.Timber + |class Example { + | fun log() { + | val bar = "bar" + | Timber.d("foo${"$"}bar") + | } |}""".trimMargin()) ) .issues(WrongTimberUsageDetector.ISSUE_BINARY) @@ -408,6 +589,16 @@ class WrongTimberUsageDetectorTest { | String bar = "bar"; | Timber.d(foo + bar); | } + |}""".trimMargin()), + kotlin(""" + |package foo + |import timber.log.Timber + |class Example { + | fun log() { + | val foo = "foo" + | val bar = "bar" + | Timber.d("${"$"}foo${"$"}bar") + | } |}""".trimMargin()) ) .issues(WrongTimberUsageDetector.ISSUE_BINARY) @@ -436,6 +627,15 @@ class WrongTimberUsageDetectorTest { | String s = "world!"; | Timber.d(true ? "Hello, " + s : "Bye"); | } + |}""".trimMargin()), + kotlin(""" + |package foo + |import timber.log.Timber + |class Example { + | fun log() { + | val s = "world!" + | Timber.d(if(true) "Hello, ${"$"}s" else "Bye") + | } |}""".trimMargin()) ) .issues(WrongTimberUsageDetector.ISSUE_BINARY) @@ -457,6 +657,14 @@ class WrongTimberUsageDetectorTest { | public void log() { | Timber.d("%s %s", "arg1"); | } + |}""".trimMargin()), + kotlin(""" + |package foo + |import timber.log.Timber + |class Example { + | fun log() { + | Timber.d("%s %s", "arg1") + | } |}""".trimMargin()) ) .issues(WrongTimberUsageDetector.ISSUE_ARG_COUNT) @@ -465,7 +673,10 @@ class WrongTimberUsageDetectorTest { |src/foo/Example.java:5: Error: Wrong argument count, format string %s %s requires 2 but format call supplies 1 [TimberArgCount] | Timber.d("%s %s", "arg1"); | ~~~~~~~~~~~~~~~~~~~~~~~~~ - |1 errors, 0 warnings""".trimMargin()) + |src/foo/Example.kt:5: Error: Wrong argument count, format string %s %s requires 2 but format call supplies 1 [TimberArgCount] + | Timber.d("%s %s", "arg1") + | ~~~~~~~~~~~~~~~~~~~~~~~~~ + |2 errors, 0 warnings""".trimMargin()) } @Test fun tooManyArgs() { @@ -478,6 +689,14 @@ class WrongTimberUsageDetectorTest { | public void log() { | Timber.d("%s", "arg1", "arg2"); | } + |}""".trimMargin()), + kotlin(""" + |package foo + |import timber.log.Timber + |class Example { + | fun log() { + | Timber.d("%s", "arg1", "arg2") + | } |}""".trimMargin()) ) .issues(WrongTimberUsageDetector.ISSUE_ARG_COUNT) @@ -486,7 +705,10 @@ class WrongTimberUsageDetectorTest { |src/foo/Example.java:5: Error: Wrong argument count, format string %s requires 1 but format call supplies 2 [TimberArgCount] | Timber.d("%s", "arg1", "arg2"); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - |1 errors, 0 warnings""".trimMargin()) + |src/foo/Example.kt:5: Error: Wrong argument count, format string %s requires 1 but format call supplies 2 [TimberArgCount] + | Timber.d("%s", "arg1", "arg2") + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + |2 errors, 0 warnings""".trimMargin()) } @Test fun wrongArgTypes() { @@ -499,6 +721,14 @@ class WrongTimberUsageDetectorTest { | public void log() { | Timber.d("%d", "arg1"); | } + |}""".trimMargin()), + kotlin(""" + |package foo + |import timber.log.Timber + |class Example { + | fun log() { + | Timber.d("%d", "arg1") + | } |}""".trimMargin()) ) .issues(WrongTimberUsageDetector.ISSUE_ARG_TYPES) @@ -507,7 +737,10 @@ class WrongTimberUsageDetectorTest { |src/foo/Example.java:5: Error: Wrong argument type for formatting argument '#1' in %d: conversion is 'd', received String (argument #2 in method call) [TimberArgTypes] | Timber.d("%d", "arg1"); | ~~~~~~ - |1 errors, 0 warnings""".trimMargin()) + |src/foo/Example.kt:5: Error: Wrong argument type for formatting argument '#1' in %d: conversion is 'd', received String (argument #2 in method call) [TimberArgTypes] + | Timber.d("%d", "arg1") + | ~~~~ + |2 errors, 0 warnings""".trimMargin()) } @Test fun tagTooLongLiteralOnly() { @@ -521,6 +754,14 @@ class WrongTimberUsageDetectorTest { | Timber.tag("abcdefghijklmnopqrstuvwx"); | } |}""".trimMargin()), + kotlin(""" + |package foo + |import timber.log.Timber + |class Example { + | fun log() { + | Timber.tag("abcdefghijklmnopqrstuvwx") + | } + |}""".trimMargin()), manifest().minSdk(25) ) .issues(WrongTimberUsageDetector.ISSUE_TAG_LENGTH) @@ -544,6 +785,15 @@ class WrongTimberUsageDetectorTest { | Timber.tag("abcdefghijklmnopqrstuvw" + field); | } |}""".trimMargin()), + kotlin(""" + |package foo + |import timber.log.Timber + |class Example { + | private val field = "x" + | fun log() { + | Timber.tag("abcdefghijklmnopqrstuvw${"$"}field") + | } + |}""".trimMargin()), manifest().minSdk(25) ) .issues(WrongTimberUsageDetector.ISSUE_TAG_LENGTH) @@ -566,6 +816,14 @@ class WrongTimberUsageDetectorTest { | Timber.tag("abcdefghijklmnopqrstuvwx"); | } |}""".trimMargin()), + kotlin(""" + |package foo + |import timber.log.Timber + |class Example { + | fun log() { + | Timber.tag("abcdefghijklmnopqrstuvwx") + | } + |}""".trimMargin()), manifest().minSdk(26) ) .issues(WrongTimberUsageDetector.ISSUE_TAG_LENGTH) @@ -573,7 +831,7 @@ class WrongTimberUsageDetectorTest { .expectClean() } - @Test fun tagTooLongLiteralPlusFieldOnlyBeforeApi26() { + @Test fun tagTooLongLiteralPlusFieldOnlyBeforeApi26() { lint() .files(TIMBER_STUB, java(""" @@ -585,6 +843,15 @@ class WrongTimberUsageDetectorTest { | Timber.tag("abcdefghijklmnopqrstuvw" + field); | } |}""".trimMargin()), + kotlin(""" + |package foo + |import timber.log.Timber + |class Example { + | private val field = "x" + | fun log() { + | Timber.tag("abcdefghijklmnopqrstuvw${"$"}field") + | } + |}""".trimMargin()), manifest().minSdk(26) ) .issues(WrongTimberUsageDetector.ISSUE_TAG_LENGTH) @@ -602,17 +869,26 @@ class WrongTimberUsageDetectorTest { | public void log() { | Timber.tag("tag").d("%s %s", "arg1"); | } + |}""".trimMargin()), + kotlin(""" + |package foo + |import timber.log.Timber + |class Example { + | fun log() { + | Timber.tag("tag").d("%s %s", "arg1") + | } |}""".trimMargin()) ) - .allowMissingSdk() - .issues(WrongTimberUsageDetector.ISSUE_ARG_COUNT) .run() .expect(""" |src/foo/Example.java:5: Error: Wrong argument count, format string %s %s requires 2 but format call supplies 1 [TimberArgCount] | Timber.tag("tag").d("%s %s", "arg1"); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - |1 errors, 0 warnings""".trimMargin()) + |src/foo/Example.kt:5: Error: Wrong argument count, format string %s %s requires 2 but format call supplies 1 [TimberArgCount] + | Timber.tag("tag").d("%s %s", "arg1") + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + |2 errors, 0 warnings""".trimMargin()) } @Test fun tooManyArgsInTag() { @@ -625,6 +901,14 @@ class WrongTimberUsageDetectorTest { | public void log() { | Timber.tag("tag").d("%s", "arg1", "arg2"); | } + |}""".trimMargin()), + kotlin(""" + |package foo + |import timber.log.Timber + |class Example { + | fun log() { + | Timber.tag("tag").d("%s", "arg1", "arg2") + | } |}""".trimMargin()) ) .issues(WrongTimberUsageDetector.ISSUE_ARG_COUNT) @@ -633,7 +917,10 @@ class WrongTimberUsageDetectorTest { |src/foo/Example.java:5: Error: Wrong argument count, format string %s requires 1 but format call supplies 2 [TimberArgCount] | Timber.tag("tag").d("%s", "arg1", "arg2"); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - |1 errors, 0 warnings""".trimMargin()) + |src/foo/Example.kt:5: Error: Wrong argument count, format string %s requires 1 but format call supplies 2 [TimberArgCount] + | Timber.tag("tag").d("%s", "arg1", "arg2") + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + |2 errors, 0 warnings""".trimMargin()) } @Test fun wrongArgTypesInTag() { @@ -646,6 +933,14 @@ class WrongTimberUsageDetectorTest { | public void log() { | Timber.tag("tag").d("%d", "arg1"); | } + |}""".trimMargin()), + kotlin(""" + |package foo + |import timber.log.Timber + |class Example { + | fun log() { + | Timber.tag("tag").d("%d", "arg1") + | } |}""".trimMargin()) ) .issues(WrongTimberUsageDetector.ISSUE_ARG_TYPES) @@ -654,7 +949,10 @@ class WrongTimberUsageDetectorTest { |src/foo/Example.java:5: Error: Wrong argument type for formatting argument '#1' in %d: conversion is 'd', received String (argument #2 in method call) [TimberArgTypes] | Timber.tag("tag").d("%d", "arg1"); | ~~~~~~ - |1 errors, 0 warnings""".trimMargin()) + |src/foo/Example.kt:5: Error: Wrong argument type for formatting argument '#1' in %d: conversion is 'd', received String (argument #2 in method call) [TimberArgTypes] + | Timber.tag("tag").d("%d", "arg1") + | ~~~~ + |2 errors, 0 warnings""".trimMargin()) } @Test fun exceptionLoggingUsingExceptionMessage() { @@ -668,6 +966,15 @@ class WrongTimberUsageDetectorTest { | Exception e = new Exception(); | Timber.d(e.getMessage()); | } + |}""".trimMargin()), + kotlin(""" + |package foo + |import timber.log.Timber + |class Example { + | fun log() { + | val e = Exception() + | Timber.d(e.message) + | } |}""".trimMargin()) ) .issues(WrongTimberUsageDetector.ISSUE_EXCEPTION_LOGGING) @@ -676,12 +983,19 @@ class WrongTimberUsageDetectorTest { |src/foo/Example.java:6: Warning: Explicitly logging exception message is redundant [TimberExceptionLogging] | Timber.d(e.getMessage()); | ~~~~~~~~~~~~~~~~~~~~~~~~ - |0 errors, 1 warnings""".trimMargin()) + |src/foo/Example.kt:6: Warning: Explicitly logging exception message is redundant [TimberExceptionLogging] + | Timber.d(e.message) + | ~~~~~~~~~~~~~~~~~~~ + |0 errors, 2 warnings""".trimMargin()) .expectFixDiffs(""" - |Fix for src/foo/Example.java line 5: Replace message with throwable: + |Fix for src/foo/Example.java line 6: Replace message with throwable: |@@ -6 +6 |- Timber.d(e.getMessage()); |+ Timber.d(e); + |Fix for src/foo/Example.kt line 6: Replace message with throwable: + |@@ -6 +6 + |- Timber.d(e.message) + |+ Timber.d(e) |""".trimMargin()) } @@ -696,6 +1010,15 @@ class WrongTimberUsageDetectorTest { | Exception e = new Exception(); | Timber.d(e, e.getMessage()); | } + |}""".trimMargin()), + kotlin(""" + |package foo + |import timber.log.Timber + |class Example { + | fun log() { + | val e = Exception() + | Timber.d(e, e.message) + | } |}""".trimMargin()) ) .issues(WrongTimberUsageDetector.ISSUE_EXCEPTION_LOGGING) @@ -704,37 +1027,15 @@ class WrongTimberUsageDetectorTest { |src/foo/Example.java:6: Warning: Explicitly logging exception message is redundant [TimberExceptionLogging] | Timber.d(e, e.getMessage()); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ - |0 errors, 1 warnings""".trimMargin()) + |src/foo/Example.kt:6: Warning: Explicitly logging exception message is redundant [TimberExceptionLogging] + | Timber.d(e, e.message) + | ~~~~~~~~~~~~~~~~~~~~~~ + |0 errors, 2 warnings""".trimMargin()) .expectFixDiffs(""" |Fix for src/foo/Example.java line 5: Remove redundant argument: |@@ -6 +6 |- Timber.d(e, e.getMessage()); |+ Timber.d(e); - |""".trimMargin()) - } - - @Test fun exceptionLoggingUsingExceptionMessageArgumentInKotlin() { - lint() - .files(TIMBER_STUB, - kt(""" - |package foo - |import timber.log.Timber - |class Example { - | fun log() { - | val e = Exception() - | Timber.d(e, e.message) - | } - |} - """.trimMargin()) - ) - .issues(WrongTimberUsageDetector.ISSUE_EXCEPTION_LOGGING) - .run() - .expect(""" - |src/foo/Example.kt:6: Warning: Explicitly logging exception message is redundant [TimberExceptionLogging] - | Timber.d(e, e.message) - | ~~~~~~~~~~~~~~~~~~~~~~ - |0 errors, 1 warnings""".trimMargin()) - .expectFixDiffs(""" |Fix for src/foo/Example.kt line 5: Remove redundant argument: |@@ -6 +6 |- Timber.d(e, e.message) @@ -754,6 +1055,16 @@ class WrongTimberUsageDetectorTest { | Exception e = new Exception(); | Timber.d(e, msg); | } + |}""".trimMargin()), + kotlin(""" + |package foo + |import timber.log.Timber + |class Example { + | fun log() { + | val msg = "Hello" + | val e = Exception() + | Timber.d(e, msg) + | } |}""".trimMargin()) ) .issues(WrongTimberUsageDetector.ISSUE_EXCEPTION_LOGGING) @@ -771,6 +1082,14 @@ class WrongTimberUsageDetectorTest { | public void log(Exception e, String message) { | Timber.d(e, message); | } + |}""".trimMargin()), + kotlin(""" + |package foo + |import timber.log.Timber + |class Example { + | fun log(e: Exception, message: String) { + | Timber.d(e, message) + | } |}""".trimMargin()) ) .issues(WrongTimberUsageDetector.ISSUE_EXCEPTION_LOGGING) @@ -791,6 +1110,17 @@ class WrongTimberUsageDetectorTest { | private String method() { | return "foo"; | } + |}""".trimMargin()), + kotlin(""" + |package foo + |import timber.log.Timber + |class Example { + | fun log(e: Exception) { + | Timber.d(e, method()) + | } + | private fun method(): String { + | return "foo" + | } |}""".trimMargin()) ) .issues(WrongTimberUsageDetector.ISSUE_EXCEPTION_LOGGING) @@ -810,6 +1140,16 @@ class WrongTimberUsageDetectorTest { | Exception e = new Exception(); | Timber.d(e, message); | } + |}""".trimMargin()), + kotlin(""" + |package foo + |import timber.log.Timber + |class Example { + | private var message = "" + | fun log() { + | val e = Exception() + | Timber.d(e, message) + | } |}""".trimMargin()) ) .issues(WrongTimberUsageDetector.ISSUE_EXCEPTION_LOGGING) @@ -829,6 +1169,16 @@ class WrongTimberUsageDetectorTest { | Exception e = new Exception(); | Timber.d(e, message); | } + |}""".trimMargin()), + kotlin(""" + |package foo + |import timber.log.Timber + |class Example { + | private val message = "" + | fun log() { + | val e = Exception() + | Timber.d(e, message) + | } |}""".trimMargin()) ) .issues(WrongTimberUsageDetector.ISSUE_EXCEPTION_LOGGING) @@ -847,6 +1197,15 @@ class WrongTimberUsageDetectorTest { | Exception e = new Exception(); | Timber.d(e, ""); | } + |}""".trimMargin()), + kotlin(""" + |package foo + |import timber.log.Timber + |class Example { + | fun log() { + | val e = Exception() + | Timber.d(e, "") + | } |}""".trimMargin()) ) .issues(WrongTimberUsageDetector.ISSUE_EXCEPTION_LOGGING) @@ -855,12 +1214,19 @@ class WrongTimberUsageDetectorTest { |src/foo/Example.java:6: Warning: Use single-argument log method instead of null/empty message [TimberExceptionLogging] | Timber.d(e, ""); | ~~~~~~~~~~~~~~~ - |0 errors, 1 warnings""".trimMargin()) + |src/foo/Example.kt:6: Warning: Use single-argument log method instead of null/empty message [TimberExceptionLogging] + | Timber.d(e, "") + | ~~~~~~~~~~~~~~~ + |0 errors, 2 warnings""".trimMargin()) .expectFixDiffs(""" - |Fix for src/foo/Example.java line 5: Remove redundant argument: + |Fix for src/foo/Example.java line 6: Remove redundant argument: |@@ -6 +6 |- Timber.d(e, ""); |+ Timber.d(e); + |Fix for src/foo/Example.kt line 6: Remove redundant argument: + |@@ -6 +6 + |- Timber.d(e, "") + |+ Timber.d(e) |""".trimMargin()) } @@ -875,6 +1241,15 @@ class WrongTimberUsageDetectorTest { | Exception e = new Exception(); | Timber.d(e, null); | } + |}""".trimMargin()), + kotlin(""" + |package foo + |import timber.log.Timber + |class Example { + | fun log() { + | val e = Exception() + | Timber.d(e, null) + | } |}""".trimMargin()) ) .issues(WrongTimberUsageDetector.ISSUE_EXCEPTION_LOGGING) @@ -883,12 +1258,19 @@ class WrongTimberUsageDetectorTest { |src/foo/Example.java:6: Warning: Use single-argument log method instead of null/empty message [TimberExceptionLogging] | Timber.d(e, null); | ~~~~~~~~~~~~~~~~~ - |0 errors, 1 warnings""".trimMargin()) + |src/foo/Example.kt:6: Warning: Use single-argument log method instead of null/empty message [TimberExceptionLogging] + | Timber.d(e, null) + | ~~~~~~~~~~~~~~~~~ + |0 errors, 2 warnings""".trimMargin()) .expectFixDiffs(""" - |Fix for src/foo/Example.java line 5: Remove redundant argument: + |Fix for src/foo/Example.java line 6: Remove redundant argument: |@@ -6 +6 |- Timber.d(e, null); |+ Timber.d(e); + |Fix for src/foo/Example.kt line 6: Remove redundant argument: + |@@ -6 +6 + |- Timber.d(e, null) + |+ Timber.d(e) |""".trimMargin()) } @@ -903,6 +1285,15 @@ class WrongTimberUsageDetectorTest { | Exception e = new Exception(); | Timber.d(e, "Valid message"); | } + |}""".trimMargin()), + kotlin(""" + |package foo + |import timber.log.Timber + |class Example { + | fun log() { + | val e = Exception() + | Timber.d(e, "Valid message") + | } |}""".trimMargin()) ) .issues(WrongTimberUsageDetector.ISSUE_EXCEPTION_LOGGING) @@ -920,6 +1311,14 @@ class WrongTimberUsageDetectorTest { | public void log() { | Timber.d("%tc", new java.util.Date()); | } + |}""".trimMargin()), + kotlin(""" + |package foo + |import timber.log.Timber + |class Example { + | fun log() { + | Timber.d("%tc", java.util.Date()) + | } |}""".trimMargin()) ) .issues(WrongTimberUsageDetector.ISSUE_ARG_TYPES) @@ -937,6 +1336,14 @@ class WrongTimberUsageDetectorTest { | public void log() { | Timber.d("%tc", System.currentTimeMillis()); | } + |}""".trimMargin()), + kotlin(""" + |package foo + |import timber.log.Timber + |class Example { + | fun log() { + | Timber.d("%tc", System.currentTimeMillis()) + | } |}""".trimMargin()) ) .issues(WrongTimberUsageDetector.ISSUE_ARG_TYPES) @@ -954,7 +1361,8 @@ class WrongTimberUsageDetectorTest { | public void log() { | Timber.d("%b", Boolean.valueOf(true)); | } - |}""".trimMargin()) + |}""".trimMargin()), + // no kotlin equivalent, since primitive wrappers do not exist ) .issues(WrongTimberUsageDetector.ISSUE_ARG_TYPES) .run() @@ -976,6 +1384,19 @@ class WrongTimberUsageDetectorTest { | Timber.d(bar.baz); | } |} + """.trimMargin()), + kotlin(""" + |package foo + |import timber.log.Timber + |class Example { + | class Bar { + | val baz = "timber" + | } + | fun log() { + | val bar = Bar() + | Timber.d(bar.baz) + | } + |} """.trimMargin()) ) .issues(WrongTimberUsageDetector.ISSUE_EXCEPTION_LOGGING) diff --git a/timber-sample/src/main/java/com/example/timber/ui/LintActivity.java b/timber-sample/src/main/java/com/example/timber/ui/JavaLintActivity.java similarity index 97% rename from timber-sample/src/main/java/com/example/timber/ui/LintActivity.java rename to timber-sample/src/main/java/com/example/timber/ui/JavaLintActivity.java index 79cf68944..ea207d643 100644 --- a/timber-sample/src/main/java/com/example/timber/ui/LintActivity.java +++ b/timber-sample/src/main/java/com/example/timber/ui/JavaLintActivity.java @@ -10,7 +10,7 @@ import static java.lang.String.format; @SuppressLint("Registered") // -public class LintActivity extends Activity { +public class JavaLintActivity extends Activity { /** * Below are some examples of how NOT to use Timber. * diff --git a/timber-sample/src/main/java/com/example/timber/ui/KotlinLintActivity.kt b/timber-sample/src/main/java/com/example/timber/ui/KotlinLintActivity.kt new file mode 100644 index 000000000..2ba740556 --- /dev/null +++ b/timber-sample/src/main/java/com/example/timber/ui/KotlinLintActivity.kt @@ -0,0 +1,76 @@ +package com.example.timber.ui + +import android.annotation.SuppressLint +import android.app.Activity +import android.os.Bundle +import android.util.Log +import timber.log.Timber +import java.lang.Exception +import java.lang.String.format + +@SuppressLint("Registered") +class KotlinLintActivity : Activity() { + /** + * Below are some examples of how NOT to use Timber. + * + * To see how a particular lint issue behaves, comment/remove its corresponding id from the set + * of SuppressLint ids below. + */ + @SuppressLint( + "LogNotTimber", + "StringFormatInTimber", + "ThrowableNotAtBeginning", + "BinaryOperationInTimber", + "TimberArgCount", + "TimberArgTypes", + "TimberTagLength", + "TimberExceptionLogging" + ) + @Suppress("RemoveRedundantQualifierName") + override fun onCreate(savedInstanceState: Bundle?) { + super.onCreate(savedInstanceState) + + // LogNotTimber + Log.d("TAG", "msg") + Log.d("TAG", "msg", Exception()) + android.util.Log.d("TAG", "msg") + android.util.Log.d("TAG", "msg", Exception()) + + // StringFormatInTimber + Timber.w(String.format("%s", getString())) + Timber.w(format("%s", getString())) + + // ThrowableNotAtBeginning + Timber.d("%s", Exception()) + + // BinaryOperationInTimber + val foo = "foo" + val bar = "bar" + Timber.d("foo" + "bar") + Timber.d("foo$bar") + Timber.d("${foo}bar") + Timber.d("$foo$bar") + + // TimberArgCount + Timber.d("%s %s", "arg0") + Timber.d("%s", "arg0", "arg1") + Timber.tag("tag").d("%s %s", "arg0") + Timber.tag("tag").d("%s", "arg0", "arg1") + + // TimberArgTypes + Timber.d("%d", "arg0") + Timber.tag("tag").d("%d", "arg0") + + // TimberTagLength + Timber.tag("abcdefghijklmnopqrstuvwx") + Timber.tag("abcdefghijklmnopqrstuvw" + "x") + + // TimberExceptionLogging + Timber.d(Exception(), Exception().message) + Timber.d(Exception(), "") + Timber.d(Exception(), null) + Timber.d(Exception().message) + } + + private fun getString() = "foo" +} \ No newline at end of file