From 9536fba5492661e546b4c74c5b0fff38af33096b Mon Sep 17 00:00:00 2001 From: Eric Milles Date: Fri, 17 Aug 2018 04:54:11 -0500 Subject: [PATCH] Prep for #678: skip pseudo-property references in rename method search - SyntheticAccesorsRenameParticipant will need to find these references - Finding them with field search of SyntheticAccessorsSearchRequestor is causing extra references and unwanted renames of actual field references --- .../search/MethodReferenceSearchTests.java | 51 ++ .../search/FieldReferenceSearchRequestor.java | 5 +- .../MethodReferenceSearchRequestor.java | 12 +- .../test/SyntheticMemberSearchTests.groovy | 37 +- .../SyntheticAccessorRenamingTests.groovy | 570 ++++++++++++++++-- .../SyntheticAccessorSearchRequestor.java | 15 +- .../SyntheticAccessorsRenameParticipant.java | 155 ++--- .../SemanticReferenceRequestor.java | 24 +- .../search/GroovyOccurrencesFinder.java | 35 +- 9 files changed, 680 insertions(+), 224 deletions(-) diff --git a/base-test/org.eclipse.jdt.groovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/MethodReferenceSearchTests.java b/base-test/org.eclipse.jdt.groovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/MethodReferenceSearchTests.java index 3d64396797..13d320789e 100644 --- a/base-test/org.eclipse.jdt.groovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/MethodReferenceSearchTests.java +++ b/base-test/org.eclipse.jdt.groovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/MethodReferenceSearchTests.java @@ -389,6 +389,57 @@ public void testStaticMethodReferenceSearch() throws Exception { assertLocation(searchRequestor.getMatch(0), contents.lastIndexOf("bar"), "bar".length()); } + @Test + public void testExplicitPropertyGetterSearch() throws Exception { + GroovyCompilationUnit bar = createUnit("foo", "Bar", + "package foo\n" + + "class Bar {\n" + + " static String string\n" + + " static String getString() {\n" + + " }\n" + + "}"); + GroovyCompilationUnit baz = createUnit("foo", "Baz", + "package foo\n" + + "import static foo.Bar.getString as blah\n" + // potential + "import static foo.Bar.getString as getS\n" + // potential + "Bar.getString()\n" + // exact + "Bar.'getString'()\n" + // exact + "str = Bar.string\n" + // potential + "str = Bar.@string\n" + + "fun = Bar.&getString\n" + // potential + "str = blah()\n" + // potential + "str = s\n" + // potential + ""); + + IMethod method = bar.getType("Bar").getMethods()[0]; + new SearchEngine().search( + SearchPattern.createPattern(method, IJavaSearchConstants.REFERENCES), + new SearchParticipant[] {SearchEngine.getDefaultSearchParticipant()}, + SearchEngine.createJavaSearchScope(new IJavaElement[] {bar.getPackageFragmentRoot()}, false), + searchRequestor, new NullProgressMonitor()); + + List matches = searchRequestor.getMatches(); + + assertEquals(8, matches.size()); + assertEquals(SearchMatch.A_INACCURATE, matches.get(0).getAccuracy()); + assertEquals(String.valueOf(baz.getContents()).indexOf("getString as blah"), matches.get(0).getOffset()); + assertEquals(SearchMatch.A_INACCURATE, matches.get(1).getAccuracy()); + assertEquals(String.valueOf(baz.getContents()).indexOf("getString as getS"), matches.get(1).getOffset()); + assertEquals(SearchMatch.A_ACCURATE, matches.get(2).getAccuracy()); + assertEquals(String.valueOf(baz.getContents()).indexOf("getString()"), matches.get(2).getOffset()); + assertEquals(SearchMatch.A_ACCURATE, matches.get(3).getAccuracy()); + assertEquals(String.valueOf(baz.getContents()).indexOf("'getString'"), matches.get(3).getOffset()); + + assertEquals(SearchMatch.A_INACCURATE, matches.get(4).getAccuracy()); + assertEquals(String.valueOf(baz.getContents()).indexOf("Bar.string") + 4, matches.get(4).getOffset()); + assertEquals(SearchMatch.A_INACCURATE, matches.get(5).getAccuracy()); + assertEquals(String.valueOf(baz.getContents()).indexOf("Bar.&getString") + 5, matches.get(5).getOffset()); + assertEquals(SearchMatch.A_INACCURATE, matches.get(6).getAccuracy()); + assertEquals(String.valueOf(baz.getContents()).lastIndexOf("blah"), matches.get(6).getOffset()); + assertEquals(SearchMatch.A_INACCURATE, matches.get(7).getAccuracy()); + assertEquals(String.valueOf(baz.getContents()).lastIndexOf("s"), matches.get(7).getOffset()); + } + @Test // https://github.com/groovy/groovy-eclipse/issues/489 public void testExplicitPropertySetterSearch() throws Exception { GroovyCompilationUnit bar = createUnit("foo", "Bar", diff --git a/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/FieldReferenceSearchRequestor.java b/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/FieldReferenceSearchRequestor.java index 5b52bbc071..86ec792605 100644 --- a/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/FieldReferenceSearchRequestor.java +++ b/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/FieldReferenceSearchRequestor.java @@ -49,7 +49,7 @@ public class FieldReferenceSearchRequestor implements ITypeRequestor { protected final String fieldName, declaringQualifiedName; protected final Set acceptedPositions = new HashSet<>(); - protected final boolean readAccess, writeAccess, findReferences, findDeclarations, findPseudoProperties; + protected final boolean readAccess, writeAccess, findReferences, findDeclarations; public FieldReferenceSearchRequestor(FieldPattern pattern, SearchRequestor requestor, SearchParticipant participant) { this.requestor = requestor; @@ -67,7 +67,6 @@ public FieldReferenceSearchRequestor(FieldPattern pattern, SearchRequestor reque writeAccess = (Boolean) ReflectionUtils.getPrivateField(VariablePattern.class, "writeAccess", pattern); findReferences = (Boolean) ReflectionUtils.getPrivateField(VariablePattern.class, "findReferences", pattern); findDeclarations = (Boolean) ReflectionUtils.getPrivateField(VariablePattern.class, "findDeclarations", pattern); - findPseudoProperties = requestor.getClass().getName().contains("SyntheticAccessorSearch"); // accessor as property } @Override @@ -117,7 +116,7 @@ public VisitStatus acceptASTNode(ASTNode node, TypeLookupResult result, IJavaEle end = node.getEnd(); // check for "foo.bar" where "bar" refers to generated/synthetic "getBar()", "isBar()" or "setBar(...)" - } else if (result.declaration instanceof MethodNode && (((MethodNode) result.declaration).isSynthetic() || findPseudoProperties)) { + } else if (result.declaration instanceof MethodNode && (((MethodNode) result.declaration).isSynthetic())) { doCheck = true; isAssignment = ((MethodNode) result.declaration).getName().startsWith("set"); start = node.getStart(); diff --git a/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/MethodReferenceSearchRequestor.java b/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/MethodReferenceSearchRequestor.java index 51a6d4c333..7e31908f2b 100644 --- a/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/MethodReferenceSearchRequestor.java +++ b/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/MethodReferenceSearchRequestor.java @@ -62,10 +62,10 @@ public class MethodReferenceSearchRequestor implements ITypeRequestor { protected final String methodName; protected final String declaringTypeName; protected final String[] parameterTypeNames; + protected final boolean findReferences, findDeclarations, skipPseudoProperties; - protected final boolean findDeclarations, findReferences; protected final Set acceptedPositions = new HashSet<>(); - protected static final int MAX_PARAMS = 10; // indices available in each boolean array of: + protected static final int MAX_PARAMS = 10; // indices available in each array of: protected final Map cachedParameterCounts = new HashMap<>(); protected final Map cachedDeclaringNameMatches = new HashMap<>(); @@ -115,6 +115,7 @@ public MethodReferenceSearchRequestor(MethodPattern pattern, SearchRequestor req findReferences = (Boolean) ReflectionUtils.getPrivateField(MethodPattern.class, "findReferences", pattern); findDeclarations = (Boolean) ReflectionUtils.getPrivateField(MethodPattern.class, "findDeclarations", pattern); + skipPseudoProperties = requestor.getClass().getName().equals("org.eclipse.jdt.internal.corext.refactoring.rename.MethodOccurenceCollector"); } protected static String[] getParameterTypeNames(MethodPattern pattern, String[] parameterTypeSignatures, IType declaringType) { @@ -209,8 +210,13 @@ public VisitStatus acceptASTNode(ASTNode node, TypeLookupResult result, IJavaEle start = ((AnnotatedNode) node).getNameStart(); end = ((AnnotatedNode) node).getNameEnd() + 1; + // check for "foo.bar" where "bar" refers to "getBar()", "isBar()" or "setBar(...)" + if (!isDeclaration && (end - start) < ((StaticMethodCallExpression) node).getMethod().length() && skipPseudoProperties) { + end = 0; + } + // check for non-synthetic match; SyntheticAccessorSearchRequestor matches "foo.bar" to "getBar()", etc. - } else if (node.getText().equals(methodName) || isNotSynthetic(node.getText(), result.declaringType)) { + } else if (node.getText().equals(methodName) || (isNotSynthetic(node.getText(), result.declaringType) && !skipPseudoProperties)) { start = node.getStart(); end = node.getEnd(); } diff --git a/ide-test/org.codehaus.groovy.eclipse.core.test/src/org/codehaus/groovy/eclipse/core/test/SyntheticMemberSearchTests.groovy b/ide-test/org.codehaus.groovy.eclipse.core.test/src/org/codehaus/groovy/eclipse/core/test/SyntheticMemberSearchTests.groovy index 33bdb6bcb6..7fd4942df3 100644 --- a/ide-test/org.codehaus.groovy.eclipse.core.test/src/org/codehaus/groovy/eclipse/core/test/SyntheticMemberSearchTests.groovy +++ b/ide-test/org.codehaus.groovy.eclipse.core.test/src/org/codehaus/groovy/eclipse/core/test/SyntheticMemberSearchTests.groovy @@ -1,3 +1,18 @@ +/* + * Copyright 2009-2018 the original author or authors. + * + * 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. + */ /* * Copyright 2009-2018 the original author or authors. * @@ -43,7 +58,7 @@ final class SyntheticMemberSearchTests extends GroovyEclipseTestSuite { gType = gUnit.getType('G') } - @Test + @Test // references to property itself and synthetic accessors void testSearchInGroovy1() { String contents = '''\ new p.G().prop @@ -54,7 +69,6 @@ final class SyntheticMemberSearchTests extends GroovyEclipseTestSuite { addGroovySource(contents, nextUnitName()) List matches = performSearch('prop') - // expecting 3 matches since the explicit property reference will not be found since it is not synthetic assertNumMatch(3, matches) assertNoMatch('run', 'prop', contents, matches) assertMatch('run', 'isProp', contents, matches) @@ -62,7 +76,7 @@ final class SyntheticMemberSearchTests extends GroovyEclipseTestSuite { assertMatch('run', 'setProp', contents, matches) } - @Test + @Test // references to pseudo-property and non-synthetic accessors void testSearchInGroovy2() { String contents = '''\ new p.G().explicit @@ -73,14 +87,14 @@ final class SyntheticMemberSearchTests extends GroovyEclipseTestSuite { addGroovySource(contents, nextUnitName()) List matches = performSearch('getExplicit') - assertNumMatch(1, matches) - assertMatch('run', 'explicit', contents, matches) + assertNumMatch(0, matches) + assertNoMatch('run', 'explicit', contents, matches) assertNoMatch('run', 'isExplicit', contents, matches) assertNoMatch('run', 'getExplicit', contents, matches) assertNoMatch('run', 'setExplicit', contents, matches) } - @Test + @Test // references to pseudo-property and non-synthetic accessors void testSearchInGroovy3() { String contents = '''\ new p.G().explicit @@ -91,14 +105,14 @@ final class SyntheticMemberSearchTests extends GroovyEclipseTestSuite { addGroovySource(contents, nextUnitName()) List matches = performSearch('setExplicit') - assertNumMatch(1, matches) - assertMatch('run', 'explicit', contents, matches) + assertNumMatch(0, matches) + assertNoMatch('run', 'explicit', contents, matches) assertNoMatch('run', 'isExplicit', contents, matches) assertNoMatch('run', 'getExplicit', contents, matches) assertNoMatch('run', 'setExplicit', contents, matches) } - @Test + @Test // references to pseudo-property and non-synthetic accessors void testSearchInGroovy4() { String contents = '''\ new p.G().explicit @@ -109,8 +123,8 @@ final class SyntheticMemberSearchTests extends GroovyEclipseTestSuite { addGroovySource(contents, nextUnitName()) List matches = performSearch('isExplicit') - assertNumMatch(1, matches) - assertMatch('run', 'explicit', contents, matches) + assertNumMatch(0, matches) + assertNoMatch('run', 'explicit', contents, matches) assertNoMatch('run', 'isExplicit', contents, matches) assertNoMatch('run', 'getExplicit', contents, matches) assertNoMatch('run', 'setExplicit', contents, matches) @@ -154,7 +168,6 @@ final class SyntheticMemberSearchTests extends GroovyEclipseTestSuite { addJavaSource(contents, 'ClassB') List matches = performSearch('prop') - // expecting 3 matches since the explicit property reference will not be found since it is not synthetic assertNumMatch(3, matches) assertNoMatch('run', 'prop', contents, matches) assertMatch('run', 'isProp()', contents, matches) diff --git a/ide-test/org.codehaus.groovy.eclipse.refactoring.test/src/org/codehaus/groovy/eclipse/refactoring/test/rename/SyntheticAccessorRenamingTests.groovy b/ide-test/org.codehaus.groovy.eclipse.refactoring.test/src/org/codehaus/groovy/eclipse/refactoring/test/rename/SyntheticAccessorRenamingTests.groovy index 315693b1d5..e105c5235f 100644 --- a/ide-test/org.codehaus.groovy.eclipse.refactoring.test/src/org/codehaus/groovy/eclipse/refactoring/test/rename/SyntheticAccessorRenamingTests.groovy +++ b/ide-test/org.codehaus.groovy.eclipse.refactoring.test/src/org/codehaus/groovy/eclipse/refactoring/test/rename/SyntheticAccessorRenamingTests.groovy @@ -76,9 +76,12 @@ final class SyntheticAccessorRenamingTests extends RenameRefactoringTestSuite { boolean foo def run() { foo - getFoo() - setFoo() isFoo() + getFoo() + setFoo(true) + this.&isFoo + this.&getFoo + this.&setFoo } } '''.stripIndent(), @@ -88,9 +91,49 @@ final class SyntheticAccessorRenamingTests extends RenameRefactoringTestSuite { boolean flar def run() { flar + isFlar() getFlar() - setFlar() + setFlar(true) + this.&isFlar + this.&getFlar + this.&setFlar + } + } + '''.stripIndent(), + )) + } + + @Test + void testSingleFileRename2() { + performRefactoringAndUndo('flar', new TestSource( + pack: 'p', name: 'First.groovy', + contents: '''\ + package p + class First { + static boolean foo + static { + foo + isFoo() + getFoo() + setFoo(true) + this.&isFoo + this.&getFoo + this.&setFoo + } + } + '''.stripIndent(), + finalContents: '''\ + package p + class First { + static boolean flar + static { + flar isFlar() + getFlar() + setFlar(true) + this.&isFlar + this.&getFlar + this.&setFlar } } '''.stripIndent(), @@ -98,21 +141,24 @@ final class SyntheticAccessorRenamingTests extends RenameRefactoringTestSuite { } @Test // don't automatically rename if the accessors are explicitly named - void testSingleFileRename2() { + void testSingleFileRename3() { performRefactoringAndUndo('flar', new TestSource( pack: 'p', name: 'First.groovy', contents: '''\ package p class First { def foo - def getFoo() { } - void setFoo() { } - def isFoo() { } + def getFoo() {} + boolean isFoo() {} + void setFoo(val) {} def run() { foo - getFoo() - setFoo() isFoo() + getFoo() + setFoo(null) + this.&isFoo + this.&getFoo + this.&setFoo } } '''.stripIndent(), @@ -120,14 +166,60 @@ final class SyntheticAccessorRenamingTests extends RenameRefactoringTestSuite { package p class First { def flar - def getFoo() { } - void setFoo() { } - def isFoo() { } + def getFoo() {} + boolean isFoo() {} + void setFoo(val) {} def run() { flar + isFoo() getFoo() - setFoo() + setFoo(null) + this.&isFoo + this.&getFoo + this.&setFoo + } + } + '''.stripIndent() + )) + } + + @Test // don't automatically rename if the accessors are explicitly named + void testSingleFileRename4() { + performRefactoringAndUndo('flar', new TestSource( + pack: 'p', name: 'First.groovy', + contents: '''\ + package p + class First { + static def foo + static def getFoo() {} + static boolean isFoo() {} + static void setFoo(val) {} + static { + foo isFoo() + getFoo() + setFoo(null) + this.&isFoo + this.&getFoo + this.&setFoo + } + } + '''.stripIndent(), + finalContents: '''\ + package p + class First { + static def flar + static def getFoo() {} + static boolean isFoo() {} + static void setFoo(val) {} + static { + flar + isFoo() + getFoo() + setFoo(null) + this.&isFoo + this.&getFoo + this.&setFoo } } '''.stripIndent() @@ -156,41 +248,76 @@ final class SyntheticAccessorRenamingTests extends RenameRefactoringTestSuite { package q def f = new p.First() f.foo - f.getFoo() - f.setFoo() f.isFoo() + f.getFoo() + f.setFoo(true) '''.stripIndent(), finalContents: '''\ package q def f = new p.First() f.flar - f.getFlar() - f.setFlar() f.isFlar() + f.getFlar() + f.setFlar(true) '''.stripIndent() )) } - @Test // don't automatically rename if the accessors are explicitly named + @Test void testMultiFileRename2() { + performRefactoringAndUndo('flar', new TestSource( + pack: 'p', name: 'First.groovy', + contents: '''\ + package p + class First { + static boolean foo + } + '''.stripIndent(), + finalContents: '''\ + package p + class First { + static boolean flar + } + '''.stripIndent() + ), new TestSource( + pack: 'q', name: 'Script.groovy', + contents: '''\ + package q + p.First.foo + p.First.isFoo() + p.First.getFoo() + p.First.setFoo(true) + '''.stripIndent(), + finalContents: '''\ + package q + p.First.flar + p.First.isFlar() + p.First.getFlar() + p.First.setFlar(true) + '''.stripIndent() + )) + } + + @Test // don't automatically rename if the accessors are explicitly named + void testMultiFileRename3() { performRefactoringAndUndo('flar', new TestSource( pack: 'p', name: 'First.groovy', contents: '''\ package p class First { def foo - def getFoo() { } - void setFoo() { } - def isFoo() { } + def getFoo() {} + boolean isFoo() {} + void setFoo(val) {} } '''.stripIndent(), finalContents: '''\ package p class First { def flar - def getFoo() { } - void setFoo() { } - def isFoo() { } + def getFoo() {} + boolean isFoo() {} + void setFoo(val) {} } '''.stripIndent() ), new TestSource( @@ -199,17 +326,58 @@ final class SyntheticAccessorRenamingTests extends RenameRefactoringTestSuite { package q def f = new p.First() f.foo - f.getFoo() - f.setFoo() f.isFoo() + f.getFoo() + f.setFoo(null) '''.stripIndent(), finalContents: '''\ package q def f = new p.First() f.foo - f.getFoo() - f.setFoo() f.isFoo() + f.getFoo() + f.setFoo(null) + '''.stripIndent() + )) + } + + @Test // don't automatically rename if the accessors are explicitly named + void testMultiFileRename4() { + performRefactoringAndUndo('flar', new TestSource( + pack: 'p', name: 'First.groovy', + contents: '''\ + package p + class First { + static def foo + static def getFoo() {} + static boolean isFoo() {} + static void setFoo(val) {} + } + '''.stripIndent(), + finalContents: '''\ + package p + class First { + static def flar + static def getFoo() {} + static boolean isFoo() {} + static void setFoo(val) {} + } + '''.stripIndent() + ), new TestSource( + pack: 'q', name: 'Script.groovy', + contents: '''\ + package q + p.First.foo + p.First.isFoo() + p.First.getFoo() + p.First.setFoo(null) + '''.stripIndent(), + finalContents: '''\ + package q + p.First.foo + p.First.isFoo() + p.First.getFoo() + p.First.setFoo(null) '''.stripIndent() )) } @@ -237,9 +405,9 @@ final class SyntheticAccessorRenamingTests extends RenameRefactoringTestSuite { class Java { void m(p.First f) { f.foo = null; - f.getFoo(); - f.setFoo(null); f.isFoo(); + f.getFoo(); + f.setFoo(true); } } '''.stripIndent(), @@ -248,9 +416,9 @@ final class SyntheticAccessorRenamingTests extends RenameRefactoringTestSuite { class Java { void m(p.First f) { f.flar = null; - f.getFlar(); - f.setFlar(null); f.isFlar(); + f.getFlar(); + f.setFlar(true); } } '''.stripIndent() @@ -265,18 +433,18 @@ final class SyntheticAccessorRenamingTests extends RenameRefactoringTestSuite { package p class First { def foo - def getFoo() { } - void setFoo(arg) { } - def isFoo() { } + def getFoo() {} + boolean isFoo() {} + void setFoo(val) {} } '''.stripIndent(), finalContents: '''\ package p class First { def flar - def getFoo() { } - void setFoo(arg) { } - def isFoo() { } + def getFoo() {} + boolean isFoo() {} + void setFoo(val) {} } '''.stripIndent() ), new TestSource( @@ -286,9 +454,9 @@ final class SyntheticAccessorRenamingTests extends RenameRefactoringTestSuite { class Java { void m(p.First f) { f.foo = null; + f.isFoo(); f.getFoo(); f.setFoo(null); - f.isFoo(); } } '''.stripIndent(), @@ -297,9 +465,9 @@ final class SyntheticAccessorRenamingTests extends RenameRefactoringTestSuite { class Java { void m(p.First f) { f.flar = null; + f.isFoo(); f.getFoo(); f.setFoo(null); - f.isFoo(); } } '''.stripIndent() @@ -313,13 +481,13 @@ final class SyntheticAccessorRenamingTests extends RenameRefactoringTestSuite { contents: '''\ package p class First { - def getFoo() { } + def getFoo() {} } '''.stripIndent(), finalContents: '''\ package p class First { - def getFlar() { } + def getFlar() {} } '''.stripIndent() ), new TestSource( @@ -366,13 +534,13 @@ final class SyntheticAccessorRenamingTests extends RenameRefactoringTestSuite { contents: '''\ package p class First { - def isFoo() { } + def isFoo() {} } '''.stripIndent(), finalContents: '''\ package p class First { - def isFlar() { } + def isFlar() {} } '''.stripIndent() ), new TestSource( @@ -419,13 +587,13 @@ final class SyntheticAccessorRenamingTests extends RenameRefactoringTestSuite { contents: '''\ package p class First { - void setFoo(value) { } + void setFoo(value) {} } '''.stripIndent(), finalContents: '''\ package p class First { - void setFlar(value) { } + void setFlar(value) {} } '''.stripIndent() ), new TestSource( @@ -472,13 +640,13 @@ final class SyntheticAccessorRenamingTests extends RenameRefactoringTestSuite { contents: '''\ package p class First { - void setFoo(value) { } + void setFoo(value) {} } '''.stripIndent(), finalContents: '''\ package p class First { - void setFooBar(value) { } + void setFooBar(value) {} } '''.stripIndent() ), new TestSource( @@ -505,13 +673,13 @@ final class SyntheticAccessorRenamingTests extends RenameRefactoringTestSuite { contents: '''\ package p class First { - static def getFoo() { } + static def getFoo() {} } '''.stripIndent(), finalContents: '''\ package p class First { - static def getFlar() { } + static def getFlar() {} } '''.stripIndent() ), new TestSource( @@ -549,20 +717,20 @@ final class SyntheticAccessorRenamingTests extends RenameRefactoringTestSuite { )) } - @Test @NotYetImplemented // this has compile errors, but it should still work + @Test // this has compile errors, but it should still work void testStaticGetterOnly2() { performRefactoringAndUndo('getFlar', new TestSource( pack: 'p', name: 'First.groovy', contents: '''\ package p class First { - static def getFoo() { } + static def getFoo() {} } '''.stripIndent(), finalContents: '''\ package p class First { - static def getFlar() { } + static def getFlar() {} } '''.stripIndent() ), new TestSource( @@ -611,13 +779,13 @@ final class SyntheticAccessorRenamingTests extends RenameRefactoringTestSuite { contents: '''\ package p class First { - static boolean isFoo() { } + static boolean isFoo() {} } '''.stripIndent(), finalContents: '''\ package p class First { - static boolean isFlar() { } + static boolean isFlar() {} } '''.stripIndent() ), new TestSource( @@ -655,20 +823,20 @@ final class SyntheticAccessorRenamingTests extends RenameRefactoringTestSuite { )) } - @Test @NotYetImplemented // this has compile errors, but it should still work + @Test // this has compile errors, but it should still work void testStaticIsserOnly2() { performRefactoringAndUndo('isFlar', new TestSource( pack: 'p', name: 'First.groovy', contents: '''\ package p class First { - static boolean isFoo() { } + static boolean isFoo() {} } '''.stripIndent(), finalContents: '''\ package p class First { - static boolean isFlar() { } + static boolean isFlar() {} } '''.stripIndent() ), new TestSource( @@ -717,13 +885,13 @@ final class SyntheticAccessorRenamingTests extends RenameRefactoringTestSuite { contents: '''\ package p class First { - static void setFoo(value) { } + static void setFoo(value) {} } '''.stripIndent(), finalContents: '''\ package p class First { - static void setFlar(value) { } + static void setFlar(value) {} } '''.stripIndent() ), new TestSource( @@ -761,20 +929,20 @@ final class SyntheticAccessorRenamingTests extends RenameRefactoringTestSuite { )) } - @Test @NotYetImplemented // this has compile errors, but it should still work + @Test // this has compile errors, but it should still work void testStaticSetterOnly2() { performRefactoringAndUndo('setFlar', new TestSource( pack: 'p', name: 'First.groovy', contents: '''\ package p class First { - static void setFoo(value) { } + static void setFoo(value) {} } '''.stripIndent(), finalContents: '''\ package p class First { - static void setFlar(value) { } + static void setFlar(value) {} } '''.stripIndent() ), new TestSource( @@ -823,13 +991,13 @@ final class SyntheticAccessorRenamingTests extends RenameRefactoringTestSuite { contents: '''\ package p class First { - static void setFoo(value) { } + static void setFoo(value) {} } '''.stripIndent(), finalContents: '''\ package p class First { - static void setFooBar(value) { } + static void setFooBar(value) {} } '''.stripIndent() ), new TestSource( @@ -933,5 +1101,275 @@ final class SyntheticAccessorRenamingTests extends RenameRefactoringTestSuite { )) } - // TODO: Repeat testFieldAndGetterAndSetterN with rename accessors enabled. + @Test + void testFieldAndGetterAndSetter3() { + renameGetters = renameSetters = true + + performRefactoringAndUndo('fooBar', new TestSource( + pack: 'p', name: 'First.groovy', + contents: '''\ + package p + class First { + private int foo = 0 + int getFoo() { return this.foo } + void setFoo(int value) { this.foo = value } + } + '''.stripIndent(), + finalContents: '''\ + package p + class First { + private int fooBar = 0 + int getFooBar() { return this.fooBar } + void setFooBar(int value) { this.fooBar = value } + } + '''.stripIndent() + ), new TestSource( + pack: 'q', name: 'Script.groovy', + contents: '''\ + package q + def m(p.First f) { + int i = f.@foo + i = f.foo + f.foo = i + } + '''.stripIndent(), + finalContents: '''\ + package q + def m(p.First f) { + int i = f.@fooBar + i = f.fooBar + f.fooBar = i + } + '''.stripIndent() + )) + } + + @Test // field and accessor types don't match exactly + void testFieldAndGetterAndSetter4() { + renameGetters = renameSetters = true + + performRefactoringAndUndo('fooBar', new TestSource( + pack: 'p', name: 'First.groovy', + contents: '''\ + package p + import java.util.concurrent.atomic.AtomicInteger + class First { + private final AtomicInteger foo = new AtomicInteger() + int getFoo() { this.foo.get() } + void setFoo(int value) { this.foo.set(value) } + } + '''.stripIndent(), + finalContents: '''\ + package p + import java.util.concurrent.atomic.AtomicInteger + class First { + private final AtomicInteger fooBar = new AtomicInteger() + int getFooBar() { this.fooBar.get() } + void setFoo(int value) { this.fooBar.set(value) } + } + '''.stripIndent() + ), new TestSource( + pack: 'q', name: 'Script.groovy', + contents: '''\ + package q + def m(p.First f) { + int i = f.@foo.get() + i = f.foo + f.foo = i + } + '''.stripIndent(), + finalContents: '''\ + package q + def m(p.First f) { + int i = f.@fooBar.get() + i = f.fooBar + f.foo = i + } + '''.stripIndent() + )) + } + + @Test + void testAliasedAccess() { + performRefactoringAndUndo('getFooBar', new TestSource( + pack: 'p', name: 'First.groovy', + contents: '''\ + package p + import static p.First.getFoo as blah + import static p.First.getFoo as getF + class First { + static def getFoo() { 'foo' } + static def foo + static { + blah() + foo + f + } + } + '''.stripIndent(), + finalContents: '''\ + package p + import static p.First.getFooBar as blah + import static p.First.getFooBar as getF + class First { + static def getFooBar() { 'foo' } + static def foo + static { + blah() + foo + f + } + } + '''.stripIndent() + )) + } + + @Test @NotYetImplemented + void testDelegateAccess() { + performRefactoringAndUndo('fooBar', new TestSource( + pack: 'p', name: 'First.groovy', + contents: '''\ + package p + class First { + def foo + } + '''.stripIndent(), + finalContents: '''\ + package p + class First { + def fooBar + } + '''.stripIndent() + ), new TestSource( + pack: 'q', name: 'Second.groovy', + contents: '''\ + package q + class Second { + @Delegate First first + } + '''.stripIndent(), + finalContents: '''\ + package q + class Second { + @Delegate First first + } + '''.stripIndent() + ), new TestSource( + pack: 'r', name: 'Script.groovy', + contents: '''\ + package r + def s = new q.Second() + s.foo + s.getFoo() + s.setFoo(null) + '''.stripIndent(), + finalContents: '''\ + package r + def s = new q.Second() + s.fooBar + s.getFooBar() + s.setFooBar(null) + '''.stripIndent() + )) + } + + @Test @NotYetImplemented + void testSubclassAccess() { + performRefactoringAndUndo('fooBar', new TestSource( + pack: 'p', name: 'First.groovy', + contents: '''\ + package p + class First { + def foo + } + '''.stripIndent(), + finalContents: '''\ + package p + class First { + def fooBar + } + '''.stripIndent() + ), new TestSource( + pack: 'q', name: 'Second.groovy', + contents: '''\ + package q + class Second extends First { + } + '''.stripIndent(), + finalContents: '''\ + package q + class Second extends First { + } + '''.stripIndent() + ), new TestSource( + pack: 'r', name: 'Script.groovy', + contents: '''\ + package r + def s = new q.Second() + s.foo + s.getFoo() + s.setFoo(null) + '''.stripIndent(), + finalContents: '''\ + package r + def s = new q.Second() + s.fooBar + s.getFooBar() + s.setFooBar(null) + '''.stripIndent() + )) + } + + @Test @NotYetImplemented + void testSubclassOverride() { + performRefactoringAndUndo('fooBar', new TestSource( + pack: 'p', name: 'First.groovy', + contents: '''\ + package p + class First { + def foo + } + '''.stripIndent(), + finalContents: '''\ + package p + class First { + def fooBar + } + '''.stripIndent() + ), new TestSource( + pack: 'q', name: 'Second.groovy', + contents: '''\ + package q + class Second extends First { + @Override + def getFoo() {} + } + '''.stripIndent(), + finalContents: '''\ + package q + class Second extends First { + @Override + def getFooBar() {} + } + '''.stripIndent() + ), new TestSource( + pack: 'r', name: 'Script.groovy', + contents: '''\ + package r + def s = new q.Second() + s.foo + s.getFoo() + s.setFoo(null) + '''.stripIndent(), + finalContents: '''\ + package r + def s = new q.Second() + s.fooBar + s.getFooBar() + s.setFooBar(null) + '''.stripIndent() + )) + } + + // TODO: .&, ::, .'getFoo', new First(foo: ...) } diff --git a/ide/org.codehaus.groovy.eclipse.core/src/org/codehaus/groovy/eclipse/core/search/SyntheticAccessorSearchRequestor.java b/ide/org.codehaus.groovy.eclipse.core/src/org/codehaus/groovy/eclipse/core/search/SyntheticAccessorSearchRequestor.java index abcad56352..4c806e50b0 100644 --- a/ide/org.codehaus.groovy.eclipse.core/src/org/codehaus/groovy/eclipse/core/search/SyntheticAccessorSearchRequestor.java +++ b/ide/org.codehaus.groovy.eclipse.core/src/org/codehaus/groovy/eclipse/core/search/SyntheticAccessorSearchRequestor.java @@ -1,5 +1,5 @@ /* - * Copyright 2009-2017 the original author or authors. + * Copyright 2009-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -47,8 +47,8 @@ */ public class SyntheticAccessorSearchRequestor { - public void findSyntheticMatches(IJavaElement element, ISearchRequestor uiRequestor, IProgressMonitor monitor) throws CoreException { - findSyntheticMatches(element, IJavaSearchConstants.REFERENCES | IJavaSearchConstants.IGNORE_RETURN_TYPE, new SearchParticipant[] {new JavaSearchParticipant()}, SearchEngine.createWorkspaceScope(), uiRequestor, monitor); + public void findSyntheticMatches(IJavaElement element, ISearchRequestor requestor, IProgressMonitor monitor) throws CoreException { + findSyntheticMatches(element, IJavaSearchConstants.REFERENCES | IJavaSearchConstants.IGNORE_RETURN_TYPE, new SearchParticipant[] {new JavaSearchParticipant()}, SearchEngine.createWorkspaceScope(), requestor, monitor); } public void findSyntheticMatches(IJavaElement element, int limitTo, SearchParticipant[] participants, IJavaSearchScope scope, final ISearchRequestor requestor, IProgressMonitor monitor) throws CoreException { @@ -102,13 +102,13 @@ private IMethod findSyntheticMember(IJavaElement element, String prefix) throws } IField field = (IField) element; - boolean isser = prefix.equals("is"); - boolean setter = prefix.equals("set"); + boolean isser = "is".equals(prefix); + boolean setter = "set".equals(prefix); if (setter && Flags.isFinal(field.getFlags())) { return null; } - if (isser && !field.getTypeSignature().equals("Z")) { + if (isser && !field.getTypeSignature().equals("Z")) { // TODO: What about "java.lang.Boolean"? return null; } @@ -138,7 +138,8 @@ private IField findSyntheticProperty(IJavaElement element) throws JavaModelExcep prefixLength = 3; } - final IField field = ((IType) element.getParent()).getField(Introspector.decapitalize(name.substring(prefixLength))); + name = Introspector.decapitalize(name.substring(prefixLength)); + final IField field = ((IType) element.getParent()).getField(name); // only return if field doesn't exist since otherwise, this method wouldn't be synthetic return field.exists() ? null : syntheticMemberProxy(IField.class, field, "Z"); // type signature appears unused } diff --git a/ide/org.codehaus.groovy.eclipse.refactoring/src/org/codehaus/groovy/eclipse/refactoring/core/rename/SyntheticAccessorsRenameParticipant.java b/ide/org.codehaus.groovy.eclipse.refactoring/src/org/codehaus/groovy/eclipse/refactoring/core/rename/SyntheticAccessorsRenameParticipant.java index 0f4171cc4c..fb0c19250e 100644 --- a/ide/org.codehaus.groovy.eclipse.refactoring/src/org/codehaus/groovy/eclipse/refactoring/core/rename/SyntheticAccessorsRenameParticipant.java +++ b/ide/org.codehaus.groovy.eclipse.refactoring/src/org/codehaus/groovy/eclipse/refactoring/core/rename/SyntheticAccessorsRenameParticipant.java @@ -15,18 +15,18 @@ */ package org.codehaus.groovy.eclipse.refactoring.core.rename; +import java.beans.Introspector; import java.util.ArrayList; -import java.util.Collections; import java.util.HashMap; import java.util.Iterator; import java.util.List; import java.util.Map; -import java.util.Map.Entry; import java.util.Set; import org.codehaus.groovy.eclipse.core.GroovyCore; import org.codehaus.groovy.eclipse.core.search.SyntheticAccessorSearchRequestor; import org.codehaus.groovy.eclipse.refactoring.core.utils.StatusHelper; +import org.codehaus.groovy.runtime.MetaClassHelper; import org.codehaus.jdt.groovy.model.GroovyNature; import org.eclipse.core.resources.IFile; import org.eclipse.core.resources.IResource; @@ -68,15 +68,10 @@ import org.eclipse.text.edits.TextEditGroup; /** - * A rename refactoring participant for renaming synthetic groovy properties and - * accessors. - * - * Renames calls to synthetic getters, setters and issers in groovy and java - * files for - * groovy properties - * - * Renames accesses to synthetic groovy properties that are backed by a getter, - * setter, and/or isser. + * A rename refactoring participant for renaming synthetic Groovy properties and + * accessors. Renames calls to synthetic getters, setters and issers in groovy + * and java files for groovy properties. Renames accesses to synthetic groovy + * properties that are backed by a getter, setter, and/or isser. */ public class SyntheticAccessorsRenameParticipant extends RenameParticipant { @@ -87,23 +82,20 @@ public class SyntheticAccessorsRenameParticipant extends RenameParticipant { @Override public RefactoringStatus checkConditions(IProgressMonitor pm, CheckConditionsContext context) throws OperationCanceledException { RefactoringStatus status = new RefactoringStatus(); - + matches = new ArrayList<>(); try { if (shouldUpdateReferences()) { - matches = findExtraReferences(SubMonitor.convert(pm, "Finding synthetic Groovy references", 10)); - } else { - matches = Collections.emptyList(); + findExtraReferences(SubMonitor.convert(pm, "Finding Groovy property references", 10)); } - checkForBinaryRefs(matches, status); - SearchResultGroup[] grouped = convert(matches); - Checks.excludeCompilationUnits(grouped, status); - status.merge(Checks.checkCompileErrorsInAffectedFiles(grouped)); - checkForPotentialRefs(matches, status); + checkForBinaryRefs(status); + checkForPotentialMatches(status); + SearchResultGroup[] groups = convertMatches(); + Checks.excludeCompilationUnits(groups, status); + status.merge(Checks.checkCompileErrorsInAffectedFiles(groups)); } catch (CoreException e) { GroovyCore.logException(e.getLocalizedMessage(), e); return RefactoringStatus.createFatalErrorStatus(e.getLocalizedMessage()); } - return status; } @@ -114,46 +106,39 @@ private boolean shouldUpdateReferences() { } else if (processor instanceof RenameMethodProcessor) { return ((RenameMethodProcessor) processor).getUpdateReferences(); } - // shouldn't get here return true; } - private void checkForPotentialRefs(List toCheck, RefactoringStatus status) { - for (SearchMatch match : toCheck) { - if (match.getAccuracy() == SearchMatch.A_INACCURATE) { - final RefactoringStatusEntry entry = new RefactoringStatusEntry(RefactoringStatus.WARNING, - RefactoringCoreMessages.RefactoringSearchEngine_potential_matches, - StatusHelper.createContext(JavaCore.createCompilationUnitFrom((IFile) match.getResource()), - new SourceRange(match.getOffset(), match.getLength()))); - status.addEntry(entry); - } - } + private void findExtraReferences(IProgressMonitor pm) throws CoreException { + SyntheticAccessorSearchRequestor requestor = new SyntheticAccessorSearchRequestor(); + requestor.findSyntheticMatches(renameTarget, matches::add, SubMonitor.convert(pm, "Find synthetic property accessors", 10)); + //something.findNonSyntheticMatches(renameTarget, matches::add, SubMonitor.convert(pm, "Find property-style uses of non-synthetic methods", 10)); } - private void checkForBinaryRefs(List toCheck, RefactoringStatus status) throws JavaModelException { + private void checkForBinaryRefs(RefactoringStatus status) throws JavaModelException { ReferencesInBinaryContext binaryRefs = new ReferencesInBinaryContext( "Elements containing binary references to refactored element ''" + renameTarget.getElementName() + "''"); - for (Iterator iter = toCheck.iterator(); iter.hasNext();) { - SearchMatch match = iter.next(); + for (Iterator it = matches.iterator(); it.hasNext();) { + SearchMatch match = it.next(); if (isBinaryElement(match.getElement())) { if (match.getAccuracy() == SearchMatch.A_ACCURATE) { // binary classpaths are often incomplete -> avoiding false // positives from inaccurate matches binaryRefs.add(match); } - iter.remove(); + it.remove(); } } binaryRefs.addErrorIfNecessary(status); } - private boolean isBinaryElement(Object element) throws JavaModelException { + private static boolean isBinaryElement(Object element) throws JavaModelException { if (element instanceof IMember) { return ((IMember) element).isBinary(); - } else if (element instanceof ICompilationUnit) { - return true; } else if (element instanceof IClassFile) { return false; + } else if (element instanceof ICompilationUnit) { + return true; } else if (element instanceof IPackageFragment) { return isBinaryElement(((IPackageFragment) element).getParent()); } else if (element instanceof IPackageFragmentRoot) { @@ -162,29 +147,32 @@ private boolean isBinaryElement(Object element) throws JavaModelException { return false; } - private SearchResultGroup[] convert(List toGroup) { - Map> groups = new HashMap<>(toGroup.size()); - for (SearchMatch searchMatch : toGroup) { - if (searchMatch.getResource() == null) { - // likely a binary match. These are handled elsewhere - continue; - } - List group = groups.get(searchMatch.getResource()); - if (group == null) { - group = new ArrayList<>(); - groups.put(searchMatch.getResource(), group); + private void checkForPotentialMatches(RefactoringStatus status) { + for (SearchMatch match : matches) { + if (match.getAccuracy() == SearchMatch.A_INACCURATE) { + final RefactoringStatusEntry entry = new RefactoringStatusEntry(RefactoringStatus.WARNING, + RefactoringCoreMessages.RefactoringSearchEngine_potential_matches, + StatusHelper.createContext(JavaCore.createCompilationUnitFrom((IFile) match.getResource()), + new SourceRange(match.getOffset(), match.getLength()))); + status.addEntry(entry); } - group.add(searchMatch); } + } + + private SearchResultGroup[] convertMatches() { + Map groups = new HashMap<>(matches.size()); - SearchResultGroup[] results = new SearchResultGroup[groups.size()]; - int i = 0; - for (Entry> group : groups.entrySet()) { - results[i++] = new SearchResultGroup(group.getKey(), group.getValue().toArray(new SearchMatch[0])); + for (SearchMatch match : matches) { + if (match.getResource() != null) { + groups.computeIfAbsent(match.getResource(), mr -> new SearchResultGroup(mr, new SearchMatch[0])).add(match); + } } - return results; + + return groups.values().toArray(new SearchResultGroup[groups.size()]); } + //-------------------------------------------------------------------------- + @Override public Change createChange(IProgressMonitor pm) throws CoreException, OperationCanceledException { CompositeChange change = new CompositeChange(getName()); @@ -216,10 +204,6 @@ protected boolean initialize(Object element) { return false; } - private String accessorName(String prefix, String name) { - return prefix + Character.toUpperCase(name.charAt(0)) + name.substring(1); - } - private void addChange(CompositeChange finalChange, IMember enclosingElement, int offset, int length, String newName) { CompilationUnitChange existingChange = findOrCreateChange(enclosingElement, finalChange); @@ -242,25 +226,6 @@ private void addChange(CompositeChange finalChange, IMember enclosingElement, in new TextEditGroup("Update synthetic Groovy accessor", occurrenceEdit))); } - private String basename(String fullName) { - int baseStart; - if (fullName.startsWith("is") && fullName.length() > 2 && Character.isUpperCase(fullName.charAt(2))) { - baseStart = 2; - } else if ((fullName.startsWith("get") || fullName.startsWith("set")) && - fullName.length() > 3 && - Character.isUpperCase(fullName.charAt(3))) { - baseStart = 3; - } else { - baseStart = -1; - } - - if (baseStart > 0) { - return Character.toLowerCase(fullName.charAt(baseStart)) + fullName.substring(baseStart + 1); - } else { - return fullName; - } - } - private void createMatchedChanges(List references, CompositeChange finalChange, Map nameMap) throws JavaModelException { for (SearchMatch searchMatch : references) { @@ -275,13 +240,6 @@ private void createMatchedChanges(List references, CompositeChange } } - private List findExtraReferences(IProgressMonitor pm) throws CoreException { - SyntheticAccessorSearchRequestor synthRequestor = new SyntheticAccessorSearchRequestor(); - final List matches = new ArrayList<>(); - synthRequestor.findSyntheticMatches(renameTarget, match -> matches.add(match), SubMonitor.convert(pm, "Find synthetic accessors", 10)); - return matches; - } - private String findMatchName(SearchMatch searchMatch, Set keySet) throws JavaModelException { IJavaElement element = JavaCore.create(searchMatch.getResource()); if (element.getElementType() == IJavaElement.COMPILATION_UNIT) { @@ -328,9 +286,9 @@ private CompilationUnitChange findOrCreateChange(IMember accessor, CompositeChan } private Map getNameMap() { - Map nameMap = new HashMap<>(); - String newBaseName = basename(getArguments().getNewName()); - String oldBaseName = basename(renameTarget.getElementName()); + Map nameMap = new HashMap<>(4); + String newBaseName = propertyName(getArguments().getNewName()); + String oldBaseName = propertyName(renameTarget.getElementName()); nameMap.put(oldBaseName, newBaseName); nameMap.put(accessorName("is", oldBaseName), accessorName("is", newBaseName)); @@ -339,4 +297,23 @@ private Map getNameMap() { return nameMap; } + + private static String accessorName(String prefix, String name) { + return prefix + MetaClassHelper.capitalize(name); + } + + private static String propertyName(String fullName) { + int prefixLength = 0; + if (fullName.startsWith("is")) { + prefixLength = 2; + } else if (fullName.startsWith("get") || fullName.startsWith("set")) { + prefixLength = 3; + } + + if (fullName.length() == prefixLength) { + return fullName; + } + + return Introspector.decapitalize(fullName.substring(prefixLength)); + } } diff --git a/ide/org.codehaus.groovy.eclipse.ui/src/org/codehaus/groovy/eclipse/editor/highlighting/SemanticReferenceRequestor.java b/ide/org.codehaus.groovy.eclipse.ui/src/org/codehaus/groovy/eclipse/editor/highlighting/SemanticReferenceRequestor.java index a70a4e02bf..9404210ba3 100644 --- a/ide/org.codehaus.groovy.eclipse.ui/src/org/codehaus/groovy/eclipse/editor/highlighting/SemanticReferenceRequestor.java +++ b/ide/org.codehaus.groovy.eclipse.ui/src/org/codehaus/groovy/eclipse/editor/highlighting/SemanticReferenceRequestor.java @@ -1,5 +1,5 @@ /* - * Copyright 2009-2017 the original author or authors. + * Copyright 2009-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,7 +20,6 @@ import org.codehaus.groovy.ast.ClassHelper; import org.codehaus.groovy.ast.ClassNode; import org.codehaus.groovy.ast.FieldNode; -import org.codehaus.groovy.ast.ImportNode; import org.codehaus.groovy.ast.MethodNode; import org.codehaus.groovy.ast.Parameter; import org.codehaus.groovy.ast.PropertyNode; @@ -42,30 +41,19 @@ public abstract class SemanticReferenceRequestor implements ITypeRequestor { protected static Position getPosition(ASTNode node) { int start, length; if (node instanceof FieldNode || node instanceof MethodNode || node instanceof PropertyNode || - (node instanceof ClassNode && ((ClassNode) node).getNameEnd() > 0)) { - AnnotatedNode an = (AnnotatedNode) node; - start = an.getNameStart(); - length = an.getNameEnd() - start + 1; + (node instanceof ClassNode && ((ClassNode) node).getNameEnd() > 0) || + node instanceof StaticMethodCallExpression) { + AnnotatedNode n = (AnnotatedNode) node; + start = n.getNameStart(); + length = n.getNameEnd() - start + 1; } else if (node instanceof Parameter) { Parameter p = (Parameter) node; start = p.getNameStart(); length = p.getNameEnd() - start; - } else if (node instanceof ImportNode) { - ClassNode clazz = ((ImportNode) node).getType(); - start = clazz.getStart(); - length = clazz.getLength(); - } else if (node instanceof StaticMethodCallExpression) { - start = node.getStart(); - length = ((StaticMethodCallExpression) node).getMethod().length(); } else if (node instanceof MethodCallExpression) { Expression e = ((MethodCallExpression) node).getMethod(); - // FIXADE : determine if we need to ignore funky method calls that - // use things like GStrings in the - // name - // if (e instanceof ConstantExpression) { start = e.getStart(); length = e.getLength(); - // } } else { start = node.getStart(); length = node.getLength(); diff --git a/ide/org.codehaus.groovy.eclipse.ui/src/org/codehaus/groovy/eclipse/search/GroovyOccurrencesFinder.java b/ide/org.codehaus.groovy.eclipse.ui/src/org/codehaus/groovy/eclipse/search/GroovyOccurrencesFinder.java index 0ffce01e05..f0faec6689 100644 --- a/ide/org.codehaus.groovy.eclipse.ui/src/org/codehaus/groovy/eclipse/search/GroovyOccurrencesFinder.java +++ b/ide/org.codehaus.groovy.eclipse.ui/src/org/codehaus/groovy/eclipse/search/GroovyOccurrencesFinder.java @@ -1,5 +1,5 @@ /* - * Copyright 2009-2017 the original author or authors. + * Copyright 2009-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -88,34 +88,17 @@ public OccurrenceLocation[] getOccurrences() { int i = 0; for (Entry entry : occurences.entrySet()) { ASTNode node = entry.getKey(); - int flag = entry.getValue(); + int flags = entry.getValue(); OccurrenceLocation occurrenceLocation; - if (node instanceof FieldNode) { - FieldNode c = (FieldNode) node; - occurrenceLocation = new OccurrenceLocation(c.getNameStart(), c.getNameEnd() - c.getNameStart() + 1, flag, "Occurrence of ''" + getElementName() + "''"); - } else if (node instanceof MethodNode) { - MethodNode c = (MethodNode) node; - occurrenceLocation = new OccurrenceLocation(c.getNameStart(), c.getNameEnd() - c.getNameStart() + 1, flag, "Occurrence of ''" + getElementName() + "''"); + if ((node instanceof ClassNode && ((ClassNode) node).getNameEnd() > 0) || node instanceof FieldNode || node instanceof MethodNode || node instanceof StaticMethodCallExpression) { + AnnotatedNode n = (AnnotatedNode) node; + occurrenceLocation = new OccurrenceLocation(n.getNameStart(), n.getNameEnd() - n.getNameStart() + 1, flags, "Occurrence of ''" + getElementName() + "''"); } else if (node instanceof Parameter) { - // should be finding the start and end of the name region only, - // but this finds the entire declaration - Parameter c = (Parameter) node; - int start = c.getNameStart(); - int length = c.getNameEnd() - c.getNameStart(); - occurrenceLocation = new OccurrenceLocation(start, length, flag, "Occurrence of ''" + getElementName() + "''"); - } else if (node instanceof ClassNode && ((ClassNode) node).getNameEnd() > 0) { - // class declaration - ClassNode c = (ClassNode) node; - occurrenceLocation = new OccurrenceLocation(c.getNameStart(), c.getNameEnd() - c.getNameStart() + 1, flag, "Occurrence of ''" + getElementName() + "''"); - } else if (node instanceof StaticMethodCallExpression) { - // special case...for static method calls, the start and end are - // of the entire expression, but we just want the name. - StaticMethodCallExpression smce = (StaticMethodCallExpression) node; - occurrenceLocation = new OccurrenceLocation(smce.getStart(), Math.min(smce.getLength(), smce.getMethod().length()), flag, "Occurrence of ''" + getElementName() + "''"); + Parameter p = (Parameter) node; + occurrenceLocation = new OccurrenceLocation(p.getNameStart(), p.getNameEnd() - p.getNameStart(), flags, "Occurrence of ''" + getElementName() + "''"); } else { - SourceRange range = getSourceRange(node); - - occurrenceLocation = new OccurrenceLocation(range.getOffset(), range.getLength(), flag, "Occurrence of ''" + getElementName() + "''"); + SourceRange r = getSourceRange(node); + occurrenceLocation = new OccurrenceLocation(r.getOffset(), r.getLength(), flags, "Occurrence of ''" + getElementName() + "''"); } locations[i++] = occurrenceLocation; }