diff options
author | 2024-02-13 16:04:59 -0800 | |
---|---|---|
committer | 2024-03-27 13:23:53 -0700 | |
commit | 3d28fff09d36886ab54a1481e497cefc7e2d513c (patch) | |
tree | 6c5febc86e472c076b477d0991d61417dc4e3b6a | |
parent | e60845e0a73b14b01c9e86323e0cd61fc3568e7b (diff) |
Create an Android Lint rule to prevent creation of log enforcement vars
The Bluetooth stack sets a process default log level, which allows the
Log framework to properly enforce our log level. Creating log
enforcement variables is unnecessary, as their usage is as well.
Tag: #feature
Bug: 315046089
Flag: EXEMPT, tooling change
Test: atest BluetoothLintCheckerTest --host
Change-Id: I4fdd1c4239294bc07f4d49457cd714899e3294d7
-rw-r--r-- | android/app/Android.bp | 1 | ||||
-rw-r--r-- | tools/lint/Android.bp | 63 | ||||
-rw-r--r-- | tools/lint/OWNERS | 3 | ||||
-rw-r--r-- | tools/lint/README.md | 50 | ||||
-rw-r--r-- | tools/lint/checks/src/com/android/bluetooth/lint/BluetoothLintCheckerRegistry.kt | 41 | ||||
-rw-r--r-- | tools/lint/checks/src/com/android/bluetooth/lint/LogEnforcementVariableCreationDetector.kt | 230 | ||||
-rw-r--r-- | tools/lint/checks/src/com/android/bluetooth/lint/Utils.kt | 31 | ||||
-rw-r--r-- | tools/lint/checks/tests/com/android/bluetooth/lint/test/LogEnforcementVariableCreationDetectorTest.kt | 857 |
8 files changed, 1276 insertions, 0 deletions
diff --git a/android/app/Android.bp b/android/app/Android.bp index a59ba7f9ac..b81205bdcb 100644 --- a/android/app/Android.bp +++ b/android/app/Android.bp @@ -300,6 +300,7 @@ android_app { "UseSparseArrays", "UseValueOf", ], + extra_check_modules: ["BluetoothLintChecker"], baseline_filename: "lint-baseline.xml", }, diff --git a/tools/lint/Android.bp b/tools/lint/Android.bp new file mode 100644 index 0000000000..8e779c95ea --- /dev/null +++ b/tools/lint/Android.bp @@ -0,0 +1,63 @@ +// 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 { + default_applicable_licenses: ["Android-Apache-2.0"], +} + +java_library_host { + name: "BluetoothLintChecker", + srcs: [ + "checks/src/**/*.java", + "checks/src/**/*.kt", + ], + plugins: ["auto_service_plugin"], + libs: [ + "auto_service_annotations", + "lint_api", + ], + kotlincflags: ["-Xjvm-default=all"], +} + +java_test_host { + name: "BluetoothLintCheckerTest", + team: "trendy_team_bluetooth", + srcs: [ + "checks/tests/**/*.java", + "checks/tests/**/*.kt", + ], + static_libs: [ + "BluetoothLintChecker", + "junit", + "lint", + "lint_tests", + ], + test_options: { + unit_test: true, + tradefed_options: [ + { + // lint bundles in some classes that were built with older versions + // of libraries, and no longer load. Since tradefed tries to load + // all classes in the jar to look for tests, it crashes loading them. + // Exclude these classes from tradefed's search. + name: "exclude-paths", + value: "org/apache", + }, + { + name: "exclude-paths", + value: "META-INF", + }, + ], + }, +} diff --git a/tools/lint/OWNERS b/tools/lint/OWNERS new file mode 100644 index 0000000000..e7ba65cd32 --- /dev/null +++ b/tools/lint/OWNERS @@ -0,0 +1,3 @@ +# Additional OWNERS for Bluetooth AndroidLint +licorne@google.com +salsavage@google.com diff --git a/tools/lint/README.md b/tools/lint/README.md new file mode 100644 index 0000000000..bbd2d24a66 --- /dev/null +++ b/tools/lint/README.md @@ -0,0 +1,50 @@ +# Bluetooth Lint Checks for AOSP + +Custom Android Lint checks are written here to be executed against Bluetooth +Java and Kotlin source code. These will appear as part of errorprone builds, +which are notably ran as part of pre/post-submit testing of Bluetooth code. + +## How to run Bluetooth lint checks against the code base + +While lint checks should be automatically run by Gerrit, you may want to run it manually on the +code base. + +Step 1: Build the lint report: +``` +m Bluetooth-lint +``` + +Step 2: Find your lint output: +``` +croot; +find out/soong/.intermediates/packages/modules/Bluetooth/ -type f -name "*lint-report*" -print; +``` + +Or: +``` +aninja -t query Bluetooth-lint +``` + +Step 3: Identify the lint report you want to view the results of, typically in the following format: +``` +out/soong/.intermediates/packages/modules/Bluetooth/android/app/Bluetooth/android_common/<run-identifier>/lint/lint-report.html +``` + +Step 4: Open the file in your favorite web browser. + +## How to run Bluetooth lint unit tests + +Unit tests can be ran with the following command: +``` +atest BluetoothLintCheckerTest --host +``` + +## Documentation + +- [Android Lint Docs](https://googlesamples.github.io/android-custom-lint-rules/) +- [Custom lint creation example from Android SystemUI team](https://g3doc.corp.google.com/company/teams/android-sysui/general_guides/writing_a_linter.md?cl=head) +- [Lint Check Unit Testing](https://googlesamples.github.io/android-custom-lint-rules/api-guide/unit-testing.md.html) +- [Android Lint source files](https://source.corp.google.com/studio-main/tools/base/lint/libs/lint-api/src/main/java/com/android/tools/lint/) +- [PSI source files](https://github.com/JetBrains/intellij-community/tree/master/java/java-psi-api/src/com/intellij/psi) +- [UAST source files](https://upsource.jetbrains.com/idea-ce/structure/idea-ce-7b9b8cc138bbd90aec26433f82cd2c6838694003/uast/uast-common/src/org/jetbrains/uast) +- [IntelliJ plugin for viewing PSI tree of files](https://plugins.jetbrains.com/plugin/227-psiviewer) diff --git a/tools/lint/checks/src/com/android/bluetooth/lint/BluetoothLintCheckerRegistry.kt b/tools/lint/checks/src/com/android/bluetooth/lint/BluetoothLintCheckerRegistry.kt new file mode 100644 index 0000000000..863fd5cf4f --- /dev/null +++ b/tools/lint/checks/src/com/android/bluetooth/lint/BluetoothLintCheckerRegistry.kt @@ -0,0 +1,41 @@ +/* + * 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.IssueRegistry +import com.android.tools.lint.client.api.Vendor +import com.android.tools.lint.detector.api.CURRENT_API +import com.google.auto.service.AutoService + +@AutoService(IssueRegistry::class) +@Suppress("UnstableApiUsage") +class BluetoothLintCheckerIssueRegistry : IssueRegistry() { + override val issues = listOf(LogEnforcementVariableCreationDetector.ISSUE) + + override val api: Int + get() = CURRENT_API + + override val minApi: Int + get() = 11 + + override val vendor = + Vendor( + vendorName = "Android", + feedbackUrl = "http://b/issues/new?component=27441", + contact = "android-bluetooth@google.com" + ) +} diff --git a/tools/lint/checks/src/com/android/bluetooth/lint/LogEnforcementVariableCreationDetector.kt b/tools/lint/checks/src/com/android/bluetooth/lint/LogEnforcementVariableCreationDetector.kt new file mode 100644 index 0000000000..3b0ca71d66 --- /dev/null +++ b/tools/lint/checks/src/com/android/bluetooth/lint/LogEnforcementVariableCreationDetector.kt @@ -0,0 +1,230 @@ +/* + * 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.LintFix +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.UField +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 +import org.jetbrains.uast.skipParenthesizedExprDown + +/** + * Lint check for creation of log enforcement variables in the Bluetooth stack + * + * Logging enforcement variables are not allowed to be _defined_, i.e.: + * + * private static final Boolean DEBUG = Log.isLoggable(TAG, Log.DEBUG); + * private static final Boolean VERBOSE = Log.isLoggable(TAG, Log.VERBOSE); + * private static final Boolean DBG = true; + * private static final Boolean VDBG = false; + * private static final Boolean BDG = FooConstants.DBG; + * + * This is because 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 confusion and inconsistency on log output, and worst case causes + * double checks against the log level each time we log. Plus, leveraging the Log framework allows + * things like runtime log level switches. + * + * The recommended fix is to _remove_ the creation of the variable. This fix is easy to make, but it + * is hard to fix all the downstream usages of the variable. + */ +class LogEnforcementVariableCreationDetector : Detector(), SourceCodeScanner { + private val LOG_ENFORCEMENT_VARS = listOf("DBG", "DEBUG", "VDBG", "VERBOSE", "D", "V") + private val LOG_ENFORCEMENT_VAR_ENDINGS = listOf("DBG", "VDBG", "DEBUG", "VERBOSE") + + companion object { + const val LOG_ENFORCEMENT_VARIABLE_USAGE_ERROR = + "Dont create log enforcement variables to enforce when a log should be made. The Log" + + " framework does this check for you. Remove this variable and update any log" + + " invocations to be unguarded." + + val ISSUE = + Issue.create( + id = "LogEnforcementVariableCreation", + briefDescription = "Do not create 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( + LogEnforcementVariableCreationDetector::class.java, + Scope.JAVA_FILE_SCOPE + ), + androidSpecific = true, + ) + } + + override fun getApplicableUastTypes(): List<Class<out UElement>> { + return listOf(UClass::class.java) + } + + override fun createUastHandler(context: JavaContext): UElementHandler? { + return object : UElementHandler() { + override fun visitClass(node: UClass) { + if (isBluetoothClass(node)) { + for (field in node.fields) { + if (checkFieldForLogEnforcementVariable(field)) { + context.report( + issue = ISSUE, + scopeClass = field, + location = context.getNameLocation(field), + message = LOG_ENFORCEMENT_VARIABLE_USAGE_ERROR, + quickfixData = + LintFix.create() + .name("Remove log enforcement variable declaration") + .replace() + .range(context.getLocation(field)) + .with("") + .build() + ) + } + } + } + } + } + } + + /* + * Checks a UField to see if its likely a declaration of a log enforcement variable + * + * Three checks are made to see if the field is a log enforcement variable: + * 1. Is the name any of the common names used in the stack. These include things like + * DBG, VDBG, DEBUG, VERBOSE, D, V, etc., or variables that _end_ in _DBG or _VDBG + * 2. Is the variable created assigned the a value based on a call to Log.isLoggable() + * 3. Is the variable created assigned the value of another log enforcement variable, based + * on the naming scheme described in check 1 above. + */ + private fun checkFieldForLogEnforcementVariable(field: UField): Boolean { + val fieldType = field.getType() + val fieldName = field.getName() + val fieldInitializer = field.uastInitializer?.skipParenthesizedExprDown() + return fieldType.canonicalText.lowercase() == "boolean" && + (isLogEnforcementVariable(fieldName) || + (fieldInitializer != null && + (checkExpressionForIsLoggableUsages(fieldInitializer) || + checkExpressionForDebugVariableUsages(fieldInitializer)))) + } + + /* + * 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) } + } + + /* + * Checks an expression to see if it uses a log enforcement variable, based on the common names + * used for log enforcement variables in the stack. + */ + private fun checkExpressionForDebugVariableUsages(expression: UExpression): Boolean { + when (expression) { + is USimpleNameReferenceExpression -> { + if (isLogEnforcementVariable(expression.identifier)) { + return true + } + } + is UUnaryExpression -> { + return checkExpressionForDebugVariableUsages(expression.operand) + } + is UBinaryExpression -> { + return checkExpressionForDebugVariableUsages(expression.leftOperand) || + checkExpressionForDebugVariableUsages(expression.rightOperand) + } + is UPolyadicExpression -> { + for (subExpression in expression.operands) { + if (checkExpressionForDebugVariableUsages(subExpression)) { + return true + } + } + return false + } + is UQualifiedReferenceExpression -> { + return checkExpressionForDebugVariableUsages(expression.selector) + } + is UParenthesizedExpression -> { + return checkExpressionForDebugVariableUsages(expression.expression) + } + } + + return false + } + + /* + * Checks an expression to see if it uses the Log.isLoggable() function + */ + private fun checkExpressionForIsLoggableUsages(expression: UExpression): Boolean { + when (expression) { + is UCallExpression -> { + val resolvedMethod = expression.resolve() + return resolvedMethod?.name == "isLoggable" && + resolvedMethod.containingClass?.qualifiedName == "android.util.Log" + } + is UUnaryExpression -> { + return checkExpressionForIsLoggableUsages(expression.operand) + } + is UBinaryExpression -> { + return checkExpressionForIsLoggableUsages(expression.leftOperand) || + checkExpressionForIsLoggableUsages(expression.rightOperand) + } + is UPolyadicExpression -> { + for (subExpression in expression.operands) { + if (checkExpressionForIsLoggableUsages(subExpression)) { + return true + } + } + return false + } + is UQualifiedReferenceExpression -> { + return checkExpressionForIsLoggableUsages(expression.receiver) || + checkExpressionForIsLoggableUsages(expression.selector) + } + is UParenthesizedExpression -> { + return checkExpressionForIsLoggableUsages(expression.expression) + } + } + return false + } +} diff --git a/tools/lint/checks/src/com/android/bluetooth/lint/Utils.kt b/tools/lint/checks/src/com/android/bluetooth/lint/Utils.kt new file mode 100644 index 0000000000..12ec820df2 --- /dev/null +++ b/tools/lint/checks/src/com/android/bluetooth/lint/Utils.kt @@ -0,0 +1,31 @@ +/* + * 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 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 +} + +/** Writes lines to debug output, visible in the isolated Java output */ +fun debug(tag: String, message: String, indent: String = "") { + println("$indent[$tag]: $message") +} diff --git a/tools/lint/checks/tests/com/android/bluetooth/lint/test/LogEnforcementVariableCreationDetectorTest.kt b/tools/lint/checks/tests/com/android/bluetooth/lint/test/LogEnforcementVariableCreationDetectorTest.kt new file mode 100644 index 0000000000..5729f46e16 --- /dev/null +++ b/tools/lint/checks/tests/com/android/bluetooth/lint/test/LogEnforcementVariableCreationDetectorTest.kt @@ -0,0 +1,857 @@ +/* + * 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.LogEnforcementVariableCreationDetector +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 LogEnforcementVariableCreationDetectorTest : LintDetectorTest() { + override fun getDetector(): Detector = LogEnforcementVariableCreationDetector() + + override fun getIssues(): List<Issue> = listOf(LogEnforcementVariableCreationDetector.ISSUE) + + override fun lint(): TestLintTask = super.lint().allowMissingSdk(true) + + @Test + fun testClassWithDVar_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 D = true; + + public Foo() { + Log.d(TAG, "created Foo without an enforcement variable"); + } + } + """ + ) + .indented(), + *stubs + ) + .issues(LogEnforcementVariableCreationDetector.ISSUE) + .run() + .expectContains( + LogEnforcementVariableCreationDetector.LOG_ENFORCEMENT_VARIABLE_USAGE_ERROR + ) + .expectContains(createErrorCountString(1, 0)) + .expectFixDiffs(createFixDiff(7, " private static final boolean D = true;")) + } + + @Test + fun testClassWithVVar_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 V = true; + + public Foo() { + Log.d(TAG, "created Foo without an enforcement variable"); + } + } + """ + ) + .indented(), + *stubs + ) + .issues(LogEnforcementVariableCreationDetector.ISSUE) + .run() + .expectContains( + LogEnforcementVariableCreationDetector.LOG_ENFORCEMENT_VARIABLE_USAGE_ERROR + ) + .expectContains(createErrorCountString(1, 0)) + .expectFixDiffs(createFixDiff(7, " private static final boolean V = true;")) + } + + @Test + fun testClassWithDbgVar_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() { + Log.d(TAG, "created Foo without an enforcement variable"); + } + } + """ + ) + .indented(), + *stubs + ) + .issues(LogEnforcementVariableCreationDetector.ISSUE) + .run() + .expectContains( + LogEnforcementVariableCreationDetector.LOG_ENFORCEMENT_VARIABLE_USAGE_ERROR + ) + .expectContains(createErrorCountString(1, 0)) + .expectFixDiffs(createFixDiff(7, " private static final boolean DBG = true;")) + } + + @Test + fun testClassWithVdbgVar_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 VDBG = true; + + public Foo() { + Log.d(TAG, "created Foo without an enforcement variable"); + } + } + """ + ) + .indented(), + *stubs + ) + .issues(LogEnforcementVariableCreationDetector.ISSUE) + .run() + .expectContains( + LogEnforcementVariableCreationDetector.LOG_ENFORCEMENT_VARIABLE_USAGE_ERROR + ) + .expectContains(createErrorCountString(1, 0)) + .expectFixDiffs(createFixDiff(7, " private static final boolean VDBG = true;")) + } + + @Test + fun testClassWithDebugVar_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 DEBUG = true; + + public Foo() { + Log.d(TAG, "created Foo without an enforcement variable"); + } + } + """ + ) + .indented(), + *stubs + ) + .issues(LogEnforcementVariableCreationDetector.ISSUE) + .run() + .expectContains( + LogEnforcementVariableCreationDetector.LOG_ENFORCEMENT_VARIABLE_USAGE_ERROR + ) + .expectContains(createErrorCountString(1, 0)) + .expectFixDiffs(createFixDiff(7, " private static final boolean DEBUG = true;")) + } + + @Test + fun testClassWithVerboseVar_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 VERBOSE = true; + + public Foo() { + Log.d(TAG, "created Foo without an enforcement variable"); + } + } + """ + ) + .indented(), + *stubs + ) + .issues(LogEnforcementVariableCreationDetector.ISSUE) + .run() + .expectContains( + LogEnforcementVariableCreationDetector.LOG_ENFORCEMENT_VARIABLE_USAGE_ERROR + ) + .expectContains(createErrorCountString(1, 0)) + .expectFixDiffs(createFixDiff(7, " private static final boolean VERBOSE = true;")) + } + + @Test + fun testClassWithDbgEndingVar_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 SHOULD_DBG = true; + + public Foo() { + Log.d(TAG, "created Foo without an enforcement variable"); + } + } + """ + ) + .indented(), + *stubs + ) + .issues(LogEnforcementVariableCreationDetector.ISSUE) + .run() + .expectContains( + LogEnforcementVariableCreationDetector.LOG_ENFORCEMENT_VARIABLE_USAGE_ERROR + ) + .expectContains(createErrorCountString(1, 0)) + .expectFixDiffs(createFixDiff(7, " private static final boolean SHOULD_DBG = true;")) + } + + @Test + fun testClassWithVdbgEndingVar_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 SHOULD_VDBG = true; + + public Foo() { + Log.d(TAG, "created Foo without an enforcement variable"); + } + } + """ + ) + .indented(), + *stubs + ) + .issues(LogEnforcementVariableCreationDetector.ISSUE) + .run() + .expectContains( + LogEnforcementVariableCreationDetector.LOG_ENFORCEMENT_VARIABLE_USAGE_ERROR + ) + .expectContains(createErrorCountString(1, 0)) + .expectFixDiffs( + createFixDiff(7, " private static final boolean SHOULD_VDBG = true;") + ) + } + + @Test + fun testClassWithMemberDbgVar_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 mDbg = true; + + public Foo() { + Log.d(TAG, "created Foo without an enforcement variable"); + } + } + """ + ) + .indented(), + *stubs + ) + .issues(LogEnforcementVariableCreationDetector.ISSUE) + .run() + .expectContains( + LogEnforcementVariableCreationDetector.LOG_ENFORCEMENT_VARIABLE_USAGE_ERROR + ) + .expectContains(createErrorCountString(1, 0)) + .expectFixDiffs(createFixDiff(7, " private static final boolean mDbg = true;")) + } + + @Test + fun testClassWithMemberDebugVar_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 mDebug = true; + + public Foo() { + Log.d(TAG, "created Foo without an enforcement variable"); + } + } + """ + ) + .indented(), + *stubs + ) + .issues(LogEnforcementVariableCreationDetector.ISSUE) + .run() + .expectContains( + LogEnforcementVariableCreationDetector.LOG_ENFORCEMENT_VARIABLE_USAGE_ERROR + ) + .expectContains(createErrorCountString(1, 0)) + .expectFixDiffs(createFixDiff(7, " private static final boolean mDebug = true;")) + } + + @Test + fun testClassWithMemberVdbgVar_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 mVdbg = true; + + public Foo() { + Log.d(TAG, "created Foo without an enforcement variable"); + } + } + """ + ) + .indented(), + *stubs + ) + .issues(LogEnforcementVariableCreationDetector.ISSUE) + .run() + .expectContains( + LogEnforcementVariableCreationDetector.LOG_ENFORCEMENT_VARIABLE_USAGE_ERROR + ) + .expectContains(createErrorCountString(1, 0)) + .expectFixDiffs(createFixDiff(7, " private static final boolean mVdbg = true;")) + } + + @Test + fun testClassWithMemberVerboseVar_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 mVerbose = true; + + public Foo() { + Log.d(TAG, "created Foo without an enforcement variable"); + } + } + """ + ) + .indented(), + *stubs + ) + .issues(LogEnforcementVariableCreationDetector.ISSUE) + .run() + .expectContains( + LogEnforcementVariableCreationDetector.LOG_ENFORCEMENT_VARIABLE_USAGE_ERROR + ) + .expectContains(createErrorCountString(1, 0)) + .expectFixDiffs(createFixDiff(7, " private static final boolean mVerbose = true;")) + } + + @Test + fun testClassWithVarAssignedToAnotherEnforcementVar_issueFound() { + lint() + .files( + java( + """ + package com.android.bluetooth; + + import android.util.Log; + import com.android.foo.FooConstants; + + public final class Foo { + private static final String TAG = Foo.class.getSimpleName(); + private static final boolean SHOULD_LOG = FooConstants.DBG; + + public Foo() { + Log.d(TAG, "created Foo without an enforcement variable"); + } + } + """ + ) + .indented(), + *stubs + ) + .issues(LogEnforcementVariableCreationDetector.ISSUE) + .run() + .expectContains( + LogEnforcementVariableCreationDetector.LOG_ENFORCEMENT_VARIABLE_USAGE_ERROR + ) + .expectContains(createErrorCountString(1, 0)) + .expectFixDiffs( + createFixDiff(8, " private static final boolean SHOULD_LOG = FooConstants.DBG;") + ) + } + + @Test + fun testClassWithVarAssignedToAnotherEnforcementVarWithBinaryOp_issueFound() { + lint() + .files( + java( + """ + package com.android.bluetooth; + + import android.util.Log; + import com.android.foo.FooConstants; + + public final class Foo { + private static final String TAG = Foo.class.getSimpleName(); + private static final boolean SHOULD_LOG = + false || FooConstants.DBG; + + public Foo() { + Log.d(TAG, "created Foo without an enforcement variable"); + } + } + """ + ) + .indented(), + *stubs + ) + .issues(LogEnforcementVariableCreationDetector.ISSUE) + .run() + .expectContains( + LogEnforcementVariableCreationDetector.LOG_ENFORCEMENT_VARIABLE_USAGE_ERROR + ) + .expectContains(createErrorCountString(1, 0)) + .expectFixDiffs( + createFixDiff( + 8, + """ private static final boolean SHOULD_LOG = + false || FooConstants.DBG;""" + ) + ) + } + + @Test + fun testClassWithLogIsLoggableVar_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 SHOULD_LOG = Log.isLoggable(TAG, Log.DEBUG); + + public Foo() { + Log.d(TAG, "created Foo without an enforcement variable"); + } + } + """ + ) + .indented(), + *stubs + ) + .issues(LogEnforcementVariableCreationDetector.ISSUE) + .run() + .expectContains( + LogEnforcementVariableCreationDetector.LOG_ENFORCEMENT_VARIABLE_USAGE_ERROR + ) + .expectContains(createErrorCountString(1, 0)) + .expectFixDiffs( + createFixDiff( + 7, + " private static final boolean SHOULD_LOG = Log.isLoggable(TAG, Log.DEBUG);" + ) + ) + } + + @Test + fun testClassWithUnaryLogIsLoggableVar_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 SHOULD_LOG = !Log.isLoggable(TAG, Log.DEBUG); + + public Foo() { + Log.d(TAG, "created Foo without an enforcement variable"); + } + } + """ + ) + .indented(), + *stubs + ) + .issues(LogEnforcementVariableCreationDetector.ISSUE) + .run() + .expectContains( + LogEnforcementVariableCreationDetector.LOG_ENFORCEMENT_VARIABLE_USAGE_ERROR + ) + .expectContains(createErrorCountString(1, 0)) + .expectFixDiffs( + createFixDiff( + 7, + " private static final boolean SHOULD_LOG = !Log.isLoggable(TAG, Log.DEBUG);" + ) + ) + } + + @Test + fun testClassWithBinaryLogIsLoggableVar_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 SHOULD_LOG = Log.isLoggable(TAG, Log.DEBUG) || true; + + public Foo() { + Log.d(TAG, "created Foo without an enforcement variable"); + } + } + """ + ) + .indented(), + *stubs + ) + .issues(LogEnforcementVariableCreationDetector.ISSUE) + .run() + .expectContains( + LogEnforcementVariableCreationDetector.LOG_ENFORCEMENT_VARIABLE_USAGE_ERROR + ) + .expectContains(createErrorCountString(1, 0)) + .expectFixDiffs( + createFixDiff( + 7, + " private static final boolean SHOULD_LOG = Log.isLoggable(TAG, Log.DEBUG) || true;" + ) + ) + } + + @Test + fun testClassWithBinaryDoubleLogIsLoggableVar_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 SHOULD_LOG = + Log.isLoggable(TAG, Log.DEBUG) || Log.isLoggable(TAG, Log.VERBOSE); + + public Foo() { + Log.d(TAG, "created Foo without an enforcement variable"); + } + } + """ + ) + .indented(), + *stubs + ) + .issues(LogEnforcementVariableCreationDetector.ISSUE) + .run() + .expectContains( + LogEnforcementVariableCreationDetector.LOG_ENFORCEMENT_VARIABLE_USAGE_ERROR + ) + .expectContains(createErrorCountString(1, 0)) + .expectFixDiffs( + createFixDiff( + 7, + """ private static final boolean SHOULD_LOG = + Log.isLoggable(TAG, Log.DEBUG) || Log.isLoggable(TAG, Log.VERBOSE);""" + ) + ) + } + + @Test + fun testClassWithMultipleBinaryLogIsLoggableVar_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 SHOULD_LOG = + false || Log.isLoggable(TAG, Log.DEBUG) || true; + + public Foo() { + Log.d(TAG, "created Foo without an enforcement variable"); + } + } + """ + ) + .indented(), + *stubs + ) + .issues(LogEnforcementVariableCreationDetector.ISSUE) + .run() + .expectContains( + LogEnforcementVariableCreationDetector.LOG_ENFORCEMENT_VARIABLE_USAGE_ERROR + ) + .expectContains(createErrorCountString(1, 0)) + .expectFixDiffs( + createFixDiff( + 7, + """ private static final boolean SHOULD_LOG = + false || Log.isLoggable(TAG, Log.DEBUG) || true;""" + ) + ) + } + + @Test + fun testClassWithParenthesizedLogIsLoggableVar_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 SHOULD_LOG = + (Log.isLoggable(TAG, Log.DEBUG) || Log.isLoggable(TAG, Log.VERBOSE)); + + public Foo() { + Log.d(TAG, "created Foo without an enforcement variable"); + } + } + """ + ) + .indented(), + *stubs + ) + .issues(LogEnforcementVariableCreationDetector.ISSUE) + .run() + .expectContains( + LogEnforcementVariableCreationDetector.LOG_ENFORCEMENT_VARIABLE_USAGE_ERROR + ) + .expectContains(createErrorCountString(1, 0)) + .expectFixDiffs( + createFixDiff( + 7, + """ private static final boolean SHOULD_LOG = + (Log.isLoggable(TAG, Log.DEBUG) || Log.isLoggable(TAG, Log.VERBOSE));""" + ) + ) + } + + @Test + fun testClassWithNoLogEnforcementVariables_noLintIssuesFound() { + 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() { + Log.d(TAG, "created Foo without an enforcement variable"); + } + } + """ + ) + .indented(), + *stubs + ) + .issues(LogEnforcementVariableCreationDetector.ISSUE) + .run() + .expectClean() + } + + @Test + fun testClassOutsideOfBluetoothPackage_noLintIssuesFound() { + lint() + .files( + java( + """ + package com.android.wifi; + + import android.util.Log; + + public final class Foo { + private static final String TAG = Foo.class.getSimpleName(); + private static final boolean DBG = true; + + public Foo() { + Log.d(TAG, "created Foo without an enforcement variable"); + } + } + """ + ) + .indented(), + *stubs + ) + .issues(LogEnforcementVariableCreationDetector.ISSUE) + .run() + .expectClean() + } + + 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.foo; + + 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: Remove log enforcement variable declaration: + @@ -$lineNumber +$lineNumber + - $minusedlines + - + """ + .trimIndent() + } +} |