You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2023/02/02 23:34:48 UTC
[groovy] branch master updated: GROOVY-10919, GROOVY-10925: read-only object fields in `@ToString` value
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push:
new 69293fb359 GROOVY-10919, GROOVY-10925: read-only object fields in `@ToString` value
69293fb359 is described below
commit 69293fb3592fb765c7ad368c1cca7df305d61ab8
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Thu Feb 2 17:20:52 2023 -0600
GROOVY-10919, GROOVY-10925: read-only object fields in `@ToString` value
---
.../transform/MapConstructorASTTransformation.java | 5 +--
.../transform/ToStringASTTransformation.java | 9 ++----
.../TupleConstructorASTTransformation.java | 9 +++---
.../groovy/transform/ImmutableTransformTest.groovy | 2 ++
.../transform/TupleConstructorTransformTest.groovy | 37 ++++++++++++++++++++++
5 files changed, 50 insertions(+), 12 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/transform/MapConstructorASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/MapConstructorASTTransformation.java
index 0944c8da5d..2361c181ff 100644
--- a/src/main/java/org/codehaus/groovy/transform/MapConstructorASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/MapConstructorASTTransformation.java
@@ -146,14 +146,15 @@ public class MapConstructorASTTransformation extends AbstractASTTransformation i
// HACK: JavaStubGenerator could have snuck in a constructor we don't want
cNode.getDeclaredConstructors().removeIf(next -> next.getFirstStatement() == null);
+ boolean includePseudoGetters = false, includePseudoSetters = allProperties, skipReadOnly = false; // GROOVY-4363
Set<String> names = new HashSet<>();
List<PropertyNode> properties;
if (includeSuperProperties || includeSuperFields) {
- properties = getAllProperties(names, cNode, cNode.getSuperClass(), includeSuperProperties, includeSuperFields, false, allProperties, true, false, false, allNames, includeStatic);
+ properties = getAllProperties(names, cNode, cNode.getSuperClass(), includeSuperProperties, includeSuperFields, includePseudoGetters, includePseudoSetters, /*super*/true, skipReadOnly, /*reverse*/false, allNames, includeStatic);
} else {
properties = new ArrayList<>();
}
- properties.addAll(getAllProperties(names, cNode, cNode, includeProperties, includeFields, false, allProperties, false, false, false, allNames, includeStatic));
+ properties.addAll(getAllProperties(names, cNode, cNode, includeProperties, includeFields, includePseudoGetters, includePseudoSetters, /*super*/false, skipReadOnly, /*reverse*/false, allNames, includeStatic));
BlockStatement body = new BlockStatement();
ClassCodeExpressionTransformer transformer = makeMapTypedArgsTransformer();
diff --git a/src/main/java/org/codehaus/groovy/transform/ToStringASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/ToStringASTTransformation.java
index d2c966573d..1228666d7b 100644
--- a/src/main/java/org/codehaus/groovy/transform/ToStringASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/ToStringASTTransformation.java
@@ -225,14 +225,11 @@ public class ToStringASTTransformation extends AbstractASTTransformation {
body.addStatement(appendS(result, constX(className + delims[0])));
Set<String> names = new HashSet<>();
- List<PropertyNode> list = getAllProperties(names, cNode, cNode, true, includeFields, allProperties, false, false, true, false, allNames, false);
- List<PropertyNode> superList;
+ boolean includeProperties = true, includePseudoGetters = allProperties, includePseudoSetters = false, skipReadOnly = false, includeStatic = false;
+ List<PropertyNode> list = getAllProperties(names, cNode, cNode, includeProperties, includeFields, includePseudoGetters, includePseudoSetters, /*super*/false, skipReadOnly, /*reverse*/false, allNames, includeStatic);
if (includeSuperProperties || includeSuperFields) {
- superList = getAllProperties(names, cNode, cNode.getSuperClass(), includeSuperProperties, includeSuperFields, allProperties, false, true, true, true, allNames, false);
- } else {
- superList = new ArrayList<>();
+ list.addAll(getAllProperties(names, cNode, cNode.getSuperClass(), includeSuperProperties, includeSuperFields, includePseudoGetters, includePseudoSetters, /*super*/true, skipReadOnly, /*reverse*/true, allNames, includeStatic));
}
- list.addAll(superList);
for (PropertyNode pNode : list) {
String name = pNode.getName();
diff --git a/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java
index ad9fb5d88c..007b8ad682 100644
--- a/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java
@@ -146,8 +146,8 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation
return;
GroovyClassLoader classLoader = compilationUnit != null ? compilationUnit.getTransformLoader() : source.getClassLoader();
PropertyHandler handler = PropertyHandler.createPropertyHandler(this, classLoader, cNode);
- if (handler == null) return;
- if (!handler.validateAttributes(this, anno)) return;
+ if (handler == null || !handler.validateAttributes(this, anno))
+ return;
Expression pre = anno.getMember("pre");
if (pre != null && !(pre instanceof ClosureExpression)) {
@@ -191,14 +191,15 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation
// no processing if explicit constructor(s) found, unless forced or ImmutableBase is in play
if (!force && !makeImmutable && hasExplicitConstructor(null, cNode)) return;
+ boolean includePseudoGetters = false, includePseudoSetters = allProperties, skipReadOnly = true;
Set<String> names = new HashSet<>();
List<PropertyNode> superList;
if (includeSuperProperties || includeSuperFields) {
- superList = getAllProperties(names, cNode.getSuperClass(), includeSuperProperties, includeSuperFields, false, allProperties, true, true);
+ superList = getAllProperties(names, cNode.getSuperClass(), includeSuperProperties, includeSuperFields, includePseudoGetters, includePseudoSetters, /*super*/true, skipReadOnly);
} else {
superList = new ArrayList<>();
}
- List<PropertyNode> list = getAllProperties(names, cNode, includeProperties, includeFields, false, allProperties, false, true);
+ List<PropertyNode> list = getAllProperties(names, cNode, includeProperties, includeFields, includePseudoGetters, includePseudoSetters, /*super*/false, skipReadOnly);
List<Parameter> params = new ArrayList<>();
List<Expression> superParams = new ArrayList<>();
diff --git a/src/test/org/codehaus/groovy/transform/ImmutableTransformTest.groovy b/src/test/org/codehaus/groovy/transform/ImmutableTransformTest.groovy
index fa024d0eea..be611b9072 100644
--- a/src/test/org/codehaus/groovy/transform/ImmutableTransformTest.groovy
+++ b/src/test/org/codehaus/groovy/transform/ImmutableTransformTest.groovy
@@ -339,6 +339,7 @@ final class ImmutableTransformTest {
'''
}
+ // GROOVY-4363
@Test
void testPrivateFieldAssignedViaConstructor() {
assertScript shell, '''
@@ -362,6 +363,7 @@ final class ImmutableTransformTest {
'''
}
+ // GROOVY-4363
@Test
void testPrivateFinalFieldAssignedViaConstructorShouldCauseError() {
shouldFail shell, ReadOnlyPropertyException, '''
diff --git a/src/test/org/codehaus/groovy/transform/TupleConstructorTransformTest.groovy b/src/test/org/codehaus/groovy/transform/TupleConstructorTransformTest.groovy
index 1eeab1cb99..42b585ed97 100644
--- a/src/test/org/codehaus/groovy/transform/TupleConstructorTransformTest.groovy
+++ b/src/test/org/codehaus/groovy/transform/TupleConstructorTransformTest.groovy
@@ -18,6 +18,7 @@
*/
package org.codehaus.groovy.transform
+import groovy.test.NotYetImplemented
import org.junit.Test
import static groovy.test.GroovyAssert.assertScript
@@ -551,4 +552,40 @@ final class TupleConstructorTransformTest {
assert new Foo().toString() == 'Foo(null, z)'
'''
}
+
+ // GROOVY-10919
+ @NotYetImplemented @Test
+ void testWithMapConstructor2() {
+ assertScript shell, '''
+ @MapConstructor(includeFields=true)
+ @TupleConstructor(includeFields=true)
+ @ToString(includeFields=true, includeNames=true)
+ class Foo {
+ private final int x
+ private int y = 1
+ private final int z
+ }
+
+ String string = new Foo(x:2, z:3)
+ assert string == 'Foo(x:2, y:1, z:3)'
+ '''
+ }
+
+ // GROOVY-10925
+ @Test
+ void testWithMapConstructor3() {
+ assertScript shell, '''
+ @MapConstructor(includeFields=true)
+ @TupleConstructor(includeFields=true)
+ @ToString(includeFields=true, includeNames=true)
+ class Foo {
+ private final int x = 1
+ private int y = 2
+ private final int z
+ }
+
+ String string = new Foo(x:3, y:3, z:3)
+ assert string == 'Foo(x:3, y:3, z:3)'
+ '''
+ }
}