Skip to content

Commit

Permalink
Fix for #1027: isX() as x; allow extension method, block category method
Browse files Browse the repository at this point in the history
  • Loading branch information
@eric-milles
eric-milles committed Feb 4, 2020
1 parent f62e1a0 commit 60209bd
Show file tree
Hide file tree
Showing 8 changed files with 258 additions and 149 deletions.
37 changes: 35 additions & 2 deletions ....core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/DGMInferencingTests.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2009-2019 the original author or authors.
* Copyright 2009-2020 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 @@ -664,7 +664,7 @@ public void testDGM50() {
//@formatter:on

assertExprType(contents, "result", "java.lang.Number");
//assertUnknownConfidence(contents, start, end, "java.lang.Object", false);
//assertUnknownConfidence(contents, contents.lastIndexOf("dgm"), contents.lastIndexOf("dgm") + 3);
}

@Test
Expand All @@ -689,6 +689,39 @@ public void testDGM51() {
assertExprType(contents, "abs", "java.lang.Integer");
}

@Test // https://github.com/groovy/groovy-eclipse/issues/1027
public void testDGM52() {
String contents =
//@formatter:off
"def result = '42'.number\n";
//@formatter:on

assertExprType(contents, "number", "java.lang.Boolean");
assertDeclType(contents, "number", "org.codehaus.groovy.runtime.StringGroovyMethods");
}

@Test // https://github.com/groovy/groovy-eclipse/issues/1027
public void testDGM53() {
String contents =
//@formatter:off
"def result = ' '.allWhitespace\n";
//@formatter:on

assertExprType(contents, "allWhitespace", "java.lang.Boolean");
assertDeclType(contents, "allWhitespace", "org.codehaus.groovy.runtime.StringGroovyMethods");
}

@Test
public void testDGM53a() {
String contents =
//@formatter:off
"def result = ' '.&allWhitespace\n";
//@formatter:on

int offset = contents.indexOf("allWhitespace");
assertUnknownConfidence(contents, offset, offset + "allWhitespace".length());
}

