Skip to content

Commit

Permalink
Prep for #678: skip pseudo-property references in rename method search
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
eric-milles committed Aug 17, 2018
1 parent 27f5e8d commit 9536fba
Show file tree
Hide file tree
Showing 9 changed files with 680 additions and 224 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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<SearchMatch> 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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public class FieldReferenceSearchRequestor implements ITypeRequestor {

protected final String fieldName, declaringQualifiedName;
protected final Set<Position> 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;
Expand All @@ -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
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<Position> 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<ClassNode, boolean[]> cachedParameterCounts = new HashMap<>();
protected final Map<ClassNode, Boolean> cachedDeclaringNameMatches = new HashMap<>();

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
*
Expand Down Expand Up @@ -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
Expand All @@ -54,15 +69,14 @@ final class SyntheticMemberSearchTests extends GroovyEclipseTestSuite {
addGroovySource(contents, nextUnitName())
List<SearchMatch> 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)
assertMatch('run', 'getProp', contents, matches)
assertMatch('run', 'setProp', contents, matches)
}

@Test
@Test // references to pseudo-property and non-synthetic accessors
void testSearchInGroovy2() {
String contents = '''\
new p.G().explicit
Expand All @@ -73,14 +87,14 @@ final class SyntheticMemberSearchTests extends GroovyEclipseTestSuite {
addGroovySource(contents, nextUnitName())
List<SearchMatch> 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
Expand All @@ -91,14 +105,14 @@ final class SyntheticMemberSearchTests extends GroovyEclipseTestSuite {
addGroovySource(contents, nextUnitName())
List<SearchMatch> 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
Expand All @@ -109,8 +123,8 @@ final class SyntheticMemberSearchTests extends GroovyEclipseTestSuite {
addGroovySource(contents, nextUnitName())
List<SearchMatch> 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)
Expand Down Expand Up @@ -154,7 +168,6 @@ final class SyntheticMemberSearchTests extends GroovyEclipseTestSuite {
addJavaSource(contents, 'ClassB')
List<SearchMatch> 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)
Expand Down
Loading

0 comments on commit 9536fba

Please sign in to comment.