Skip to content

Commit 4ce3223

Browse files
csadilekpocmo
authored andcommitted
Closes mozilla-mobile#1058: Lint Log check leaks into embedding applications
1 parent a126b08 commit 4ce3223

File tree

3 files changed

+75
-7
lines changed

3 files changed

+75
-7
lines changed

components/tooling/lint/src/main/java/mozilla/components/tooling/lint/LintLogChecks.kt

+17-7
Original file line numberDiff line numberDiff line change
@@ -13,28 +13,38 @@ import com.android.tools.lint.detector.api.Scope
1313
import com.android.tools.lint.detector.api.Severity
1414
import com.intellij.psi.PsiMethod
1515
import org.jetbrains.uast.UCallExpression
16+
import org.jetbrains.uast.getContainingUClass
1617
import java.util.EnumSet
1718

18-
private const val ANDROID_LOG_CLASS = "android.util.Log"
19+
internal const val ANDROID_LOG_CLASS = "android.util.Log"
20+
internal const val ERROR_MESSAGE = "Using Android Log instead of base component"
1921

2022
/**
2123
* Custom lint checks related to logging.
2224
*/
2325
class LintLogChecks : Detector(), Detector.UastScanner {
26+
private val componentPackages = listOf("mozilla.components", "org.mozilla.telemetry", "org.mozilla.samples")
27+
2428
override fun getApplicableMethodNames() = listOf("v", "d", "i", "w", "e")
2529

2630
override fun visitMethod(context: JavaContext, node: UCallExpression, method: PsiMethod) {
2731
if (context.evaluator.isMemberInClass(method, ANDROID_LOG_CLASS)) {
28-
context.report(
29-
ISSUE_LOG_USAGE,
30-
node,
31-
context.getLocation(node),
32-
"Using Android Log instead of base component")
32+
val inComponentPackage = componentPackages.any {
33+
node.methodIdentifier?.getContainingUClass()?.qualifiedName?.startsWith(it) == true
34+
}
35+
36+
if (inComponentPackage) {
37+
context.report(
38+
ISSUE_LOG_USAGE,
39+
node,
40+
context.getLocation(node),
41+
ERROR_MESSAGE)
42+
}
3343
}
3444
}
3545

3646
companion object {
37-
private val ISSUE_LOG_USAGE = Issue.create(
47+
internal val ISSUE_LOG_USAGE = Issue.create(
3848
"LogUsage",
3949
"Log/Logger from base component should be used.",
4050
"""The Log or Logger class from the base component should be used for logging instead of
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
/* This Source Code Form is subject to the terms of the Mozilla Public
2+
* License, v. 2.0. If a copy of the MPL was not distributed with this
3+
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
4+
5+
package mozilla.components.tooling.lint
6+
7+
import com.android.tools.lint.client.api.JavaEvaluator
8+
import com.android.tools.lint.detector.api.JavaContext
9+
import com.intellij.psi.PsiMethod
10+
import mozilla.components.tooling.lint.LintLogChecks.Companion.ISSUE_LOG_USAGE
11+
import org.jetbrains.uast.UCallExpression
12+
import org.jetbrains.uast.UClass
13+
import org.jetbrains.uast.UIdentifier
14+
import org.jetbrains.uast.getContainingUClass
15+
import org.junit.Test
16+
import org.mockito.Mockito.`when`
17+
import org.mockito.Mockito.mock
18+
import org.mockito.Mockito.never
19+
import org.mockito.Mockito.times
20+
import org.mockito.Mockito.verify
21+
22+
class LintLogChecksTest {
23+
24+
@Test
25+
fun `report log error in components code only`() {
26+
val evaluator = mock(JavaEvaluator::class.java)
27+
val context = mock(JavaContext::class.java)
28+
val node = mock(UCallExpression::class.java)
29+
val method = mock(PsiMethod::class.java)
30+
val methodIdentifier = mock(UIdentifier::class.java)
31+
val clazz = mock(UClass::class.java)
32+
33+
`when`(evaluator.isMemberInClass(method, ANDROID_LOG_CLASS)).thenReturn(true)
34+
`when`(context.evaluator).thenReturn(evaluator)
35+
36+
val logCheck = LintLogChecks()
37+
logCheck.visitMethod(context, node, method)
38+
verify(context, never()).report(ISSUE_LOG_USAGE, node, context.getLocation(node), ERROR_MESSAGE)
39+
40+
`when`(node.methodIdentifier).thenReturn(methodIdentifier)
41+
logCheck.visitMethod(context, node, method)
42+
verify(context, never()).report(ISSUE_LOG_USAGE, node, context.getLocation(node), ERROR_MESSAGE)
43+
44+
`when`(methodIdentifier.getContainingUClass()).thenReturn(clazz)
45+
logCheck.visitMethod(context, node, method)
46+
verify(context, never()).report(ISSUE_LOG_USAGE, node, context.getLocation(node), ERROR_MESSAGE)
47+
48+
`when`(clazz.qualifiedName).thenReturn("com.some.app.Class")
49+
logCheck.visitMethod(context, node, method)
50+
verify(context, never()).report(ISSUE_LOG_USAGE, node, context.getLocation(node), ERROR_MESSAGE)
51+
52+
`when`(clazz.qualifiedName).thenReturn("mozilla.components.some.Class")
53+
logCheck.visitMethod(context, node, method)
54+
verify(context, times(1)).report(ISSUE_LOG_USAGE, node, context.getLocation(node), ERROR_MESSAGE)
55+
}
56+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
mock-maker-inline
2+
// This allows mocking final classes (classes are final by default in Kotlin)

0 commit comments

Comments
 (0)