@Test // GRECLIPSE-1131
public void testDGMClosure1() {
String contents =
Expand Down
52 changes: 50 additions & 2 deletions ...ests.builder/src/org/eclipse/jdt/core/groovy/tests/search/MethodReferenceSearchTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -515,8 +515,7 @@ public void testExplicitPropertyGetterSearch1() throws Exception {
"package foo\n" +
"class Bar {\n" +
" static String string\n" +
" static String getString() {\n" +
" }\n" +
" static String getString() {}\n" +
"}\n");
GroovyCompilationUnit baz = createUnit("foo", "Baz",
"package foo\n" +
Expand Down Expand Up @@ -652,6 +651,55 @@ public void testExplicitPropertyGetterSearch3() throws Exception {
assertEquals(offset, match.getOffset());
}

@Test // https://github.com/groovy/groovy-eclipse/issues/1027
public void testExplicitPropertyGetterSearch4() throws Exception {
GroovyCompilationUnit bar = createUnit("foo", "Bar",
"package foo\n" +
"class Bar {\n" +
"//static Boolean bool\n" + //TODO
" static Boolean isBool() {}\n" +
"}\n");
GroovyCompilationUnit baz = createUnit("foo", "Baz",
"package foo\n" +
"import static foo.Bar.isBool as blah\n" + // exact
"import static foo.Bar.isBool as isXy\n" + // exact
"flag = Bar.isBool()\n" + // exact
"flag = Bar.'isBool'()\n" + // exact
"flag = Bar.bool\n" + // exact
"flag = Bar.@bool\n" +
"func = Bar.&isBool\n" + // potential (may evaluate as category method)
"flag = blah()\n" + // exact
"flag = xy\n"); // exact

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_ACCURATE, matches.get(0).getAccuracy());
assertEquals(String.valueOf(baz.getContents()).indexOf("isBool as blah"), matches.get(0).getOffset());
assertEquals(SearchMatch.A_ACCURATE, matches.get(1).getAccuracy());
assertEquals(String.valueOf(baz.getContents()).indexOf("isBool as isXy"), matches.get(1).getOffset());
assertEquals(SearchMatch.A_ACCURATE, matches.get(2).getAccuracy());
assertEquals(String.valueOf(baz.getContents()).indexOf("isBool()"), matches.get(2).getOffset());
assertEquals(SearchMatch.A_ACCURATE, matches.get(3).getAccuracy());
assertEquals(String.valueOf(baz.getContents()).indexOf("'isBool'"), matches.get(3).getOffset());

assertEquals(SearchMatch.A_ACCURATE, matches.get(4).getAccuracy());
assertEquals(String.valueOf(baz.getContents()).indexOf("Bar.bool") + 4, matches.get(4).getOffset());
assertEquals(SearchMatch.A_INACCURATE, matches.get(5).getAccuracy());
assertEquals(String.valueOf(baz.getContents()).indexOf("Bar.&isBool") + 5, matches.get(5).getOffset());
assertEquals(SearchMatch.A_ACCURATE, matches.get(6).getAccuracy());
assertEquals(String.valueOf(baz.getContents()).lastIndexOf("blah"), matches.get(6).getOffset());
assertEquals(SearchMatch.A_ACCURATE, matches.get(7).getAccuracy());
assertEquals(String.valueOf(baz.getContents()).lastIndexOf("xy"), matches.get(7).getOffset());
}

@Test
public void testExplicitPropertySetterSearch1() throws Exception {
GroovyCompilationUnit bar = createUnit("foo", "Bar",
Expand Down
12 changes: 6 additions & 6 deletions ...ore.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/TraitInferencingTests.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2009-2019 the original author or authors.
* Copyright 2009-2020 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 @@ -213,17 +213,17 @@ public void testProperty12() {
public void testProperty13() {
String source =
"trait T {\n" +
" Number number\n" +
" Number thing\n" +
"}\n" +
"class C implements T {\n" +
" void meth() {\n" +
" println T.super.number\n" +
" println T.super.thing\n" +
" }\n" +
"}\n";

// TODO: assertDeclType(source, "number", "T");
// TODO: assertExprType(source, "number", "java.lang.Number");
assertUnknownConfidence(source, source.lastIndexOf("number"), source.lastIndexOf("number") + "number".length());
// TODO: assertDeclType(source, "thing", "T");
// TODO: assertExprType(source, "thing", "java.lang.Number");
assertUnknownConfidence(source, source.lastIndexOf("thing"), source.lastIndexOf("thing") + "thing".length());
}

@Test
Expand Down
13 changes: 9 additions & 4 deletions ...e.jdt.groovy.core/src/org/codehaus/jdt/groovy/internal/compiler/ast/GroovyClassScope.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2009-2019 the original author or authors.
* Copyright 2009-2020 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 @@ -135,11 +135,16 @@ protected MethodBinding[] augmentMethodBindings(MethodBinding[] methodBindings)

String name = property.getName(), capitalizedName = MetaClassHelper.capitalize(name);

createGetterMethod(name, "get" + capitalizedName, modifiers, methodBindings)
.ifPresent(groovyMethods::add);

if (ClassHelper.boolean_TYPE.equals(property.getType())) {
createGetterMethod(name, "is" + capitalizedName, modifiers, methodBindings)
.ifPresent(binding -> {
groovyMethods.add(binding);
// GROOVY-9382: no getter generated if isser declared
createGetterMethod(name, "get" + capitalizedName, modifiers, methodBindings)
.ifPresent(groovyMethods::add);
});
} else {
createGetterMethod(name, "get" + capitalizedName, modifiers, methodBindings)
.ifPresent(groovyMethods::add);
}

Expand Down
43 changes: 22 additions & 21 deletions base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/AccessorSupport.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,14 @@
public enum AccessorSupport {
GETTER("get"), SETTER("set"), ISSER("is"), NONE("");

private final String prefix;

AccessorSupport(String prefix) {
this.prefix = prefix;
}

private final String prefix;

//

public boolean isAccessor() {
return (this != NONE);
}
Expand All @@ -51,19 +53,35 @@ public boolean isAccessorKind(MethodNode node, boolean isCategory) {
case SETTER:
return parameters != null && parameters.length == (!isCategory ? 1 : 2) && (!isCategory || !isVargs(parameters));
case ISSER:
return !isCategory && (parameters == null || parameters.length == 0) && ClassHelper.boolean_TYPE.equals(node.getReturnType());
return (parameters == null || parameters.length == (!isCategory ? 0 : 1)) && ClassHelper.boolean_TYPE == ClassHelper.getUnwrapper(node.getReturnType());
default:
return false;
}
}

public String createAccessorName(String name) {
if (!name.startsWith(GETTER.prefix) && !name.startsWith(SETTER.prefix) && name.length() > 0) {
if (isAccessor() && !name.isEmpty() && !name.startsWith(GETTER.prefix) && !name.startsWith(SETTER.prefix)) {
return this.prefix + Character.toUpperCase(name.charAt(0)) + (name.length() > 1 ? name.substring(1) : "");
}
return null;
}

public static AccessorSupport create(String methodName, boolean isCategory) {
AccessorSupport accessor = AccessorSupport.NONE;

for (AccessorSupport kind : AccessorSupport.values()) {
if (kind.isAccessor() &&
methodName.startsWith(kind.prefix) &&
methodName.length() > kind.prefix.length() &&
Character.isUpperCase(methodName.charAt(kind.prefix.length()))) {
accessor = kind;
break;
}
}

return accessor;
}

public static AccessorSupport findAccessorKind(MethodNode node, boolean isCategory) {
AccessorSupport accessor = create(node.getName(), isCategory);
return accessor.isAccessorKind(node, isCategory) ? accessor : NONE;
Expand Down Expand Up @@ -124,21 +142,4 @@ public static boolean isSetter(MethodNode node) {
return node.getParameters().length == 1 &&
(node.getName().startsWith("set") && node.getName().length() > 3);
}

public static AccessorSupport create(String methodName, boolean isCategory) {
AccessorSupport accessor = AccessorSupport.NONE;
// is is allowed only for non-category methods
if (!isCategory && methodName.length() > 2 && methodName.startsWith("is") &&
Character.isUpperCase(methodName.charAt(2))) {
accessor = AccessorSupport.ISSER;
}

if (!accessor.isAccessor()) {
if (methodName.length() > 3 && (methodName.startsWith("get") || methodName.startsWith("set")) &&
Character.isUpperCase(methodName.charAt(3))) {
accessor = methodName.charAt(0) == 'g' ? AccessorSupport.GETTER : AccessorSupport.SETTER;
}
}
return accessor;
}
}
37 changes: 18 additions & 19 deletions base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/CategoryTypeLookup.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2009-2019 the original author or authors.
* Copyright 2009-2020 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 @@ -33,6 +33,7 @@

/**
* Looks up the type of an expression in the currently applicable categories.
* <p>
* Note: DefaultGroovyMethods are always considered to be an applicable category.
*/
public class CategoryTypeLookup implements ITypeLookup {
Expand All @@ -48,29 +49,27 @@ public TypeLookupResult lookupType(Expression node, VariableScope scope, ClassNo
//
List<MethodNode> candidates = new ArrayList<>();

for (ClassNode category : scope.getCategoryNames()) {
if (scope.isMethodCall() || isMethodPointer) {
if (isMethodPointer || scope.isMethodCall()) {
for (ClassNode category : scope.getCategoryNames()) {
for (MethodNode method : category.getMethods(simpleName)) {
if (isCompatibleCategoryMethod(method, selfType, scope)) {
candidates.add(method);
}
}
}
String getterName = AccessorSupport.GETTER.createAccessorName(simpleName);
if (getterName != null && !isMethodPointer) {
for (MethodNode method : category.getMethods(getterName)) {
if (AccessorSupport.findAccessorKind(method, true) == AccessorSupport.GETTER &&
isCompatibleCategoryMethod(method, selfType, scope)) {
candidates.add(method);
}
}
}
String setterName = AccessorSupport.SETTER.createAccessorName(simpleName);
if (setterName != null && !isMethodPointer) {
for (MethodNode method : category.getMethods(setterName)) {
if (AccessorSupport.findAccessorKind(method, true) == AccessorSupport.SETTER &&
isCompatibleCategoryMethod(method, selfType, scope)) {
candidates.add(method);
}
if (!isMethodPointer) {
for (AccessorSupport kind : AccessorSupport.values()) {
String methodName = kind.createAccessorName(simpleName);
if (methodName != null) {
for (ClassNode category : scope.getCategoryNames()) {
for (MethodNode method : category.getMethods(methodName)) {
if (kind.isAccessorKind(method, true) && isCompatibleCategoryMethod(method, selfType, scope) &&
// GROOVY-5245: isPropName() methods cannot be used for bean-style property expressions
(kind != AccessorSupport.ISSER || isDefaultGroovyMethod(method, scope) || isDefaultGroovyStaticMethod(method, scope))) {
candidates.add(method);
}
}
}
}
}
Expand All @@ -85,7 +84,7 @@ public TypeLookupResult lookupType(Expression node, VariableScope scope, ClassNo
MethodNode method = selectBestMatch(candidates, argumentTypes);

TypeLookupResult result = new TypeLookupResult(method.getReturnType(), method.getDeclaringClass(), method,
isDefaultGroovyMethod(method, scope) ? TypeConfidence.LOOSELY_INFERRED : TypeConfidence.INFERRED, scope);
isDefaultGroovyMethod(method, scope) ? TypeConfidence.LOOSELY_INFERRED : TypeConfidence.INFERRED, scope);
result.isGroovy = true; // enable semantic highlighting as Groovy method
return result;
}
Expand Down
Loading

0 comments on commit 60209bd

Please sign in to comment.