diff --git a/plugins/groovy/resources/inspectionDescriptions/GroovyAccessibility.html b/plugins/groovy/resources/inspectionDescriptions/GroovyAccessibility.html new file mode 100644 index 000000000000..4a1c9cc15c75 --- /dev/null +++ b/plugins/groovy/resources/inspectionDescriptions/GroovyAccessibility.html @@ -0,0 +1,6 @@ + + + +This inspection reports all references which exceed access rights. +
Powered by InspectorGroovy + \ No newline at end of file diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/GroovyBundle.properties b/plugins/groovy/src/org/jetbrains/plugins/groovy/GroovyBundle.properties index ce1156ed033a..7d372d9b73c1 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/GroovyBundle.properties +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/GroovyBundle.properties @@ -178,7 +178,7 @@ change.modifier=Make ''{0}'' {1} change.modifier.not=Make ''{0}'' not {1} change.modifier.family.name=Change modifiers # suppress inspection "UnusedProperty" -packageLocal.visibility.presentation=property +packageLocal.visibility.presentation=default visible # suppress inspection "UnusedProperty" protected.visibility.presentation=protected # suppress inspection "UnusedProperty" diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/GroovyAnnotator.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/GroovyAnnotator.java index 258571806c4a..9704abc4cfb7 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/GroovyAnnotator.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/GroovyAnnotator.java @@ -25,16 +25,16 @@ import com.intellij.lang.annotation.AnnotationHolder; import com.intellij.lang.annotation.Annotator; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.project.IndexNotReadyException; -import com.intellij.openapi.project.Project; import com.intellij.openapi.util.TextRange; import com.intellij.openapi.util.text.StringUtil; import com.intellij.openapi.vfs.VirtualFile; import com.intellij.psi.*; import com.intellij.psi.infos.CandidateInfo; import com.intellij.psi.search.searches.SuperMethodsSearch; -import com.intellij.psi.util.*; -import com.intellij.util.ArrayUtil; -import com.intellij.util.IncorrectOperationException; +import com.intellij.psi.util.InheritanceUtil; +import com.intellij.psi.util.MethodSignature; +import com.intellij.psi.util.MethodSignatureBackedByPsiMethod; +import com.intellij.psi.util.PsiTreeUtil; import com.intellij.util.containers.HashSet; import com.intellij.util.containers.MultiMap; import gnu.trove.THashSet; @@ -185,13 +185,13 @@ public class GroovyAnnotator extends GroovyElementVisitor implements Annotator { if (resolved instanceof PsiMember) { highlightMemberResolved(myHolder, referenceExpression, ((PsiMember)resolved)); } - if (!resolveResult.isAccessible()) { + /*if (!resolveResult.isAccessible()) { String message = GroovyBundle.message("cannot.access", referenceExpression.getReferenceName()); final Annotation annotation = myHolder.createWarningAnnotation(getElementToHighlight(referenceExpression), message); if (resolved instanceof PsiMember) { registerAccessFix(annotation, referenceExpression, ((PsiMember)resolved)); } - } + }*/ //todo uncomment when correct isStatic() is working if (!resolveResult.isStaticsOK() && resolved instanceof PsiModifierListOwner) { @@ -252,6 +252,7 @@ public class GroovyAnnotator extends GroovyElementVisitor implements Annotator { } } + /* private static void registerAccessFix(Annotation annotation, PsiElement place, PsiMember refElement) { if (refElement instanceof PsiCompiledElement) return; PsiModifierList modifierList = refElement.getModifierList(); @@ -284,6 +285,7 @@ public class GroovyAnnotator extends GroovyElementVisitor implements Annotator { LOG.error(e); } } + */ private static void registerStaticImportFix(GrReferenceExpression referenceExpression, Annotation annotation) { final String referenceName = referenceExpression.getReferenceName(); @@ -580,7 +582,7 @@ public class GroovyAnnotator extends GroovyElementVisitor implements Annotator { checkDefaultMapConstructor(myHolder, argList, constructor); } - if (!constructorResolveResult.isAccessible()) { +/* if (!constructorResolveResult.isAccessible()) { String message = GroovyBundle.message("cannot.access", PsiFormatUtil.formatMethod((PsiMethod)constructor, PsiSubstitutor.EMPTY, PsiFormatUtil.SHOW_NAME | PsiFormatUtil.SHOW_TYPE | @@ -590,7 +592,7 @@ public class GroovyAnnotator extends GroovyElementVisitor implements Annotator { )); final Annotation annotation = myHolder.createWarningAnnotation(getElementToHighlight(refElement), message); registerAccessFix(annotation, refElement, ((PsiMember)constructor)); - } + }*/ } else { diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/GroovyFix.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/GroovyFix.java index 49ea83d4bf0a..1309857a4275 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/GroovyFix.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/GroovyFix.java @@ -35,6 +35,8 @@ import org.jetbrains.plugins.groovy.lang.psi.api.util.GrStatementOwner; public abstract class GroovyFix implements LocalQuickFix { + public static final GroovyFix[] EMPTY_ARRAY = new GroovyFix[0]; + //to appear in "Apply Fix" statement when multiple Quick Fixes exist @NotNull public String getFamilyName() { diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/GroovyInspectionBundle.properties b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/GroovyInspectionBundle.properties index 6fa06f83eb3a..8f08a7f701cf 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/GroovyInspectionBundle.properties +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/GroovyInspectionBundle.properties @@ -59,3 +59,7 @@ pointless.boolean.quickfix=Simplify Cannot.perform.undo.operation=Cannot perform undo operation Undo.disable=Undo disabled field.already.defined=Variables with field names + +access.to.inaccessible.element=Access to inaccessible element + +cannot.reference.nonstatic=Cannot reference nonstatic symbol ''{0}'' from static context \ No newline at end of file diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/GroovyInspectionProvider.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/GroovyInspectionProvider.java index e89d519e06bc..cf7199c1f789 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/GroovyInspectionProvider.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/GroovyInspectionProvider.java @@ -135,6 +135,7 @@ public class GroovyInspectionProvider implements InspectionToolProvider, Applica GroovyInfiniteRecursionInspection.class, GroovyDivideByZeroInspection.class, GroovyResultOfObjectAllocationIgnoredInspection.class, + GroovyAccessibilityInspection.class, GroovyClassNamingConventionInspection.class, GroovyInterfaceNamingConventionInspection.class, diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/bugs/GroovyAccessibilityInspection.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/bugs/GroovyAccessibilityInspection.java new file mode 100644 index 000000000000..a1d885d4a0dc --- /dev/null +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/bugs/GroovyAccessibilityInspection.java @@ -0,0 +1,253 @@ +/* + * Copyright 2000-2011 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.plugins.groovy.codeInspection.bugs; + +import com.intellij.codeInspection.ProblemDescriptor; +import com.intellij.openapi.diagnostic.Logger; +import com.intellij.openapi.project.Project; +import com.intellij.psi.*; +import com.intellij.psi.util.PsiFormatUtil; +import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.util.ArrayUtil; +import com.intellij.util.IncorrectOperationException; +import org.jetbrains.annotations.Nls; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.plugins.groovy.GroovyBundle; +import org.jetbrains.plugins.groovy.codeInspection.BaseInspection; +import org.jetbrains.plugins.groovy.codeInspection.BaseInspectionVisitor; +import org.jetbrains.plugins.groovy.codeInspection.GroovyFix; +import org.jetbrains.plugins.groovy.codeInspection.GroovyInspectionBundle; +import org.jetbrains.plugins.groovy.lang.psi.GrReferenceElement; +import org.jetbrains.plugins.groovy.lang.psi.GroovyFile; +import org.jetbrains.plugins.groovy.lang.psi.api.GroovyResolveResult; +import org.jetbrains.plugins.groovy.lang.psi.api.statements.GrConstructorInvocation; +import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrConstructorCall; +import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrNewExpression; +import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrReferenceExpression; +import org.jetbrains.plugins.groovy.lang.psi.api.types.GrCodeReferenceElement; + +import java.util.ArrayList; +import java.util.List; + +/** + * @author Maxim.Medvedev + */ +public class GroovyAccessibilityInspection extends BaseInspection { + private static final Logger LOG = Logger.getInstance("org.jetbrains.plugins.groovy.codeInspection.bugs.GroovyAccessibilityInspection"); + + @Override + protected BaseInspectionVisitor buildVisitor() { + return new MyVisitor(); + } + + @Nls + @NotNull + @Override + public String getGroupDisplayName() { + return PROBABLE_BUGS; + } + + @Nls + @NotNull + @Override + public String getDisplayName() { + return GroovyInspectionBundle.message("access.to.inaccessible.element"); + } + + @Override + protected String buildErrorString(Object... args) { + return GroovyBundle.message("cannot.access", args); + } + + @Override + protected GroovyFix[] buildFixes(PsiElement location) { + if (!(location instanceof GrReferenceElement || location instanceof GrConstructorCall)) { + location = location.getParent(); + } + + final GroovyResolveResult resolveResult; + if (location instanceof GrConstructorCall) { + resolveResult = ((GrConstructorCall)location).advancedResolve(); + } + else { + resolveResult = ((GrReferenceElement)location).advancedResolve(); + } + + final PsiElement element = resolveResult.getElement(); + if (!(element instanceof PsiMember)) return GroovyFix.EMPTY_ARRAY; + final PsiMember refElement = (PsiMember)element; + + if (refElement instanceof PsiCompiledElement) return GroovyFix.EMPTY_ARRAY; + + PsiModifierList modifierList = refElement.getModifierList(); + if (modifierList == null) return GroovyFix.EMPTY_ARRAY; + + List fixes = new ArrayList(); + try { + Project project = refElement.getProject(); + JavaPsiFacade facade = JavaPsiFacade.getInstance(project); + PsiModifierList modifierListCopy = facade.getElementFactory().createFieldFromText("int a;", null).getModifierList(); + modifierListCopy.setModifierProperty(PsiModifier.STATIC, modifierList.hasModifierProperty(PsiModifier.STATIC)); + @Modifier String minModifier = PsiModifier.PROTECTED; + if (refElement.hasModifierProperty(PsiModifier.PROTECTED)) { + minModifier = PsiModifier.PUBLIC; + } + String[] modifiers = {PsiModifier.PROTECTED, PsiModifier.PUBLIC, PsiModifier.PACKAGE_LOCAL}; + PsiClass accessObjectClass = PsiTreeUtil.getParentOfType(location, PsiClass.class, false); + if (accessObjectClass == null) { + accessObjectClass = ((GroovyFile)location.getContainingFile()).getScriptClass(); + } + for (int i = ArrayUtil.indexOf(modifiers, minModifier); i < modifiers.length; i++) { + String modifier = modifiers[i]; + modifierListCopy.setModifierProperty(modifier, true); + if (facade.getResolveHelper().isAccessible(refElement, modifierListCopy, location, accessObjectClass, null)) { + fixes.add(new GrModifierFix(refElement, refElement.getModifierList(), modifier, true)); + } + } + } + catch (IncorrectOperationException e) { + LOG.error(e); + } + return fixes.toArray(new GroovyFix[fixes.size()]); + } + + private static class GrModifierFix extends GroovyFix { + private PsiMember myMember; + private PsiModifierList myModifierList; + private String myModifier; + private boolean myDoSet; + + public GrModifierFix(@NotNull PsiMember member, + @NotNull PsiModifierList modifierList, + String modifier, + boolean doSet) { + myMember = member; + myModifierList = modifierList; + myModifier = modifier; + myDoSet = doSet; + } + + @Override + protected void doFix(Project project, ProblemDescriptor descriptor) throws IncorrectOperationException { + myModifierList.setModifierProperty(myModifier, myDoSet); + } + + @NotNull + @Override + public String getName() { + String name; + final PsiClass containingClass = myMember.getContainingClass(); + String containingClassName; + if (containingClass != null) { + containingClassName = containingClass.getName() + "."; + } + else { + containingClassName = ""; + } + + name = containingClassName + myMember.getName(); + + String modifierText = toPresentableText(myModifier); + + if (myDoSet) { + return GroovyBundle.message("change.modifier", name, modifierText); + } + else { + return GroovyBundle.message("change.modifier.not", name, modifierText); + } + } + } + + private static String toPresentableText(String modifier) { + return GroovyBundle.message(modifier + ".visibility.presentation"); + } + + @Override + public boolean isEnabledByDefault() { + return true; + } + + private static class MyVisitor extends BaseInspectionVisitor { + @Override + public void visitCodeReferenceElement(GrCodeReferenceElement refElement) { + super.visitCodeReferenceElement(refElement); + checkRef(refElement); + } + + @Override + public void visitReferenceExpression(GrReferenceExpression ref) { + super.visitReferenceExpression(ref); + checkRef(ref); + } + + @Override + public void visitNewExpression(GrNewExpression newExpression) { + checkConstructorCall(newExpression); + } + + private void checkConstructorCall(GrConstructorCall call) { + final GroovyResolveResult result = call.advancedResolve(); + if (result.getElement() == null) return; + final PsiElement constructor = result.getElement(); + if (!(constructor instanceof PsiMethod)) return; + if (!result.isAccessible()) { + + PsiElement refElement = null; + if (call instanceof GrNewExpression) { + refElement = ((GrNewExpression)call).getReferenceElement(); + } + else if (call instanceof GrConstructorInvocation) { + refElement = ((GrConstructorInvocation)call).getThisOrSuperKeyword(); + } + if (refElement == null) { + refElement = call; + } + + + registerError(refElement, + PsiFormatUtil.formatMethod((PsiMethod)constructor, PsiSubstitutor.EMPTY, + PsiFormatUtil.SHOW_NAME | + PsiFormatUtil.SHOW_TYPE | + PsiFormatUtil.TYPE_AFTER | + PsiFormatUtil.SHOW_PARAMETERS, + PsiFormatUtil.SHOW_TYPE + )); + } + } + + @Override + public void visitConstructorInvocation(GrConstructorInvocation invocation) { + super.visitConstructorInvocation(invocation); + checkConstructorCall(invocation); + } + + private void checkRef(GrReferenceElement ref) { + final GroovyResolveResult result = ref.advancedResolve(); + if (result == null) return; + if (result.getElement() == null) return; + if (!result.isAccessible()) { + registerError(getErrorLocation(ref), ref.getReferenceName()); + } + } + + @NotNull + private static PsiElement getErrorLocation(GrReferenceElement ref) { + final PsiElement nameElement = ref.getReferenceNameElement(); + if (nameElement != null) return nameElement; + return ref; + } + } +} diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyHighlightingTest.java b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyHighlightingTest.java index e2693ef2ab23..5f7ffd506860 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyHighlightingTest.java +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyHighlightingTest.java @@ -20,6 +20,7 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.plugins.groovy.codeInspection.GroovyImportsTracker; import org.jetbrains.plugins.groovy.codeInspection.assignment.GroovyAssignabilityCheckInspection; import org.jetbrains.plugins.groovy.codeInspection.assignment.GroovyUncheckedAssignmentOfMemberOfRawTypeInspection; +import org.jetbrains.plugins.groovy.codeInspection.bugs.GroovyAccessibilityInspection; import org.jetbrains.plugins.groovy.codeInspection.bugs.GroovyResultOfObjectAllocationIgnoredInspection; import org.jetbrains.plugins.groovy.codeInspection.control.GroovyTrivialConditionalInspection; import org.jetbrains.plugins.groovy.codeInspection.control.GroovyTrivialIfInspection; @@ -262,12 +263,16 @@ public class GroovyHighlightingTest extends LightCodeInsightFixtureTestCase { public void testTupleTypeAssignments() throws Exception{doTest(new GroovyAssignabilityCheckInspection());} public void testUnusedImportsForImportsOnDemand() throws Exception { - doTest(); + doTest(new GroovyAccessibilityInspection()); final Set unusedImportStatements = GroovyImportsTracker.getInstance(getProject()).getUnusedImportStatements(((GroovyFile)myFixture.getFile())); assertEquals(0, unusedImportStatements.size()); } + public void testInaccessibleConstructorCall() { + doTest(new GroovyAccessibilityInspection()); + } + public void testSignatureIsNotApplicableToList() throws Exception { doTest(new GroovyAssignabilityCheckInspection()); } diff --git a/plugins/groovy/testdata/highlighting/InaccessibleConstructorCall.groovy b/plugins/groovy/testdata/highlighting/InaccessibleConstructorCall.groovy new file mode 100644 index 000000000000..ab047c55d7d5 --- /dev/null +++ b/plugins/groovy/testdata/highlighting/InaccessibleConstructorCall.groovy @@ -0,0 +1,9 @@ +class Base { + private Base(int i){} +} + +class Extension extends Base { + def Extension() { + super(1) + } +} \ No newline at end of file