From 56a075eab656520bea950bad1c6ea2e8bccddcf3 Mon Sep 17 00:00:00 2001 From: Vyacheslav Gerasimov Date: Thu, 24 Aug 2017 16:33:52 +0300 Subject: [PATCH] UAST: Fix annotation arguments processing multiple unnamed arguments represented as value named expression with array initializer call kind for array in annotation argument should be "array initializer" instead of "method call" #KT-16600 Fixed Target Versions 1.1.5 --- .../android/lint/AbstractKotlinLintTest.kt | 4 +- .../android/lint/RequiresPermission.java | 133 ++++++++++++++++++ .../android/lint/supportAnnotation.kt | 22 ++- license/README.md | 4 + .../kotlin/declarations/KotlinUAnnotation.kt | 49 ++++--- .../KotlinUFunctionCallExpression.kt | 20 ++- .../expressions/KotlinUNamedExpression.kt | 96 +++++++++++++ .../testData/AnnotationParameters.kt | 15 +- .../testData/AnnotationParameters.log.txt | 32 +++++ .../testData/AnnotationParameters.render.txt | 18 +++ 10 files changed, 361 insertions(+), 32 deletions(-) create mode 100644 idea/testData/android/lint/RequiresPermission.java create mode 100644 plugins/uast-kotlin/src/org/jetbrains/uast/kotlin/expressions/KotlinUNamedExpression.kt diff --git a/idea/idea-android/tests/org/jetbrains/kotlin/android/lint/AbstractKotlinLintTest.kt b/idea/idea-android/tests/org/jetbrains/kotlin/android/lint/AbstractKotlinLintTest.kt index cbd8a90e27e..8efc853d4ca 100644 --- a/idea/idea-android/tests/org/jetbrains/kotlin/android/lint/AbstractKotlinLintTest.kt +++ b/idea/idea-android/tests/org/jetbrains/kotlin/android/lint/AbstractKotlinLintTest.kt @@ -43,7 +43,7 @@ abstract class AbstractKotlinLintTest : KotlinAndroidTestCase() { val ktFile = File(path) val fileText = ktFile.readText() val mainInspectionClassName = findStringWithPrefixes(fileText, "// INSPECTION_CLASS: ") ?: error("Empty class name") - val dependency = InTextDirectivesUtils.findStringWithPrefixes(fileText, "// DEPENDENCY: ") + val dependencies = InTextDirectivesUtils.findLinesWithPrefixesRemoved(fileText, "// DEPENDENCY: ") val inspectionClassNames = mutableListOf(mainInspectionClassName) for (i in 2..100) { @@ -71,7 +71,7 @@ abstract class AbstractKotlinLintTest : KotlinAndroidTestCase() { val virtualFile = myFixture.copyFileToProject(ktFile.absolutePath, "src/${PathUtil.getFileName(path)}") myFixture.configureFromExistingVirtualFile(virtualFile) - if (dependency != null) { + dependencies.forEach { dependency -> val (dependencyFile, dependencyTargetPath) = dependency.split(" -> ").map(String::trim) myFixture.copyFileToProject("${PathUtil.getParentPath(path)}/$dependencyFile", "src/$dependencyTargetPath") } diff --git a/idea/testData/android/lint/RequiresPermission.java b/idea/testData/android/lint/RequiresPermission.java new file mode 100644 index 00000000000..1ff19646485 --- /dev/null +++ b/idea/testData/android/lint/RequiresPermission.java @@ -0,0 +1,133 @@ +/* + * Copyright (C) 2015 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 android.support.annotation; + +import java.lang.annotation.Retention; +import java.lang.annotation.Target; + +import static java.lang.annotation.ElementType.ANNOTATION_TYPE; +import static java.lang.annotation.ElementType.CONSTRUCTOR; +import static java.lang.annotation.ElementType.FIELD; +import static java.lang.annotation.ElementType.METHOD; +import static java.lang.annotation.ElementType.PARAMETER; +import static java.lang.annotation.RetentionPolicy.CLASS; + +/** + * Denotes that the annotated element requires (or may require) one or more permissions. + *

