Skip to content

Commit

Permalink
Fix for #788: match inner class type name
Browse files Browse the repository at this point in the history
  • Loading branch information
@eric-milles
eric-milles committed Jan 7, 2019
1 parent fddde85 commit b99a032
Show file tree
Hide file tree
Showing 4 changed files with 135 additions and 62 deletions.
127 changes: 124 additions & 3 deletions ...tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/FieldReferenceSearchTests.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2009-2018 the original author or authors.
* Copyright 2009-2019 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.
Expand All @@ -15,7 +15,16 @@
*/
package org.eclipse.jdt.core.groovy.tests.search;

import static org.junit.Assert.assertEquals;

import org.codehaus.jdt.groovy.model.GroovyCompilationUnit;
import org.eclipse.jdt.core.IField;
import org.eclipse.jdt.core.IJavaElement;
import org.eclipse.jdt.core.groovy.tests.MockPossibleMatch;
import org.eclipse.jdt.core.search.IJavaSearchConstants;
import org.eclipse.jdt.core.search.SearchPattern;
import org.eclipse.jdt.groovy.search.ITypeRequestor;
import org.eclipse.jdt.groovy.search.TypeRequestorFactory;
import org.junit.Test;

public final class FieldReferenceSearchTests extends SearchTestSuite {
Expand Down Expand Up @@ -182,15 +191,84 @@ public void testFieldReadsInScript1() throws Exception {
"f.xxx = f.xxx");
}

@Test // https://github.com/groovy/groovy-eclipse/issues/788
public void testFindInnerClassField1() throws Exception {
String contents =
"class Outer {\n" +
" static class Inner1 {\n" +
" String something\n" +
" }\n" +
" static class Inner2 {\n" +
" String something\n" +
" void meth(Inner1 param) {\n" +
" param.with {\n" +
" something\n" +
" owner.something\n" +
" println \"Something: $something\"\n" +
" println \"Something else: $owner.something\"\n" +
" }\n" +
" }\n" +
" }\n" +
"}\n";
int offset = contents.indexOf("with {");
GroovyCompilationUnit unit = createUnit("Outer", contents);
IField field = findType("Outer", unit).getType("Inner1").getField("something");

MockPossibleMatch possibleMatch = new MockPossibleMatch(unit);
ITypeRequestor typeRequestor = new TypeRequestorFactory().createRequestor(possibleMatch,
SearchPattern.createPattern(field, IJavaSearchConstants.REFERENCES), searchRequestor);
factory.createVisitor(possibleMatch).visitCompilationUnit(typeRequestor);

assertEquals("Should have found 2 matches, but found:\n" + searchRequestor.printMatches(), 2, searchRequestor.getMatches().size());
assertLocation(searchRequestor.getMatch(0), contents.indexOf("something", offset), "something".length());
assertLocation(searchRequestor.getMatch(1), contents.indexOf("$something", offset) + 1, "something".length());
}

@Test // https://github.com/groovy/groovy-eclipse/issues/788
public void testFindInnerClassField2() throws Exception {
String contents =
"class Outer {\n" +
" static class Inner1 {\n" +
" String something\n" +
" }\n" +
" static class Inner2 {\n" +
" String something\n" +
" void meth(Inner1 param) {\n" +
" param.with {\n" +
" something\n" +
" owner.something\n" +
" println \"Something: $something\"\n" +
" println \"Something else: $owner.something\"\n" +
" }\n" +
" }\n" +
" }\n" +
"}\n";
int offset = contents.indexOf("with {");
GroovyCompilationUnit unit = createUnit("Outer", contents);
IField field = findType("Outer", unit).getType("Inner2").getField("something");

MockPossibleMatch possibleMatch = new MockPossibleMatch(unit);
ITypeRequestor typeRequestor = new TypeRequestorFactory().createRequestor(possibleMatch,
SearchPattern.createPattern(field, IJavaSearchConstants.REFERENCES), searchRequestor);
factory.createVisitor(possibleMatch).visitCompilationUnit(typeRequestor);

assertEquals("Should have found 2 matches, but found:\n" + searchRequestor.printMatches(), 2, searchRequestor.getMatches().size());
assertLocation(searchRequestor.getMatch(0), contents.indexOf("owner.something", offset) + 6, "something".length());
assertLocation(searchRequestor.getMatch(1), contents.indexOf("$owner.something", offset) + 7, "something".length());
}

//--------------------------------------------------------------------------

