From a59edc4e4a824c61d6c6dde4162d4d595092db5c Mon Sep 17 00:00:00 2001 From: "Anna.Kozlova" Date: Tue, 27 Dec 2016 13:58:03 +0100 Subject: [PATCH] accessibility check for annotation parameters: treat parameters as they are outside the class (IDEA-165904) --- .../analysis/AnnotationsHighlightUtil.java | 49 ------------------- .../impl/analysis/HighlightVisitorImpl.java | 1 - .../impl/source/resolve/JavaResolveUtil.java | 25 ++++++++-- ...dUsedInAnnotationParameterOfInheritor.java | 4 ++ .../ClassObjectAccessibility.java | 2 +- ...ightInaccessibleFromClassModifierList.java | 2 +- .../privateInaccessibleConstant.java | 2 +- .../LightAdvHighlightingFixtureTest.java | 6 +++ 8 files changed, 34 insertions(+), 57 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advFixture/ProtectedFieldUsedInAnnotationParameterOfInheritor.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/AnnotationsHighlightUtil.java b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/AnnotationsHighlightUtil.java index 021668ee1225..6579e63f1521 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/AnnotationsHighlightUtil.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/AnnotationsHighlightUtil.java @@ -445,55 +445,6 @@ public class AnnotationsHighlightUtil { return ref; } - static HighlightInfo checkForeignInnerClassesUsed(final PsiAnnotation annotation) { - final HighlightInfo[] infos = new HighlightInfo[1]; - final PsiAnnotationOwner owner = annotation.getOwner(); - if (owner instanceof PsiModifierList) { - final PsiElement parent = ((PsiModifierList)owner).getParent(); - if (parent instanceof PsiClass) { - annotation.accept(new JavaRecursiveElementWalkingVisitor() { - @Override - public void visitElement(PsiElement element) { - if (infos[0] != null) return; - super.visitElement(element); - } - - @Override - public void visitClassObjectAccessExpression(PsiClassObjectAccessExpression expression) { - super.visitClassObjectAccessExpression(expression); - final PsiTypeElement operand = expression.getOperand(); - final PsiClass classType = PsiUtil.resolveClassInType(operand.getType()); - if (classType != null) { - checkAccessibility(operand.getInnermostComponentReferenceElement(), classType, HighlightUtil.formatClass(classType)); - } - } - - @Override - public void visitReferenceExpression(PsiReferenceExpression expression) { - super.visitReferenceExpression(expression); - final PsiElement resolve = expression.resolve(); - if (resolve instanceof PsiField) { - checkAccessibility(expression, (PsiMember)resolve, HighlightUtil.formatField((PsiField)resolve)); - } - } - - private void checkAccessibility(PsiJavaCodeReferenceElement expression, PsiMember resolve, String memberString) { - if (resolve.hasModifierProperty(PsiModifier.PRIVATE) && - PsiTreeUtil.isAncestor(parent, resolve, true)) { - String description = JavaErrorMessages.message("private.symbol", - memberString, - HighlightUtil.formatClass((PsiClass)parent)); - infos[0] = - HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR).range(expression).descriptionAndTooltip(description).create(); - HighlightUtil.registerAccessQuickFixAction(resolve, expression, infos[0], null); - } - } - }); - } - } - return infos[0]; - } - @Nullable static HighlightInfo checkAnnotationType(PsiAnnotation annotation) { PsiJavaCodeReferenceElement nameReferenceElement = annotation.getNameReferenceElement(); diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java index 294505c27770..72dfedbbf09e 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java @@ -241,7 +241,6 @@ public class HighlightVisitorImpl extends JavaElementVisitor implements Highligh if (!myHolder.hasErrorResults()) myHolder.add(AnnotationsHighlightUtil.checkMissingAttributes(annotation)); if (!myHolder.hasErrorResults()) myHolder.add(AnnotationsHighlightUtil.checkTargetAnnotationDuplicates(annotation)); if (!myHolder.hasErrorResults()) myHolder.add(AnnotationsHighlightUtil.checkDuplicateAnnotations(annotation, myLanguageLevel)); - if (!myHolder.hasErrorResults()) myHolder.add(AnnotationsHighlightUtil.checkForeignInnerClassesUsed(annotation)); if (!myHolder.hasErrorResults()) myHolder.add(AnnotationsHighlightUtil.checkFunctionalInterface(annotation, myLanguageLevel)); if (!myHolder.hasErrorResults()) myHolder.add(AnnotationsHighlightUtil.checkRepeatableAnnotation(annotation)); if (CommonClassNames.JAVA_LANG_OVERRIDE.equals(annotation.getQualifiedName())) { diff --git a/java/java-psi-impl/src/com/intellij/psi/impl/source/resolve/JavaResolveUtil.java b/java/java-psi-impl/src/com/intellij/psi/impl/source/resolve/JavaResolveUtil.java index e97b5d6312a4..f9c4db15693e 100644 --- a/java/java-psi-impl/src/com/intellij/psi/impl/source/resolve/JavaResolveUtil.java +++ b/java/java-psi-impl/src/com/intellij/psi/impl/source/resolve/JavaResolveUtil.java @@ -122,9 +122,15 @@ public class JavaResolveUtil { if (memberClass == null) { return false; } - // if resolving supertype reference, skip its containing class with getContextClass - PsiClass contextClass = member instanceof PsiClass ? getContextClass(place) - : PsiTreeUtil.getContextOfType(place, PsiClass.class, false); + PsiClass contextClass; + if (member instanceof PsiClass) { + // if resolving supertype reference, skip its containing class with getContextClass + contextClass = getContextClass(place); + } + else { + contextClass = PsiTreeUtil.getContextOfType(place, PsiClass.class, false); + if (isInClassAnnotationParameterList(place, contextClass)) return false; + } while (contextClass != null) { if (InheritanceUtil.isInheritorOrSelf(contextClass, memberClass, true)) { if (member instanceof PsiClass || @@ -156,7 +162,8 @@ public class JavaResolveUtil { if (fileResolveScope == null) { PsiClass placeTopLevelClass = getTopLevelClass(place, null); PsiClass memberTopLevelClass = getTopLevelClass(memberClass, null); - return manager.areElementsEquivalent(placeTopLevelClass, memberTopLevelClass); + return manager.areElementsEquivalent(placeTopLevelClass, memberTopLevelClass) && + !isInClassAnnotationParameterList(place, PsiTreeUtil.getContextOfType(place, PsiClass.class, false)); } else { return fileResolveScope instanceof PsiClass && @@ -186,6 +193,16 @@ public class JavaResolveUtil { return true; } + private static boolean isInClassAnnotationParameterList(@NotNull PsiElement place, @Nullable PsiClass contextClass) { + if (contextClass != null) { + PsiAnnotation annotation = PsiTreeUtil.getContextOfType(place, PsiAnnotation.class, true); + if (annotation != null && contextClass.getModifierList() == annotation.getOwner()) { + return true; + } + } + return false; + } + private static boolean ignoreReferencedElementAccessibility(PsiFile placeFile) { return placeFile instanceof FileResolveScopeProvider && ((FileResolveScopeProvider) placeFile).ignoreReferencedElementAccessibility() && diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advFixture/ProtectedFieldUsedInAnnotationParameterOfInheritor.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advFixture/ProtectedFieldUsedInAnnotationParameterOfInheritor.java new file mode 100644 index 000000000000..f39e09c76e0f --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advFixture/ProtectedFieldUsedInAnnotationParameterOfInheritor.java @@ -0,0 +1,4 @@ +import a.A; + +@SuppressWarnings(A.A_FOO) +class B extends A {} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting6/ClassObjectAccessibility.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting6/ClassObjectAccessibility.java index 929b107d53f6..5dda31b8b426 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting6/ClassObjectAccessibility.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting6/ClassObjectAccessibility.java @@ -1,4 +1,4 @@ -@SomeAnnotation(Foo.Bar.class) +@SomeAnnotation(Foo.Bar.class) public class Foo{ private static class Bar { } diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting7/HighlightInaccessibleFromClassModifierList.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting7/HighlightInaccessibleFromClassModifierList.java index e02db8fb902f..974d6ee9c6e6 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting7/HighlightInaccessibleFromClassModifierList.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting7/HighlightInaccessibleFromClassModifierList.java @@ -1,4 +1,4 @@ -@SuppressWarnings(ThisClass.FOO) +@SuppressWarnings(ThisClass.FOO) public class ThisClass { private static final String FOO = "foo"; } \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/annotations/privateInaccessibleConstant.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/annotations/privateInaccessibleConstant.java index 435810d00b44..ef49b407ae26 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/annotations/privateInaccessibleConstant.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/annotations/privateInaccessibleConstant.java @@ -1,4 +1,4 @@ -@FooAnnotation(Foo.BAR) +@FooAnnotation(Foo.BAR) class Foo { private static final String BAR = "bar"; } diff --git a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/LightAdvHighlightingFixtureTest.java b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/LightAdvHighlightingFixtureTest.java index 5b3ecddbe758..07d0cd173feb 100644 --- a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/LightAdvHighlightingFixtureTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/LightAdvHighlightingFixtureTest.java @@ -99,6 +99,12 @@ public class LightAdvHighlightingFixtureTest extends LightCodeInsightFixtureTest myFixture.checkHighlighting(); } + public void testProtectedFieldUsedInAnnotationParameterOfInheritor() throws Exception { + myFixture.addClass("package a; public class A {protected final static String A_FOO = \"A\";}"); + myFixture.configureByFile(getTestName(false) + ".java"); + myFixture.checkHighlighting(); + } + @Override protected String getBasePath() { return JavaTestUtil.getRelativeJavaTestDataPath() + "/codeInsight/daemonCodeAnalyzer/advFixture";