From 0dd8bd15b777ca20398df452c5bb7e9a321b5639 Mon Sep 17 00:00:00 2001 From: Maxim Medvedev Date: Fri, 28 Jan 2011 12:25:13 +0300 Subject: [PATCH 1/6] tests for Range type check --- .../GroovyInspectionBundle.properties | 3 +- .../bugs/GroovyRangeTypeCheckInspection.java | 113 +++++++++++------- .../groovy/lang/resolve/ResolveUtil.java | 2 +- .../bugs/GroovyRangeTypeCheckTest.groovy | 68 +++++++++++ .../groovy/lang/GroovyHighlightingTest.java | 5 + .../inspections/rangeTypeCheck/All.groovy | 5 + .../rangeTypeCheck/All_after.groovy | 16 +++ .../inspections/rangeTypeCheck/Next.groovy | 13 ++ .../rangeTypeCheck/Next_after.groovy | 16 +++ .../rangeTypeCheck/NotComparable.groovy | 10 ++ .../rangeTypeCheck/NotComparable_after.groovy | 14 +++ .../testdata/highlighting/RangeType.groovy | 4 + 12 files changed, 225 insertions(+), 44 deletions(-) create mode 100644 plugins/groovy/test/org/jetbrains/plugins/groovy/codeInspection/bugs/GroovyRangeTypeCheckTest.groovy create mode 100644 plugins/groovy/testdata/groovy/inspections/rangeTypeCheck/All.groovy create mode 100644 plugins/groovy/testdata/groovy/inspections/rangeTypeCheck/All_after.groovy create mode 100644 plugins/groovy/testdata/groovy/inspections/rangeTypeCheck/Next.groovy create mode 100644 plugins/groovy/testdata/groovy/inspections/rangeTypeCheck/Next_after.groovy create mode 100644 plugins/groovy/testdata/groovy/inspections/rangeTypeCheck/NotComparable.groovy create mode 100644 plugins/groovy/testdata/groovy/inspections/rangeTypeCheck/NotComparable_after.groovy create mode 100644 plugins/groovy/testdata/highlighting/RangeType.groovy 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 23a144c9d449..841a4d840152 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/GroovyInspectionBundle.properties +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/GroovyInspectionBundle.properties @@ -68,4 +68,5 @@ type.doesnt.contain.method=Type ''{0}'' cannot be iterated in range because it d incorrect.range.argument=Incorrect range arguments type.doesnt.implemnt.comparable=Type ''{0}'' doesnt implement Comparable add.method=Add method ''{0}()'' to class ''{1}'' -implement.class=Implement {0} \ No newline at end of file +implement.class=Implement {0} +fix.class=Fix class {0} \ No newline at end of file diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/bugs/GroovyRangeTypeCheckInspection.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/bugs/GroovyRangeTypeCheckInspection.java index 23864216a17a..af384d9ce3cd 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/bugs/GroovyRangeTypeCheckInspection.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/bugs/GroovyRangeTypeCheckInspection.java @@ -37,6 +37,7 @@ import org.jetbrains.plugins.groovy.lang.lexer.GroovyTokenTypes; import org.jetbrains.plugins.groovy.lang.psi.GrReferenceElement; import org.jetbrains.plugins.groovy.lang.psi.GroovyPsiElementFactory; import org.jetbrains.plugins.groovy.lang.psi.api.GroovyResolveResult; +import org.jetbrains.plugins.groovy.lang.psi.api.auxiliary.modifiers.GrModifier; import org.jetbrains.plugins.groovy.lang.psi.api.statements.blocks.GrCodeBlock; import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.arithmetic.GrRangeExpression; import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.GrReferenceList; @@ -78,34 +79,60 @@ public class GroovyRangeTypeCheckInspection extends BaseInspection { } @Override - protected GroovyFix[] buildFixes(PsiElement location) { + protected GroovyFix buildFix(PsiElement location) { final GrRangeExpression range = (GrRangeExpression)location; final PsiType type = range.getType(); - List fixes = new ArrayList(3); + final List fixes = new ArrayList(3); if (type instanceof GrRangeType) { PsiType iterationType = ((GrRangeType)type).getIterationType(); - if (!(iterationType instanceof PsiClassType)) return GroovyFix.EMPTY_ARRAY; + if (!(iterationType instanceof PsiClassType)) return null; final PsiClass psiClass = ((PsiClassType)iterationType).resolve(); - if (!(psiClass instanceof GrTypeDefinition)) return GroovyFix.EMPTY_ARRAY; + if (!(psiClass instanceof GrTypeDefinition)) return null; - final GroovyResolveResult[] nexts = ResolveUtil.getMethodCandidates(iterationType, "next", range, PsiType.EMPTY_ARRAY); - final GroovyResolveResult[] previouses = ResolveUtil.getMethodCandidates(iterationType, "previous", range, PsiType.EMPTY_ARRAY); + final GroovyResolveResult[] nexts = ResolveUtil.getMethodCandidates(iterationType, "next", range); + final GroovyResolveResult[] previouses = ResolveUtil.getMethodCandidates(iterationType, "previous", range); + final GroovyResolveResult[] compareTos = ResolveUtil.getMethodCandidates(iterationType, "compareTo", range, iterationType); - if (nexts.length == 0) { + if (countImplementations(psiClass, nexts)==0) { fixes.add(new AddMethodFix("next", (GrTypeDefinition)psiClass)); } - if (previouses.length == 0) { + if (countImplementations(psiClass, previouses) == 0) { fixes.add(new AddMethodFix("previous", (GrTypeDefinition)psiClass)); } - if (!InheritanceUtil.isInheritor(type, CommonClassNames.JAVA_LANG_COMPARABLE)) { + if (!InheritanceUtil.isInheritor(iterationType, CommonClassNames.JAVA_LANG_COMPARABLE) || + countImplementations(psiClass, compareTos) == 0) { fixes.add(new AddClassToExtends((GrTypeDefinition)psiClass, CommonClassNames.JAVA_LANG_COMPARABLE)); } + + return new GroovyFix() { + @Override + protected void doFix(Project project, ProblemDescriptor descriptor) throws IncorrectOperationException { + for (GroovyFix fix : fixes) { + fix.applyFix(project, descriptor); + } + } + + @NotNull + @Override + public String getName() { + return GroovyInspectionBundle.message("fix.class", psiClass.getName()); + } + }; } - return fixes.toArray(new GroovyFix[fixes.size()]); + return null; } + private static int countImplementations(PsiClass clazz, GroovyResolveResult[] methods) { + if (clazz.isInterface()) return methods.length; + int result = 0; + for (GroovyResolveResult method : methods) { + final PsiElement el = method.getElement(); + if (el instanceof PsiMethod && !((PsiMethod)el).hasModifierProperty(GrModifier.ABSTRACT)) result++; + } + return result; + } @Override protected String buildErrorString(Object... args) { @@ -225,32 +252,6 @@ public class GroovyRangeTypeCheckInspection extends BaseInspection { GrReferenceList list; final GroovyPsiElementFactory factory = GroovyPsiElementFactory.getInstance(project); - if (myPsiClass.isInterface()) { - list = myPsiClass.getExtendsClause(); - if (list == null) { - list = factory.createExtendsClause(); - - PsiElement anchor = myPsiClass.getImplementsClause(); - if (anchor==null) { - anchor = myPsiClass.getBody(); - } - if (anchor == null) return; - list = (GrReferenceList)myPsiClass.addBefore(list, anchor); - myPsiClass.getNode().addLeaf(GroovyTokenTypes.mWS, " ", anchor.getNode()); - myPsiClass.getNode().addLeaf(GroovyTokenTypes.mWS, " ", list.getNode()); - } - } - else { - list = myPsiClass.getImplementsClause(); - if (list == null) { - list = factory.createImplementsClause(); - PsiElement anchor = myPsiClass.getBody(); - if (anchor == null) return; - list = (GrReferenceList)myPsiClass.addBefore(list, anchor); - myPsiClass.getNode().addLeaf(GroovyTokenTypes.mWS, " ", list.getNode()); - myPsiClass.getNode().addLeaf(GroovyTokenTypes.mWS, " ", anchor.getNode()); - } - } final PsiClass comparable = JavaPsiFacade.getInstance(project).findClass(CommonClassNames.JAVA_LANG_COMPARABLE, myPsiClass.getResolveScope()); @@ -270,11 +271,40 @@ public class GroovyRangeTypeCheckInspection extends BaseInspection { } } - final PsiElement ref = - list - .add(factory.createReferenceElementFromText(myInterfaceName + (addTypeParam ? "<" + generateTypeText(myPsiClass) + ">" : ""))); - PsiUtil.shortenReference((GrReferenceElement)ref); + if (!InheritanceUtil.isInheritor(myPsiClass, CommonClassNames.JAVA_LANG_COMPARABLE)) { + if (myPsiClass.isInterface()) { + list = myPsiClass.getExtendsClause(); + if (list == null) { + list = factory.createExtendsClause(); + PsiElement anchor = myPsiClass.getImplementsClause(); + if (anchor == null) { + anchor = myPsiClass.getBody(); + } + if (anchor == null) return; + list = (GrReferenceList)myPsiClass.addBefore(list, anchor); + myPsiClass.getNode().addLeaf(GroovyTokenTypes.mWS, " ", anchor.getNode()); + myPsiClass.getNode().addLeaf(GroovyTokenTypes.mWS, " ", list.getNode()); + } + } + else { + list = myPsiClass.getImplementsClause(); + if (list == null) { + list = factory.createImplementsClause(); + PsiElement anchor = myPsiClass.getBody(); + if (anchor == null) return; + list = (GrReferenceList)myPsiClass.addBefore(list, anchor); + myPsiClass.getNode().addLeaf(GroovyTokenTypes.mWS, " ", list.getNode()); + myPsiClass.getNode().addLeaf(GroovyTokenTypes.mWS, " ", anchor.getNode()); + } + } + + + final PsiElement ref = + list + .add(factory.createReferenceElementFromText(myInterfaceName + (addTypeParam ? "<" + generateTypeText(myPsiClass) + ">" : ""))); + PsiUtil.shortenReference((GrReferenceElement)ref); + } if (comparable != null && !myPsiClass.isInterface()) { GroovyOverrideImplementUtil .generateImplementation(null, myPsiClass.getContainingFile(), myPsiClass, comparable.getMethods()[0], substitutor); @@ -283,8 +313,7 @@ public class GroovyRangeTypeCheckInspection extends BaseInspection { @NotNull @Override - public String getName - () { + public String getName() { return GroovyInspectionBundle.message("implement.class", myInterfaceName); } } diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/resolve/ResolveUtil.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/resolve/ResolveUtil.java index a207430fc4fa..14caa5c9b638 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/resolve/ResolveUtil.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/resolve/ResolveUtil.java @@ -585,7 +585,7 @@ public class ResolveUtil { public static GroovyResolveResult[] getMethodCandidates(@NotNull PsiType thisType, @Nullable String methodName, @NotNull GroovyPsiElement place, - @Nullable PsiType[] argumentTypes) { + @Nullable PsiType... argumentTypes) { if (methodName != null) { MethodResolverProcessor processor = new MethodResolverProcessor(methodName, place, false, thisType, argumentTypes, PsiType.EMPTY_ARRAY); diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/codeInspection/bugs/GroovyRangeTypeCheckTest.groovy b/plugins/groovy/test/org/jetbrains/plugins/groovy/codeInspection/bugs/GroovyRangeTypeCheckTest.groovy new file mode 100644 index 000000000000..2a97a95ead20 --- /dev/null +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/codeInspection/bugs/GroovyRangeTypeCheckTest.groovy @@ -0,0 +1,68 @@ +/* + * 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.InspectionManager +import com.intellij.codeInspection.ProblemDescriptor +import com.intellij.codeInspection.ProblemHighlightType +import com.intellij.openapi.application.ApplicationManager +import com.intellij.psi.PsiElement +import com.intellij.psi.impl.source.PostprocessReformattingAspect +import com.intellij.psi.util.PsiTreeUtil +import com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase +import org.jetbrains.plugins.groovy.codeInspection.GroovyFix +import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.arithmetic.GrRangeExpression +import org.jetbrains.plugins.groovy.util.TestUtils + +/** + * @author Maxim.Medvedev + */ +class GroovyRangeTypeCheckTest extends LightCodeInsightFixtureTestCase { + @Override + protected String getBasePath() { + return TestUtils.getTestDataPath() + 'groovy/inspections/rangeTypeCheck' + } + + public void doTest() { + myFixture.configureByFile getTestName(false) + '.groovy' + + final int offset = myFixture.getEditor().getCaretModel().getOffset() + final PsiElement atCaret = myFixture.getFile().findElementAt(offset) + final GrRangeExpression range = PsiTreeUtil.getParentOfType(atCaret, GrRangeExpression) + final GroovyRangeTypeCheckInspection inspection = new GroovyRangeTypeCheckInspection() + + GroovyFix fix = inspection.buildFix(range) + + final ProblemDescriptor descriptor = InspectionManager.getInstance(project).createProblemDescriptor(range, "bla-bla", fix, ProblemHighlightType.WEAK_WARNING); + ApplicationManager.application.runWriteAction( + new Runnable() { + @Override + void run() { + fix.applyFix myFixture.project, descriptor + PostprocessReformattingAspect.getInstance(getProject()).doPostponedFormatting(); + } + }) + + + myFixture.checkResultByFile getTestName(false) + '_after.groovy' + } + + void testNotComparable() {doTest()} + + void testNext() {doTest()} + + void testAll() {doTest()} +} 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 5f7ffd506860..975444532582 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyHighlightingTest.java +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyHighlightingTest.java @@ -21,6 +21,7 @@ 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.GroovyRangeTypeCheckInspection; import org.jetbrains.plugins.groovy.codeInspection.bugs.GroovyResultOfObjectAllocationIgnoredInspection; import org.jetbrains.plugins.groovy.codeInspection.control.GroovyTrivialConditionalInspection; import org.jetbrains.plugins.groovy.codeInspection.control.GroovyTrivialIfInspection; @@ -311,4 +312,8 @@ public class GroovyHighlightingTest extends LightCodeInsightFixtureTestCase { } public void testMapParamWithNoArgs() {doTest(new GroovyAssignabilityCheckInspection());} + + public void testRangeType() { + doTest(new GroovyRangeTypeCheckInspection()); + } } \ No newline at end of file diff --git a/plugins/groovy/testdata/groovy/inspections/rangeTypeCheck/All.groovy b/plugins/groovy/testdata/groovy/inspections/rangeTypeCheck/All.groovy new file mode 100644 index 000000000000..d08541ace832 --- /dev/null +++ b/plugins/groovy/testdata/groovy/inspections/rangeTypeCheck/All.groovy @@ -0,0 +1,5 @@ +class Foo { + +} + +print new Foo()..new Foo() \ No newline at end of file diff --git a/plugins/groovy/testdata/groovy/inspections/rangeTypeCheck/All_after.groovy b/plugins/groovy/testdata/groovy/inspections/rangeTypeCheck/All_after.groovy new file mode 100644 index 000000000000..5c864d6fa7b3 --- /dev/null +++ b/plugins/groovy/testdata/groovy/inspections/rangeTypeCheck/All_after.groovy @@ -0,0 +1,16 @@ +class Foo implements Comparable { + + def Foo next() { + return null //To change body of implemented methods use File | Settings | File Templates. + } + + def Foo previous() { + return null //To change body of implemented methods use File | Settings | File Templates. + } + + int compareTo(Foo o) { + return 0 //To change body of implemented methods use File | Settings | File Templates. + } +} + +print new Foo()..new Foo() \ No newline at end of file diff --git a/plugins/groovy/testdata/groovy/inspections/rangeTypeCheck/Next.groovy b/plugins/groovy/testdata/groovy/inspections/rangeTypeCheck/Next.groovy new file mode 100644 index 000000000000..4a040802f42e --- /dev/null +++ b/plugins/groovy/testdata/groovy/inspections/rangeTypeCheck/Next.groovy @@ -0,0 +1,13 @@ +class Foo implements Comparable { + @Override + int compareTo(Foo o) { + return 0 + } + + def previous() { + return this + } + +} + +print new Foo()..new Foo() \ No newline at end of file diff --git a/plugins/groovy/testdata/groovy/inspections/rangeTypeCheck/Next_after.groovy b/plugins/groovy/testdata/groovy/inspections/rangeTypeCheck/Next_after.groovy new file mode 100644 index 000000000000..e75b918ab633 --- /dev/null +++ b/plugins/groovy/testdata/groovy/inspections/rangeTypeCheck/Next_after.groovy @@ -0,0 +1,16 @@ +class Foo implements Comparable { + @Override + int compareTo(Foo o) { + return 0 + } + + def previous() { + return this + } + + def Foo next() { + return null //To change body of implemented methods use File | Settings | File Templates. + } +} + +print new Foo()..new Foo() \ No newline at end of file diff --git a/plugins/groovy/testdata/groovy/inspections/rangeTypeCheck/NotComparable.groovy b/plugins/groovy/testdata/groovy/inspections/rangeTypeCheck/NotComparable.groovy new file mode 100644 index 000000000000..863622a54070 --- /dev/null +++ b/plugins/groovy/testdata/groovy/inspections/rangeTypeCheck/NotComparable.groovy @@ -0,0 +1,10 @@ +class Foo { + def next() {return this} + def previous() {return this} +} + +class X { + def foo() { + final ObjectRange range = new Foo()..new Foo() + } +} diff --git a/plugins/groovy/testdata/groovy/inspections/rangeTypeCheck/NotComparable_after.groovy b/plugins/groovy/testdata/groovy/inspections/rangeTypeCheck/NotComparable_after.groovy new file mode 100644 index 000000000000..636af2a7f8c5 --- /dev/null +++ b/plugins/groovy/testdata/groovy/inspections/rangeTypeCheck/NotComparable_after.groovy @@ -0,0 +1,14 @@ +class Foo implements Comparable { + def next() {return this} + def previous() {return this} + + int compareTo(Foo o) { + return 0 //To change body of implemented methods use File | Settings | File Templates. + } +} + +class X { + def foo() { + final ObjectRange range = new Foo()..new Foo() + } +} diff --git a/plugins/groovy/testdata/highlighting/RangeType.groovy b/plugins/groovy/testdata/highlighting/RangeType.groovy new file mode 100644 index 000000000000..3c2fb4060fb4 --- /dev/null +++ b/plugins/groovy/testdata/highlighting/RangeType.groovy @@ -0,0 +1,4 @@ +class Foo { +} + +print new Foo()..new Foo() \ No newline at end of file From 19c7f638c756d8b08e8b75d6dc77a077cac9eadc Mon Sep 17 00:00:00 2001 From: Maxim Medvedev Date: Fri, 28 Jan 2011 12:50:00 +0300 Subject: [PATCH 2/6] Highlight ambiguous code blocks in Groovy code --- .../groovy/annotator/GroovyAnnotator.java | 18 ++++++++++++++++-- .../groovy/lang/GroovyHighlightingTest.java | 1 + .../AmbiguousCodeBlockInMethodCall.groovy | 1 + .../DefinitionUsedInClosure.groovy | 2 +- 4 files changed, 19 insertions(+), 3 deletions(-) create mode 100644 plugins/groovy/testdata/highlighting/AmbiguousCodeBlockInMethodCall.groovy 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 0ae805d0c2aa..f6f2b82fda57 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/GroovyAnnotator.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/annotator/GroovyAnnotator.java @@ -709,13 +709,27 @@ public class GroovyAnnotator extends GroovyElementVisitor implements Annotator { public void visitClosure(GrClosableBlock closure) { super.visitClosure(closure); if (!closure.hasParametersSection()) { - final PsiElement parent = closure.getParent(); - if (parent instanceof GrCodeBlock || parent instanceof GroovyFile) { + if (!checkClosureForAmbiguous(closure)) { myHolder.createErrorAnnotation(closure, GroovyBundle.message("ambiguous.code.block")); } } } + private static boolean checkClosureForAmbiguous(GrClosableBlock closure) { + PsiElement place; + PsiElement parent = closure; + do { + place = parent; + parent = place.getParent(); + } + while (!(parent instanceof GrContinueStatement || parent instanceof GrCodeBlock || parent instanceof GroovyFile)); + while (place != null) { + if (place == closure) return false; + place = place.getFirstChild(); + } + return true; + } + @Override public void visitSuperExpression(GrSuperReferenceExpression superExpression) { checkThisOrSuperReferenceExpression(superExpression, myHolder); 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 975444532582..a68cfb9474a8 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyHighlightingTest.java +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyHighlightingTest.java @@ -254,6 +254,7 @@ public class GroovyHighlightingTest extends LightCodeInsightFixtureTestCase { public void testPutIncorrectValueToMap() throws Exception {doTest(new GroovyAssignabilityCheckInspection());} public void testAmbiguousCodeBlock() throws Exception {doTest();} + public void testAmbiguousCodeBlockInMethodCall() throws Exception {doTest();} public void testNotAmbiguousClosableBlock() throws Exception {doTest();} public void testDuplicateParameterInClosableBlock() throws Exception {doTest();} diff --git a/plugins/groovy/testdata/highlighting/AmbiguousCodeBlockInMethodCall.groovy b/plugins/groovy/testdata/highlighting/AmbiguousCodeBlockInMethodCall.groovy new file mode 100644 index 000000000000..e41a4d5b62f1 --- /dev/null +++ b/plugins/groovy/testdata/highlighting/AmbiguousCodeBlockInMethodCall.groovy @@ -0,0 +1 @@ +{print "foo"}.call() \ No newline at end of file diff --git a/plugins/groovy/testdata/highlighting/DefinitionUsedInClosure.groovy b/plugins/groovy/testdata/highlighting/DefinitionUsedInClosure.groovy index 13a1e6662152..b92dcb576267 100644 --- a/plugins/groovy/testdata/highlighting/DefinitionUsedInClosure.groovy +++ b/plugins/groovy/testdata/highlighting/DefinitionUsedInClosure.groovy @@ -1,7 +1,7 @@ class A { def r() { def a = 0; - { + {-> a.intValue() }.call() } From 034bcc56c9c2fc92f884fc7b9fd0ecdfc6c49937 Mon Sep 17 00:00:00 2001 From: Denis Zhdanov Date: Mon, 31 Jan 2011 12:40:43 +0300 Subject: [PATCH 3/6] IDEA-59536 New variation of "chop down if long" for annotations 1. Method annotation wrap is preferred to parameters wrap; 2. Corresponding test is added; --- .../psi/formatter/java/AbstractJavaBlock.java | 32 ++++++++++++++++++- .../formatter/java/JavaFormatterWrapTest.java | 15 +++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/java/java-impl/src/com/intellij/psi/formatter/java/AbstractJavaBlock.java b/java/java-impl/src/com/intellij/psi/formatter/java/AbstractJavaBlock.java index e6f14c03dc91..5da4faeb8fcf 100644 --- a/java/java-impl/src/com/intellij/psi/formatter/java/AbstractJavaBlock.java +++ b/java/java-impl/src/com/intellij/psi/formatter/java/AbstractJavaBlock.java @@ -474,7 +474,16 @@ public abstract class AbstractJavaBlock extends AbstractBlock implements JavaBlo } else if (childType == JavaTokenType.LPARENTH && nodeType == JavaElementType.PARAMETER_LIST) { - final Wrap wrap = Wrap.createWrap(getWrapType(mySettings.METHOD_PARAMETERS_WRAP), false); + final Wrap wrap; + Wrap reservedWrap = getReservedWrap(JavaElementType.MODIFIER_LIST); + // There is a possible case that particular annotated method definition is too long. We may wrap either after annotation + // or after opening lbrace then. Our strategy is to wrap after annotation whenever possible. + if (reservedWrap == null) { + wrap = Wrap.createWrap(getWrapType(mySettings.METHOD_PARAMETERS_WRAP), false); + } + else { + wrap = Wrap.createChildWrap(reservedWrap, getWrapType(mySettings.METHOD_PARAMETERS_WRAP), false); + } child = processParenthesisBlock(result, child, WrappingStrategy.createDoNotWrapCommaStrategy(wrap), mySettings.ALIGN_MULTILINE_PARAMETERS); @@ -533,6 +542,19 @@ public abstract class AbstractJavaBlock extends AbstractBlock implements JavaBlo myAnnotationWrap = null; } } + else if (childType == JavaElementType.PARAMETER_LIST && nodeType == JavaElementType.METHOD) { + // We prefer wrapping after method annotation to wrapping method parameter list, hence, deliver target wrap object + // to child block if necessary. + if (!result.isEmpty()) { + Block firstChildBlock = result.get(0); + if (firstChildBlock instanceof AbstractJavaBlock) { + AbstractJavaBlock childJavaBlock = (AbstractJavaBlock)firstChildBlock; + if (firstChildIsAnnotation(childJavaBlock.getNode())) { + javaBlock.setReservedWrap(childJavaBlock.getReservedWrap(JavaElementType.MODIFIER_LIST), JavaElementType.MODIFIER_LIST); + } + } + } + } } result.add(block); @@ -747,6 +769,14 @@ public abstract class AbstractJavaBlock extends AbstractBlock implements JavaBlo } + private static boolean firstChildIsAnnotation(final ASTNode child) { + ASTNode current = child.getFirstChildNode(); + while (current != null && current.getElementType() == TokenType.WHITE_SPACE) { + current = current.getTreeNext(); + } + return current != null && current.getElementType() == JavaElementType.ANNOTATION; + } + private static boolean lastChildIsAnnotation(final ASTNode child) { ASTNode current = child.getLastChildNode(); while (current != null && current.getElementType() == TokenType.WHITE_SPACE) { diff --git a/java/java-tests/testSrc/com/intellij/psi/formatter/java/JavaFormatterWrapTest.java b/java/java-tests/testSrc/com/intellij/psi/formatter/java/JavaFormatterWrapTest.java index f60b7feb1e94..493ddd23ab85 100644 --- a/java/java-tests/testSrc/com/intellij/psi/formatter/java/JavaFormatterWrapTest.java +++ b/java/java-tests/testSrc/com/intellij/psi/formatter/java/JavaFormatterWrapTest.java @@ -17,6 +17,7 @@ package com.intellij.psi.formatter.java; import com.intellij.openapi.util.TextRange; import com.intellij.psi.codeStyle.CodeStyleSettings; +import com.intellij.psi.codeStyle.CommonCodeStyleSettings; /** * Is intended to hold specific java formatting tests for 'wrapping' settings. @@ -224,4 +225,18 @@ public class JavaFormatterWrapTest extends AbstractJavaFormatterTest { "}" ); } + + public void testWrapMethodAnnotationBeforeParams() throws Exception { + // Inspired by IDEA-59536 + getSettings().RIGHT_MARGIN = 90; + getSettings().METHOD_ANNOTATION_WRAP = CommonCodeStyleSettings.WRAP_AS_NEEDED; + getSettings().METHOD_PARAMETERS_WRAP = CommonCodeStyleSettings.WRAP_AS_NEEDED; + + doClassTest( + "@SuppressWarnings({\"SomeInspectionIWantToIgnore\"}) public void doSomething(int x, int y) {}", + "@SuppressWarnings({\"SomeInspectionIWantToIgnore\"})\n" + + "public void doSomething(int x, int y) {" + + "\n}" + ); + } } From 83f3ffc9ef726ae7f2643051b532838e427e7d3b Mon Sep 17 00:00:00 2001 From: Maxim Medvedev Date: Mon, 31 Jan 2011 12:47:31 +0300 Subject: [PATCH 4/6] resolve accessors from DefaultGroovyMethods --- .../GrReferenceExpressionImpl.java | 65 ++++++++++--------- .../processors/AccessorResolverProcessor.java | 2 +- .../resolve/processors/ResolverProcessor.java | 2 +- .../groovy/lang/GroovyHighlightingTest.java | 4 ++ .../highlighting/ResolveMetaClass.groovy | 6 ++ 5 files changed, 46 insertions(+), 33 deletions(-) create mode 100644 plugins/groovy/testdata/highlighting/ResolveMetaClass.groovy diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/statements/expressions/GrReferenceExpressionImpl.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/statements/expressions/GrReferenceExpressionImpl.java index c20426953418..55bd88601944 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/statements/expressions/GrReferenceExpressionImpl.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/statements/expressions/GrReferenceExpressionImpl.java @@ -144,10 +144,11 @@ public class GrReferenceExpressionImpl extends GrReferenceElementImpl implements for (String accessorName : accessorNames) { AccessorResolverProcessor accessorResolver = new AccessorResolverProcessor(accessorName, this, !isLValue); resolveImpl(accessorResolver); - final GroovyResolveResult[] candidates = accessorResolver.getCandidates(); //can be only one candidate - if (candidates.length == 1 && candidates[0].isStaticsOK()) { - if (isPropertyName || candidates[0].getElement() instanceof GrAccessorMethod) { - return candidates; + final GroovyResolveResult[] candidates = accessorResolver.getCandidates(); //can be only one correct candidate + if (candidates.length > 0 && candidates[candidates.length - 1].isStaticsOK()) { + if (isPropertyName || candidates[candidates.length - 1].getElement() instanceof GrAccessorMethod) { + if (candidates.length == 1) return candidates; + return new GroovyResolveResult[]{candidates[candidates.length - 1]}; } } else { @@ -627,43 +628,45 @@ public class GrReferenceExpressionImpl extends GrReferenceElementImpl implements } } - private void resolveImpl(ResolverProcessor processor) { + private boolean resolveImpl(ResolverProcessor processor) { GrExpression qualifier = getQualifierExpression(); if (qualifier == null) { ResolveUtil.treeWalkUp(this, processor, true); if (!processor.hasCandidates()) { qualifier = PsiImplUtil.getRuntimeQualifier(this); if (qualifier != null) { - processQualifier(processor, qualifier); + if (!processQualifier(processor, qualifier)) return false; } } } else { if (getDotTokenType() != GroovyTokenTypes.mSPREAD_DOT) { - processQualifier(processor, qualifier); + if (!processQualifier(processor, qualifier)) return false; } else { - processQualifierForSpreadDot(processor, qualifier); + if (!processQualifierForSpreadDot(processor, qualifier)) return false; } if (qualifier instanceof GrReferenceExpression && "class".equals(((GrReferenceExpression)qualifier).getReferenceName()) || qualifier instanceof GrThisReferenceExpression) { - processIfJavaLangClass(processor, qualifier.getType(), qualifier); + return processIfJavaLangClass(processor, qualifier.getType(), qualifier); } } + return true; } - private void processIfJavaLangClass(ResolverProcessor processor, PsiType type, GroovyPsiElement resolveContext) { + private boolean processIfJavaLangClass(ResolverProcessor processor, PsiType type, GroovyPsiElement resolveContext) { if (type instanceof PsiClassType) { final PsiClass psiClass = ((PsiClassType)type).resolve(); if (psiClass != null && CommonClassNames.JAVA_LANG_CLASS.equals(psiClass.getQualifiedName())) { final PsiType[] params = ((PsiClassType)type).getParameters(); if (params.length == 1) { - processClassQualifierType(processor, params[0], resolveContext); + if (!processClassQualifierType(processor, params[0], resolveContext)) return false; } } } + return true; } - private void processQualifierForSpreadDot(ResolverProcessor processor, GrExpression qualifier) { + private boolean processQualifierForSpreadDot(ResolverProcessor processor, GrExpression qualifier) { PsiType qualifierType = qualifier.getType(); if (qualifierType instanceof PsiClassType) { PsiClassType.ClassResolveResult result = ((PsiClassType) qualifierType).resolveGenerics(); @@ -676,37 +679,38 @@ public class GrReferenceExpressionImpl extends GrReferenceElementImpl implements if (substitutor != null) { PsiType componentType = substitutor.substitute(collection.getTypeParameters()[0]); if (componentType != null) { - processClassQualifierType(processor, componentType, qualifier); + return processClassQualifierType(processor, componentType, qualifier); } } } } } else if (qualifierType instanceof PsiArrayType) { - processClassQualifierType(processor, ((PsiArrayType) qualifierType).getComponentType(), qualifier); + return processClassQualifierType(processor, ((PsiArrayType) qualifierType).getComponentType(), qualifier); } + return true; } - private void processQualifier(ResolverProcessor processor, GrExpression qualifier) { + private boolean processQualifier(ResolverProcessor processor, GrExpression qualifier) { PsiType qualifierType = qualifier.getType(); if (qualifierType == null) { if (qualifier instanceof GrReferenceExpression) { PsiElement resolved = ((GrReferenceExpression) qualifier).resolve(); if (resolved instanceof PsiPackage) { - if (!resolved.processDeclarations(processor, ResolveState.initial().put(ResolverProcessor.RESOLVE_CONTEXT, qualifier), null, this)) //noinspection UnnecessaryReturnStatement - return; + return resolved.processDeclarations(processor, ResolveState.initial().put(ResolverProcessor.RESOLVE_CONTEXT, qualifier), null, + this); } else { qualifierType = TypesUtil.getJavaLangObject(this); - processClassQualifierType(processor, qualifierType, qualifier); + return processClassQualifierType(processor, qualifierType, qualifier); } } } else { if (qualifierType instanceof PsiIntersectionType) { for (PsiType conjunct : ((PsiIntersectionType) qualifierType).getConjuncts()) { - processClassQualifierType(processor, conjunct, qualifier); + if (!processClassQualifierType(processor, conjunct, qualifier)) return false; } } else { - processClassQualifierType(processor, qualifierType, qualifier); + if (!processClassQualifierType(processor, qualifierType, qualifier)) return false; if (qualifier instanceof GrReferenceExpression) { PsiElement resolved = ((GrReferenceExpression) qualifier).resolve(); if (resolved instanceof PsiClass) { //omitted .class @@ -720,17 +724,18 @@ public class GrReferenceExpressionImpl extends GrReferenceElementImpl implements substitutor = substitutor.put(typeParameters[0], qualifierType); state = state.put(PsiSubstitutor.KEY, substitutor); } - if (!javaLangClass.processDeclarations(processor, state, null, this)) return; + if (!javaLangClass.processDeclarations(processor, state, null, this)) return false; PsiType javaLangClassType = JavaPsiFacade.getInstance(getProject()).getElementFactory().createType(javaLangClass, substitutor); - ResolveUtil.processNonCodeMethods(javaLangClassType, processor, this, state); + if (!ResolveUtil.processNonCodeMethods(javaLangClassType, processor, this, state)) return false; } } } } } + return true; } - private void processClassQualifierType(ResolverProcessor processor, PsiType qualifierType, GroovyPsiElement resolveContext) { + private boolean processClassQualifierType(ResolverProcessor processor, PsiType qualifierType, GroovyPsiElement resolveContext) { final ResolveState state; if (qualifierType instanceof PsiClassType) { PsiClassType.ClassResolveResult qualifierResult = ((PsiClassType)qualifierType).resolveGenerics(); @@ -738,27 +743,25 @@ public class GrReferenceExpressionImpl extends GrReferenceElementImpl implements state = ResolveState.initial().put(PsiSubstitutor.KEY, qualifierResult.getSubstitutor()) .put(ResolverProcessor.RESOLVE_CONTEXT, resolveContext); if (qualifierClass != null) { - if (!qualifierClass.processDeclarations(processor, state, null, this)) { - return; - } + if (!qualifierClass.processDeclarations(processor, state, null, this)) return false; } } else if (qualifierType instanceof PsiArrayType) { final GrTypeDefinition arrayClass = GroovyPsiManager.getInstance(getProject()).getArrayClass(); state = ResolveState.initial(); - if (!arrayClass.processDeclarations(processor, state, null, this)) return; + if (!arrayClass.processDeclarations(processor, state, null, this)) return false; } else if (qualifierType instanceof PsiIntersectionType) { for (PsiType conjunct : ((PsiIntersectionType)qualifierType).getConjuncts()) { - processClassQualifierType(processor, conjunct, resolveContext); + if (!processClassQualifierType(processor, conjunct, resolveContext)) return false; } - return; + return true; } else { state = ResolveState.initial(); } - if (!ResolveUtil.processCategoryMembers(this, processor)) return; - ResolveUtil.processNonCodeMethods(qualifierType, processor, this, state); + if (!ResolveUtil.processCategoryMembers(this, processor)) return false; + return ResolveUtil.processNonCodeMethods(qualifierType, processor, this, state); } public MethodResolverProcessor runMethodResolverProcessor(String referenceName, PsiType[] argTypes, final boolean allVariants) { diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/resolve/processors/AccessorResolverProcessor.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/resolve/processors/AccessorResolverProcessor.java index b62139cf5ae7..5a57d4187ffc 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/resolve/processors/AccessorResolverProcessor.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/resolve/processors/AccessorResolverProcessor.java @@ -63,7 +63,7 @@ public class AccessorResolverProcessor extends ResolverProcessor { final GroovyPsiElement resolveContext = state.get(RESOLVE_CONTEXT); boolean isStaticsOK = isStaticsOK(method, resolveContext); addCandidate(new GroovyResolveResultImpl(method, resolveContext, substitutor, isAccessible, isStaticsOK, myIsPropertyInvoked)); - return !isAccessible; + return !isAccessible || !isStaticsOK; } private static boolean usedInCategory(GroovyPsiElement resolveContext) { diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/resolve/processors/ResolverProcessor.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/resolve/processors/ResolverProcessor.java index a7a918fa9a24..3f6884d90fa7 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/resolve/processors/ResolverProcessor.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/resolve/processors/ResolverProcessor.java @@ -90,7 +90,7 @@ public class ResolverProcessor implements PsiScopeProcessor, NameHint, ClassHint final GroovyPsiElement resolveContext = state.get(RESOLVE_CONTEXT); boolean isStaticsOK = isStaticsOK(namedElement, resolveContext); addCandidate(new GroovyResolveResultImpl(namedElement, resolveContext, substitutor, isAccessible, isStaticsOK)); - return !isAccessible; + return !isAccessible || !isStaticsOK; } return true; 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 a68cfb9474a8..780b018aa688 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyHighlightingTest.java +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyHighlightingTest.java @@ -317,4 +317,8 @@ public class GroovyHighlightingTest extends LightCodeInsightFixtureTestCase { public void testRangeType() { doTest(new GroovyRangeTypeCheckInspection()); } + + public void testResolveMetaClass() { + doTest(new GroovyAccessibilityInspection()); + } } \ No newline at end of file diff --git a/plugins/groovy/testdata/highlighting/ResolveMetaClass.groovy b/plugins/groovy/testdata/highlighting/ResolveMetaClass.groovy new file mode 100644 index 000000000000..0eaa5b023193 --- /dev/null +++ b/plugins/groovy/testdata/highlighting/ResolveMetaClass.groovy @@ -0,0 +1,6 @@ +class MyClass { + private field +} + +print MyClass.metaClass.getMethods() +print MyClass.field \ No newline at end of file From 7897cd7e0c0b1f3a99bd5ba4e3565e72776d159c Mon Sep 17 00:00:00 2001 From: Konstantin Bulenkov Date: Mon, 31 Jan 2011 12:53:22 +0300 Subject: [PATCH 5/6] IDEA-64501 Context menu keyboard button shows context menu, but focus is still in editor --- .../src/com/intellij/openapi/editor/impl/EditorImpl.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/platform/platform-impl/src/com/intellij/openapi/editor/impl/EditorImpl.java b/platform/platform-impl/src/com/intellij/openapi/editor/impl/EditorImpl.java index af207585d4a5..45c1d6e8c2dd 100644 --- a/platform/platform-impl/src/com/intellij/openapi/editor/impl/EditorImpl.java +++ b/platform/platform-impl/src/com/intellij/openapi/editor/impl/EditorImpl.java @@ -3354,7 +3354,10 @@ public final class EditorImpl extends UserDataHolderBase implements EditorEx, Hi } private void requestFocus() { - IdeFocusManager.getInstance(myProject).requestFocus(myEditorComponent, true); + final IdeFocusManager focusManager = IdeFocusManager.getInstance(myProject); + if (focusManager.getFocusOwner() != myEditorComponent) { //IDEA-64501 + focusManager.requestFocus(myEditorComponent, true); + } } private void validateMousePointer(MouseEvent e) { From 6889a473c61c0ada2c523f47a4b1ac36a3596c75 Mon Sep 17 00:00:00 2001 From: Denis Zhdanov Date: Mon, 31 Jan 2011 12:55:15 +0300 Subject: [PATCH 6/6] Checking if target offset belongs to document range; --- .../openapi/editor/GenericLineWrapPositionStrategy.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/platform/platform-api/src/com/intellij/openapi/editor/GenericLineWrapPositionStrategy.java b/platform/platform-api/src/com/intellij/openapi/editor/GenericLineWrapPositionStrategy.java index c32530cc9f3d..49a11039ef65 100644 --- a/platform/platform-api/src/com/intellij/openapi/editor/GenericLineWrapPositionStrategy.java +++ b/platform/platform-api/src/com/intellij/openapi/editor/GenericLineWrapPositionStrategy.java @@ -99,10 +99,10 @@ public class GenericLineWrapPositionStrategy implements LineWrapPositionStrategy // Try to find target offset that is beyond preferred offset. // Note that we don't consider symbol weights here and just break on the first appropriate position. - if (!allowToBeyondMaxPreferredOffset || maxPreferredOffset >= text.length() - 1) { + if (!allowToBeyondMaxPreferredOffset) { return maxPreferredOffset; } - for (int i = maxPreferredOffsetToUse + 1; i < endOffset; i++) { + for (int i = Math.min(maxPreferredOffsetToUse + 1, text.length() - 1); i < endOffset; i++) { char c = text.charAt(i); if (c == '\n') { return i + 1;