Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 18814965 authored by mattgilbride's avatar mattgilbride
Browse files

SimpleManualPermissionEnforcementDetector: minor cleanup 

- rename issue constant to match issue ID
- move more logic into EnforcePermissionFix helper rather than detector
  itself
- fix indentation in test

Bug: 232058525
Test: SimpleManualPermissionEnforcementDetectorTest
Change-Id: I9b037806711cc0e9d113e308bfb1f802b56d4034
parent 4a49ec0d

tools/lint/framework/checks/src/main/java/com/google/android/lint/AndroidFrameworkIssueRegistry.kt

+1 −1
Original line number Diff line number Diff line
@@ -40,7 +40,7 @@ class AndroidFrameworkIssueRegistry : IssueRegistry() { 
        EnforcePermissionDetector.ISSUE_MISSING_ENFORCE_PERMISSION,

        EnforcePermissionDetector.ISSUE_MISMATCHING_ENFORCE_PERMISSION,

        EnforcePermissionHelperDetector.ISSUE_ENFORCE_PERMISSION_HELPER,

        SimpleManualPermissionEnforcementDetector.ISSUE_USE_ENFORCE_PERMISSION_ANNOTATION,

        SimpleManualPermissionEnforcementDetector.ISSUE_SIMPLE_MANUAL_PERMISSION_ENFORCEMENT,

        SaferParcelChecker.ISSUE_UNSAFE_API_USAGE,

        PackageVisibilityDetector.ISSUE_PACKAGE_NAME_NO_PACKAGE_VISIBILITY_FILTERS,

        RegisterReceiverFlagDetector.ISSUE_RECEIVER_EXPORTED_FLAG,

tools/lint/global/checks/src/main/java/com/google/android/lint/AndroidGlobalIssueRegistry.kt

+1 −1
Original line number Diff line number Diff line
@@ -31,7 +31,7 @@ class AndroidGlobalIssueRegistry : IssueRegistry() { 
            EnforcePermissionDetector.ISSUE_MISSING_ENFORCE_PERMISSION,

            EnforcePermissionDetector.ISSUE_MISMATCHING_ENFORCE_PERMISSION,

            EnforcePermissionHelperDetector.ISSUE_ENFORCE_PERMISSION_HELPER,

            SimpleManualPermissionEnforcementDetector.ISSUE_USE_ENFORCE_PERMISSION_ANNOTATION,

            SimpleManualPermissionEnforcementDetector.ISSUE_SIMPLE_MANUAL_PERMISSION_ENFORCEMENT,

    )



    override val api: Int

tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionFix.kt

+26 −2
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@ 
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.android.tools.lint.detector.api.getUMethod

import com.google.android.lint.hasPermissionNameAnnotation
@@ -38,11 +39,34 @@ data class EnforcePermissionFix( 
    val locations: List<Location>,

    val permissionNames: List<String>

) {

    val annotation: String

    fun toLintFix(annotationLocation: Location): LintFix {

        val removeFixes = this.locations.map {

            LintFix.create()

                .replace()

                .reformat(true)

                .range(it)

                .with("")

                .autoFix()

                .build()

        }



        val annotateFix = LintFix.create()

            .annotate(this.annotation)

            .range(annotationLocation)

            .autoFix()

            .build()



        return LintFix.create().composite(annotateFix, *removeFixes.toTypedArray())

    }



    private val annotation: String

        get() {

            val quotedPermissions = permissionNames.joinToString(", ") { """"$it"""" }



            val annotationParameter =

                if (permissionNames.size > 1) "allOf={$quotedPermissions}" else quotedPermissions

                if (permissionNames.size > 1) "allOf={$quotedPermissions}"

                else quotedPermissions



            return "@$ANNOTATION_ENFORCE_PERMISSION($annotationParameter)"

        }

tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/SimpleManualPermissionEnforcementDetector.kt

+12 −23
Original line number Diff line number Diff line
@@ -28,6 +28,7 @@ import org.jetbrains.uast.UElement 
import org.jetbrains.uast.UIfExpression

import org.jetbrains.uast.UMethod

import org.jetbrains.uast.UQualifiedReferenceExpression

import org.jetbrains.uast.skipParenthesizedExprDown



/**

 * Looks for methods implementing generated AIDL interface stubs
@@ -44,29 +45,16 @@ class SimpleManualPermissionEnforcementDetector : AidlImplementationDetector() { 
            interfaceName: String,

            body: UBlockExpression

    ) {

        val fix = accumulateSimplePermissionCheckFixes(body, context) ?: return



        val javaRemoveFixes = fix.locations.map {

            fix()

                    .replace()

                    .reformat(true)

                    .range(it)

                    .with("")

                    .autoFix()

                    .build()

        }



        val javaAnnotateFix = fix()

                .annotate(fix.annotation)

                .range(context.getLocation(node))

                .autoFix()

                .build()

        val enforcePermissionFix = accumulateSimplePermissionCheckFixes(body, context) ?: return

        val lintFix = enforcePermissionFix.toLintFix(context.getLocation(node))

        val message =

                "$interfaceName permission check can be converted to @EnforcePermission annotation"



        context.report(

                ISSUE_USE_ENFORCE_PERMISSION_ANNOTATION,

                fix.locations.last(),

                "$interfaceName permission check can be converted to @EnforcePermission annotation",

                fix().composite(*javaRemoveFixes.toTypedArray(), javaAnnotateFix)

                ISSUE_SIMPLE_MANUAL_PERMISSION_ENFORCEMENT,

                enforcePermissionFix.locations.last(),

                message,

                lintFix

        )

    }
@@ -89,7 +77,8 @@ class SimpleManualPermissionEnforcementDetector : AidlImplementationDetector() { 
            EnforcePermissionFix? {

        val singleFixes = mutableListOf<EnforcePermissionFix>()

        for (expression in methodBody.expressions) {

            singleFixes.add(getPermissionCheckFix(expression, context) ?: break)

            singleFixes.add(getPermissionCheckFix(expression.skipParenthesizedExprDown(), context)

                    ?: break)

        }

        return when (singleFixes.size) {

            0 -> null
@@ -133,7 +122,7 @@ class SimpleManualPermissionEnforcementDetector : AidlImplementationDetector() { 
        """.trimIndent()



        @JvmField

        val ISSUE_USE_ENFORCE_PERMISSION_ANNOTATION = Issue.create(

        val ISSUE_SIMPLE_MANUAL_PERMISSION_ENFORCEMENT = Issue.create(

                id = "SimpleManualPermissionEnforcement",

                briefDescription = "Manual permission check can be @EnforcePermission annotation",

                explanation = EXPLANATION,

tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/SimpleManualPermissionEnforcementDetectorTest.kt

+67 −68
Original line number Diff line number Diff line
@@ -18,7 +18,6 @@ package com.google.android.lint.aidl 


import com.android.tools.lint.checks.infrastructure.LintDetectorTest

import com.android.tools.lint.checks.infrastructure.TestLintTask

import com.android.tools.lint.checks.infrastructure.TestMode

import com.android.tools.lint.detector.api.Detector

import com.android.tools.lint.detector.api.Issue
@@ -27,7 +26,7 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() { 
    override fun getDetector(): Detector = SimpleManualPermissionEnforcementDetector()

    override fun getIssues(): List<Issue> = listOf(

            SimpleManualPermissionEnforcementDetector

            .ISSUE_USE_ENFORCE_PERMISSION_ANNOTATION

            .ISSUE_SIMPLE_MANUAL_PERMISSION_ENFORCEMENT

    )



    override fun lint(): TestLintTask = super.lint().allowMissingSdk()
@@ -217,7 +216,7 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() { 
    }



    fun testPermissionHelper() {

        lint().skipTestModes(TestMode.PARENTHESIZED).files(

        lint().files(

            java(

                """

                import android.content.Context;
@@ -261,7 +260,7 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() { 
    }



    fun testPermissionHelperAllOf() {

        lint().skipTestModes(TestMode.PARENTHESIZED).files(

        lint().files(

            java(

                """

                import android.content.Context;
@@ -309,7 +308,7 @@ class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() { 




    fun testPermissionHelperNested() {

        lint().skipTestModes(TestMode.PARENTHESIZED).files(

        lint().files(

            java(

                """

                import android.content.Context;