Skip to content

Commit

Permalink
Fix for #421: immutable explicit defaults
Browse files Browse the repository at this point in the history
  • Loading branch information
@eric-milles
eric-milles committed Oct 20, 2022
1 parent f3e05e8 commit 587103b
Show file tree
Hide file tree
Showing 7 changed files with 123 additions and 5 deletions.
22 changes: 22 additions & 0 deletions ...roovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/xform/CanonicalTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,4 +231,26 @@ public void testCanonical9() {

runConformTest(sources, "Outer$Inner(foo:bar, baz:null)");
}

@Test // GROOVY-10238
public void testCanonical10() {
//@formatter:off
String[] sources = {
"Main.groovy",
"class Outer {\n" +
" @groovy.transform.CompileStatic\n" +
" @groovy.transform.Canonical\n" +
" static class Inner {\n" +
" Map<String,Object> map = [:].withDefault { new Object() }\n" + // NoSuchMethodError: java.util.Map.withDefault(Lgroovy/lang/Closure;)
" }\n" +
"}\n" +
"def obj = new Outer.Inner()\n" +
"assert obj.toString() == 'Outer$Inner([:])'\n" +
"assert obj.map['foo'] != null\n" +
"assert obj.toString() != 'Outer$Inner([:])'\n" ,
};
//@formatter:on

runConformTest(sources);
}
}
46 changes: 45 additions & 1 deletion ...roovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/xform/ImmutableTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public void testImmutable2() {
String[] sources = {
"Main.java",
"public class Main {\n" +
" public static void main(String... args) {\n" +
" public static void main(String[] args) {\n" +
" System.out.print(new Foo(\"one\", \"two\"));\n" +
" }\n" +
"}\n",
Expand All @@ -92,4 +92,48 @@ public void testImmutable2() {

runConformTest(sources, "Foo(one, two)");
}

@Test // https://github.com/groovy/groovy-eclipse/issues/421
public void testImmutable3() {
//@formatter:off
String[] sources = {
"Main.java",
"public class Main {\n" +
" public static void main(String[] args) {\n" +
" System.out.print(new Foo(\"one\", \"two\"));\n" +
" }\n" +
"}\n",

"Foo.groovy",
"@groovy.transform.Immutable(defaults=false)\n" +
"class Foo {\n" +
" String bar, baz\n" +
"}\n",
};
//@formatter:on

runConformTest(sources, "Foo(one, two)");
}

@Test // https://github.com/groovy/groovy-eclipse/issues/421
public void testImmutable4() {
//@formatter:off
String[] sources = {
"Main.java",
"public class Main {\n" +
" public static void main(String[] args) {\n" +
" System.out.print(new Foo(\"one\"));\n" +
" }\n" +
"}\n",

"Foo.groovy",
"@groovy.transform.Immutable(defaults=true,noArg=false)\n" + //TODO: GROOVY-10790
"class Foo {\n" +
" String bar, baz = \"two\"\n" +
"}\n",
};
//@formatter:on

runConformTest(sources, "Foo(one, two)");
}
}
47 changes: 47 additions & 0 deletions ...ore.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/xform/TupleConstructorTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -262,4 +262,51 @@ public void testTupleConstructor9() {

runConformTest(sources, "Foo(baz:two, bar:one)");
}

@Test
public void testTupleConstructor10() {
//@formatter:off
String[] sources = {
"Main.groovy",
"@groovy.transform.TupleConstructor(includeFields=true)\n" +
"class Person {\n" +
" String firstName = 'John'\n" +
" private String lastName = 'Doe'\n" +
" String getLastName() { lastName }\n" +
"}\n" +
"def p = new Person()\n" +
"assert p.firstName == 'John'\n" +
"assert p.lastName == 'Doe'\n" +
"p = new Person('Jane')\n" +
"assert p.firstName == 'Jane'\n" +
"assert p.lastName == 'Doe'\n",
};
//@formatter:on

runConformTest(sources);
}

@Test
public void testTupleConstructor11() {
//@formatter:off
String[] sources = {
"Main.groovy",
"@groovy.transform.TupleConstructor(force=true)\n" +
"class Person {\n" +
" String firstName, lastName\n" +
" Person(Person that) {\n" +
" this.firstName = that.firstName\n" +
" }\n" +
"}\n" +
"def p = new Person('John', 'Doe')\n" +
"assert p.firstName == 'John'\n" +
"assert p.lastName == 'Doe'\n" +
"p = new Person(p)\n" +
"assert p.firstName == 'John'\n" +
"assert p.lastName == null\n",
};
//@formatter:on

runConformTest(sources);
}
}
1 change: 1 addition & 0 deletions base/org.codehaus.groovy25/src/org/codehaus/groovy/classgen/asm/InvocationWriter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1051,6 +1051,7 @@ private void makeMOPBasedConstructorCall(List<ConstructorNode> constructors, Con
if (sameHashNode!=null) {
controller.getSourceUnit().addError(
new SyntaxException("Unable to compile class "+controller.getClassNode().getName() + " due to hash collision in constructors", call.getLineNumber(), call.getColumnNumber()));
return; // GRECLIPSE add
}
}
Label[] targets = new Label[constructors.size()];
Expand Down
1 change: 1 addition & 0 deletions base/org.codehaus.groovy30/src/org/codehaus/groovy/classgen/asm/InvocationWriter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1026,6 +1026,7 @@ private void makeMOPBasedConstructorCall(final List<ConstructorNode> constructor
if (sameHashNode != null) {
controller.getSourceUnit().addError(new SyntaxException(
"Unable to compile class "+controller.getClassNode().getName() + " due to hash collision in constructors", call.getLineNumber(), call.getColumnNumber()));
return; // GRECLIPSE add
}
}
Label[] targets = new Label[constructors.size()];
Expand Down
1 change: 0 additions & 1 deletion ...odehaus.groovy40/src/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,6 @@ private static Parameter createParam(FieldNode fNode, String name, DefaultsMode
private static Expression providedOrDefaultInitialValue(final FieldNode fNode) {
ClassNode fType = fNode.getType();
Expression init = fNode.getInitialExpression();
fNode.setInitialValueExpression(null); // GROOVY-10238
if (init == null || (ClassHelper.isPrimitiveType(fType) && ExpressionUtils.isNullConstant(init))) {
init = defaultValueX(fType);
}
Expand Down
10 changes: 7 additions & 3 deletions ...e/src/org/codehaus/jdt/groovy/internal/compiler/ast/GroovyCompilationUnitDeclaration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import java.util.stream.Stream;

import groovy.lang.GroovyRuntimeException;
import groovy.transform.CompilationUnitAware;
import groovy.transform.PackageScopeTarget;

import org.codehaus.groovy.GroovyBugError;
Expand Down Expand Up @@ -2957,7 +2958,7 @@ private List<ConstructorNode> getDeclaredAndGeneratedConstructors(ClassNode clas
AnnotationNode anno = new AnnotationNode(ClassHelper.make(groovy.transform.TupleConstructor.class));
if (isType("groovy.transform.Immutable", annotationType))
anno.addMember("defaults", ConstantExpression.FALSE);
attributes.forEach((name, value) -> anno.addMember(name, value));
attributes.forEach((name, value) -> anno.setMember(name, value));

// create type that intercepts generated constructors
ClassNode type = new ClassNode(classNode.getName(), 0, null) {
Expand All @@ -2981,9 +2982,12 @@ public void addConstructor(ConstructorNode ctor) {
}
};

ASTTransformation astt = new org.codehaus.groovy.transform.TupleConstructorASTTransformation();
ASTTransformation astt = new org.codehaus.groovy.transform.TupleConstructorASTTransformation() {
@Override public void addError(String msg, org.codehaus.groovy.ast.ASTNode node) {}
};
((CompilationUnitAware) astt).setCompilationUnit(unitDeclaration.compilationUnit);
// apply TupleConstructorASTTransformation early to generate constructor(s)
astt.visit(new org.codehaus.groovy.ast.ASTNode[] {anno, type}, sourceUnit);
astt.visit(new org.codehaus.groovy.ast.ASTNode[] {anno, type}, null);

if (!generated.isEmpty()) {
constructorNodes = new ArrayList<>(constructorNodes);
Expand Down

0 comments on commit 587103b

Please sign in to comment.