From 96431b11ca53ec6dcb85f9e2f25b2208a540f95d Mon Sep 17 00:00:00 2001 From: Thiébaud Weksteen Date: Thu, 31 Aug 2023 11:17:58 +1000 Subject: Move AidlImplementationDetector from global to common Bug: 298285238 Test: m AndroidGlobalLintChecker Change-Id: I4e90a0452caaf69ec5da680b7955b16eb1387871 --- .../lint/aidl/AidlImplementationDetector.kt | 52 +++++++ .../java/com/google/android/lint/aidl/Constants.kt | 149 +++++++++++++++++++++ .../android/lint/aidl/EnforcePermissionUtils.kt | 96 +++++++++++++ .../lint/aidl/AidlImplementationDetector.kt | 52 ------- .../java/com/google/android/lint/aidl/Constants.kt | 149 --------------------- .../android/lint/aidl/EnforcePermissionUtils.kt | 96 ------------- 6 files changed, 297 insertions(+), 297 deletions(-) create mode 100644 tools/lint/common/src/main/java/com/google/android/lint/aidl/AidlImplementationDetector.kt create mode 100644 tools/lint/common/src/main/java/com/google/android/lint/aidl/Constants.kt create mode 100644 tools/lint/common/src/main/java/com/google/android/lint/aidl/EnforcePermissionUtils.kt delete mode 100644 tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/AidlImplementationDetector.kt delete mode 100644 tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/Constants.kt delete mode 100644 tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionUtils.kt diff --git a/tools/lint/common/src/main/java/com/google/android/lint/aidl/AidlImplementationDetector.kt b/tools/lint/common/src/main/java/com/google/android/lint/aidl/AidlImplementationDetector.kt new file mode 100644 index 000000000000..ab6d871d6ea6 --- /dev/null +++ b/tools/lint/common/src/main/java/com/google/android/lint/aidl/AidlImplementationDetector.kt @@ -0,0 +1,52 @@ +/* + * Copyright (C) 2022 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.google.android.lint.aidl + +import com.android.tools.lint.client.api.UElementHandler +import com.android.tools.lint.detector.api.Detector +import com.android.tools.lint.detector.api.JavaContext +import com.android.tools.lint.detector.api.SourceCodeScanner +import org.jetbrains.uast.UBlockExpression +import org.jetbrains.uast.UElement +import org.jetbrains.uast.UMethod + +/** + * Abstract class for detectors that look for methods implementing + * generated AIDL interface stubs + */ +abstract class AidlImplementationDetector : Detector(), SourceCodeScanner { + override fun getApplicableUastTypes(): List> = + listOf(UMethod::class.java) + + override fun createUastHandler(context: JavaContext): UElementHandler = AidlStubHandler(context) + + private inner class AidlStubHandler(val context: JavaContext) : UElementHandler() { + override fun visitMethod(node: UMethod) { + val interfaceName = getContainingAidlInterface(context, node) + .takeUnless(EXCLUDED_CPP_INTERFACES::contains) ?: return + val body = (node.uastBody as? UBlockExpression) ?: return + visitAidlMethod(context, node, interfaceName, body) + } + } + + abstract fun visitAidlMethod( + context: JavaContext, + node: UMethod, + interfaceName: String, + body: UBlockExpression, + ) +} diff --git a/tools/lint/common/src/main/java/com/google/android/lint/aidl/Constants.kt b/tools/lint/common/src/main/java/com/google/android/lint/aidl/Constants.kt new file mode 100644 index 000000000000..e03d92ab44a0 --- /dev/null +++ b/tools/lint/common/src/main/java/com/google/android/lint/aidl/Constants.kt @@ -0,0 +1,149 @@ +/* + * Copyright (C) 2022 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.google.android.lint.aidl + +const val ANNOTATION_ENFORCE_PERMISSION = "android.annotation.EnforcePermission" +const val ANNOTATION_REQUIRES_NO_PERMISSION = "android.annotation.RequiresNoPermission" +const val ANNOTATION_PERMISSION_MANUALLY_ENFORCED = "android.annotation.PermissionManuallyEnforced" + +val AIDL_PERMISSION_ANNOTATIONS = listOf( + ANNOTATION_ENFORCE_PERMISSION, + ANNOTATION_REQUIRES_NO_PERMISSION, + ANNOTATION_PERMISSION_MANUALLY_ENFORCED +) + +const val BINDER_CLASS = "android.os.Binder" +const val IINTERFACE_INTERFACE = "android.os.IInterface" + +const val AIDL_PERMISSION_HELPER_SUFFIX = "_enforcePermission" + +/** + * If a non java (e.g. c++) backend is enabled, the @EnforcePermission + * annotation cannot be used. At time of writing, the mechanism + * is not implemented for non java backends. + * TODO: b/242564874 (have lint know which interfaces have the c++ backend enabled) + * rather than hard coding this list? + */ +val EXCLUDED_CPP_INTERFACES = listOf( + "AdbTransportType", + "FingerprintAndPairDevice", + "IAdbCallback", + "IAdbManager", + "PairDevice", + "IStatsBootstrapAtomService", + "StatsBootstrapAtom", + "StatsBootstrapAtomValue", + "FixedSizeArrayExample", + "PlaybackTrackMetadata", + "RecordTrackMetadata", + "SinkMetadata", + "SourceMetadata", + "IUpdateEngineStable", + "IUpdateEngineStableCallback", + "AudioCapabilities", + "ConfidenceLevel", + "ModelParameter", + "ModelParameterRange", + "Phrase", + "PhraseRecognitionEvent", + "PhraseRecognitionExtra", + "PhraseSoundModel", + "Properties", + "RecognitionConfig", + "RecognitionEvent", + "RecognitionMode", + "RecognitionStatus", + "SoundModel", + "SoundModelType", + "Status", + "IThermalService", + "IPowerManager", + "ITunerResourceManager", + // b/278147400 + "IActivityManager", + "IUidObserver", + "IDrm", + "IVsyncCallback", + "IVsyncService", + "ICallback", + "IIPCTest", + "ISafeInterfaceTest", + "IGpuService", + "IConsumerListener", + "IGraphicBufferConsumer", + "ITransactionComposerListener", + "SensorEventConnection", + "SensorServer", + "ICamera", + "ICameraClient", + "ICameraRecordingProxy", + "ICameraRecordingProxyListener", + "ICrypto", + "IOMXObserver", + "IStreamListener", + "IStreamSource", + "IAudioService", + "IDataSource", + "IDrmClient", + "IMediaCodecList", + "IMediaDrmService", + "IMediaExtractor", + "IMediaExtractorService", + "IMediaHTTPConnection", + "IMediaHTTPService", + "IMediaLogService", + "IMediaMetadataRetriever", + "IMediaMetricsService", + "IMediaPlayer", + "IMediaPlayerClient", + "IMediaPlayerService", + "IMediaRecorder", + "IMediaRecorderClient", + "IMediaResourceMonitor", + "IMediaSource", + "IRemoteDisplay", + "IRemoteDisplayClient", + "IResourceManagerClient", + "IResourceManagerService", + "IComplexTypeInterface", + "IPermissionController", + "IPingResponder", + "IProcessInfoService", + "ISchedulingPolicyService", + "IStringConstants", + "IObbActionListener", + "IStorageEventListener", + "IStorageManager", + "IStorageShutdownObserver", + "IPersistentVrStateCallbacks", + "IVrManager", + "IVrStateCallbacks", + "ISurfaceComposer", + "IMemory", + "IMemoryHeap", + "IProcfsInspector", + "IAppOpsCallback", + "IAppOpsService", + "IBatteryStats", + "IResultReceiver", + "IShellCallback", + "IDrmManagerService", + "IDrmServiceListener", + "IAAudioClient", + "IAAudioService", + "VtsFuzzer", +) diff --git a/tools/lint/common/src/main/java/com/google/android/lint/aidl/EnforcePermissionUtils.kt b/tools/lint/common/src/main/java/com/google/android/lint/aidl/EnforcePermissionUtils.kt new file mode 100644 index 000000000000..d41fee3fc0dc --- /dev/null +++ b/tools/lint/common/src/main/java/com/google/android/lint/aidl/EnforcePermissionUtils.kt @@ -0,0 +1,96 @@ +/* + * Copyright (C) 2022 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.google.android.lint.aidl + +import com.android.tools.lint.detector.api.JavaContext +import com.android.tools.lint.detector.api.LintFix +import com.android.tools.lint.detector.api.Location +import com.intellij.psi.PsiClass +import com.intellij.psi.PsiReferenceList +import org.jetbrains.uast.UMethod + +/** + * Given a UMethod, determine if this method is + * the entrypoint to an interface generated by AIDL, + * returning the interface name if so, otherwise returning null + */ +fun getContainingAidlInterface(context: JavaContext, node: UMethod): String? { + if (!isContainedInSubclassOfStub(context, node)) return null + for (superMethod in node.findSuperMethods()) { + for (extendsInterface in superMethod.containingClass?.extendsList?.referenceElements + ?: continue) { + if (extendsInterface.qualifiedName == IINTERFACE_INTERFACE) { + return superMethod.containingClass?.name + } + } + } + return null +} + +fun isContainedInSubclassOfStub(context: JavaContext, node: UMethod?): Boolean { + var superClass = node?.containingClass?.superClass + while (superClass != null) { + if (isStub(context, superClass)) return true + superClass = superClass.superClass + } + return false +} + +fun isStub(context: JavaContext, psiClass: PsiClass?): Boolean { + if (psiClass == null) return false + if (psiClass.name != "Stub") return false + if (!context.evaluator.isStatic(psiClass)) return false + if (!context.evaluator.isAbstract(psiClass)) return false + + if (!hasSingleAncestor(psiClass.extendsList, BINDER_CLASS)) return false + + val parent = psiClass.parent as? PsiClass ?: return false + if (!hasSingleAncestor(parent.extendsList, IINTERFACE_INTERFACE)) return false + + val parentName = parent.qualifiedName ?: return false + if (!hasSingleAncestor(psiClass.implementsList, parentName)) return false + + return true +} + +private fun hasSingleAncestor(references: PsiReferenceList?, qualifiedName: String) = + references != null && + references.referenceElements.size == 1 && + references.referenceElements[0].qualifiedName == qualifiedName + +fun getHelperMethodCallSourceString(node: UMethod) = "${node.name}$AIDL_PERMISSION_HELPER_SUFFIX()" + +fun getHelperMethodFix( + node: UMethod, + manualCheckLocation: Location, + prepend: Boolean = true +): LintFix { + val helperMethodSource = getHelperMethodCallSourceString(node) + val indent = " ".repeat(manualCheckLocation.start?.column ?: 0) + val newText = "$helperMethodSource;${if (prepend) "\n\n$indent" else ""}" + + val fix = LintFix.create() + .replace() + .range(manualCheckLocation) + .with(newText) + .reformat(true) + .autoFix() + + if (prepend) fix.beginning() + + return fix.build() +} diff --git a/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/AidlImplementationDetector.kt b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/AidlImplementationDetector.kt deleted file mode 100644 index ab6d871d6ea6..000000000000 --- a/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/AidlImplementationDetector.kt +++ /dev/null @@ -1,52 +0,0 @@ -/* - * Copyright (C) 2022 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.google.android.lint.aidl - -import com.android.tools.lint.client.api.UElementHandler -import com.android.tools.lint.detector.api.Detector -import com.android.tools.lint.detector.api.JavaContext -import com.android.tools.lint.detector.api.SourceCodeScanner -import org.jetbrains.uast.UBlockExpression -import org.jetbrains.uast.UElement -import org.jetbrains.uast.UMethod - -/** - * Abstract class for detectors that look for methods implementing - * generated AIDL interface stubs - */ -abstract class AidlImplementationDetector : Detector(), SourceCodeScanner { - override fun getApplicableUastTypes(): List> = - listOf(UMethod::class.java) - - override fun createUastHandler(context: JavaContext): UElementHandler = AidlStubHandler(context) - - private inner class AidlStubHandler(val context: JavaContext) : UElementHandler() { - override fun visitMethod(node: UMethod) { - val interfaceName = getContainingAidlInterface(context, node) - .takeUnless(EXCLUDED_CPP_INTERFACES::contains) ?: return - val body = (node.uastBody as? UBlockExpression) ?: return - visitAidlMethod(context, node, interfaceName, body) - } - } - - abstract fun visitAidlMethod( - context: JavaContext, - node: UMethod, - interfaceName: String, - body: UBlockExpression, - ) -} diff --git a/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/Constants.kt b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/Constants.kt deleted file mode 100644 index e03d92ab44a0..000000000000 --- a/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/Constants.kt +++ /dev/null @@ -1,149 +0,0 @@ -/* - * Copyright (C) 2022 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.google.android.lint.aidl - -const val ANNOTATION_ENFORCE_PERMISSION = "android.annotation.EnforcePermission" -const val ANNOTATION_REQUIRES_NO_PERMISSION = "android.annotation.RequiresNoPermission" -const val ANNOTATION_PERMISSION_MANUALLY_ENFORCED = "android.annotation.PermissionManuallyEnforced" - -val AIDL_PERMISSION_ANNOTATIONS = listOf( - ANNOTATION_ENFORCE_PERMISSION, - ANNOTATION_REQUIRES_NO_PERMISSION, - ANNOTATION_PERMISSION_MANUALLY_ENFORCED -) - -const val BINDER_CLASS = "android.os.Binder" -const val IINTERFACE_INTERFACE = "android.os.IInterface" - -const val AIDL_PERMISSION_HELPER_SUFFIX = "_enforcePermission" - -/** - * If a non java (e.g. c++) backend is enabled, the @EnforcePermission - * annotation cannot be used. At time of writing, the mechanism - * is not implemented for non java backends. - * TODO: b/242564874 (have lint know which interfaces have the c++ backend enabled) - * rather than hard coding this list? - */ -val EXCLUDED_CPP_INTERFACES = listOf( - "AdbTransportType", - "FingerprintAndPairDevice", - "IAdbCallback", - "IAdbManager", - "PairDevice", - "IStatsBootstrapAtomService", - "StatsBootstrapAtom", - "StatsBootstrapAtomValue", - "FixedSizeArrayExample", - "PlaybackTrackMetadata", - "RecordTrackMetadata", - "SinkMetadata", - "SourceMetadata", - "IUpdateEngineStable", - "IUpdateEngineStableCallback", - "AudioCapabilities", - "ConfidenceLevel", - "ModelParameter", - "ModelParameterRange", - "Phrase", - "PhraseRecognitionEvent", - "PhraseRecognitionExtra", - "PhraseSoundModel", - "Properties", - "RecognitionConfig", - "RecognitionEvent", - "RecognitionMode", - "RecognitionStatus", - "SoundModel", - "SoundModelType", - "Status", - "IThermalService", - "IPowerManager", - "ITunerResourceManager", - // b/278147400 - "IActivityManager", - "IUidObserver", - "IDrm", - "IVsyncCallback", - "IVsyncService", - "ICallback", - "IIPCTest", - "ISafeInterfaceTest", - "IGpuService", - "IConsumerListener", - "IGraphicBufferConsumer", - "ITransactionComposerListener", - "SensorEventConnection", - "SensorServer", - "ICamera", - "ICameraClient", - "ICameraRecordingProxy", - "ICameraRecordingProxyListener", - "ICrypto", - "IOMXObserver", - "IStreamListener", - "IStreamSource", - "IAudioService", - "IDataSource", - "IDrmClient", - "IMediaCodecList", - "IMediaDrmService", - "IMediaExtractor", - "IMediaExtractorService", - "IMediaHTTPConnection", - "IMediaHTTPService", - "IMediaLogService", - "IMediaMetadataRetriever", - "IMediaMetricsService", - "IMediaPlayer", - "IMediaPlayerClient", - "IMediaPlayerService", - "IMediaRecorder", - "IMediaRecorderClient", - "IMediaResourceMonitor", - "IMediaSource", - "IRemoteDisplay", - "IRemoteDisplayClient", - "IResourceManagerClient", - "IResourceManagerService", - "IComplexTypeInterface", - "IPermissionController", - "IPingResponder", - "IProcessInfoService", - "ISchedulingPolicyService", - "IStringConstants", - "IObbActionListener", - "IStorageEventListener", - "IStorageManager", - "IStorageShutdownObserver", - "IPersistentVrStateCallbacks", - "IVrManager", - "IVrStateCallbacks", - "ISurfaceComposer", - "IMemory", - "IMemoryHeap", - "IProcfsInspector", - "IAppOpsCallback", - "IAppOpsService", - "IBatteryStats", - "IResultReceiver", - "IShellCallback", - "IDrmManagerService", - "IDrmServiceListener", - "IAAudioClient", - "IAAudioService", - "VtsFuzzer", -) diff --git a/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionUtils.kt b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionUtils.kt deleted file mode 100644 index d41fee3fc0dc..000000000000 --- a/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionUtils.kt +++ /dev/null @@ -1,96 +0,0 @@ -/* - * Copyright (C) 2022 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.google.android.lint.aidl - -import com.android.tools.lint.detector.api.JavaContext -import com.android.tools.lint.detector.api.LintFix -import com.android.tools.lint.detector.api.Location -import com.intellij.psi.PsiClass -import com.intellij.psi.PsiReferenceList -import org.jetbrains.uast.UMethod - -/** - * Given a UMethod, determine if this method is - * the entrypoint to an interface generated by AIDL, - * returning the interface name if so, otherwise returning null - */ -fun getContainingAidlInterface(context: JavaContext, node: UMethod): String? { - if (!isContainedInSubclassOfStub(context, node)) return null - for (superMethod in node.findSuperMethods()) { - for (extendsInterface in superMethod.containingClass?.extendsList?.referenceElements - ?: continue) { - if (extendsInterface.qualifiedName == IINTERFACE_INTERFACE) { - return superMethod.containingClass?.name - } - } - } - return null -} - -fun isContainedInSubclassOfStub(context: JavaContext, node: UMethod?): Boolean { - var superClass = node?.containingClass?.superClass - while (superClass != null) { - if (isStub(context, superClass)) return true - superClass = superClass.superClass - } - return false -} - -fun isStub(context: JavaContext, psiClass: PsiClass?): Boolean { - if (psiClass == null) return false - if (psiClass.name != "Stub") return false - if (!context.evaluator.isStatic(psiClass)) return false - if (!context.evaluator.isAbstract(psiClass)) return false - - if (!hasSingleAncestor(psiClass.extendsList, BINDER_CLASS)) return false - - val parent = psiClass.parent as? PsiClass ?: return false - if (!hasSingleAncestor(parent.extendsList, IINTERFACE_INTERFACE)) return false - - val parentName = parent.qualifiedName ?: return false - if (!hasSingleAncestor(psiClass.implementsList, parentName)) return false - - return true -} - -private fun hasSingleAncestor(references: PsiReferenceList?, qualifiedName: String) = - references != null && - references.referenceElements.size == 1 && - references.referenceElements[0].qualifiedName == qualifiedName - -fun getHelperMethodCallSourceString(node: UMethod) = "${node.name}$AIDL_PERMISSION_HELPER_SUFFIX()" - -fun getHelperMethodFix( - node: UMethod, - manualCheckLocation: Location, - prepend: Boolean = true -): LintFix { - val helperMethodSource = getHelperMethodCallSourceString(node) - val indent = " ".repeat(manualCheckLocation.start?.column ?: 0) - val newText = "$helperMethodSource;${if (prepend) "\n\n$indent" else ""}" - - val fix = LintFix.create() - .replace() - .range(manualCheckLocation) - .with(newText) - .reformat(true) - .autoFix() - - if (prepend) fix.beginning() - - return fix.build() -} -- cgit v1.2.3-59-g8ed1b From e29165131ae1955825e4ccea33d0bbfddd28c360 Mon Sep 17 00:00:00 2001 From: Thiébaud Weksteen Date: Thu, 31 Aug 2023 11:22:13 +1000 Subject: Add utility lint for metrics on @EnforcePermission Bug: 298285238 Test: lint_fix --print --no-fix --check AnnotatedAidlCounter --lint-module AndroidUtilsLintChecker services.autofill Test: atest --host AndroidUtilsLintCheckerTest Change-Id: I7876e44cabc006de9b996f84477e15071cb95203 --- tools/lint/common/Android.bp | 27 ++++++ tools/lint/framework/Android.bp | 21 +---- tools/lint/global/Android.bp | 21 +---- tools/lint/utils/Android.bp | 45 ++++++++++ .../android/lint/AndroidUtilsIssueRegistry.kt | 43 ++++++++++ .../android/lint/aidl/AnnotatedAidlCounter.kt | 96 +++++++++++++++++++++ .../android/lint/aidl/AnnotatedAidlCounterTest.kt | 99 ++++++++++++++++++++++ 7 files changed, 312 insertions(+), 40 deletions(-) create mode 100644 tools/lint/utils/Android.bp create mode 100644 tools/lint/utils/checks/src/main/java/com/google/android/lint/AndroidUtilsIssueRegistry.kt create mode 100644 tools/lint/utils/checks/src/main/java/com/google/android/lint/aidl/AnnotatedAidlCounter.kt create mode 100644 tools/lint/utils/checks/src/test/java/com/google/android/lint/aidl/AnnotatedAidlCounterTest.kt diff --git a/tools/lint/common/Android.bp b/tools/lint/common/Android.bp index 898f88b8759c..8bfbfe5f60b3 100644 --- a/tools/lint/common/Android.bp +++ b/tools/lint/common/Android.bp @@ -27,3 +27,30 @@ java_library_host { libs: ["lint_api"], kotlincflags: ["-Xjvm-default=all"], } + +java_defaults { + name: "AndroidLintCheckerTestDefaults", + srcs: ["checks/src/test/java/**/*.kt"], + static_libs: [ + "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/framework/Android.bp b/tools/lint/framework/Android.bp index 30a6daaef2a4..5acdf436ae08 100644 --- a/tools/lint/framework/Android.bp +++ b/tools/lint/framework/Android.bp @@ -37,28 +37,9 @@ java_library_host { java_test_host { name: "AndroidFrameworkLintCheckerTest", + defaults: ["AndroidLintCheckerTestDefaults"], srcs: ["checks/src/test/java/**/*.kt"], static_libs: [ "AndroidFrameworkLintChecker", - "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/global/Android.bp b/tools/lint/global/Android.bp index bedb7bd78a29..3e74171814ab 100644 --- a/tools/lint/global/Android.bp +++ b/tools/lint/global/Android.bp @@ -38,28 +38,9 @@ java_library_host { java_test_host { name: "AndroidGlobalLintCheckerTest", + defaults: ["AndroidLintCheckerTestDefaults"], srcs: ["checks/src/test/java/**/*.kt"], static_libs: [ "AndroidGlobalLintChecker", - "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/utils/Android.bp b/tools/lint/utils/Android.bp new file mode 100644 index 000000000000..75e8d6863c20 --- /dev/null +++ b/tools/lint/utils/Android.bp @@ -0,0 +1,45 @@ +// Copyright (C) 2023 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 { + // See: http://go/android-license-faq + // A large-scale-change added 'default_applicable_licenses' to import + // all of the 'license_kinds' from "frameworks_base_license" + // to get the below license kinds: + // SPDX-license-identifier-Apache-2.0 + default_applicable_licenses: ["frameworks_base_license"], +} + +java_library_host { + name: "AndroidUtilsLintChecker", + srcs: ["checks/src/main/java/**/*.kt"], + plugins: ["auto_service_plugin"], + libs: [ + "auto_service_annotations", + "lint_api", + ], + static_libs: [ + "AndroidCommonLint", + ], + kotlincflags: ["-Xjvm-default=all"], +} + +java_test_host { + name: "AndroidUtilsLintCheckerTest", + defaults: ["AndroidLintCheckerTestDefaults"], + srcs: ["checks/src/test/java/**/*.kt"], + static_libs: [ + "AndroidUtilsLintChecker", + ], +} diff --git a/tools/lint/utils/checks/src/main/java/com/google/android/lint/AndroidUtilsIssueRegistry.kt b/tools/lint/utils/checks/src/main/java/com/google/android/lint/AndroidUtilsIssueRegistry.kt new file mode 100644 index 000000000000..fa61c42ef8e6 --- /dev/null +++ b/tools/lint/utils/checks/src/main/java/com/google/android/lint/AndroidUtilsIssueRegistry.kt @@ -0,0 +1,43 @@ +/* + * Copyright (C) 2023 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.google.android.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.android.lint.aidl.AnnotatedAidlCounter +import com.google.auto.service.AutoService + +@AutoService(IssueRegistry::class) +@Suppress("UnstableApiUsage") +class AndroidUtilsIssueRegistry : IssueRegistry() { + override val issues = listOf( + AnnotatedAidlCounter.ISSUE_ANNOTATED_AIDL_COUNTER, + ) + + override val api: Int + get() = CURRENT_API + + override val minApi: Int + get() = 8 + + override val vendor: Vendor = Vendor( + vendorName = "Android", + feedbackUrl = "http://b/issues/new?component=315013", + contact = "tweek@google.com" + ) +} diff --git a/tools/lint/utils/checks/src/main/java/com/google/android/lint/aidl/AnnotatedAidlCounter.kt b/tools/lint/utils/checks/src/main/java/com/google/android/lint/aidl/AnnotatedAidlCounter.kt new file mode 100644 index 000000000000..f0ec3f44f6c4 --- /dev/null +++ b/tools/lint/utils/checks/src/main/java/com/google/android/lint/aidl/AnnotatedAidlCounter.kt @@ -0,0 +1,96 @@ +/* + * Copyright (C) 2023 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.google.android.lint.aidl + +import com.android.tools.lint.detector.api.Category +import com.android.tools.lint.detector.api.Context +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.Location +import com.android.tools.lint.detector.api.Scope +import com.android.tools.lint.detector.api.Severity +import org.jetbrains.uast.UBlockExpression +import org.jetbrains.uast.UMethod + +import java.util.TreeMap + +/** + * Count the number of AIDL interfaces. Reports the number of annotated and + * non-annotated methods. + */ +@Suppress("UnstableApiUsage") +class AnnotatedAidlCounter : AidlImplementationDetector() { + + private data class Stat( + var unannotated: Int = 0, + var enforced: Int = 0, + var notRequired: Int = 0, + ) + + private var packagesStats: TreeMap = TreeMap() + + override fun visitAidlMethod( + context: JavaContext, + node: UMethod, + interfaceName: String, + body: UBlockExpression + ) { + val packageName = context.uastFile?.packageName ?: "" + var packageStat = packagesStats.getOrDefault(packageName, Stat()) + when { + node.hasAnnotation(ANNOTATION_ENFORCE_PERMISSION) -> packageStat.enforced += 1 + node.hasAnnotation(ANNOTATION_REQUIRES_NO_PERMISSION) -> packageStat.notRequired += 1 + else -> packageStat.unannotated += 1 + } + packagesStats.put(packageName, packageStat) + // context.driver.client.log(null, "%s.%s#%s".format(packageName, interfaceName, node.name)) + } + + override fun afterCheckRootProject(context: Context) { + var total = Stat() + for ((packageName, stat) in packagesStats) { + context.client.log(null, "package $packageName => $stat") + total.unannotated += stat.unannotated + total.enforced += stat.enforced + total.notRequired += stat.notRequired + } + val location = Location.create(context.project.dir) + context.report( + ISSUE_ANNOTATED_AIDL_COUNTER, + location, + "module ${context.project.name} => $total" + ) + } + + companion object { + + @JvmField + val ISSUE_ANNOTATED_AIDL_COUNTER = Issue.create( + id = "AnnotatedAidlCounter", + briefDescription = "Statistics on the number of annotated AIDL methods.", + explanation = "", + category = Category.SECURITY, + priority = 5, + severity = Severity.INFORMATIONAL, + implementation = Implementation( + AnnotatedAidlCounter::class.java, + Scope.JAVA_FILE_SCOPE + ), + ) + } +} diff --git a/tools/lint/utils/checks/src/test/java/com/google/android/lint/aidl/AnnotatedAidlCounterTest.kt b/tools/lint/utils/checks/src/test/java/com/google/android/lint/aidl/AnnotatedAidlCounterTest.kt new file mode 100644 index 000000000000..692b7da243c9 --- /dev/null +++ b/tools/lint/utils/checks/src/test/java/com/google/android/lint/aidl/AnnotatedAidlCounterTest.kt @@ -0,0 +1,99 @@ +/* + * Copyright (C) 2023 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.google.android.lint.aidl + +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 + +@Suppress("UnstableApiUsage") +class AnnotatedAidlCounterTest : LintDetectorTest() { + override fun getDetector(): Detector = AnnotatedAidlCounter() + + override fun getIssues(): List = listOf( + AnnotatedAidlCounter.ISSUE_ANNOTATED_AIDL_COUNTER, + ) + + override fun lint(): TestLintTask = super.lint().allowMissingSdk(true) + + /** No issue scenario */ + + fun testDoesNotDetectIssuesCorrectAnnotationOnMethod() { + lint().files(java( + """ + package test.pkg; + import android.annotation.EnforcePermission; + public class TestClass2 extends IFooMethod.Stub { + @Override + @EnforcePermission(android.Manifest.permission.READ_PHONE_STATE) + public void testMethod() {} + } + """).indented(), + *stubs + ) + .run() + .expect(""" + app: Information: module app => Stat(unannotated=0, enforced=1, notRequired=0) [AnnotatedAidlCounter] + 0 errors, 0 warnings + """) + } + + // A service with permission annotation on the method. + private val interfaceIFooMethodStub: TestFile = java( + """ + public interface IFooMethod extends android.os.IInterface { + public static abstract class Stub extends android.os.Binder implements IFooMethod {} + @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE) + public void testMethod(); + } + """ + ).indented() + + // A service without any permission annotation. + private val interfaceIBarStub: TestFile = java( + """ + public interface IBar extends android.os.IInterface { + public static abstract class Stub extends android.os.Binder implements IBar { + @Override + public void testMethod() {} + } + public void testMethod(); + } + """ + ).indented() + + private val manifestPermissionStub: TestFile = java( + """ + package android.Manifest; + class permission { + public static final String READ_PHONE_STATE = "android.permission.READ_PHONE_STATE"; + } + """ + ).indented() + + private val enforcePermissionAnnotationStub: TestFile = java( + """ + package android.annotation; + public @interface EnforcePermission {} + """ + ).indented() + + private val stubs = arrayOf(interfaceIFooMethodStub, interfaceIBarStub, + manifestPermissionStub, enforcePermissionAnnotationStub) +} -- cgit v1.2.3-59-g8ed1b From 5c149e2df4c8f4bff1826b44cdb99a2ec54d6923 Mon Sep 17 00:00:00 2001 From: Thiébaud Weksteen Date: Mon, 4 Sep 2023 09:29:35 +1000 Subject: Support multiple modules for lint_fix Test: lint_fix --print --no-fix --check AnnotatedAidlCounter --lint-module AndroidUtilsLintChecker services.autofill services.usage Change-Id: I08f5aa3e74db8ff24544bbdd3edb0635a0d2fb52 --- tools/lint/fix/soong_lint_fix.py | 103 +++++++++++++++++++++++++-------------- 1 file changed, 66 insertions(+), 37 deletions(-) diff --git a/tools/lint/fix/soong_lint_fix.py b/tools/lint/fix/soong_lint_fix.py index cd4d778d1dec..acc0ad043171 100644 --- a/tools/lint/fix/soong_lint_fix.py +++ b/tools/lint/fix/soong_lint_fix.py @@ -29,6 +29,39 @@ PATH_PREFIX = "out/soong/.intermediates" PATH_SUFFIX = "android_common/lint" FIX_ZIP = "suggested-fixes.zip" + +class SoongModule: + """A Soong module to lint. + + The constructor takes the name of the module (for example, + "framework-minus-apex"). find() must be called to extract the intermediate + module path from Soong's module-info.json + """ + def __init__(self, name): + self._name = name + + def find(self, module_info): + """Finds the module in the loaded module_info.json.""" + if self._name not in module_info: + raise Exception(f"Module {self._name} not found!") + + partial_path = module_info[self._name]["path"][0] + print(f"Found module {partial_path}/{self._name}.") + self._path = f"{PATH_PREFIX}/{partial_path}/{self._name}/{PATH_SUFFIX}" + + @property + def name(self): + return self._name + + @property + def lint_report(self): + return f"{self._path}/lint-report.txt" + + @property + def suggested_fixes(self): + return f"{self._path}/{FIX_ZIP}" + + class SoongLintFix: """ This class creates a command line tool that will @@ -53,16 +86,14 @@ class SoongLintFix: self._parser = _setup_parser() self._args = None self._kwargs = None - self._path = None - self._target = None - + self._modules = [] - def run(self, additional_setup=None, custom_fix=None): + def run(self): """ Run the script """ self._setup() - self._find_module() + self._find_modules() self._lint() if not self._args.no_fix: @@ -87,8 +118,6 @@ class SoongLintFix: os.chdir(ANDROID_BUILD_TOP) - - def _find_module(self): print("Refreshing soong modules...") try: os.mkdir(ANDROID_PRODUCT_OUT) @@ -97,48 +126,47 @@ class SoongLintFix: subprocess.call(f"{SOONG_UI} --make-mode {PRODUCT_OUT}/module-info.json", **self._kwargs) print("done.") + + def _find_modules(self): with open(f"{ANDROID_PRODUCT_OUT}/module-info.json") as f: module_info = json.load(f) - if self._args.module not in module_info: - sys.exit(f"Module {self._args.module} not found!") - - module_path = module_info[self._args.module]["path"][0] - print(f"Found module {module_path}/{self._args.module}.") - - self._path = f"{PATH_PREFIX}/{module_path}/{self._args.module}/{PATH_SUFFIX}" - self._target = f"{self._path}/lint-report.txt" - + for module_name in self._args.modules: + module = SoongModule(module_name) + module.find(module_info) + self._modules.append(module) def _lint(self): print("Cleaning up any old lint results...") - try: - os.remove(f"{self._target}") - os.remove(f"{self._path}/{FIX_ZIP}") - except FileNotFoundError: - pass + for module in self._modules: + try: + os.remove(f"{module.lint_report}") + os.remove(f"{module.suggested_fixes}") + except FileNotFoundError: + pass print("done.") - print(f"Generating {self._target}") - subprocess.call(f"{SOONG_UI} --make-mode {self._target}", **self._kwargs) + target = " ".join([ module.lint_report for module in self._modules ]) + print(f"Generating {target}") + subprocess.call(f"{SOONG_UI} --make-mode {target}", **self._kwargs) print("done.") - def _fix(self): - print("Copying suggested fixes to the tree...") - with zipfile.ZipFile(f"{self._path}/{FIX_ZIP}") as zip: - for name in zip.namelist(): - if name.startswith("out") or not name.endswith(".java"): - continue - with zip.open(name) as src, open(f"{ANDROID_BUILD_TOP}/{name}", "wb") as dst: - shutil.copyfileobj(src, dst) + for module in self._modules: + print(f"Copying suggested fixes for {module.name} to the tree...") + with zipfile.ZipFile(f"{module.suggested_fixes}") as zip: + for name in zip.namelist(): + if name.startswith("out") or not name.endswith(".java"): + continue + with zip.open(name) as src, open(f"{ANDROID_BUILD_TOP}/{name}", "wb") as dst: + shutil.copyfileobj(src, dst) print("done.") - def _print(self): - print("### lint-report.txt ###", end="\n\n") - with open(self._target, "r") as f: - print(f.read()) + for module in self._modules: + print(f"### lint-report.txt {module.name} ###", end="\n\n") + with open(module.lint_report, "r") as f: + print(f.read()) def _setup_parser(): @@ -151,7 +179,8 @@ def _setup_parser(): **Gotcha**: You must have run `source build/envsetup.sh` and `lunch` first. """, formatter_class=argparse.RawTextHelpFormatter) - parser.add_argument('module', + parser.add_argument('modules', + nargs='+', help='The soong build module to run ' '(e.g. framework-minus-apex or services.core.unboosted)') @@ -170,4 +199,4 @@ def _setup_parser(): return parser if __name__ == "__main__": - SoongLintFix().run() \ No newline at end of file + SoongLintFix().run() -- cgit v1.2.3-59-g8ed1b