From 29b9521596d92b1888e5c15455a14caec95f2de3 Mon Sep 17 00:00:00 2001 From: Vyacheslav Gerasimov Date: Wed, 26 Oct 2016 13:05:57 +0300 Subject: [PATCH] Android lint check for api requirements for classes in catch clause (KT-13243) Added test for #KT-14047, #KT-13243 Fixed, #KT-14047 Fixed --- .../tools/klint/checks/ApiDetector.java | 99 ++++++++++--------- .../klint/IntellijApiDetector.java | 80 +++++---------- .../AbstractKotlinLintTest.kt | 3 +- plugins/uast-kotlin/testData/lint/apiCheck.kt | 18 +++- 4 files changed, 95 insertions(+), 105 deletions(-) diff --git a/plugins/lint/lint-checks/src/com/android/tools/klint/checks/ApiDetector.java b/plugins/lint/lint-checks/src/com/android/tools/klint/checks/ApiDetector.java index a1e01c80fa2..7ed9b6ca038 100644 --- a/plugins/lint/lint-checks/src/com/android/tools/klint/checks/ApiDetector.java +++ b/plugins/lint/lint-checks/src/com/android/tools/klint/checks/ApiDetector.java @@ -2023,7 +2023,7 @@ public class ApiDetector extends ResourceXmlDetector public boolean visitTryExpression(UTryExpression statement) { List resourceList = statement.getResources(); //noinspection VariableNotUsedInsideIf - if (!resourceList.isEmpty()) { + if (resourceList != null && !resourceList.isEmpty()) { int api = 19; // minSdk for try with resources int minSdk = getMinSdk(mContext); @@ -2061,53 +2061,6 @@ public class ApiDetector extends ResourceXmlDetector return super.visitTryExpression(statement); } - private Location getCatchParametersLocation(JavaContext context, UCatchClause catchClause) { - List types = catchClause.getTypeReferences(); - if (types.isEmpty()) { - return Location.NONE; - } - - Location first = context.getUastLocation(types.get(0)); - if (types.size() < 2) { - return first; - } - - Location last = context.getUastLocation(types.get(types.size() - 1)); - File file = first.getFile(); - Position start = first.getStart(); - Position end = last.getEnd(); - - if (start == null) { - return Location.create(file); - } - - return Location.create(file, start, end); - } - - private boolean isMultiCatchReflectiveOperationException(UCatchClause catchClause) { - List types = catchClause.getTypes(); - if (types.size() < 2) { - return false; - } - - for (PsiType t : types) { - if(!isSubclassOfReflectiveOperationException(t)) { - return false; - } - } - - return true; - } - - private boolean isSubclassOfReflectiveOperationException(PsiType type) { - for (PsiType t : type.getSuperTypes()) { - if (REFLECTIVE_OPERATION_EXCEPTION.equals(t.getCanonicalText())) { - return true; - } - } - return false; - } - private void checkCatchTypeElement(@NonNull UTryExpression statement, @NonNull UTypeReferenceExpression typeReference, @Nullable PsiType type) { @@ -2236,7 +2189,7 @@ public class ApiDetector extends ResourceXmlDetector return false; } - private static int getTargetApi(@Nullable UElement scope) { + public static int getTargetApi(@Nullable UElement scope) { while (scope != null) { if (scope instanceof PsiModifierListOwner) { PsiModifierList modifierList = ((PsiModifierListOwner) scope).getModifierList(); @@ -2737,6 +2690,45 @@ public class ApiDetector extends ResourceXmlDetector return null; } + + public static Location getCatchParametersLocation(JavaContext context, UCatchClause catchClause) { + List types = catchClause.getTypeReferences(); + if (types.isEmpty()) { + return Location.NONE; + } + + Location first = context.getUastLocation(types.get(0)); + if (types.size() < 2) { + return first; + } + + Location last = context.getUastLocation(types.get(types.size() - 1)); + File file = first.getFile(); + Position start = first.getStart(); + Position end = last.getEnd(); + + if (start == null) { + return Location.create(file); + } + + return Location.create(file, start, end); + } + + public static boolean isMultiCatchReflectiveOperationException(UCatchClause catchClause) { + List types = catchClause.getTypes(); + if (types.size() < 2) { + return false; + } + + for (PsiType t : types) { + if(!isSubclassOfReflectiveOperationException(t)) { + return false; + } + } + + return true; + } + private static boolean isAndedWithConditional(UElement element, int api, @Nullable UElement before) { if (element instanceof UBinaryExpression) { UBinaryExpression inner = (UBinaryExpression) element; @@ -2782,4 +2774,13 @@ public class ApiDetector extends ResourceXmlDetector return false; } + + private static boolean isSubclassOfReflectiveOperationException(PsiType type) { + for (PsiType t : type.getSuperTypes()) { + if (REFLECTIVE_OPERATION_EXCEPTION.equals(t.getCanonicalText())) { + return true; + } + } + return false; + } } diff --git a/plugins/lint/lint-idea/src/org/jetbrains/android/inspections/klint/IntellijApiDetector.java b/plugins/lint/lint-idea/src/org/jetbrains/android/inspections/klint/IntellijApiDetector.java index 8a1201258c4..67ad9ed7a1c 100644 --- a/plugins/lint/lint-idea/src/org/jetbrains/android/inspections/klint/IntellijApiDetector.java +++ b/plugins/lint/lint-idea/src/org/jetbrains/android/inspections/klint/IntellijApiDetector.java @@ -29,6 +29,7 @@ import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; import org.jetbrains.uast.*; import org.jetbrains.uast.expressions.UInstanceExpression; +import org.jetbrains.uast.expressions.UTypeReferenceExpression; import org.jetbrains.uast.util.UastExpressionUtils; import org.jetbrains.uast.visitor.AbstractUastVisitor; import org.jetbrains.uast.visitor.UastVisitor; @@ -419,11 +420,22 @@ public class IntellijApiDetector extends ApiDetector { } for (UCatchClause catchClause : statement.getCatchClauses()) { - for (PsiParameter parameter : catchClause.getParameters()) { - PsiTypeElement typeElement = parameter.getTypeElement(); - if (typeElement != null) { - checkCatchTypeElement(statement, typeElement, typeElement.getType()); - } + + // Special case reflective operation exception which can be implicitly used + // with multi-catches: see issue 153406 + int minSdk = getMinSdk(myContext); + if(minSdk < 19 && isMultiCatchReflectiveOperationException(catchClause)) { + String message = String.format("Multi-catch with these reflection exceptions requires API level 19 (current min is %d) " + + "because they get compiled to the common but new super type `ReflectiveOperationException`. " + + "As a workaround either create individual catch statements, or catch `Exception`.", + minSdk); + + myContext.report(UNSUPPORTED, getCatchParametersLocation(myContext, catchClause), message); + continue; + } + + for (UTypeReferenceExpression typeReference : catchClause.getTypeReferences()) { + checkCatchTypeElement(statement, typeReference, typeReference.getType()); } } @@ -431,36 +443,14 @@ public class IntellijApiDetector extends ApiDetector { } private void checkCatchTypeElement(@NonNull UTryExpression statement, - @NotNull PsiTypeElement typeElement, + @NonNull UTypeReferenceExpression typeReference, @Nullable PsiType type) { PsiClass resolved = null; - if (type instanceof PsiDisjunctionType) { - PsiDisjunctionType disjunctionType = (PsiDisjunctionType)type; - type = disjunctionType.getLeastUpperBound(); - if (type instanceof PsiClassType) { - resolved = ((PsiClassType)type).resolve(); - } - for (PsiElement child : typeElement.getChildren()) { - if (child instanceof PsiTypeElement) { - PsiTypeElement childTypeElement = (PsiTypeElement)child; - PsiType childType = childTypeElement.getType(); - if (!type.equals(childType)) { - checkCatchTypeElement(statement, childTypeElement, childType); - } - } - } - } else if (type instanceof PsiClassReferenceType) { - PsiClassReferenceType referenceType = (PsiClassReferenceType)type; - resolved = referenceType.resolve(); - } else if (type instanceof PsiClassType) { - resolved = ((PsiClassType)type).resolve(); + if (type instanceof PsiClassType) { + resolved = ((PsiClassType) type).resolve(); } if (resolved != null) { - String signature = IntellijLintUtils.getInternalName(resolved); - if (signature == null) { - return; - } - + String signature = myContext.getEvaluator().getInternalName(resolved); int api = mApiDatabase.getClassVersion(signature); if (api == -1) { return; @@ -469,31 +459,15 @@ public class IntellijApiDetector extends ApiDetector { if (api <= minSdk) { return; } - if (mySeenTargetApi) { - int target = getTargetApi(statement, myFile); - if (target != -1) { - if (api <= target) { - return; - } - } - } - if (mySeenSuppress && IntellijLintUtils.isSuppressed(statement, myFile, UNSUPPORTED)) { + int target = getTargetApi(statement); + if (target != -1 && api <= target) { return; } - Location location; - location = IntellijLintUtils.getLocation(myContext.file, typeElement); - String fqcn = resolved.getName(); - String message = String.format("Class requires API level %1$d (current min is %2$d): %3$s", api, minSdk, fqcn); - - // Special case reflective operation exception which can be implicitly used - // with multi-catches: see issue 153406 - if (api == 19 && "ReflectiveOperationException".equals(fqcn)) { - message = String.format("Multi-catch with these reflection exceptions requires API level 19 (current min is %2$d) " + - "because they get compiled to the common but new super type `ReflectiveOperationException`. " + - "As a workaround either create individual catch statements, or catch `Exception`.", - api, minSdk); - } + Location location = myContext.getUastLocation(typeReference); + String fqcn = resolved.getQualifiedName(); + String message = String.format("Class requires API level %1$d (current min is %2$d): %3$s", + api, minSdk, fqcn); myContext.report(UNSUPPORTED, location, message); } } diff --git a/plugins/uast-kotlin/test/org.jetbrains.kotlin.uast/AbstractKotlinLintTest.kt b/plugins/uast-kotlin/test/org.jetbrains.kotlin.uast/AbstractKotlinLintTest.kt index 25750cb366a..6ee5605ce10 100644 --- a/plugins/uast-kotlin/test/org.jetbrains.kotlin.uast/AbstractKotlinLintTest.kt +++ b/plugins/uast-kotlin/test/org.jetbrains.kotlin.uast/AbstractKotlinLintTest.kt @@ -63,10 +63,9 @@ abstract class AbstractKotlinLintTest : KotlinAndroidTestCase() { } } - val virtualFile = myFixture.copyFileToProject(ktFile.absolutePath, "src/" + getTestName(true) + ".kt"); + val virtualFile = myFixture.copyFileToProject(ktFile.absolutePath, "src/" + getTestName(true) + ".kt") myFixture.configureFromExistingVirtualFile(virtualFile) - myFixture.doHighlighting() myFixture.checkHighlighting(true, false, false) } diff --git a/plugins/uast-kotlin/testData/lint/apiCheck.kt b/plugins/uast-kotlin/testData/lint/apiCheck.kt index 65017f80a28..17420c25ac8 100644 --- a/plugins/uast-kotlin/testData/lint/apiCheck.kt +++ b/plugins/uast-kotlin/testData/lint/apiCheck.kt @@ -1,4 +1,5 @@ -// INSPECTION_CLASS: org.jetbrains.android.inspections.klint.AndroidLintInspectionToolProvider$AndroidKLintNewApiInspection + +// INSPECTION_CLASS: org.jetbrains.android.inspections.klint.AndroidLintInspectionToolProvider$AndroidKLintNewApiInspection // INSPECTION_CLASS2: org.jetbrains.android.inspections.klint.AndroidLintInspectionToolProvider$AndroidKLintInlinedApiInspection // INSPECTION_CLASS3: org.jetbrains.android.inspections.klint.AndroidLintInspectionToolProvider$AndroidKLintOverrideInspection @@ -13,6 +14,7 @@ import android.view.ViewGroup import android.view.ViewGroup.LayoutParams import android.app.Activity import android.app.ApplicationErrorReport +import android.graphics.Path import android.graphics.PorterDuff import android.graphics.Rect import android.os.Build @@ -25,6 +27,7 @@ import android.os.Build.VERSION_CODES import android.os.Build.VERSION_CODES.ICE_CREAM_SANDWICH import android.os.Build.VERSION_CODES.JELLY_BEAN import android.os.Bundle +import android.system.ErrnoException import android.widget.TextView @Suppress("SENSELESS_COMPARISON", "UNUSED_EXPRESSION", "UsePropertyAccessSyntax", "UNUSED_VARIABLE", "unused", "UNUSED_PARAMETER", "DEPRECATION") @@ -275,6 +278,19 @@ class ApiCallTest: Activity() { } } + fun testCatch() { + try { + + } catch (e: ErrnoException) { + + } + } + + fun testOverload() { + // this overloaded addOval available only on API Level 21 + Path().addOval(0f, 0f, 0f, 0f, Path.Direction.CW) + } + // Return type internal // API 14 val gridLayout: GridLayout?