+ * Example of requiring a single permission: + * 


+ *   @RequiresPermission(Manifest.permission.SET_WALLPAPER)
+ *   public abstract void setWallpaper(Bitmap bitmap) throws IOException;
+ *
+ *   @RequiresPermission(ACCESS_COARSE_LOCATION)
+ *   public abstract Location getLastKnownLocation(String provider);
+ *
+ * Example of requiring at least one permission from a set: + * 

+ *   @RequiresPermission(anyOf = {ACCESS_COARSE_LOCATION, ACCESS_FINE_LOCATION})
+ *   public abstract Location getLastKnownLocation(String provider);
+ *
+ * Example of requiring multiple permissions: + * 

+ *   @RequiresPermission(allOf = {ACCESS_COARSE_LOCATION, ACCESS_FINE_LOCATION})
+ *   public abstract Location getLastKnownLocation(String provider);
+ *
+ * Example of requiring separate read and write permissions for a content provider: + * 

+ *   @RequiresPermission.Read(@RequiresPermission(READ_HISTORY_BOOKMARKS))
+ *   @RequiresPermission.Write(@RequiresPermission(WRITE_HISTORY_BOOKMARKS))
+ *   public static final Uri BOOKMARKS_URI = Uri.parse("content://browser/bookmarks");
+ *
+ *

+ * When specified on a parameter, the annotation indicates that the method requires + * a permission which depends on the value of the parameter. For example, consider + * {@code android.app.Activity.startActivity(android.content.Intent)}: + * 

{@code
+ *   public void startActivity(@RequiresPermission Intent intent) { ... }
+ * }
+ * Notice how there are no actual permission names listed in the annotation. The actual + * permissions required will depend on the particular intent passed in. For example, + * the code may look like this: + * 
{@code
+ *   Intent intent = new Intent(Intent.ACTION_CALL);
+ *   startActivity(intent);
+ * }
+ * and the actual permission requirement for this particular intent is described on + * the Intent name itself: + * 

