This is an automated email from the ASF dual-hosted git repository. paulk pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 80530316a15340cb900766069604d3cdc24a669c Author: Paul King <[email protected]> AuthorDate: Fri Oct 22 18:13:58 2021 +1000 GROOVY-10298: Refine records to not use system properties --- src/main/groovy/groovy/transform/RecordType.groovy | 4 +- .../java/groovy/transform/TupleConstructor.java | 9 +++ .../apache/groovy/parser/antlr4/AstBuilder.java | 65 ++++------------------ .../java/org/codehaus/groovy/ast/PropertyNode.java | 6 +- .../org/codehaus/groovy/classgen/Verifier.java | 22 +++----- .../transform/NamedVariantASTTransformation.java | 24 ++++---- .../transform/RecordTypeASTTransformation.java | 34 ++++++----- .../TupleConstructorASTTransformation.java | 30 ++++++++-- .../core/RecordDeclaration_10x.groovy | 11 +++- .../core/RecordDeclaration_11x.groovy | 2 +- .../core/RecordDeclaration_12x.groovy | 10 ++-- .../org/codehaus/groovy/classgen/RecordTest.groovy | 26 +++++---- 12 files changed, 116 insertions(+), 127 deletions(-) diff --git a/src/main/groovy/groovy/transform/RecordType.groovy b/src/main/groovy/groovy/transform/RecordType.groovy index e5d0528..044fc27 100644 --- a/src/main/groovy/groovy/transform/RecordType.groovy +++ b/src/main/groovy/groovy/transform/RecordType.groovy @@ -72,7 +72,6 @@ import java.lang.annotation.Target * @see ImmutableOptions * @see PropertyOptions * @see TupleConstructor - * @see MapConstructor * @see KnownImmutable * @see POJO * @since 4.0.0 @@ -80,8 +79,7 @@ import java.lang.annotation.Target @RecordBase @ImmutableOptions @PropertyOptions(propertyHandler = ImmutablePropertyHandler) -@TupleConstructor(defaults = false) -@MapConstructor +@TupleConstructor(namedVariant = true) @KnownImmutable @POJO @CompileStatic diff --git a/src/main/java/groovy/transform/TupleConstructor.java b/src/main/java/groovy/transform/TupleConstructor.java index 770ae9b..e0527da 100644 --- a/src/main/java/groovy/transform/TupleConstructor.java +++ b/src/main/java/groovy/transform/TupleConstructor.java @@ -325,6 +325,15 @@ public @interface TupleConstructor { boolean allProperties() default false; /** + * If true, add a map-based named-arg variant. + * Similar to using {@code @MapConstructor} but handles just the parameters handled by the generated tuple constructor. + * Setting {@code namedVariant=true} is incompatible with using {@code @MapConstructor}. + * + * @since 4.0.0 + */ + boolean namedVariant() default false; + + /** * If specified, must match the "id" attribute in a VisibilityOptions annotation to enable a custom visibility. * * @since 2.5.0 diff --git a/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java b/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java index 9ed4453..1cfb080 100644 --- a/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java +++ b/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java @@ -21,7 +21,6 @@ package org.apache.groovy.parser.antlr4; import groovy.lang.Tuple2; import groovy.lang.Tuple3; import groovy.transform.CompileStatic; -import groovy.transform.MapConstructor; import groovy.transform.NonSealed; import groovy.transform.Sealed; import groovy.transform.Trait; @@ -41,7 +40,6 @@ import org.antlr.v4.runtime.misc.ParseCancellationException; import org.antlr.v4.runtime.tree.ParseTree; import org.antlr.v4.runtime.tree.TerminalNode; import org.apache.groovy.ast.tools.AnnotatedNodeUtils; -import org.apache.groovy.internal.util.Function; import org.apache.groovy.parser.antlr4.GroovyParser.AdditiveExprAltContext; import org.apache.groovy.parser.antlr4.GroovyParser.AndExprAltContext; import org.apache.groovy.parser.antlr4.GroovyParser.AnnotatedQualifiedClassNameContext; @@ -237,7 +235,6 @@ import org.codehaus.groovy.ast.NodeMetaDataHandler; import org.codehaus.groovy.ast.PackageNode; import org.codehaus.groovy.ast.Parameter; import org.codehaus.groovy.ast.PropertyNode; -import org.codehaus.groovy.ast.RecordComponentNode; import org.codehaus.groovy.ast.expr.AnnotationConstantExpression; import org.codehaus.groovy.ast.expr.ArgumentListExpression; import org.codehaus.groovy.ast.expr.ArrayExpression; @@ -361,19 +358,14 @@ import static org.apache.groovy.parser.antlr4.GroovyParser.SUB; import static org.apache.groovy.parser.antlr4.GroovyParser.VAR; import static org.apache.groovy.parser.antlr4.util.PositionConfigureUtils.configureAST; import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveVoid; -import static org.codehaus.groovy.ast.tools.GeneralUtils.args; -import static org.codehaus.groovy.ast.tools.GeneralUtils.assignS; import static org.codehaus.groovy.ast.tools.GeneralUtils.assignX; -import static org.codehaus.groovy.ast.tools.GeneralUtils.block; import static org.codehaus.groovy.ast.tools.GeneralUtils.callX; -import static org.codehaus.groovy.ast.tools.GeneralUtils.castX; import static org.codehaus.groovy.ast.tools.GeneralUtils.cloneParams; import static org.codehaus.groovy.ast.tools.GeneralUtils.closureX; import static org.codehaus.groovy.ast.tools.GeneralUtils.constX; import static org.codehaus.groovy.ast.tools.GeneralUtils.declS; import static org.codehaus.groovy.ast.tools.GeneralUtils.listX; import static org.codehaus.groovy.ast.tools.GeneralUtils.localVarX; -import static org.codehaus.groovy.ast.tools.GeneralUtils.mapX; import static org.codehaus.groovy.ast.tools.GeneralUtils.returnS; import static org.codehaus.groovy.ast.tools.GeneralUtils.stmt; import static org.codehaus.groovy.ast.tools.GeneralUtils.varX; @@ -1967,10 +1959,9 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> { if (!methodName.equals(className)) { createParsingFailedException("Compact constructor should have the same name with record: " + className, ctx.methodName()); } - ClassNode returnType = ClassHelper.MAP_TYPE.getPlainNodeReference(); Parameter[] header = classNode.getNodeMetaData(RECORD_HEADER); - Objects.requireNonNull(classNode, "record header should not be null"); + Objects.requireNonNull(header, "record header should not be null"); final Parameter[] parameters = cloneParams(header); Statement code = this.visitMethodBody(ctx.methodBody()); @@ -1985,51 +1976,18 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> { } }); - Statement block = block( - stmt(callX(closureX(code), "call")), - returnS(mapX(Arrays.stream(parameters).map(p -> { - String parameterName = p.getName(); - return new MapEntryExpression(constX(parameterName), varX(parameterName)); - }).collect(Collectors.toList()))) - ); - MethodNode methodNode = classNode.addSyntheticMethod(RECORD_COMPACT_CONSTRUCTOR_NAME, Opcodes.ACC_PRIVATE, - returnType, parameters, ClassNode.EMPTY_ARRAY, block); - - ConstructorNode dummyConstructorNode = configureAST(new ConstructorNode(modifierManager.getClassMemberModifiersOpValue(), block), ctx); - modifierManager.validate(dummyConstructorNode); - - modifierManager.attachAnnotations(methodNode); - attachMapConstructorAnnotationToRecord(classNode, parameters); - attachTupleConstructorAnnotationToRecord(classNode, parameters); - - return configureAST(methodNode, ctx); - } - - private void attachMapConstructorAnnotationToRecord(ClassNode classNode, Parameter[] parameters) { - doAttachConstructorAnnotationToRecord(classNode, MapConstructor.class, - parameters, p -> castX(p.getOriginType(), callX(varX("args"), "get", args(constX(p.getName()))))); - } - - private void attachTupleConstructorAnnotationToRecord(ClassNode classNode, Parameter[] parameters) { - doAttachConstructorAnnotationToRecord(classNode, TupleConstructor.class, - parameters, p -> castX(p.getOriginType(), varX(p.getName()))); + attachTupleConstructorAnnotationToRecord(classNode, parameters, code); + return null; } - private void doAttachConstructorAnnotationToRecord(ClassNode classNode, Class<?> annotationClass, Parameter[] parameters, Function<? super Parameter, ? extends Expression> mapper) { - AnnotationNode tupleConstructorAnnotationNode = new AnnotationNode(ClassHelper.makeCached(annotationClass)); - List<Expression> argExpressionList = - Arrays.stream(parameters) - .map(mapper::apply) - .collect(Collectors.toList()); - final String resultVarName = "$r" + System.nanoTime(); - tupleConstructorAnnotationNode.setMember("pre", closureX(block( - declS(localVarX(resultVarName), castX(ClassHelper.MAP_TYPE.getPlainNodeReference(), callX(varX("this"), RECORD_COMPACT_CONSTRUCTOR_NAME, args(argExpressionList)))), - assignS( - new TupleExpression(Arrays.stream(parameters).map(p -> varX(p.getName())).collect(Collectors.toList())), - listX(Arrays.stream(parameters).map(p -> castX(p.getOriginType(), callX(varX(resultVarName), "get", args(constX(p.getName()))))).collect(Collectors.toList())) - ) - ))); - classNode.addAnnotation(tupleConstructorAnnotationNode); + private void attachTupleConstructorAnnotationToRecord(ClassNode classNode, Parameter[] parameters, Statement block) { + ClassNode tupleConstructorType = ClassHelper.makeCached(TupleConstructor.class); + List<AnnotationNode> annos = classNode.getAnnotations(tupleConstructorType); + AnnotationNode tupleConstructor = annos.isEmpty() ? new AnnotationNode(tupleConstructorType) : annos.get(0); + tupleConstructor.setMember("pre", closureX(block)); + if (annos.isEmpty()) { + classNode.addAnnotation(tupleConstructor); + } } @Override @@ -5260,7 +5218,6 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> { private static final String PARAMETER_CONTEXT = "_PARAMETER_CONTEXT"; private static final String IS_RECORD_GENERATED = "_IS_RECORD_GENERATED"; private static final String RECORD_HEADER = "_RECORD_HEADER"; - private static final String RECORD_COMPACT_CONSTRUCTOR_NAME = "$compactInit"; private static final String RECORD_TYPE_NAME = "groovy.transform.RecordType"; private static final ClassNode RECORD_TYPE_CLASS = ClassHelper.makeWithoutCaching(RECORD_TYPE_NAME); } diff --git a/src/main/java/org/codehaus/groovy/ast/PropertyNode.java b/src/main/java/org/codehaus/groovy/ast/PropertyNode.java index 0a6add9..012a1ca 100644 --- a/src/main/java/org/codehaus/groovy/ast/PropertyNode.java +++ b/src/main/java/org/codehaus/groovy/ast/PropertyNode.java @@ -38,7 +38,7 @@ public class PropertyNode extends AnnotatedNode implements Variable { private Statement setterBlock; private String getterName = null; private String setterName = null; - private final int modifiers; + private int modifiers; public PropertyNode( String name, int modifiers, ClassNode type, ClassNode owner, @@ -117,6 +117,10 @@ public class PropertyNode extends AnnotatedNode implements Variable { return modifiers; } + public void setModifiers(int modifiers) { + this.modifiers = modifiers; + } + @Override public String getName() { return field.getName(); diff --git a/src/main/java/org/codehaus/groovy/classgen/Verifier.java b/src/main/java/org/codehaus/groovy/classgen/Verifier.java index 9a925db..41e5047 100644 --- a/src/main/java/org/codehaus/groovy/classgen/Verifier.java +++ b/src/main/java/org/codehaus/groovy/classgen/Verifier.java @@ -814,25 +814,17 @@ public class Verifier implements GroovyClassVisitor, Opcodes { } if (getterBlock != null) { - boolean toVisitGetter = true; - if (classNode.isRecord()) { - boolean isGetterDefined = classNode.getDeclaredMethods(name).stream() - .anyMatch(MethodNodeUtils::isGetterCandidate); - toVisitGetter = !isGetterDefined; - } - - if (toVisitGetter) { - visitGetter(node, field, getterBlock, getterModifiers, getterName); + visitGetter(node, field, getterBlock, getterModifiers, getterName); - if (node.getGetterName() == null && getterName.startsWith("get") && isPrimitiveBoolean(node.getType())) { - String altGetterName = "is" + capitalize(name); - MethodNode altGetter = classNode.getGetterMethod(altGetterName, !node.isStatic()); - if (methodNeedsReplacement(altGetter)) { - visitGetter(node, field, getterBlock, getterModifiers, altGetterName); - } + if (node.getGetterName() == null && getterName.startsWith("get") && isPrimitiveBoolean(node.getType())) { + String altGetterName = "is" + capitalize(name); + MethodNode altGetter = classNode.getGetterMethod(altGetterName, !node.isStatic()); + if (methodNeedsReplacement(altGetter)) { + visitGetter(node, field, getterBlock, getterModifiers, altGetterName); } } } + if (setterBlock != null) { Parameter[] setterParameterTypes = {new Parameter(node.getType(), "value")}; MethodNode setter = new MethodNode(setterName, accessorModifiers, ClassHelper.VOID_TYPE, setterParameterTypes, ClassNode.EMPTY_ARRAY, setterBlock); diff --git a/src/main/java/org/codehaus/groovy/transform/NamedVariantASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/NamedVariantASTTransformation.java index f13fca7..33d8c4d 100644 --- a/src/main/java/org/codehaus/groovy/transform/NamedVariantASTTransformation.java +++ b/src/main/java/org/codehaus/groovy/transform/NamedVariantASTTransformation.java @@ -121,7 +121,7 @@ public class NamedVariantASTTransformation extends AbstractASTTransformation { } else { for (Parameter fromParam : fromParams) { if (!annoFound) { - if (!processImplicitNamedParam(mNode, mapParam, args, propNames, fromParam, coerce)) return; + if (!processImplicitNamedParam(this, mNode, mapParam, args, propNames, fromParam, coerce)) return; } else if (AnnotatedNodeUtils.hasAnnotation(fromParam, NAMED_PARAM_TYPE)) { if (!processExplicitNamedParam(mNode, mapParam, inner, args, propNames, fromParam, coerce)) return; } else if (AnnotatedNodeUtils.hasAnnotation(fromParam, NAMED_DELEGATE_TYPE)) { @@ -130,18 +130,18 @@ public class NamedVariantASTTransformation extends AbstractASTTransformation { VariableExpression arg = varX(fromParam); Expression argOrDefault = fromParam.hasInitialExpression() ? elvisX(arg, fromParam.getDefaultValue()) : arg; args.addExpression(asType(argOrDefault, fromParam.getType(), coerce)); - if (hasDuplicates(mNode, propNames, fromParam.getName())) return; + if (hasDuplicates(this, mNode, propNames, fromParam.getName())) return; genParams.add(fromParam); } } } - createMapVariant(mNode, anno, mapParam, genParams, cNode, inner, args, propNames); + createMapVariant(this, mNode, anno, mapParam, genParams, cNode, inner, args, propNames); } - private boolean processImplicitNamedParam(final MethodNode mNode, final Parameter mapParam, final ArgumentListExpression args, final List<String> propNames, final Parameter fromParam, boolean coerce) { + static boolean processImplicitNamedParam(final ErrorCollecting xform, final MethodNode mNode, final Parameter mapParam, final ArgumentListExpression args, final List<String> propNames, final Parameter fromParam, boolean coerce) { boolean required = fromParam.hasInitialExpression(); String name = fromParam.getName(); - if (hasDuplicates(mNode, propNames, name)) return false; + if (hasDuplicates(xform, mNode, propNames, name)) return false; AnnotationNode namedParam = new AnnotationNode(NAMED_PARAM_TYPE); namedParam.addMember("value", constX(name)); namedParam.addMember("type", classX(fromParam.getType())); @@ -163,7 +163,7 @@ public class NamedVariantASTTransformation extends AbstractASTTransformation { if (getMemberValue(namedParam, "type") == null) { namedParam.addMember("type", classX(fromParam.getType())); } - if (hasDuplicates(mNode, propNames, name)) return false; + if (hasDuplicates(this, mNode, propNames, name)) return false; // TODO: Check specified type is assignable from declared param type? //ClassNode type = getMemberClassValue(namedParam, "type"); if (required) { @@ -193,7 +193,7 @@ public class NamedVariantASTTransformation extends AbstractASTTransformation { Set<String> names = new HashSet<>(); List<PropertyNode> props = getAllProperties(names, fromParam.getType(), true, false, false, true, false, true); for (String next : names) { - if (hasDuplicates(mNode, propNames, next)) return false; + if (hasDuplicates(this, mNode, propNames, next)) return false; } List<MapEntryExpression> entries = new ArrayList<>(); for (PropertyNode pNode : props) { @@ -212,16 +212,16 @@ public class NamedVariantASTTransformation extends AbstractASTTransformation { return true; } - private boolean hasDuplicates(final MethodNode mNode, final List<String> propNames, final String next) { + private static boolean hasDuplicates(final ErrorCollecting xform, final MethodNode mNode, final List<String> propNames, final String next) { if (propNames.contains(next)) { - addError("Error during " + NAMED_VARIANT + " processing. Duplicate property '" + next + "' found.", mNode); + xform.addError("Error during " + NAMED_VARIANT + " processing. Duplicate property '" + next + "' found.", mNode); return true; } propNames.add(next); return false; } - private void createMapVariant(final MethodNode mNode, final AnnotationNode anno, final Parameter mapParam, final List<Parameter> genParams, final ClassNode cNode, final BlockStatement inner, final ArgumentListExpression args, final List<String> propNames) { + static void createMapVariant(final ErrorCollecting xform, final MethodNode mNode, final AnnotationNode anno, final Parameter mapParam, final List<Parameter> genParams, final ClassNode cNode, final BlockStatement inner, final ArgumentListExpression args, final List<String> propNames) { Parameter namedArgKey = param(STRING_TYPE, "namedArgKey"); inner.addStatement( new ForStatement( @@ -233,7 +233,7 @@ public class NamedVariantASTTransformation extends AbstractASTTransformation { Parameter[] genParamsArray = genParams.toArray(Parameter.EMPTY_ARRAY); // TODO account for default params giving multiple signatures if (cNode.hasMethod(mNode.getName(), genParamsArray)) { - addError("Error during " + NAMED_VARIANT + " processing. Class " + cNode.getNameWithoutPackage() + + xform.addError("Error during " + NAMED_VARIANT + " processing. Class " + cNode.getNameWithoutPackage() + " already has a named-arg " + (mNode instanceof ConstructorNode ? "constructor" : "method") + " of type " + genParams, mNode); return; @@ -264,7 +264,7 @@ public class NamedVariantASTTransformation extends AbstractASTTransformation { } } - private Expression asType(Expression expression, ClassNode classNode, boolean coerce) { + private static Expression asType(Expression expression, ClassNode classNode, boolean coerce) { if (coerce) { return asX(classNode, expression); } else { diff --git a/src/main/java/org/codehaus/groovy/transform/RecordTypeASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/RecordTypeASTTransformation.java index aa900ab..eab9fd1 100644 --- a/src/main/java/org/codehaus/groovy/transform/RecordTypeASTTransformation.java +++ b/src/main/java/org/codehaus/groovy/transform/RecordTypeASTTransformation.java @@ -23,6 +23,7 @@ import groovy.transform.CompilationUnitAware; import groovy.transform.RecordBase; import groovy.transform.RecordTypeMode; import groovy.transform.options.PropertyHandler; +import org.apache.groovy.ast.tools.MethodNodeUtils; import org.codehaus.groovy.ast.ASTNode; import org.codehaus.groovy.ast.AnnotatedNode; import org.codehaus.groovy.ast.AnnotationNode; @@ -134,8 +135,6 @@ public class RecordTypeASTTransformation extends AbstractASTTransformation imple addError(message + " when attempting to create a native record", cNode); } - List<PropertyNode> newProperties = new ArrayList<>(); - String cName = cNode.getName(); if (!checkNotInterface(cNode, MY_TYPE_NAME)) return; makeClassFinal(this, cNode); @@ -143,15 +142,8 @@ public class RecordTypeASTTransformation extends AbstractASTTransformation imple final List<PropertyNode> pList = getInstanceProperties(cNode); for (PropertyNode pNode : pList) { - adjustPropertyForImmutability(pNode, newProperties, handler); - } - // replace props with final version of self - for (PropertyNode oldProp : newProperties) { - cNode.getProperties().remove(oldProp); - PropertyNode newProp = new PropertyNode(oldProp.getField(), oldProp.getModifiers() | ACC_FINAL, oldProp.getGetterBlock(), null); - newProp.setGetterName(oldProp.getGetterNameOrDefault()); - cNode.removeField(oldProp.getField().getName()); - cNode.addProperty(newProp); + adjustPropertyForImmutability(cNode, pNode, handler); + pNode.setModifiers(pNode.getModifiers() | ACC_FINAL); } final List<FieldNode> fList = cNode.getFields(); for (FieldNode fNode : fList) { @@ -166,7 +158,10 @@ public class RecordTypeASTTransformation extends AbstractASTTransformation imple if (isNative) { createRecordToString(cNode); } else { - ToStringASTTransformation.createToString(cNode, false, false, null, null, true, false, true, true); + ToStringASTTransformation.createToString(cNode, false, false, null, + null, true, false, false, true, + false, false, false, false, false, + new String[]{"[", "]", "=", ", "}); } } @@ -310,15 +305,18 @@ public class RecordTypeASTTransformation extends AbstractASTTransformation imple } } - private static void adjustPropertyForImmutability(PropertyNode pNode, List<PropertyNode> newNodes, PropertyHandler handler) { + private static void adjustPropertyForImmutability(ClassNode cNode, PropertyNode pNode, PropertyHandler handler) { final FieldNode fNode = pNode.getField(); fNode.setModifiers((pNode.getModifiers() & (~ACC_PUBLIC)) | ACC_FINAL | ACC_PRIVATE); - Statement getter = handler.createPropGetter(pNode); - if (getter != null) { - pNode.setGetterBlock(getter); - pNode.setGetterName(pNode.getName()); + boolean isGetterDefined = cNode.getDeclaredMethods(pNode.getName()).stream() + .anyMatch(MethodNodeUtils::isGetterCandidate); + if (!isGetterDefined) { + Statement getter = handler.createPropGetter(pNode); + if (getter != null) { + pNode.setGetterBlock(getter); + pNode.setGetterName(pNode.getName()); + } } - newNodes.add(pNode); } @Override diff --git a/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java index 5a1e156..bc1d883 100644 --- a/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java +++ b/src/main/java/org/codehaus/groovy/transform/TupleConstructorASTTransformation.java @@ -34,6 +34,7 @@ import org.codehaus.groovy.ast.ConstructorNode; import org.codehaus.groovy.ast.FieldNode; import org.codehaus.groovy.ast.Parameter; import org.codehaus.groovy.ast.PropertyNode; +import org.codehaus.groovy.ast.expr.ArgumentListExpression; import org.codehaus.groovy.ast.expr.ClosureExpression; import org.codehaus.groovy.ast.expr.ConstantExpression; import org.codehaus.groovy.ast.expr.Expression; @@ -43,6 +44,7 @@ import org.codehaus.groovy.ast.stmt.BlockStatement; import org.codehaus.groovy.ast.stmt.EmptyStatement; import org.codehaus.groovy.ast.stmt.ExpressionStatement; import org.codehaus.groovy.ast.stmt.Statement; +import org.codehaus.groovy.ast.tools.GenericsUtils; import org.codehaus.groovy.classgen.VariableScopeVisitor; import org.codehaus.groovy.control.CompilationUnit; import org.codehaus.groovy.control.CompilePhase; @@ -59,6 +61,7 @@ import static org.apache.groovy.ast.tools.ClassNodeUtils.addGeneratedConstructor import static org.apache.groovy.ast.tools.ClassNodeUtils.hasExplicitConstructor; import static org.apache.groovy.ast.tools.ConstructorNodeUtils.checkPropNamesS; import static org.apache.groovy.ast.tools.VisibilityUtils.getVisibility; +import static org.codehaus.groovy.ast.ClassHelper.MAP_TYPE; import static org.codehaus.groovy.ast.ClassHelper.isObjectType; import static org.codehaus.groovy.ast.ClassHelper.make; import static org.codehaus.groovy.ast.ClassHelper.makeWithoutCaching; @@ -70,16 +73,19 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.constX; import static org.codehaus.groovy.ast.tools.GeneralUtils.copyStatementsWithSuperAdjustment; import static org.codehaus.groovy.ast.tools.GeneralUtils.ctorX; import static org.codehaus.groovy.ast.tools.GeneralUtils.defaultValueX; +import static org.codehaus.groovy.ast.tools.GeneralUtils.elvisX; import static org.codehaus.groovy.ast.tools.GeneralUtils.equalsNullX; import static org.codehaus.groovy.ast.tools.GeneralUtils.getAllProperties; import static org.codehaus.groovy.ast.tools.GeneralUtils.ifElseS; import static org.codehaus.groovy.ast.tools.GeneralUtils.ifS; +import static org.codehaus.groovy.ast.tools.GeneralUtils.param; import static org.codehaus.groovy.ast.tools.GeneralUtils.params; import static org.codehaus.groovy.ast.tools.GeneralUtils.propX; import static org.codehaus.groovy.ast.tools.GeneralUtils.stmt; import static org.codehaus.groovy.ast.tools.GeneralUtils.throwS; import static org.codehaus.groovy.ast.tools.GeneralUtils.varX; import static org.codehaus.groovy.transform.ImmutableASTTransformation.makeImmutable; +import static org.codehaus.groovy.transform.NamedVariantASTTransformation.processImplicitNamedParam; import static org.objectweb.asm.Opcodes.ACC_PUBLIC; /** @@ -92,6 +98,7 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation static final Class MY_CLASS = TupleConstructor.class; static final ClassNode MY_TYPE = make(MY_CLASS); static final String MY_TYPE_NAME = "@" + MY_TYPE.getNameWithoutPackage(); + private static final String NAMED_ARGS = "__namedArgs"; private static final ClassNode LHMAP_TYPE = makeWithoutCaching(LinkedHashMap.class, false); private static final ClassNode POJO_TYPE = make(POJO.class); @@ -120,6 +127,7 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation boolean includeSuperFields = memberHasValue(anno, "includeSuperFields", true); boolean includeSuperProperties = memberHasValue(anno, "includeSuperProperties", true); boolean allProperties = memberHasValue(anno, "allProperties", true); + boolean namedVariant = memberHasValue(anno, "namedVariant", true); List<String> excludes = getMemberStringList(anno, "excludes"); List<String> includes = getMemberStringList(anno, "includes"); boolean allNames = memberHasValue(anno, "allNames", true); @@ -145,7 +153,7 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation } createConstructor(this, anno, cNode, includeFields, includeProperties, includeSuperFields, includeSuperProperties, - excludes, includes, allNames, allProperties, + excludes, includes, allNames, allProperties, namedVariant, sourceUnit, handler, (ClosureExpression) pre, (ClosureExpression) post); if (pre != null) { @@ -159,7 +167,7 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation private static void createConstructor(AbstractASTTransformation xform, AnnotationNode anno, ClassNode cNode, boolean includeFields, boolean includeProperties, boolean includeSuperFields, boolean includeSuperProperties, - List<String> excludes, final List<String> includes, boolean allNames, boolean allProperties, + List<String> excludes, final List<String> includes, boolean allNames, boolean allProperties, boolean namedVariant, SourceUnit sourceUnit, PropertyHandler handler, ClosureExpression pre, ClosureExpression post) { boolean callSuper = xform.memberHasValue(anno, "callSuper", true); boolean force = xform.memberHasValue(anno, "force", true); @@ -245,7 +253,19 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation boolean hasMapCons = AnnotatedNodeUtils.hasAnnotation(cNode, MapConstructorASTTransformation.MY_TYPE); int modifiers = getVisibility(anno, cNode, ConstructorNode.class, ACC_PUBLIC); - addGeneratedConstructor(cNode, modifiers, params.toArray(Parameter.EMPTY_ARRAY), ClassNode.EMPTY_ARRAY, body); + ConstructorNode consNode = addGeneratedConstructor(cNode, modifiers, params.toArray(Parameter.EMPTY_ARRAY), ClassNode.EMPTY_ARRAY, body); + if (namedVariant) { + BlockStatement inner = new BlockStatement(); + Parameter mapParam = param(GenericsUtils.nonGeneric(MAP_TYPE), NAMED_ARGS); + List<Parameter> genParams = new ArrayList<>(); + genParams.add(mapParam); + ArgumentListExpression args = new ArgumentListExpression(); + List<String> propNames = new ArrayList<>(); + for (Parameter p : params) { + if (!processImplicitNamedParam(xform, consNode, mapParam, args, propNames, p,false)) return; + } + NamedVariantASTTransformation.createMapVariant(xform, consNode, anno, mapParam, genParams, cNode, inner, args, propNames); + } if (sourceUnit != null && !body.isEmpty()) { VariableScopeVisitor scopeVisitor = new VariableScopeVisitor(sourceUnit); @@ -294,9 +314,9 @@ public class TupleConstructorASTTransformation extends AbstractASTTransformation } public static void addSpecialMapConstructors(int modifiers, ClassNode cNode, String message, boolean addNoArg) { - Parameter[] parameters = params(new Parameter(LHMAP_TYPE, "__namedArgs")); + Parameter[] parameters = params(new Parameter(LHMAP_TYPE, NAMED_ARGS)); BlockStatement code = new BlockStatement(); - VariableExpression namedArgs = varX("__namedArgs"); + VariableExpression namedArgs = varX(NAMED_ARGS); namedArgs.setAccessedVariable(parameters[0]); code.addStatement(ifElseS(equalsNullX(namedArgs), illegalArgumentBlock(message), diff --git a/src/test-resources/core/RecordDeclaration_10x.groovy b/src/test-resources/core/RecordDeclaration_10x.groovy index f1de04b..05296d3 100644 --- a/src/test-resources/core/RecordDeclaration_10x.groovy +++ b/src/test-resources/core/RecordDeclaration_10x.groovy @@ -18,6 +18,7 @@ */ package core [email protected](defaults=false) record Point(int x, int y, String color) { public Point { x = -x; @@ -26,11 +27,11 @@ record Point(int x, int y, String color) { } public Point(int x, int y) { - this(x, y, "Blue"); + this(x, y, 'Blue'); } } -def p1 = new Point(5, 10, "Green") +def p1 = new Point(5, 10, 'Green') assert -5 == p1.x() assert 10 == p1.y() assert 'GREEN' == p1.color() @@ -39,3 +40,9 @@ def p2 = new Point(0, 20) assert 0 == p2.x() assert 20 == p2.y() assert 'BLUE' == p2.color() + +def p3 = new Point(x: 100, y: 200, color: 'Red') +assert -100 == p3.x() +assert 200 == p3.y() +assert 'RED' == p3.color() + diff --git a/src/test-resources/core/RecordDeclaration_11x.groovy b/src/test-resources/core/RecordDeclaration_11x.groovy index 961cdee..5d4c10a 100644 --- a/src/test-resources/core/RecordDeclaration_11x.groovy +++ b/src/test-resources/core/RecordDeclaration_11x.groovy @@ -18,7 +18,7 @@ */ package core [email protected] [email protected](defaults=false, namedVariant=false) record Point(int x, int y, String color) { public Point { x = -x; diff --git a/src/test-resources/core/RecordDeclaration_12x.groovy b/src/test-resources/core/RecordDeclaration_12x.groovy index 2f54939..702ae45 100644 --- a/src/test-resources/core/RecordDeclaration_12x.groovy +++ b/src/test-resources/core/RecordDeclaration_12x.groovy @@ -19,12 +19,14 @@ package core @groovy.transform.CompileStatic [email protected](defaults=false) record Point(int x, int y, String color) { public Point { - x = -x; - if (x < 0) return - Objects.requireNonNull(color); - color = color.toUpperCase(); + x = -x + if (x >= 0) { + Objects.requireNonNull(color) + color = color.toUpperCase() + } } public Point(int x, int y) { diff --git a/src/test/groovy/org/codehaus/groovy/classgen/RecordTest.groovy b/src/test/groovy/org/codehaus/groovy/classgen/RecordTest.groovy index 6a3200d..9036d92 100644 --- a/src/test/groovy/org/codehaus/groovy/classgen/RecordTest.groovy +++ b/src/test/groovy/org/codehaus/groovy/classgen/RecordTest.groovy @@ -109,6 +109,20 @@ class RecordTest { } @Test + void testRecordWithDefaultParams() { + assertScript ''' + record Point(int i = 5, int j = 10) {} + assert new Point().toString() == 'Point[i=5, j=10]' + assert new Point(50).toString() == 'Point[i=50, j=10]' + assert new Point(50, 100).toString() == 'Point[i=50, j=100]' + assert new Point([:]).toString() == 'Point[i=5, j=10]' + assert new Point(i: 50).toString() == 'Point[i=50, j=10]' + assert new Point(j: 100).toString() == 'Point[i=5, j=100]' + assert new Point(i: 50, j: 100).toString() == 'Point[i=50, j=100]' + ''' + } + + @Test void testNativeRecordOnJDK16plus() { assumeTrue(isAtLeastJdk('16.0')) assertScript ''' @@ -304,12 +318,6 @@ class RecordTest { def r = new Record('someRecord', 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20) def expected = 'Record[name=someRecord, x0=0, x1=-1, x2=2, x3=3, x4=4, x5=5, x6=6, x7=7, x8=8, x9=9, x10=10, x11=11, x12=12, x13=13, x14=14, x15=15, x16=16, x17=17, x18=18, x19=19, x20=20]' assert expected == r.toString() - - def ms = r.getClass().getDeclaredMethods().grep(m -> m.name == '$compactInit') - assert 1 == ms.size() - def m = ms[0] - assert m.isSynthetic() - assert 22 == m.getParameterCount() ''' assertScript ''' import groovy.transform.* @@ -326,12 +334,6 @@ class RecordTest { def r = new Record('someRecord', 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20) def expected = 'Record(name:someRecord, x0:0, x1:-1, x2:2, x3:3, x4:4, x5:5, x6:6, x7:7, x8:8, x9:9, x10:10, x11:11, x12:12, x13:13, x14:14, x15:15, x16:16, x17:17, x18:18, x19:19, x20:20)' assert expected == r.toString() - - def ms = r.getClass().getDeclaredMethods().grep(m -> m.name == '$compactInit') - assert 1 == ms.size() - def m = ms[0] - assert m.isSynthetic() - assert 22 == m.getParameterCount() ''' }