protected void doTestForTwoFieldReferencesInGString(String secondContents) throws Exception {
super.doTestForTwoFieldReferencesInGString(FIRST_CONTENTS_CLASS_FOR_FIELDS, secondContents, "xxx");
private static final String FIRST_CONTENTS_CLASS_FOR_FIELDS = "class First {\n def xxx\n}";

private void doTestForTwoFieldReferencesInGString(String secondContents) throws Exception {
doTestForTwoFieldReferencesInGString(FIRST_CONTENTS_CLASS_FOR_FIELDS, secondContents, "xxx");
}

private void doTestForTwoFieldWritesInScript(String secondContents) throws Exception {
doTestForTwoFieldReferences(FIRST_CONTENTS_CLASS_FOR_FIELDS, secondContents, true, 3, "xxx", IJavaSearchConstants.WRITE_ACCESSES);
}

private void doTestForTwoFieldReadsInScript(String secondContents) throws Exception {
doTestForTwoFieldReferences(FIRST_CONTENTS_CLASS_FOR_FIELDS, secondContents, true, 3, "xxx", IJavaSearchConstants.READ_ACCESSES);
}
Expand All @@ -206,4 +284,47 @@ private void doTestForTwoFieldReferencesInScriptWithQuotes(String secondContents
private void doTestForTwoFieldReferencesInClass(String secondContents) throws Exception {
doTestForTwoFieldReferences(FIRST_CONTENTS_CLASS_FOR_FIELDS, secondContents, false, 0, "xxx");
}

private void doTestForTwoFieldReferences(String firstContents, String secondContents, boolean contentsIsScript, int offsetInParent, String matchName) throws Exception {
doTestForTwoFieldReferences(firstContents, secondContents, contentsIsScript, offsetInParent, matchName, IJavaSearchConstants.REFERENCES);
}

private void doTestForTwoFieldReferences(String firstContents, String secondContents, boolean contentsIsScript, int offsetInParent, String matchName, int searchFlags) throws Exception {
String firstClassName = "First";
String secondClassName = "Second";
String matchedFieldName = "xxx";
GroovyCompilationUnit first = createUnit(firstClassName, firstContents);
IField firstField = findType(firstClassName, first).getField(matchedFieldName);
SearchPattern pattern = SearchPattern.createPattern(firstField, searchFlags);

GroovyCompilationUnit second = createUnit(secondClassName, secondContents);
IJavaElement firstMatchEnclosingElement;
IJavaElement secondMatchEnclosingElement;
if (contentsIsScript) {
firstMatchEnclosingElement = findType(secondClassName, second).getChildren()[offsetInParent];
secondMatchEnclosingElement = findType(secondClassName, second).getChildren()[offsetInParent];
} else {
firstMatchEnclosingElement = findType(secondClassName, second).getChildren()[offsetInParent];
secondMatchEnclosingElement = findType(secondClassName, second).getChildren()[offsetInParent+2];
}
// match is enclosed in run method (for script), or x method for class

checkMatches(secondContents, matchName, pattern, second, firstMatchEnclosingElement, secondMatchEnclosingElement);
}

// as above, but enclosing element is always the first child of the enclosing type
private void doTestForTwoFieldReferencesInGString(String firstContents, String secondContents, String matchName) throws Exception {
String firstClassName = "First";
String secondClassName = "Second";
String matchedFieldName = "xxx";
GroovyCompilationUnit first = createUnit(firstClassName, firstContents);
IField firstField = findType(firstClassName, first).getField(matchedFieldName);
SearchPattern pattern = SearchPattern.createPattern(firstField, IJavaSearchConstants.REFERENCES);

GroovyCompilationUnit second = createUnit(secondClassName, secondContents);
IJavaElement firstMatchEnclosingElement = findType(secondClassName, second).getChildren()[0];
IJavaElement secondMatchEnclosingElement = findType(secondClassName, second).getChildren()[0];

checkMatches(secondContents, matchName, pattern, second, firstMatchEnclosingElement, secondMatchEnclosingElement);
}
}
55 changes: 4 additions & 51 deletions ...oovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/SearchTestSuite.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2009-2018 the original author or authors.
* Copyright 2009-2019 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.
Expand Down Expand Up @@ -28,7 +28,6 @@
import org.eclipse.core.runtime.Path;
import org.eclipse.core.runtime.jobs.Job;
import org.eclipse.jdt.core.ICompilationUnit;
import org.eclipse.jdt.core.IField;
import org.eclipse.jdt.core.IJavaElement;
import org.eclipse.jdt.core.IJavaProject;
import org.eclipse.jdt.core.ILocalVariable;
Expand Down Expand Up @@ -141,7 +140,6 @@ protected void assertLocation(SearchMatch match, int start, int length) {

protected final static String FIRST_CONTENTS_CLASS = "class First {}";
protected final static String FIRST_CONTENTS_INTERFACE = "interface First {}";
protected final static String FIRST_CONTENTS_CLASS_FOR_FIELDS = "class First { def xxx }";
protected final static String FIRST_CONTENTS_CLASS_FOR_METHODS = "class First { def xxx() { } }";
protected final static String FIRST_CONTENTS_CLASS_FOR_METHODS2 = "class First { def xxx() { } \n def xxx(arg) { } }";

Expand All @@ -167,51 +165,6 @@ protected void doTestForTwoTypeReferences(String firstContents, String secondCon
checkMatches(secondContents, firstClassName, pattern, second, firstMatchEnclosingElement, secondMatchEnclosingElement);
}

protected void doTestForTwoFieldReferences(String firstContents, String secondContents, boolean contentsIsScript, int offsetInParent, String matchName) throws JavaModelException {
doTestForTwoFieldReferences(firstContents, secondContents, contentsIsScript, offsetInParent, matchName, IJavaSearchConstants.REFERENCES);
}

protected void doTestForTwoFieldReferences(String firstContents, String secondContents, boolean contentsIsScript, int offsetInParent, String matchName, int searchFlags) throws JavaModelException {
String firstClassName = "First";
String secondClassName = "Second";
String matchedFieldName = "xxx";
GroovyCompilationUnit first = createUnit(firstClassName, firstContents);
IField firstField = findType(firstClassName, first).getField(matchedFieldName);
SearchPattern pattern = SearchPattern.createPattern(firstField, searchFlags);

GroovyCompilationUnit second = createUnit(secondClassName, secondContents);
IJavaElement firstMatchEnclosingElement;
IJavaElement secondMatchEnclosingElement;
if (contentsIsScript) {
firstMatchEnclosingElement = findType(secondClassName, second).getChildren()[offsetInParent];
secondMatchEnclosingElement = findType(secondClassName, second).getChildren()[offsetInParent];
} else {
firstMatchEnclosingElement = findType(secondClassName, second).getChildren()[offsetInParent];
secondMatchEnclosingElement = findType(secondClassName, second).getChildren()[offsetInParent+2];
}
// match is enclosed in run method (for script), or x method for class

checkMatches(secondContents, matchName, pattern, second,
firstMatchEnclosingElement, secondMatchEnclosingElement);
}

// as above, but enclosing element is always the first child of the enclosing type
protected void doTestForTwoFieldReferencesInGString(String firstContents, String secondContents, String matchName) throws JavaModelException {
String firstClassName = "First";
String secondClassName = "Second";
String matchedFieldName = "xxx";
GroovyCompilationUnit first = createUnit(firstClassName, firstContents);
IField firstField = findType(firstClassName, first).getField(matchedFieldName);
SearchPattern pattern = SearchPattern.createPattern(firstField, IJavaSearchConstants.REFERENCES);

GroovyCompilationUnit second = createUnit(secondClassName, secondContents);
IJavaElement firstMatchEnclosingElement = findType(secondClassName, second).getChildren()[0];
IJavaElement secondMatchEnclosingElement = findType(secondClassName, second).getChildren()[0];

checkMatches(secondContents, matchName, pattern, second,
firstMatchEnclosingElement, secondMatchEnclosingElement);
}

protected void doTestForTwoMethodReferences(String firstContents, String secondContents, boolean contentsIsScript, int offsetInParent, String matchName) throws JavaModelException {
String firstClassName = "First";
String secondClassName = "Second";
Expand Down Expand Up @@ -275,7 +228,7 @@ protected List<SearchMatch> getAllMatches(String firstContents, String secondCon
return searchRequestor.getMatches();
}

private IType findType(String firstClassName, GroovyCompilationUnit first) {
protected IType findType(String firstClassName, GroovyCompilationUnit first) {
IType type = first.getType(firstClassName);
if (! type.exists()) {
try {
Expand Down Expand Up @@ -305,7 +258,7 @@ protected void doTestForVarReferences(String contents, int offsetInParent, Strin
checkLocalVarMatches(contents, matchName, pattern, unit, matchLocations);
}

private void checkLocalVarMatches(String contents, String matchName, SearchPattern pattern, GroovyCompilationUnit unit, MatchRegion[] matchLocations) {
protected void checkLocalVarMatches(String contents, String matchName, SearchPattern pattern, GroovyCompilationUnit unit, MatchRegion[] matchLocations) {
MockPossibleMatch match = new MockPossibleMatch(unit);
ITypeRequestor typeRequestor = new TypeRequestorFactory().createRequestor(match, pattern, searchRequestor);
TypeInferencingVisitorWithRequestor visitor = factory.createVisitor(match);
Expand All @@ -319,7 +272,7 @@ private void checkLocalVarMatches(String contents, String matchName, SearchPatte
}
}

private void checkMatches(String secondContents, String matchText, SearchPattern pattern, GroovyCompilationUnit second, IJavaElement firstMatchEnclosingElement, IJavaElement secondMatchEnclosingElement) {
protected void checkMatches(String secondContents, String matchText, SearchPattern pattern, GroovyCompilationUnit second, IJavaElement firstMatchEnclosingElement, IJavaElement secondMatchEnclosingElement) {
MockPossibleMatch match = new MockPossibleMatch(second);
ITypeRequestor typeRequestor = new TypeRequestorFactory().createRequestor(match, pattern, searchRequestor);
TypeInferencingVisitorWithRequestor visitor = factory.createVisitor(match);
Expand Down
11 changes: 5 additions & 6 deletions ...ipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/FieldReferenceSearchRequestor.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2009-2018 the original author or authors.
* Copyright 2009-2019 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.
Expand Down Expand Up @@ -47,7 +47,7 @@ public class FieldReferenceSearchRequestor implements ITypeRequestor {
protected final SearchRequestor requestor;
protected final SearchParticipant participant;

protected final String fieldName, declaringQualifiedName;
protected final String fieldName, declaringTypeName;
protected final Set<Position> acceptedPositions = new HashSet<>();
protected final boolean readAccess, writeAccess, findReferences, findDeclarations;

Expand All @@ -61,7 +61,7 @@ public FieldReferenceSearchRequestor(FieldPattern pattern, SearchRequestor reque
String declaringSimpleName = ((arr == null || arr.length == 0) ? "" : String.valueOf(arr));
arr = ReflectionUtils.getPrivateField(FieldPattern.class, "declaringQualification", pattern);
String declaringQualification = ((arr == null || arr.length == 0) ? "" : (String.valueOf(arr) + "."));
declaringQualifiedName = declaringQualification + declaringSimpleName;
declaringTypeName = declaringQualification + declaringSimpleName;

readAccess = (Boolean) ReflectionUtils.getPrivateField(VariablePattern.class, "readAccess", pattern);
writeAccess = (Boolean) ReflectionUtils.getPrivateField(VariablePattern.class, "writeAccess", pattern);
Expand Down Expand Up @@ -155,16 +155,15 @@ public VisitStatus acceptASTNode(ASTNode node, TypeLookupResult result, IJavaEle
return VisitStatus.CONTINUE;
}

// recursively check the hierarchy
private boolean qualifiedNameMatches(ClassNode declaringType) {
if (declaringType == null) {
// no declaring type; probably a variable declaration
return false;
} else if (declaringQualifiedName.isEmpty()) {
} else if (declaringTypeName.isEmpty()) {
// no type specified, accept all
return true;
}
return declaringType.getName().equals(declaringQualifiedName);
return declaringType.getName().replace('$', '.').equals(declaringTypeName);
}

private int getAccuracy(TypeConfidence confidence, boolean isCompleteMatch) {
Expand Down
4 changes: 2 additions & 2 deletions ...pse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/MethodReferenceSearchRequestor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -335,11 +335,11 @@ private boolean matchOnName(ClassNode declaringType) {
return false;
}
String declaringTypeName = declaringType.getName().replace('$', '.');
if (declaringTypeName.equals("java.lang.Object") && declaringType.getDeclaredMethods(methodName).isEmpty()) {
if ("java.lang.Object".equals(declaringTypeName) && declaringType.getDeclaredMethods(methodName).isEmpty()) {
// local variables have a declaring type of Object; don't accidentally return them as a match
return false;
}
if (this.declaringTypeName == null || this.declaringTypeName.length() == 0) {
if (this.declaringTypeName == null || this.declaringTypeName.isEmpty()) {
// no type specified, accept all
return true;
}
Expand Down

0 comments on commit b99a032

Please sign in to comment.