+ *   @RequiresPermission(Manifest.permission.CALL_PHONE)
+ *   public static final String ACTION_CALL = "android.intent.action.CALL";
+ *
+ */ +@Retention(CLASS) +@Target({ANNOTATION_TYPE,METHOD,CONSTRUCTOR,FIELD,PARAMETER}) +public @interface RequiresPermission { + /** + * The name of the permission that is required, if precisely one permission + * is required. If more than one permission is required, specify either + * {@link #allOf()} or {@link #anyOf()} instead. + *

+ * If specified, {@link #anyOf()} and {@link #allOf()} must both be null. + */ + String value() default ""; + + /** + * Specifies a list of permission names that are all required. + *

+ * If specified, {@link #anyOf()} and {@link #value()} must both be null. + */ + String[] allOf() default {}; + + /** + * Specifies a list of permission names where at least one is required + *

+ * If specified, {@link #allOf()} and {@link #value()} must both be null. + */ + String[] anyOf() default {}; + + /** + * If true, the permission may not be required in all cases (e.g. it may only be + * enforced on certain platforms, or for certain call parameters, etc. + */ + boolean conditional() default false; + + /** + * Specifies that the given permission is required for read operations. + *

+ * When specified on a parameter, the annotation indicates that the method requires + * a permission which depends on the value of the parameter (and typically + * the corresponding field passed in will be one of a set of constants which have + * been annotated with a {@code @RequiresPermission} annotation.) + */ + @Target({FIELD, METHOD, PARAMETER}) + @interface Read { + RequiresPermission value() default @RequiresPermission; + } + + /** + * Specifies that the given permission is required for write operations. + *

+ * When specified on a parameter, the annotation indicates that the method requires + * a permission which depends on the value of the parameter (and typically + * the corresponding field passed in will be one of a set of constants which have + * been annotated with a {@code @RequiresPermission} annotation.) + */ + @Target({FIELD, METHOD, PARAMETER}) + @interface Write { + RequiresPermission value() default @RequiresPermission; + } +} diff --git a/idea/testData/android/lint/supportAnnotation.kt b/idea/testData/android/lint/supportAnnotation.kt index 4f10424aa66..d6fae75c4c2 100644 --- a/idea/testData/android/lint/supportAnnotation.kt +++ b/idea/testData/android/lint/supportAnnotation.kt @@ -1,7 +1,11 @@ // INSPECTION_CLASS: org.jetbrains.android.inspections.klint.AndroidLintInspectionToolProvider$AndroidKLintSupportAnnotationUsageInspection // DEPENDENCY: IntRange.java -> android/support/annotation/IntRange.java +// DEPENDENCY: RequiresPermission.java -> android/support/annotation/RequiresPermission.java + import android.support.annotation.IntRange +import android.support.annotation.RequiresPermission +import android.Manifest import android.view.View const val constantVal = 0L @@ -13,4 +17,20 @@ fun invalidRange1a(): Int = 5 fun invalidRange0b(): Int = 5 @IntRange(from = 10, to = constantVal) -fun invalidRange1b(): Int = 5 \ No newline at end of file +fun invalidRange1b(): Int = 5 + + +// should be ok, KT-16600 +@RequiresPermission(anyOf = arrayOf(Manifest.permission.ACCESS_CHECKIN_PROPERTIES, + Manifest.permission.ACCESS_FINE_LOCATION)) +fun needsPermissions1() { } + +// should be ok, KT-16600 +@RequiresPermission(Manifest.permission.ACCESS_CHECKIN_PROPERTIES) +fun needsPermissions2() { } + +// error +@RequiresPermission( + value = Manifest.permission.ACCESS_CHECKIN_PROPERTIES, + anyOf = arrayOf(Manifest.permission.ACCESS_CHECKIN_PROPERTIES, Manifest.permission.ACCESS_FINE_LOCATION)) +fun needsPermissions3() { } \ No newline at end of file diff --git a/license/README.md b/license/README.md index 2947be9aa48..6eea52cdfc9 100644 --- a/license/README.md +++ b/license/README.md @@ -109,6 +109,10 @@ any distributions of the compiler, libraries or plugin: - Path: idea/testData/android/lint/IntRange.java - License: Apache 2 (license/third_party/aosp_license.txt) - Origin: Copyright (C) 2011-15 The Android Open Source Project + + - Path: idea/testData/android/lint/RequiresPermission.java + - License: Apache 2 (license/third_party/aosp_license.txt) + - Origin: Copyright (C) 2011-15 The Android Open Source Project - Path: libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/allOpenSpring/src/org/springframework/stereotype/Component.java - License: Apache 2 (license/third_party/testdata/spring_license.txt) diff --git a/plugins/uast-kotlin/src/org/jetbrains/uast/kotlin/declarations/KotlinUAnnotation.kt b/plugins/uast-kotlin/src/org/jetbrains/uast/kotlin/declarations/KotlinUAnnotation.kt index 55a8f9009f3..5fac24f2852 100644 --- a/plugins/uast-kotlin/src/org/jetbrains/uast/kotlin/declarations/KotlinUAnnotation.kt +++ b/plugins/uast-kotlin/src/org/jetbrains/uast/kotlin/declarations/KotlinUAnnotation.kt @@ -1,30 +1,39 @@ package org.jetbrains.uast.kotlin import com.intellij.psi.PsiClass -import com.intellij.psi.PsiElement +import org.jetbrains.kotlin.descriptors.annotations.AnnotationDescriptor import org.jetbrains.kotlin.psi.KtAnnotationEntry +import org.jetbrains.kotlin.psi.KtParameter import org.jetbrains.kotlin.resolve.BindingContext +import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall +import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall import org.jetbrains.kotlin.resolve.descriptorUtil.annotationClass +import org.jetbrains.kotlin.resolve.source.getPsi import org.jetbrains.uast.* class KotlinUAnnotation( override val psi: KtAnnotationEntry, override val uastParent: UElement? ) : UAnnotation { - private val resolvedAnnotation by lz { psi.analyze()[BindingContext.ANNOTATION, psi] } + private val resolvedAnnotation: AnnotationDescriptor? by lz { psi.analyze()[BindingContext.ANNOTATION, psi] } + + private val resolvedCall: ResolvedCall<*>? by lz { psi.getResolvedCall(psi.analyze()) } override val qualifiedName: String? get() = resolvedAnnotation?.fqName?.asString() - override val attributeValues by lz { - val context = getUastContext() - psi.valueArguments.map { arg -> - val name = arg.getArgumentName()?.asName?.asString() ?: "" - KotlinUNamedExpression(name, this).apply { - val value = arg.getArgumentExpression()?.let { context.convertElement(it, this) } as? UExpression - expression = value ?: UastEmptyExpression + override val attributeValues: List by lz { + resolvedCall?.valueArguments?.entries?.mapNotNull { + val arguments = it.value.arguments + val name = it.key.name.asString() + when { + arguments.size == 1 -> + KotlinUNamedExpression.create(name, arguments.first(), this) + arguments.size > 1 -> + KotlinUNamedExpression.create(name, arguments, this) + else -> null } - } + } ?: emptyList() } override fun resolve(): PsiClass? { @@ -32,20 +41,16 @@ class KotlinUAnnotation( return descriptor.toSource()?.getMaybeLightElement(this) as? PsiClass } - //TODO - override fun findAttributeValue(name: String?) = findDeclaredAttributeValue(name) + override fun findAttributeValue(name: String?): UExpression? = + findDeclaredAttributeValue(name) override fun findDeclaredAttributeValue(name: String?): UExpression? { - return attributeValues.firstOrNull { it.name == (name ?: "value") }?.expression + return attributeValues.find { + it.name == name || + (name == null && it.name == "value") || + (name == "value" && it.name == null) + }?.expression } + } -class KotlinUNamedExpression(override val name: String, override val uastParent: UElement?) : UNamedExpression { - override lateinit var expression: UExpression - - override val annotations: List - get() = emptyList() - - override val psi: PsiElement? - get() = null -} diff --git a/plugins/uast-kotlin/src/org/jetbrains/uast/kotlin/expressions/KotlinUFunctionCallExpression.kt b/plugins/uast-kotlin/src/org/jetbrains/uast/kotlin/expressions/KotlinUFunctionCallExpression.kt index f9190e5a682..9737cd4eca7 100644 --- a/plugins/uast-kotlin/src/org/jetbrains/uast/kotlin/expressions/KotlinUFunctionCallExpression.kt +++ b/plugins/uast-kotlin/src/org/jetbrains/uast/kotlin/expressions/KotlinUFunctionCallExpression.kt @@ -23,9 +23,9 @@ import org.jetbrains.kotlin.asJava.LightClassUtil import org.jetbrains.kotlin.asJava.toLightClass import org.jetbrains.kotlin.descriptors.ConstructorDescriptor import org.jetbrains.kotlin.descriptors.DeclarationDescriptor -import org.jetbrains.kotlin.psi.KtCallExpression -import org.jetbrains.kotlin.psi.KtClassOrObject -import org.jetbrains.kotlin.psi.KtFunction +import org.jetbrains.kotlin.psi.* +import org.jetbrains.kotlin.psi.psiUtil.parents +import org.jetbrains.kotlin.resolve.CompileTimeConstantUtils import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall import org.jetbrains.uast.* @@ -89,9 +89,11 @@ class KotlinUFunctionCallExpression( override val returnType: PsiType? get() = getExpressionType() - override val kind by lz { - when (resolvedCall?.resultingDescriptor) { - is ConstructorDescriptor -> UastCallKind.CONSTRUCTOR_CALL + override val kind: UastCallKind by lz { + val resolvedCall = resolvedCall ?: return@lz UastCallKind.METHOD_CALL + when { + resolvedCall.resultingDescriptor is ConstructorDescriptor -> UastCallKind.CONSTRUCTOR_CALL + this.isAnnotationArgumentArrayInitializer() -> UastCallKind.NESTED_ARRAY_INITIALIZER else -> UastCallKind.METHOD_CALL } } @@ -118,4 +120,10 @@ class KotlinUFunctionCallExpression( visitor.afterVisitCallExpression(this) } + + private fun isAnnotationArgumentArrayInitializer(): Boolean { + val resolvedCall = resolvedCall ?: return false + // KtAnnotationEntry -> KtValueArgumentList -> KtValueArgument -> arrayOf call + return psi.parents.elementAtOrNull(2) is KtAnnotationEntry && CompileTimeConstantUtils.isArrayFunctionCall(resolvedCall) + } } \ No newline at end of file diff --git a/plugins/uast-kotlin/src/org/jetbrains/uast/kotlin/expressions/KotlinUNamedExpression.kt b/plugins/uast-kotlin/src/org/jetbrains/uast/kotlin/expressions/KotlinUNamedExpression.kt new file mode 100644 index 00000000000..1e80dfcf2c1 --- /dev/null +++ b/plugins/uast-kotlin/src/org/jetbrains/uast/kotlin/expressions/KotlinUNamedExpression.kt @@ -0,0 +1,96 @@ +/* + * Copyright 2010-2017 JetBrains s.r.o. + * + * 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 org.jetbrains.uast.kotlin + +import com.intellij.psi.PsiElement +import com.intellij.psi.PsiType +import org.jetbrains.kotlin.psi.ValueArgument +import org.jetbrains.uast.* + +class KotlinUNamedExpression private constructor( + override val name: String?, + override val uastParent: UElement?, + expressionProducer: (UElement) -> UExpression +) : UNamedExpression { + override val expression: UExpression by lz { expressionProducer(this) } + + override val annotations: List = emptyList() + + override val psi: PsiElement? = null + + companion object { + internal fun create(name: String?, valueArgument: ValueArgument, uastParent: UElement?): UNamedExpression { + val expression = valueArgument.getArgumentExpression() + return KotlinUNamedExpression(name, uastParent) { expressionParent -> + expression?.let { expressionParent.getLanguagePlugin().convert(it, expressionParent) } ?: UastEmptyExpression + } + } + + internal fun create( + name: String?, + valueArguments: List, + uastParent: UElement?): UNamedExpression { + return KotlinUNamedExpression(name, uastParent) { expressionParent -> + object : KotlinAbstractUExpression(), UCallExpression { + override val uastParent: UElement? = expressionParent + + override val kind: UastCallKind = UastCallKind.NESTED_ARRAY_INITIALIZER + + override val valueArguments: List by lz { + valueArguments.map { + it.getArgumentExpression()?.let { argumentExpression -> + getLanguagePlugin().convert(argumentExpression, this) + } ?: UastEmptyExpression + } + } + + override val valueArgumentCount: Int + get() = valueArguments.size + + override val psi: PsiElement? + get() = null + + override val methodIdentifier: UIdentifier? + get() = null + + override val classReference: UReferenceExpression? + get() = null + + override val methodName: String? + get() = null + + override val typeArgumentCount: Int + get() = 0 + + override val typeArguments: List + get() = emptyList() + + override val returnType: PsiType? + get() = null + + override fun resolve() = null + + override val receiver: UExpression? + get() = null + + override val receiverType: PsiType? + get() = null + } + } + } + } +} \ No newline at end of file diff --git a/plugins/uast-kotlin/testData/AnnotationParameters.kt b/plugins/uast-kotlin/testData/AnnotationParameters.kt index 1b9e57b5bbc..9d3cb1cea30 100644 --- a/plugins/uast-kotlin/testData/AnnotationParameters.kt +++ b/plugins/uast-kotlin/testData/AnnotationParameters.kt @@ -1,5 +1,18 @@ annotation class IntRange(val from: Long, val to: Long) +annotation class RequiresPermission(val anyOf: IntArray) + +annotation class WithDefaultValue(val value: Int = 42) + +annotation class SuppressLint(vararg val value: String) + +@RequiresPermission(anyOf = intArrayOf(1, 2, 3)) @IntRange(from = 10, to = 0) -fun foo(): Int = 5 \ No newline at end of file +@WithDefaultValue +@SuppressLint("Lorem") +fun foo(): Int = 5 + +@IntRange(0, 100) +@SuppressLint("Lorem", "Ipsum", "Dolor") +fun bar() = Unit \ No newline at end of file diff --git a/plugins/uast-kotlin/testData/AnnotationParameters.log.txt b/plugins/uast-kotlin/testData/AnnotationParameters.log.txt index 54e2d24c1cd..e786df2eb65 100644 --- a/plugins/uast-kotlin/testData/AnnotationParameters.log.txt +++ b/plugins/uast-kotlin/testData/AnnotationParameters.log.txt @@ -1,12 +1,44 @@ UFile (package = ) UClass (name = AnnotationParametersKt) UAnnotationMethod (name = foo) + UAnnotation (fqName = RequiresPermission) + UNamedExpression (name = anyOf) + UCallExpression (kind = UastCallKind(name='array_initializer'), argCount = 3)) + UIdentifier (Identifier (intArrayOf)) + USimpleNameReferenceExpression (identifier = intArrayOf) + ULiteralExpression (value = 1) + ULiteralExpression (value = 2) + ULiteralExpression (value = 3) UAnnotation (fqName = IntRange) UNamedExpression (name = from) ULiteralExpression (value = 10) UNamedExpression (name = to) ULiteralExpression (value = 0) + UAnnotation (fqName = WithDefaultValue) + UAnnotation (fqName = SuppressLint) + UNamedExpression (name = value) + ULiteralExpression (value = "Lorem") ULiteralExpression (value = 5) + UAnnotationMethod (name = bar) + UAnnotation (fqName = IntRange) + UNamedExpression (name = from) + ULiteralExpression (value = 0) + UNamedExpression (name = to) + ULiteralExpression (value = 100) + UAnnotation (fqName = SuppressLint) + UNamedExpression (name = value) + UCallExpression (kind = UastCallKind(name='array_initializer'), argCount = 3)) + ULiteralExpression (value = "Lorem") + ULiteralExpression (value = "Ipsum") + ULiteralExpression (value = "Dolor") + USimpleNameReferenceExpression (identifier = Unit) UClass (name = IntRange) UAnnotationMethod (name = from) UAnnotationMethod (name = to) + UClass (name = RequiresPermission) + UAnnotationMethod (name = anyOf) + UClass (name = WithDefaultValue) + UAnnotationMethod (name = value) + ULiteralExpression (value = 42) + UClass (name = SuppressLint) + UAnnotationMethod (name = value) diff --git a/plugins/uast-kotlin/testData/AnnotationParameters.render.txt b/plugins/uast-kotlin/testData/AnnotationParameters.render.txt index 4803ace7f95..69cffc86a7e 100644 --- a/plugins/uast-kotlin/testData/AnnotationParameters.render.txt +++ b/plugins/uast-kotlin/testData/AnnotationParameters.render.txt @@ -1,9 +1,27 @@ public final class AnnotationParametersKt { + @RequiresPermission(anyOf = intArrayOf(1, 2, 3)) @IntRange(from = 10, to = 0) + @WithDefaultValue + @SuppressLint(value = "Lorem") public static final fun foo() : int = 5 + @IntRange(from = 0, to = 100) + @SuppressLint(value = ("Lorem", "Ipsum", "Dolor")) + public static final fun bar() : void = Unit } public abstract annotation IntRange { public abstract fun from() : long = UastEmptyExpression public abstract fun to() : long = UastEmptyExpression } + +public abstract annotation RequiresPermission { + public abstract fun anyOf() : int[] = UastEmptyExpression +} + +public abstract annotation WithDefaultValue { + public abstract fun value() : int = UastEmptyExpression +} + +public abstract annotation SuppressLint { + public abstract fun value() : java.lang.String[] = UastEmptyExpression +}