From 346386316d62318a108f4cfd44edf0888892672d Mon Sep 17 00:00:00 2001 From: Alexey Kudravtsev Date: Fri, 19 Feb 2016 13:34:17 +0300 Subject: [PATCH] IDEA-151875 "Access can be private" suggested for abstract method --- .../AccessCanBeTightenedInspection.java | 66 ++++++++----------- .../AccessCanBeTightenedInspectionTest.java | 11 ++++ 2 files changed, 38 insertions(+), 39 deletions(-) diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/visibility/AccessCanBeTightenedInspection.java b/java/java-analysis-impl/src/com/intellij/codeInspection/visibility/AccessCanBeTightenedInspection.java index d89c92bd8b77..38084ab8ac94 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/visibility/AccessCanBeTightenedInspection.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/visibility/AccessCanBeTightenedInspection.java @@ -24,14 +24,11 @@ import com.intellij.codeInspection.deadCode.UnusedDeclarationInspectionBase; import com.intellij.openapi.progress.EmptyProgressIndicator; import com.intellij.openapi.project.Project; import com.intellij.openapi.util.Comparing; -import com.intellij.openapi.util.Condition; import com.intellij.profile.codeInspection.InspectionProjectProfileManager; import com.intellij.psi.*; import com.intellij.psi.util.ClassUtil; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.PsiUtil; -import com.intellij.usageView.UsageInfo; -import com.intellij.util.Processor; import com.intellij.util.VisibilityUtil; import com.intellij.util.containers.ContainerUtil; import com.siyeh.ig.fixes.ChangeModifierFix; @@ -149,37 +146,31 @@ class AccessCanBeTightenedInspection extends BaseJavaBatchLocalInspectionTool { final PsiPackage memberPackage = memberDirectory == null ? null : JavaDirectoryService.getInstance().getPackage(memberDirectory); log(member.getName()+ ": checking effective level for "+member); boolean result = - UnusedSymbolUtil.processUsages(project, memberFile, member, new EmptyProgressIndicator(), null, new Processor() { - @Override - public boolean process(UsageInfo info) { - foundUsage.set(true); - PsiFile psiFile = info.getFile(); - if (psiFile == null) return true; - if (!(psiFile instanceof PsiJavaFile)) { - log(" refd from " + psiFile.getName() + "; set to public"); - maxLevel.set(PsiUtil.ACCESS_LEVEL_PUBLIC); - if (memberClass != null) { - childMembersAreUsedOutsideMyPackage.add(memberClass); - } - return false; // referenced from XML, has to be public - } - //int offset = info.getNavigationOffset(); - //if (offset == -1) return true; - PsiElement element = info.getElement(); - if (element == null) return true; - @PsiUtil.AccessLevel - int level = getEffectiveLevel(element, psiFile, member, memberFile, memberClass, memberPackage); - log(" ref in file " + psiFile.getName() + "; level = " + PsiUtil.getAccessModifier(level) + "; (" + element + ")"); - while (true) { - int oldLevel = maxLevel.get(); - if (level <= oldLevel || maxLevel.compareAndSet(oldLevel, level)) break; - } - if (level == PsiUtil.ACCESS_LEVEL_PUBLIC && memberClass != null) { + UnusedSymbolUtil.processUsages(project, memberFile, member, new EmptyProgressIndicator(), null, info -> { + foundUsage.set(true); + PsiFile psiFile = info.getFile(); + if (psiFile == null) return true; + if (!(psiFile instanceof PsiJavaFile)) { + log(" refd from " + psiFile.getName() + "; set to public"); + maxLevel.set(PsiUtil.ACCESS_LEVEL_PUBLIC); + if (memberClass != null) { childMembersAreUsedOutsideMyPackage.add(memberClass); } - - return level != PsiUtil.ACCESS_LEVEL_PUBLIC; + return false; // referenced from XML, has to be public } + //int offset = info.getNavigationOffset(); + //if (offset == -1) return true; + PsiElement element = info.getElement(); + if (element == null) return true; + @PsiUtil.AccessLevel + int level = getEffectiveLevel(element, psiFile, member, memberFile, memberClass, memberPackage); + log(" ref in file " + psiFile.getName() + "; level = " + PsiUtil.getAccessModifier(level) + "; (" + element + ")"); + maxLevel.getAndAccumulate(level, Math::max); + if (level == PsiUtil.ACCESS_LEVEL_PUBLIC && memberClass != null) { + childMembersAreUsedOutsideMyPackage.add(memberClass); + } + + return level != PsiUtil.ACCESS_LEVEL_PUBLIC; }); if (!foundUsage.get()) { @@ -201,12 +192,8 @@ class AccessCanBeTightenedInspection extends BaseJavaBatchLocalInspectionTool { return; // e.g. some public method is used outside my package (without importing class) } PsiElement toHighlight = currentLevel == PsiUtil.ACCESS_LEVEL_PACKAGE_LOCAL ? ((PsiNameIdentifierOwner)member).getNameIdentifier() : ContainerUtil.find( - memberModifierList.getChildren(), new Condition() { - @Override - public boolean value(PsiElement element) { - return element instanceof PsiKeyword && element.getText().equals(PsiUtil.getAccessModifier(currentLevel)); - } - }); + memberModifierList.getChildren(), + element -> element instanceof PsiKeyword && element.getText().equals(PsiUtil.getAccessModifier(currentLevel))); assert toHighlight != null : member +" ; " + ((PsiNameIdentifierOwner)member).getNameIdentifier() + "; "+ memberModifierList.getText(); myHolder.registerProblem(toHighlight, "Access can be " + VisibilityUtil.toPresentableText(maxModifier), new ChangeModifierFix(maxModifier)); } @@ -220,6 +207,7 @@ class AccessCanBeTightenedInspection extends BaseJavaBatchLocalInspectionTool { PsiClass memberClass, PsiPackage memberPackage) { PsiClass aClass = PsiTreeUtil.getParentOfType(element, PsiClass.class); + boolean isAbstractMember = member.hasModifierProperty(PsiModifier.ABSTRACT); if (memberClass != null && PsiTreeUtil.isAncestor(aClass, memberClass, false) || aClass != null && PsiTreeUtil.isAncestor(memberClass, aClass, false)) { // access from the same file can be via private @@ -232,8 +220,8 @@ class AccessCanBeTightenedInspection extends BaseJavaBatchLocalInspectionTool { return suggestPackageLocal(member); } - return myVisibilityInspection.SUGGEST_PRIVATE_FOR_INNERS || - !isInnerClass(memberClass) ? PsiUtil.ACCESS_LEVEL_PRIVATE : suggestPackageLocal(member); + return !isAbstractMember && (myVisibilityInspection.SUGGEST_PRIVATE_FOR_INNERS || + !isInnerClass(memberClass)) ? PsiUtil.ACCESS_LEVEL_PRIVATE : suggestPackageLocal(member); } //if (file == memberFile) { // return PsiUtil.ACCESS_LEVEL_PACKAGE_LOCAL; diff --git a/plugins/InspectionGadgets/testsrc/com/intellij/codeInspection/visibility/AccessCanBeTightenedInspectionTest.java b/plugins/InspectionGadgets/testsrc/com/intellij/codeInspection/visibility/AccessCanBeTightenedInspectionTest.java index 1ef9f804903d..d016d939fd1f 100644 --- a/plugins/InspectionGadgets/testsrc/com/intellij/codeInspection/visibility/AccessCanBeTightenedInspectionTest.java +++ b/plugins/InspectionGadgets/testsrc/com/intellij/codeInspection/visibility/AccessCanBeTightenedInspectionTest.java @@ -117,6 +117,17 @@ public class AccessCanBeTightenedInspectionTest extends LightInspectionTestCase "}"); } + public void testDoNotSuggestPrivateForAbstractIDEA151875() { + doTest("class C {\n" + + " abstract static class Inner {\n" + + " abstract void foo();\n"+ + " }\n" + + " void f(Inner i) {\n" + + " i.foo();\n" + + " }\n"+ + "}"); + } + @Override protected LocalInspectionTool getInspection() { VisibilityInspection inspection = new VisibilityInspection();