Skip to content

Commit

Permalink
Fix for #1269: update boolean property method(s) detection / generation
Browse files Browse the repository at this point in the history 
GROOVY-1736, GROOVY-6097, GROOVY-9382
  • Loading branch information
@eric-milles
eric-milles committed Jun 17, 2021
1 parent 5423f47 commit b339a5c
Show file tree
Hide file tree
Showing 5 changed files with 235 additions and 27 deletions.
145 changes: 145 additions & 0 deletions ...ovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/InferencingTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1472,6 +1472,151 @@ public void testSuperPropertyReference5() {
assertUnknown(contents, "value");
}

@Test
public void testSuperPropertyReference6() {
for (String qual : new String[] {"", "this.", "super."}) {
String contents =
"class A {\n" +
" boolean isValue() {}\n" +
" boolean getValue() {}\n" +
"}\n" +
"class B extends A {\n" +
" void test() {\n" +
" " + qual + "value\n" +
" }\n" +
"}";
int offset = contents.lastIndexOf("value");
assertDeclaration(contents, offset, offset + 5, "A", "getValue", DeclarationKind.METHOD);
}
}

@Test // GROOVY-1736
public void testSuperPropertyReference6a() {
for (String qual : new String[] {"", "this.", "super."}) {
String contents =
"class A {\n" +
" boolean isValue() {}\n" +
"}\n" +
"class B extends A {\n" +
" boolean getValue() {}\n" +
" void test() {\n" +
" " + qual + "value\n" +
" }\n" +
"}";
int offset = contents.lastIndexOf("value");
if (qual.startsWith("super")) {
assertUnknownConfidence(contents, offset, offset + 5);
} else {
assertDeclaration(contents, offset, offset + 5, "B", "getValue", DeclarationKind.METHOD);
}
}
}

@Test
public void testSuperPropertyReference6b() {
for (String qual : new String[] {"", "this.", "super."}) {
String contents =
"class A {\n" +
" boolean value\n" +
"}\n" +
"class B extends A {\n" +
" boolean getValue() {}\n" +
" void test() {\n" +
" " + qual + "value\n" +
" }\n" +
"}";
int offset = contents.lastIndexOf("value");
if (qual.startsWith("super")) {
assertDeclaration(contents, offset, offset + 5, "A", "value", DeclarationKind.PROPERTY);
} else {
assertDeclaration(contents, offset, offset + 5, "B", "getValue", DeclarationKind.METHOD);
}
}
}

@Test // isX only applies to [Bb]oolean
public void testSuperPropertyReference7() {
for (String qual : new String[] {"", "this.", "super."}) {
String contents =
"class A {\n" +
" Number isValue() {}\n" +
" Number getValue() {}\n" +
"}\n" +
"class B extends A {\n" +
" void test() {\n" +
" " + qual + "value\n" +
" }\n" +
"}";
int offset = contents.lastIndexOf("value");
assertDeclaration(contents, offset, offset + 5, "A", "getValue", DeclarationKind.METHOD);
}
}

@Test
public void testSuperPropertyReference8() {
for (String qual : new String[] {"", "this.", "super."}) {
String contents =
"class A {\n" +
" boolean value\n" +
" boolean getValue() {}\n" + // no isValue() generated
"}\n" +
"class B extends A {\n" +
" void test() {\n" +
" " + qual + "value\n" +
" " + qual + "isValue()\n" +
" }\n" +
"}";
assertUnknown(contents, "isValue");
int offset = contents.lastIndexOf("value");
assertDeclaration(contents, offset, offset + 5, "A", "getValue", DeclarationKind.METHOD);
}
}

@Test // GROOVY-6097
public void testSuperPropertyReference9() {
for (String qual : new String[] {"", "this.", "super."}) {
String contents =
"class A {\n" +
" boolean value\n" +
" boolean isValue() {}\n" + // no getValue() generated
"}\n" +
"class B extends A {\n" +
" void test() {\n" +
" " + qual + "value\n" +
" " + qual + "getValue()\n" +
" }\n" +
"}";
assertUnknown(contents, "getValue");
if (qual.startsWith("super")) {
assertUnknown(contents, "value");
} else {
int offset = contents.lastIndexOf("value");
assertDeclaration(contents, offset, offset + 5, "A", "isValue", DeclarationKind.METHOD);
}
}
}

