summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Sal Savage <salsavage@google.com> 2024-02-27 14:55:14 -0800
committer Sal Savage <salsavage@google.com> 2024-04-01 14:59:31 -0700
commitef098768cec2af64f71dec206ab9d85a9e8407cd (patch)
treed47baa322799f7a859a53db547ad43906ac43463
parent5ef0c449fa60bbba69a13b1b7c923937e5909583 (diff)
Create an Android Lint rule to find guarded log invocations
The Bluetooth stack sets a process default log level, which allows the Android Log framework to properly enforce wheth a log invocation fires based on the set log level for the tag. You do not need to guard individual log invocations anymore. This lint rule looks for log lines inside conditionals (if, else if, or even else), where the conditionals use variables that appear to be log enforcement variables as part of their logic. Errors are generated for those usages. Because this is mostly name based, this is very much best effort, and will have to be updated as patterns in the stack evolve. This was able to find all the violations in the stack that existed before our refactor though. Usages of Log.isLoggable() in conditionals are reduced to warnings, as there are a few valid usages to prevent expensive log operations in code. In general, we expect these usages to be minimal and hope developers will consider altering log statements to avoid usages. Run this lint rule together with others using `m Bluetooth-lint`. Results are available in txt/xml/html format in the build output artifacts. See the README for more details. Tag: #feature Bug: 315046089 Flag: EXEMPT, tooling change Test: atest BluetoothLintCheckerTest --host Change-Id: I175f908f97567ed6638af6026f4caf83acd57e60
-rw-r--r--tools/lint/checks/src/com/android/bluetooth/lint/BluetoothLintCheckerRegistry.kt7
-rw-r--r--tools/lint/checks/src/com/android/bluetooth/lint/GuardedLogLineDetector.kt294
-rw-r--r--tools/lint/checks/src/com/android/bluetooth/lint/Utils.kt4
-rw-r--r--tools/lint/checks/tests/com/android/bluetooth/lint/test/GuardedLogLineDetectorTest.kt572
4 files changed, 874 insertions, 3 deletions
diff --git a/tools/lint/checks/src/com/android/bluetooth/lint/BluetoothLintCheckerRegistry.kt b/tools/lint/checks/src/com/android/bluetooth/lint/BluetoothLintCheckerRegistry.kt
index 863fd5cf4f..94dc4add29 100644
--- a/tools/lint/checks/src/com/android/bluetooth/lint/BluetoothLintCheckerRegistry.kt
+++ b/tools/lint/checks/src/com/android/bluetooth/lint/BluetoothLintCheckerRegistry.kt
@@ -24,7 +24,12 @@ import com.google.auto.service.AutoService
@AutoService(IssueRegistry::class)
@Suppress("UnstableApiUsage")
class BluetoothLintCheckerIssueRegistry : IssueRegistry() {
- override val issues = listOf(LogEnforcementVariableCreationDetector.ISSUE)
+ override val issues =
+ listOf(
+ LogEnforcementVariableCreationDetector.ISSUE,
+ GuardedLogLineDetector.ISSUE,
+ GuardedLogLineDetector.WARNING
+ )
override val api: Int
get() = CURRENT_API
diff --git a/tools/lint/checks/src/com/android/bluetooth/lint/GuardedLogLineDetector.kt b/tools/lint/checks/src/com/android/bluetooth/lint/GuardedLogLineDetector.kt
new file mode 100644
index 0000000000..4a4f0e17c9
--- /dev/null
+++ b/tools/lint/checks/src/com/android/bluetooth/lint/GuardedLogLineDetector.kt
@@ -0,0 +1,294 @@
+/*
+ * Copyright (C) 2024 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.bluetooth.lint
+
+import com.android.tools.lint.client.api.UElementHandler
+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.Scope
+import com.android.tools.lint.detector.api.Severity
+import com.android.tools.lint.detector.api.SourceCodeScanner
+import org.jetbrains.uast.UBinaryExpression
+import org.jetbrains.uast.UCallExpression
+import org.jetbrains.uast.UClass
+import org.jetbrains.uast.UElement
+import org.jetbrains.uast.UExpression
+import org.jetbrains.uast.UIfExpression
+import org.jetbrains.uast.UParenthesizedExpression
+import org.jetbrains.uast.UPolyadicExpression
+import org.jetbrains.uast.UQualifiedReferenceExpression
+import org.jetbrains.uast.USimpleNameReferenceExpression
+import org.jetbrains.uast.UUnaryExpression
+
+/**
+ * Lint check for guarded log lines
+ *
+ * Logging enforcement variables are not allowed to be _used_. i.e.:
+ *
+ * if (DBG) {
+ * Log.d(TAG, "message");
+ * }
+ * if (Log.isLoggable(TAG, Log.DEBUG)) {
+ * Log.d(TAG, "message");
+ * }
+ * if (foo != null) {
+ * // ...
+ * } else if (DBG) {
+ * Log.d(TAG, "foo was null");
+ * }
+ * if (!DBG) {
+ * // ...
+ * } else {
+ * Log.d(TAG, "foo was null");
+ * }
+ * if (DBG) {
+ * if (foo != null) {
+ * Log.d(TAG, "foo was null");
+ * }
+ * }
+ */
+class GuardedLogLineDetector : Detector(), SourceCodeScanner {
+ private val LOG_ENFORCEMENT_VARS = listOf("DBG", "DEBUG", "VDBG", "VERBOSE", "D", "V")
+ private val LOG_ENFORCEMENT_VAR_ENDINGS = listOf("_DBG", "_VDBG")
+ private val LOG_LOGGING_FUNCTIONS = listOf("wtf", "e", "w", "i", "d", "v")
+
+ enum class LogEnforcementType {
+ NONE,
+ VARIABLE,
+ IS_LOGGABLE
+ }
+
+ companion object {
+ const val GUARDED_LOG_INVOCATION_ERROR =
+ "Do not guard log invocations with if blocks using log enforcement variables or" +
+ " isLoggable(). The Log framework does this check for you. Remove the surrounding if" +
+ " block and call to log completely unguarded"
+
+ val ISSUE =
+ Issue.create(
+ id = "GuardedLogInvocation",
+ briefDescription =
+ "Do not guard log invocations with if blocks using log enforcement variables",
+ explanation =
+ "The BT stack defines a process default log level, which allows the Android" +
+ " Log framework (For Java, Kotlin, _and_ Native) to properly enforce log" +
+ " levels for us. Using our own variables for enforcement causes inconsistency" +
+ " in log output and double checks against the log level each time we log." +
+ " Please delete this variable and use the Log functions unguarded in your" +
+ " code.",
+ category = Category.CORRECTNESS,
+ severity = Severity.ERROR,
+ implementation =
+ Implementation(GuardedLogLineDetector::class.java, Scope.JAVA_FILE_SCOPE),
+ androidSpecific = true,
+ )
+
+ val WARNING =
+ Issue.create(
+ id = "GuardedLogInvocation",
+ briefDescription =
+ "Guarding log invocations with calls to Log#isLoggable() should be used" +
+ " rarely, if ever. Please reconsider what you're logging and/or if it needs" +
+ "to be guarded in the first place.",
+ explanation =
+ "The BT stack defines a process default log level, which allows the Android" +
+ " Log framework (For Java, Kotlin, _and_ Native) to properly enforce log" +
+ " levels for us. Using Log#isLoggable() calls to guard invocations is at the" +
+ " very least redunant. It's also typically used in patterns where non-log" +
+ " code is guarded, like string builders and loops. In rare cases, we've " +
+ " even seen abuse of log level checking to hide different logic/behavior, or " +
+ " forms of debug, like writing to disk. Please reconsider what you're logging" +
+ " and if it should be guarded be Log#isLoggable().",
+ category = Category.CORRECTNESS,
+ severity = Severity.WARNING,
+ implementation =
+ Implementation(GuardedLogLineDetector::class.java, Scope.JAVA_FILE_SCOPE),
+ androidSpecific = true,
+ )
+ }
+
+ override fun getApplicableUastTypes(): List<Class<out UElement>> {
+ return listOf(UCallExpression::class.java)
+ }
+
+ override fun createUastHandler(context: JavaContext): UElementHandler? {
+ return object : UElementHandler() {
+ override fun visitCallExpression(node: UCallExpression) {
+ val callingClass = findOwningUClass(node)
+ if (!isBluetoothClass(callingClass)) {
+ return
+ }
+
+ if (!isLoggingFunction(node)) {
+ return
+ }
+
+ var ifStatement = findNextContainingUIfExpression(node.uastParent)
+ while (ifStatement != null) {
+ var enforcementType = isExpressionWithLogEnforcement(ifStatement.condition)
+ if (enforcementType == LogEnforcementType.VARIABLE) {
+ context.report(
+ issue = ISSUE,
+ location = context.getNameLocation(ifStatement),
+ message = GUARDED_LOG_INVOCATION_ERROR,
+ )
+ return
+ } else if (enforcementType == LogEnforcementType.IS_LOGGABLE) {
+ context.report(
+ issue = WARNING,
+ location = context.getNameLocation(ifStatement),
+ message = GUARDED_LOG_INVOCATION_ERROR,
+ )
+ return
+ }
+
+ ifStatement = findNextContainingUIfExpression(ifStatement.uastParent)
+ }
+ }
+ }
+ }
+
+ /** Traverse the element tree upward to find the closest UClass to a given expression */
+ private fun findOwningUClass(node: UElement?): UClass? {
+ if (node == null) {
+ return null
+ }
+
+ if (node is UClass) {
+ return node
+ }
+
+ return findOwningUClass(node.uastParent)
+ }
+
+ /*
+ * Returns the most recent parent IfExpression for a given expression, or null if the expression
+ * is not contained in an IfExpression
+ */
+ private fun findNextContainingUIfExpression(node: UElement?): UIfExpression? {
+ if (node == null) {
+ return null
+ }
+
+ if (node is UIfExpression) {
+ return node
+ }
+
+ return findNextContainingUIfExpression(node.uastParent)
+ }
+
+ /*
+ * Determines if the given Expression contains any usages of Log.isLoggable, or any variables
+ * that are likely log enforcement variables
+ */
+ private fun isExpressionWithLogEnforcement(node: UExpression): LogEnforcementType {
+ when (node) {
+ // A simple class or local variable reference, i.e. "DBG" or "VDBG"
+ is USimpleNameReferenceExpression -> {
+ if (isLogEnforcementVariable(node.identifier)) {
+ return LogEnforcementType.VARIABLE
+ }
+ return LogEnforcementType.NONE
+ }
+
+ // An actual function call, i.e. "isLoggable()" part of Log.isLoggable()
+ is UCallExpression -> {
+ if (isLoggableFunction(node)) {
+ return LogEnforcementType.IS_LOGGABLE
+ }
+ return LogEnforcementType.NONE
+ }
+
+ // A unary operation on another expression, i.e. "!DBG" or "!Log.isLoggable()""
+ is UUnaryExpression -> {
+ return isExpressionWithLogEnforcement(node.operand)
+ }
+
+ // A binary operation on another expression, i.e. "DBG || Log.isLoggable()"
+ is UBinaryExpression -> {
+ val leftEnforcementType = isExpressionWithLogEnforcement(node.leftOperand)
+ if (leftEnforcementType != LogEnforcementType.NONE) {
+ return leftEnforcementType
+ }
+
+ val rightEnforcementType = isExpressionWithLogEnforcement(node.rightOperand)
+ if (rightEnforcementType != LogEnforcementType.NONE) {
+ return rightEnforcementType
+ }
+
+ return LogEnforcementType.NONE
+ }
+
+ // A conditional expression with multiple operators, i.e. "mFoo || DBG && i < 6"
+ is UPolyadicExpression -> {
+ for (subExpression in node.operands) {
+ var enforcementType = isExpressionWithLogEnforcement(subExpression)
+ if (enforcementType != LogEnforcementType.NONE) {
+ return enforcementType
+ }
+ }
+ return LogEnforcementType.NONE
+ }
+
+ // A function compound call, i.e. "Log.isLoggable()""
+ is UQualifiedReferenceExpression -> {
+ return isExpressionWithLogEnforcement(node.selector)
+ }
+
+ // An expression surrounded by parenthesis, i.e. "(DBG || Log.isLoggable())"
+ is UParenthesizedExpression -> {
+ return isExpressionWithLogEnforcement(node.expression)
+ }
+ }
+ return LogEnforcementType.NONE
+ }
+
+ /*
+ * Determines if the given call is one to any of the various Log framework calls that write a
+ * log line to logcat, i.e. wtf, e, w, i, d, or v
+ */
+ private fun isLoggingFunction(node: UCallExpression): Boolean {
+ val resolvedMethod = node.resolve()
+ val methodClassName = resolvedMethod?.containingClass?.qualifiedName
+ val methodName = resolvedMethod?.name
+ return methodClassName == "android.util.Log" && methodName in LOG_LOGGING_FUNCTIONS
+ }
+
+ /** Determines if the given call is one to Log.isLoggable() */
+ private fun isLoggableFunction(node: UCallExpression): Boolean {
+ val resolvedMethod = node.resolve()
+ val methodClassName = resolvedMethod?.containingClass?.qualifiedName
+ val methodName = resolvedMethod?.name
+ return methodClassName == "android.util.Log" && methodName == "isLoggable"
+ }
+
+ /*
+ * Checks a string variable name to see if its one of the common names used in the stack for
+ * log enforcement variables.
+ *
+ * These include things like DBG, VDBG, DEBUG, VERBOSE, D, V, etc., or variables that _end_ in
+ * _DBG or _VDBG
+ */
+ private fun isLogEnforcementVariable(name: String): Boolean {
+ val nameUpper = name.uppercase()
+ return nameUpper in LOG_ENFORCEMENT_VARS ||
+ LOG_ENFORCEMENT_VAR_ENDINGS.any { nameUpper.endsWith(it) }
+ }
+}
diff --git a/tools/lint/checks/src/com/android/bluetooth/lint/Utils.kt b/tools/lint/checks/src/com/android/bluetooth/lint/Utils.kt
index 12ec820df2..9acf2af5ac 100644
--- a/tools/lint/checks/src/com/android/bluetooth/lint/Utils.kt
+++ b/tools/lint/checks/src/com/android/bluetooth/lint/Utils.kt
@@ -21,8 +21,8 @@ import org.jetbrains.uast.UClass
val BLUETOOTH_PLATFORM_PACKAGE = "com.android.bluetooth"
/** Returns true */
-fun isBluetoothClass(node: UClass): Boolean {
- return node.qualifiedName?.startsWith(BLUETOOTH_PLATFORM_PACKAGE) ?: false
+fun isBluetoothClass(node: UClass?): Boolean {
+ return node?.qualifiedName?.startsWith(BLUETOOTH_PLATFORM_PACKAGE) ?: false
}
/** Writes lines to debug output, visible in the isolated Java output */
diff --git a/tools/lint/checks/tests/com/android/bluetooth/lint/test/GuardedLogLineDetectorTest.kt b/tools/lint/checks/tests/com/android/bluetooth/lint/test/GuardedLogLineDetectorTest.kt
new file mode 100644
index 0000000000..e6088b6c97
--- /dev/null
+++ b/tools/lint/checks/tests/com/android/bluetooth/lint/test/GuardedLogLineDetectorTest.kt
@@ -0,0 +1,572 @@
+/*
+ * Copyright (C) 2024 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.bluetooth.lint.test
+
+import com.android.bluetooth.lint.GuardedLogLineDetector
+import com.android.tools.lint.checks.infrastructure.LintDetectorTest
+import com.android.tools.lint.checks.infrastructure.TestFile
+import com.android.tools.lint.checks.infrastructure.TestLintTask
+import com.android.tools.lint.detector.api.Detector
+import com.android.tools.lint.detector.api.Issue
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.JUnit4
+
+@Suppress("UnstableApiUsage")
+@RunWith(JUnit4::class)
+class GuardedLogLineDetectorTest : LintDetectorTest() {
+ override fun getDetector(): Detector = GuardedLogLineDetector()
+
+ override fun getIssues(): List<Issue> = listOf(GuardedLogLineDetector.ISSUE)
+
+ override fun lint(): TestLintTask = super.lint().allowMissingSdk(true)
+
+ @Test
+ fun testUnguardedLogStatements_noIssuesFound() {
+ lint()
+ .files(
+ java(
+ """
+package com.android.bluetooth;
+
+import android.util.Log;
+
+public final class Foo {
+ private static final String TAG = Foo.class.getSimpleName();
+ private static final boolean DBG = true;
+
+ public Foo() {
+ init(6);
+ }
+
+ public void init(int i) {
+ Log.v(TAG, "Log as v");
+ Log.d(TAG, "Log as d");
+ Log.i(TAG, "Log as i");
+ Log.w(TAG, "Log as w");
+ Log.e(TAG, "Log as e");
+ Log.wtf(TAG, "Log as a");
+ }
+}
+ """
+ ),
+ *stubs
+ )
+ .run()
+ .expectClean()
+ }
+
+ @Test
+ fun testUnguardedLogStatements_inSafeIfStatement_noIssuesFound() {
+ lint()
+ .files(
+ java(
+ """
+package com.android.bluetooth;
+
+import android.util.Log;
+
+public final class Foo {
+ private static final String TAG = Foo.class.getSimpleName();
+ private static final boolean DBG = true;
+
+ public Foo() {
+ init(6);
+ }
+
+ public void init(int i) {
+ if (i > 6) {
+ Log.v(TAG, "Log as v");
+ Log.d(TAG, "Log as d");
+ Log.i(TAG, "Log as i");
+ Log.w(TAG, "Log as w");
+ Log.e(TAG, "Log as e");
+ Log.wtf(TAG, "Log as a");
+ }
+ }
+}
+ """
+ ),
+ *stubs
+ )
+ .run()
+ .expectClean()
+ }
+
+ @Test
+ fun testGuardedLogWithIsLoggable_warningFound() {
+ lint()
+ .files(
+ java(
+ """
+package com.android.bluetooth;
+
+import android.util.Log;
+
+public final class Foo {
+ private static final String TAG = Foo.class.getSimpleName();
+
+ public Foo() {
+ init(6);
+ }
+
+ public void init(int i) {
+ if (Log.isLoggable(TAG, Log.DEBUG)) {
+ Log.d(TAG, "Log as v");
+ }
+ }
+}
+ """
+ ),
+ *stubs
+ )
+ .issues(GuardedLogLineDetector.WARNING)
+ .run()
+ .expectContains(GuardedLogLineDetector.GUARDED_LOG_INVOCATION_ERROR)
+ .expectContains(createErrorCountString(0, 1))
+ }
+
+ @Test
+ fun testGuardedLogWithDbgVariable_issueFound() {
+ lint()
+ .files(
+ java(
+ """
+package com.android.bluetooth;
+
+import android.util.Log;
+
+public final class Foo {
+ private static final String TAG = Foo.class.getSimpleName();
+ private static final boolean DBG = true;
+
+ public Foo() {
+ init(6);
+ }
+
+ public void init(int i) {
+ if (DBG) {
+ Log.d(TAG, "Log as v");
+ }
+ }
+}
+ """
+ ),
+ *stubs
+ )
+ .issues(GuardedLogLineDetector.ISSUE)
+ .run()
+ .expectContains(GuardedLogLineDetector.GUARDED_LOG_INVOCATION_ERROR)
+ .expectContains(createErrorCountString(1, 0))
+ }
+
+ @Test
+ fun testGuardedLogWithUnaryDbgVariable_issueFound() {
+ lint()
+ .files(
+ java(
+ """
+package com.android.bluetooth;
+
+import android.util.Log;
+
+public final class Foo {
+ private static final String TAG = Foo.class.getSimpleName();
+ private static final boolean DBG = true;
+
+ public Foo() {
+ init(6);
+ }
+
+ public void init(int i) {
+ if (!DBG) {
+ Log.d(TAG, "Log as v");
+ }
+ }
+}
+ """
+ ),
+ *stubs
+ )
+ .issues(GuardedLogLineDetector.ISSUE)
+ .run()
+ .expectContains(GuardedLogLineDetector.GUARDED_LOG_INVOCATION_ERROR)
+ .expectContains(createErrorCountString(1, 0))
+ }
+
+ @Test
+ fun testGuardedLogWithBinaryDbgVariable_issueFound() {
+ lint()
+ .files(
+ java(
+ """
+package com.android.bluetooth;
+
+import android.util.Log;
+
+public final class Foo {
+ private static final String TAG = Foo.class.getSimpleName();
+ private static final boolean DBG = true;
+
+ public Foo() {
+ init(6);
+ }
+
+ public void init(int i) {
+ if (true || DBG) {
+ Log.d(TAG, "Log as v");
+ }
+ }
+}
+ """
+ ),
+ *stubs
+ )
+ .issues(GuardedLogLineDetector.ISSUE)
+ .run()
+ .expectContains(GuardedLogLineDetector.GUARDED_LOG_INVOCATION_ERROR)
+ .expectContains(createErrorCountString(1, 0))
+ }
+
+ @Test
+ fun testGuardedLogWithPolyadicDbgVariable_issueFound() {
+ lint()
+ .files(
+ java(
+ """
+package com.android.bluetooth;
+
+import android.util.Log;
+
+public final class Foo {
+ private static final String TAG = Foo.class.getSimpleName();
+ private static final boolean DBG = true;
+
+ public Foo() {
+ init(6);
+ }
+
+ public void init(int i) {
+ if (true || DBG || Log.isLoggable(TAG, Log.DEBUG)) {
+ Log.d(TAG, "Log as v");
+ }
+ }
+}
+ """
+ ),
+ *stubs
+ )
+ .issues(GuardedLogLineDetector.ISSUE)
+ .run()
+ .expectContains(GuardedLogLineDetector.GUARDED_LOG_INVOCATION_ERROR)
+ .expectContains(createErrorCountString(1, 0))
+ }
+
+ @Test
+ fun testGuardedLogWithParenthesizedDbgVariable_issueFound() {
+ lint()
+ .files(
+ java(
+ """
+package com.android.bluetooth;
+
+import android.util.Log;
+
+public final class Foo {
+ private static final String TAG = Foo.class.getSimpleName();
+ private static final boolean DBG = true;
+
+ public Foo() {
+ init(6);
+ }
+
+ public void init(int i) {
+ if ((true || DBG || Log.isLoggable(TAG, Log.DEBUG))) {
+ Log.d(TAG, "Log as v");
+ }
+ }
+}
+ """
+ ),
+ *stubs
+ )
+ .issues(GuardedLogLineDetector.ISSUE)
+ .run()
+ .expectContains(GuardedLogLineDetector.GUARDED_LOG_INVOCATION_ERROR)
+ .expectContains(createErrorCountString(1, 0))
+ }
+
+ @Test
+ fun testGuardedLogInNestedIfWithDbgVariable_issueFound() {
+ lint()
+ .files(
+ java(
+ """
+package com.android.bluetooth;
+
+import android.util.Log;
+
+public final class Foo {
+ private static final String TAG = Foo.class.getSimpleName();
+ private static final boolean DBG = true;
+
+ public Foo() {
+ init(6);
+ }
+
+ public void init(int i) {
+ if ( i > 6) {
+ if (DBG) {
+ Log.d(TAG, "Log as v");
+ }
+ }
+ }
+}
+ """
+ ),
+ *stubs
+ )
+ .issues(GuardedLogLineDetector.ISSUE)
+ .run()
+ .expectContains(GuardedLogLineDetector.GUARDED_LOG_INVOCATION_ERROR)
+ .expectContains(createErrorCountString(1, 0))
+ }
+
+ @Test
+ fun testGuardedLogInIfElseWithDbgVariableInElseIf_issueFound() {
+ lint()
+ .files(
+ java(
+ """
+package com.android.bluetooth;
+
+import android.util.Log;
+
+public final class Foo {
+ private static final String TAG = Foo.class.getSimpleName();
+ private static final boolean DBG = true;
+
+ public Foo() {
+ init(6);
+ }
+
+ public void init(int i) {
+ if (i > 6) {
+ return;
+ } else if (DBG) {
+ Log.d(TAG, "Log as d");
+ }
+ }
+}
+ """
+ ),
+ *stubs
+ )
+ .issues(GuardedLogLineDetector.ISSUE)
+ .run()
+ .expectContains(GuardedLogLineDetector.GUARDED_LOG_INVOCATION_ERROR)
+ .expectContains(createErrorCountString(1, 0))
+ }
+
+ @Test
+ fun testGuardedLogInIfElseWithDbgVariableInIf_issueFound() {
+ lint()
+ .files(
+ java(
+ """
+package com.android.bluetooth;
+
+import android.util.Log;
+
+public final class Foo {
+ private static final String TAG = Foo.class.getSimpleName();
+ private static final boolean DBG = true;
+
+ public Foo() {
+ init(6);
+ }
+
+ public void init(int i) {
+ if (!DBG) {
+ return;
+ } else {
+ Log.d(TAG, "Log as d");
+ }
+ }
+}
+ """
+ ),
+ *stubs
+ )
+ .issues(GuardedLogLineDetector.ISSUE)
+ .run()
+ .expectContains(GuardedLogLineDetector.GUARDED_LOG_INVOCATION_ERROR)
+ .expectContains(createErrorCountString(1, 0))
+ }
+
+ @Test
+ fun testGuardedLogInNestedIfWithDbgVariableOuterIfAndLogInInner_issueFound() {
+ lint()
+ .files(
+ java(
+ """
+package com.android.bluetooth;
+
+import android.util.Log;
+
+public final class Foo {
+ private static final String TAG = Foo.class.getSimpleName();
+ private static final boolean DBG = true;
+
+ public Foo() {
+ init(6);
+ }
+
+ public void init(int i) {
+ if (DBG) {
+ if (i > 6) {
+ Log.d(TAG, "Log as d");
+ }
+ }
+ }
+}
+ """
+ ),
+ *stubs
+ )
+ .issues(GuardedLogLineDetector.ISSUE)
+ .run()
+ .expectContains(GuardedLogLineDetector.GUARDED_LOG_INVOCATION_ERROR)
+ .expectContains(createErrorCountString(1, 0))
+ }
+
+ @Test
+ fun testGuardedLogInNestedIfWithIsLoggableOuterIfAndLogInInner_warningFound() {
+ lint()
+ .files(
+ java(
+ """
+package com.android.bluetooth;
+
+import android.util.Log;
+
+public final class Foo {
+ private static final String TAG = Foo.class.getSimpleName();
+ private static final boolean DBG = true;
+
+ public Foo() {
+ init(6);
+ }
+
+ public void init(int i) {
+ if (Log.isLoggable(TAG, Log.DEBUG)) {
+ if (i > 6) {
+ Log.d(TAG, "Log as d");
+ }
+ }
+ }
+}
+ """
+ ),
+ *stubs
+ )
+ .issues(GuardedLogLineDetector.WARNING)
+ .run()
+ .expectContains(GuardedLogLineDetector.GUARDED_LOG_INVOCATION_ERROR)
+ .expectContains(createErrorCountString(0, 1))
+ }
+
+ private val logFramework: TestFile =
+ java(
+ """
+ package android.util;
+ public class Log {
+ public static final int ASSERT = 7;
+ public static final int ERROR = 6;
+ public static final int WARN = 5;
+ public static final int INFO = 4;
+ public static final int DEBUG = 3;
+ public static final int VERBOSE = 2;
+
+ public static Boolean isLoggable(String tag, int level) {
+ return true;
+ }
+
+ public static int wtf(String msg) {
+ return 1;
+ }
+
+ public static int e(String msg) {
+ return 1;
+ }
+
+ public static int w(String msg) {
+ return 1;
+ }
+
+ public static int i(String msg) {
+ return 1;
+ }
+
+ public static int d(String msg) {
+ return 1;
+ }
+
+ public static int v(String msg) {
+ return 1;
+ }
+ }
+ """
+ )
+ .indented()
+
+ private val constantsHelper: TestFile =
+ java(
+ """
+ package com.android.bluetooth;
+
+ public class FooConstants {
+ public static final String TAG = "FooConstants";
+ public static final boolean DBG = true;
+ public static final boolean VDBG = Log.isLoggable(TAG, Log.VERBOSE);
+ }
+ """
+ )
+ .indented()
+
+ private val stubs =
+ arrayOf(
+ logFramework,
+ constantsHelper,
+ )
+
+ private fun createErrorCountString(errors: Int, warnings: Int): String {
+ return "%d errors, %d warnings".format(errors, warnings)
+ }
+
+ private fun createFixDiff(lineNumber: Int, lines: String): String {
+ // All lines are removed. Add enough spaces to match the below indenting
+ val minusedlines = lines.replace("\n ", "\n - ")
+ return """
+ Fix for src/com/android/bluetooth/Foo.java line $lineNumber: Update log tag initialization:
+ @@ -$lineNumber +$lineNumber
+ - $minusedlines
+ -
+ """
+ .trimIndent()
+ }
+}