@Test
public void testSuperPropertyReference9a() {
for (String qual : new String[] {"", "this.", "super."}) {
String contents =
"class A {\n" +
" Boolean value\n" +
" Boolean isValue() {}\n" + // getValue() generated
"}\n" +
"class B extends A {\n" +
" void test() {\n" +
" " + qual + "value\n" +
" " + qual + "getValue()\n" +
" }\n" +
"}";
int offset = contents.lastIndexOf("value");
assertDeclaration(contents, offset, offset + 5, "A", "value", DeclarationKind.PROPERTY);
/**/offset = contents.lastIndexOf("getValue");
assertDeclaration(contents, offset, offset + 8, "A", "getValue", DeclarationKind.METHOD);
}
}

@Test
public void testSuperClassMethod1() {
String contents =
Expand Down
103 changes: 79 additions & 24 deletions ...vy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/basic/GroovySimpleTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5074,19 +5074,17 @@ public void testExtendingGroovyObjects_clinit() {
public void testGroovyPropertyAccessors1() {
//@formatter:off
String[] sources = {
"p/C.java",
"package p;\n" +
"C.java",
"public class C {\n" +
" public static void main(String[] argv) {\n" +
" G o = new G();\n" +
" System.out.print(o.isB());\n" +
" System.out.print(o.getB());\n" +
" G pogo = new G();\n" +
" System.out.print(pogo.isB());\n" +
" System.out.print(pogo.getB());\n" +
" }\n" +
"}\n",

"p/G.groovy",
"package p;\n" +
"public class G {\n" +
"G.groovy",
"class G {\n" +
" boolean b\n" +
"}\n",
};
Expand All @@ -5099,20 +5097,18 @@ public void testGroovyPropertyAccessors1() {
public void testGroovyPropertyAccessors2() {
//@formatter:off
String[] sources = {
"p/C.java",
"package p;\n" +
"C.java",
"public class C {\n" +
" public static void main(String[] argv) {\n" +
" G o = new G();\n" +
" System.out.print(o.getB());\n" +
" o.setB(true);\n" +
" System.out.print(o.getB());\n" +
" G pogo = new G();\n" +
" System.out.print(pogo.getB());\n" +
" pogo.setB(true);\n" +
" System.out.print(pogo.getB());\n" +
" }\n" +
"}\n",

"p/G.groovy",
"package p;\n" +
"public class G {\n" +
"G.groovy",
"class G {\n" +
" boolean b\n" +
"}\n",
};
Expand All @@ -5121,12 +5117,71 @@ public void testGroovyPropertyAccessors2() {
runConformTest(sources, "falsetrue");
}

@Test // @Deprecated should be propagated to accessors
@Test
public void testGroovyPropertyAccessors3() {
//@formatter:off
String[] sources = {
"p/G.groovy",
"package p;\n" +
"C.java",
"public class C {\n" +
" public static void main(String[] argv) {\n" +
" G pogo = new G();\n" +
" System.out.print(pogo.isB());\n" +
" System.out.print(pogo.getB());\n" +
" }\n" +
"}\n",

"G.groovy",
"class G {\n" +
" boolean b\n" +
" boolean isB() {}\n" +
"}\n",
};
//@formatter:on

runNegativeTest(sources,
"----------\n" +
"1. ERROR in C.java (at line 5)\n" +
"\tSystem.out.print(pogo.getB());\n" +
"\t ^^^^\n" +
"The method getB() is undefined for the type G\n" +
"----------\n");
}

@Test
public void testGroovyPropertyAccessors4() {
//@formatter:off
String[] sources = {
"C.java",
"public class C {\n" +
" public static void main(String[] argv) {\n" +
" G pogo = new G();\n" +
" System.out.print(pogo.isB());\n" +
" System.out.print(pogo.getB());\n" +
" }\n" +
"}\n",

"G.groovy",
"class G {\n" +
" boolean b\n" +
" boolean getB() {}\n" +
"}\n",
};
//@formatter:on

runNegativeTest(sources,
"----------\n" +
"1. ERROR in C.java (at line 4)\n" +
"\tSystem.out.print(pogo.isB());\n" +
"\t ^^^\n" +
"The method isB() is undefined for the type G\n" +
"----------\n");
}

@Test // @Deprecated should be propagated to accessors
public void testGroovyPropertyAccessors5() {
//@formatter:off
String[] sources = {
"G.groovy",
"class G {\n" +
" @Deprecated\n" +
" boolean flag\n" +
Expand All @@ -5136,19 +5191,19 @@ public void testGroovyPropertyAccessors3() {

runNegativeTest(sources, "");

checkDisassemblyFor("p/G.class",
checkDisassemblyFor("G.class",
" @java.lang.Deprecated\n" +
" private boolean flag;\n");

checkDisassemblyFor("p/G.class",
checkDisassemblyFor("G.class",
" @java.lang.Deprecated\n @groovy.transform.Generated\n" +
" public boolean isFlag();\n");

checkDisassemblyFor("p/G.class",
checkDisassemblyFor("G.class",
" @java.lang.Deprecated\n @groovy.transform.Generated\n" +
" public boolean getFlag();\n");

checkDisassemblyFor("p/G.class",
checkDisassemblyFor("G.class",
" @java.lang.Deprecated\n @groovy.transform.Generated\n" +
" public void setFlag(boolean arg0);\n");
}
Expand Down
4 changes: 3 additions & 1 deletion ...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
Expand Up @@ -129,10 +129,12 @@ protected MethodBinding[] augmentMethodBindings(final MethodBinding[] methodBind
String capitalizedName = MetaClassHelper.capitalize(property.getName());

if (property.getType().equals(ClassHelper.boolean_TYPE)) {
if (!createGetterMethod(property, "get" + capitalizedName, modifiers, methodBindings).isPresent()) {
continue; // only generate accessor method(s) if one or both are not explicitly declared
}
createGetterMethod(property, "is" + capitalizedName, modifiers, methodBindings)
.ifPresent(binding -> {
groovyMethods.add(binding);
// GROOVY-9382: no getter generated if isser declared
createGetterMethod(property, "get" + capitalizedName, modifiers, methodBindings)
.ifPresent(groovyMethods::add);
});
Expand Down
6 changes: 4 additions & 2 deletions ...lipse.jdt.groovy.core/src/org/codehaus/jdt/groovy/internal/compiler/ast/JDTClassNode.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -302,14 +302,16 @@ private void initializeMembers() {
for (PropertyNode pNode : getProperties()) {
String pName = pNode.getName(), capitalizedName = MetaClassHelper.capitalize(pName);
int mMods = Flags.AccPublic | (pNode.getModifiers() & Flags.AccStatic);
if (ClassHelper.boolean_TYPE.equals(pNode.getType())) {
if (pNode.getType().equals(ClassHelper.boolean_TYPE)) {
// only generate accessor method(s) if one or both are not explicitly declared
if (getDeclaredMethod("get" + capitalizedName, Parameter.EMPTY_ARRAY) != null) continue;

MethodNode mNode = addMethod("is" + capitalizedName, mMods, pNode.getType(), Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null);
if (!(mNode instanceof JDTNode)) {
mNode.setNameStart(pNode.getField().getNameStart());
mNode.setNameEnd(pNode.getField().getNameEnd());
mNode.setSynthetic(true);

// GROOVY-9382: include "getter" if "isser" was not declared
mNode = addMethod("get" + capitalizedName, mMods, pNode.getType(), Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null);
if (!(mNode instanceof JDTNode)) {
mNode.setNameStart(pNode.getField().getNameStart());
Expand Down
4 changes: 4 additions & 0 deletions base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/SimpleTypeLookup.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,10 @@ protected TypeLookupResult findTypeForNameWithKnownObjectExpression(final String
} else if (method.isPrivate() && isThisObjectExpression(scope) && isNotThisOrOuterClass(declaringType, resolvedDeclaringType)) {
// "this.method()" reference to private method of super class yields MissingMethodException; "super.method()" is okay
confidence = TypeConfidence.UNKNOWN;
} else if (method.getName().startsWith("is") && !name.startsWith("is") && isSuperObjectExpression(scope)) {
// GROOVY-1736, GROOVY-6097: "super.name" => "super.getName()" in AsmClassGenerator
confidence = TypeConfidence.UNKNOWN;
declaration = null;
} else if (isLooseMatch(scope.getMethodCallArgumentTypes(), method.getParameters()) &&
!(isStaticObjectExpression && isStaticReferenceToUnambiguousMethod(scope, name, declaringType)) &&
!(AccessorSupport.isGetter(method) && !scope.isMethodCall() && scope.getEnclosingNode() instanceof PropertyExpression)) {
Expand Down

0 comments on commit b339a5c

Please sign in to comment.