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 7240a0402cf9c143cf84845321989df0e2fb0fbf Author: Paul King <[email protected]> AuthorDate: Thu Oct 21 21:15:34 2021 +1000 GROOVY-10298: Refine records to not use system properties --- src/main/groovy/groovy/transform/RecordType.groovy | 5 +- src/main/java/groovy/transform/RecordBase.java | 4 + .../java/groovy/transform/RecordTypeMode.java} | 31 ++-- .../apache/groovy/parser/antlr4/AstBuilder.java | 25 ++-- .../java/org/codehaus/groovy/ast/ClassNode.java | 9 +- .../groovy/classgen/AsmClassGenerator.java | 3 +- .../groovy/classgen/asm/WriterController.java | 6 - .../groovy/control/CompilerConfiguration.java | 26 ---- .../transform/RecordTypeASTTransformation.java | 166 ++++++++++++++++++++- .../transform/stc/StaticTypeCheckingVisitor.java | 35 +++-- .../core/RecordDeclaration_03x.groovy | 1 - .../core/RecordDeclaration_08x.groovy | 2 +- .../core/RecordDeclaration_09x.groovy | 2 +- .../org/codehaus/groovy/classgen/RecordTest.groovy | 69 ++++++++- 14 files changed, 290 insertions(+), 94 deletions(-) diff --git a/src/main/groovy/groovy/transform/RecordType.groovy b/src/main/groovy/groovy/transform/RecordType.groovy index 3cc1fc7..e5d0528 100644 --- a/src/main/groovy/groovy/transform/RecordType.groovy +++ b/src/main/groovy/groovy/transform/RecordType.groovy @@ -68,8 +68,6 @@ import java.lang.annotation.Target * </ul> * Record-like classes are particularly useful for data structures. * - * @see ToString - * @see EqualsAndHashCode * @see RecordBase * @see ImmutableOptions * @see PropertyOptions @@ -80,14 +78,13 @@ import java.lang.annotation.Target * @since 4.0.0 */ @RecordBase -@ToString(cache = true, includeNames = true) -@EqualsAndHashCode(cache = true, useCanEqual = false) @ImmutableOptions @PropertyOptions(propertyHandler = ImmutablePropertyHandler) @TupleConstructor(defaults = false) @MapConstructor @KnownImmutable @POJO +@CompileStatic @AnnotationCollector(mode = AnnotationCollectorMode.PREFER_EXPLICIT_MERGED) @Retention(RetentionPolicy.RUNTIME) @Target([ElementType.TYPE]) diff --git a/src/main/java/groovy/transform/RecordBase.java b/src/main/java/groovy/transform/RecordBase.java index 1fdfb0a..1265b99 100644 --- a/src/main/java/groovy/transform/RecordBase.java +++ b/src/main/java/groovy/transform/RecordBase.java @@ -39,4 +39,8 @@ import java.lang.annotation.Target; @Target({ElementType.TYPE}) @GroovyASTTransformationClass("org.codehaus.groovy.transform.RecordTypeASTTransformation") public @interface RecordBase { + /** + * Mode to use when creating record type classes. + */ + RecordTypeMode mode() default RecordTypeMode.AUTO; } diff --git a/src/test-resources/core/RecordDeclaration_03x.groovy b/src/main/java/groovy/transform/RecordTypeMode.java similarity index 63% copy from src/test-resources/core/RecordDeclaration_03x.groovy copy to src/main/java/groovy/transform/RecordTypeMode.java index 99d036d..da2c967 100644 --- a/src/test-resources/core/RecordDeclaration_03x.groovy +++ b/src/main/java/groovy/transform/RecordTypeMode.java @@ -16,16 +16,27 @@ * specific language governing permissions and limitations * under the License. */ -package core +package groovy.transform; [email protected] -record Fruit(String name, double price) {} +/** + * Intended mode to use for records when using the {@code @RecordType} annotation (or {@code record} keyword). + * + * @since 4.0.0 + * @see RecordType + */ +public enum RecordTypeMode { + /** + * Produce a record-like class. + */ + EMULATE, [email protected] -void test() { - def apple = new Fruit('Apple', 11.6D) - assert 'Apple' == apple.name() - assert 11.6 == apple.price() -} + /** + * Produce a Java-like "native" record (JEP 359/384/395). + */ + NATIVE, -test() + /** + * Produce native records when compiling for a suitable target bytecode (JDK16+). + */ + AUTO +} 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 00eb43a..9ed4453 100644 --- a/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java +++ b/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java @@ -40,6 +40,7 @@ import org.antlr.v4.runtime.misc.Interval; 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; @@ -1572,7 +1573,7 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> { classNode.addAnnotation(new AnnotationNode(ClassHelper.makeCached(Trait.class))); } if (isRecord) { - classNode.addAnnotation(new AnnotationNode(ClassHelper.makeWithoutCaching(RECORD_TYPE_NAME))); + classNode.addAnnotation(new AnnotationNode(RECORD_TYPE_CLASS)); } classNode.addAnnotations(modifierManager.getAnnotations()); @@ -1603,21 +1604,18 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> { this.initUsingGenerics(classNode); this.hackMixins(classNode); - } else if (isEnum) { + } else if (isEnum || isRecord) { classNode.setInterfaces(this.visitTypeList(ctx.is)); this.initUsingGenerics(classNode); + if (isRecord) { + transformRecordHeaderToProperties(ctx, classNode); + } } else if (isAnnotation) { classNode.setModifiers(classNode.getModifiers() | Opcodes.ACC_INTERFACE | Opcodes.ACC_ABSTRACT | Opcodes.ACC_ANNOTATION); classNode.addInterface(ClassHelper.Annotation_TYPE); this.hackMixins(classNode); - } else if (isRecord) { - classNode.setModifiers(classNode.getModifiers() | Opcodes.ACC_RECORD | Opcodes.ACC_FINAL); - classNode.setRecordComponentNodes(this.transformRecordHeaderToProperties(ctx, classNode)); - classNode.setInterfaces(this.visitTypeList(ctx.is)); - this.initUsingGenerics(classNode); - } else { throw createParsingFailedException("Unsupported class declaration: " + ctx.getText(), ctx); } @@ -1652,12 +1650,11 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> { return classNode; } - private List<RecordComponentNode> transformRecordHeaderToProperties(ClassDeclarationContext ctx, ClassNode classNode) { + private void transformRecordHeaderToProperties(ClassDeclarationContext ctx, ClassNode classNode) { Parameter[] parameters = this.visitFormalParameters(ctx.formalParameters()); classNode.putNodeMetaData(RECORD_HEADER, parameters); final int n = parameters.length; - List<RecordComponentNode> components = new ArrayList<>(n); for (int i = 0; i < n; i += 1) { Parameter parameter = parameters[i]; FormalParameterContext parameterCtx = parameter.getNodeMetaData(PARAMETER_CONTEXT); @@ -1666,10 +1663,7 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> { PropertyNode propertyNode = declareProperty(parameterCtx, parameterModifierManager, originType, classNode, i, parameter, parameter.getName(), parameter.getModifiers(), parameter.getInitialExpression()); propertyNode.getField().putNodeMetaData(IS_RECORD_GENERATED, Boolean.TRUE); - - components.add(new RecordComponentNode(classNode, parameter.getName(), originType, parameter.getAnnotations())); } - return components; } @SuppressWarnings("unchecked") @@ -1958,7 +1952,7 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> { ClassNode classNode = ctx.getNodeMetaData(CLASS_DECLARATION_CLASS_NODE); Objects.requireNonNull(classNode, "classNode should not be null"); - if (!classNode.isRecord()) { + if (!AnnotatedNodeUtils.hasAnnotation(classNode, RECORD_TYPE_CLASS)) { createParsingFailedException("Only `record` can have compact constructor", ctx); } @@ -5266,6 +5260,7 @@ 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_TYPE_NAME = "groovy.transform.RecordType"; 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/ClassNode.java b/src/main/java/org/codehaus/groovy/ast/ClassNode.java index e0c04ea..1774289 100644 --- a/src/main/java/org/codehaus/groovy/ast/ClassNode.java +++ b/src/main/java/org/codehaus/groovy/ast/ClassNode.java @@ -165,6 +165,7 @@ public class ClassNode extends AnnotatedNode { private List<ClassNode> permittedSubclasses = new ArrayList<>(4); private List<AnnotationNode> typeAnnotations = Collections.emptyList(); private List<RecordComponentNode> recordComponentNodes = Collections.emptyList(); + private boolean isRecord = false; /** * The AST Transformations to be applied during compilation. @@ -1360,14 +1361,14 @@ public class ClassNode extends AnnotatedNode { } /** - * Checks if the {@link ClassNode} instance represents {@code record} + * Checks if the {@link ClassNode} instance represents a native {@code record}. + * Check instead for the {@code RecordType} annotation if looking for records and record-like classes. * - * @return {@code true} if the instance represents {@code record} + * @return {@code true} if the instance represents a native {@code record} * @since 4.0.0 */ public boolean isRecord() { - List<RecordComponentNode> recordComponentNodes = getRecordComponentNodes(); - return null != recordComponentNodes && !recordComponentNodes.isEmpty(); + return getUnresolvedSuperClass() != null && "java.lang.Record".equals(getUnresolvedSuperClass().getName()); } /** diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java index 616fcb0..52796e7 100644 --- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java +++ b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java @@ -384,8 +384,7 @@ public class AsmClassGenerator extends ClassGenerator { } } - if (bytecodeVersion >= Opcodes.V16 && classNode.isRecord() && - context.getCompileUnit().getConfig().isRecordsNative()) { + if (classNode.isRecord()) { visitRecordComponents(classNode); } classVisitor.visitEnd(); diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/WriterController.java b/src/main/java/org/codehaus/groovy/classgen/asm/WriterController.java index d34cc3b..a71bf44 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/WriterController.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/WriterController.java @@ -19,7 +19,6 @@ package org.codehaus.groovy.classgen.asm; import org.codehaus.groovy.GroovyBugError; -import org.codehaus.groovy.ast.ClassHelper; import org.codehaus.groovy.ast.ClassNode; import org.codehaus.groovy.ast.ConstructorNode; import org.codehaus.groovy.ast.InterfaceHelperClassNode; @@ -98,11 +97,6 @@ public class WriterController { this.bytecodeVersion = chooseBytecodeVersion(invokedynamic, config.isPreviewFeatures(), config.getTargetBytecode()); - if (bytecodeVersion >= Opcodes.V16 && cn.isRecord() && config.isRecordsNative()) { - // `java.lang.Record` has been added since JDK16 - cn.setSuperClass(ClassHelper.makeWithoutCaching(RECORD_CLASS_NAME)); - } - if (invokedynamic) { this.invocationWriter = new InvokeDynamicWriter(this); this.callSiteWriter = new IndyCallSiteWriter(this); diff --git a/src/main/java/org/codehaus/groovy/control/CompilerConfiguration.java b/src/main/java/org/codehaus/groovy/control/CompilerConfiguration.java index af83287..3d04bd3 100644 --- a/src/main/java/org/codehaus/groovy/control/CompilerConfiguration.java +++ b/src/main/java/org/codehaus/groovy/control/CompilerConfiguration.java @@ -389,12 +389,6 @@ public class CompilerConfiguration { private boolean previewFeatures; /** - * Whether Record class information is natively generated for target bytecode >= 16 (JEP 359/384/395). - * Otherwise record-like classes are produced. - */ - private boolean recordsNative; - - /** * Whether Sealed class information is natively generated for target bytecode >= 17 (JEP 360/397/409) */ private boolean sealedNative; @@ -474,7 +468,6 @@ public class CompilerConfiguration { warningLevel = WarningMessage.LIKELY_ERRORS; parameters = getBooleanSafe("groovy.parameters"); previewFeatures = getBooleanSafe("groovy.preview.features"); - recordsNative = !getBooleanSafe("groovy.records.native.disable"); sealedNative = !getBooleanSafe("groovy.sealed.native.disable"); sealedAnnotations = !getBooleanSafe("groovy.sealed.annotations.disable"); logClassgen = getBooleanSafe("groovy.log.classgen"); @@ -525,7 +518,6 @@ public class CompilerConfiguration { setMinimumRecompilationInterval(configuration.getMinimumRecompilationInterval()); setTargetBytecode(configuration.getTargetBytecode()); setPreviewFeatures(configuration.isPreviewFeatures()); - setRecordsNative(configuration.isRecordsNative()); setSealedNative(configuration.isSealedNative()); setSealedAnnotations(configuration.isSealedAnnotations()); setLogClassgen(configuration.isLogClassgen()); @@ -1143,24 +1135,6 @@ public class CompilerConfiguration { } /** - * Whether Record class information is natively generated (JEP 359/384/395). - * - * @return recordsNative - */ - public boolean isRecordsNative() { - return recordsNative; - } - - /** - * Sets whether Record class information is natively generated (JEP 359/384/395). - * - * @param recordsNative whether record classes extend java.lang.Record for target bytecode >= 16 - */ - public void setRecordsNative(final boolean recordsNative) { - this.recordsNative = recordsNative; - } - - /** * Whether Sealed class information is natively generated (JEP 360/397/409). * * @return sealedNative diff --git a/src/main/java/org/codehaus/groovy/transform/RecordTypeASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/RecordTypeASTTransformation.java index ecdde13..aa900ab 100644 --- a/src/main/java/org/codehaus/groovy/transform/RecordTypeASTTransformation.java +++ b/src/main/java/org/codehaus/groovy/transform/RecordTypeASTTransformation.java @@ -21,6 +21,7 @@ package org.codehaus.groovy.transform; import groovy.lang.GroovyClassLoader; import groovy.transform.CompilationUnitAware; import groovy.transform.RecordBase; +import groovy.transform.RecordTypeMode; import groovy.transform.options.PropertyHandler; import org.codehaus.groovy.ast.ASTNode; import org.codehaus.groovy.ast.AnnotatedNode; @@ -29,32 +30,53 @@ import org.codehaus.groovy.ast.ClassHelper; import org.codehaus.groovy.ast.ClassNode; import org.codehaus.groovy.ast.FieldNode; import org.codehaus.groovy.ast.InnerClassNode; +import org.codehaus.groovy.ast.Parameter; import org.codehaus.groovy.ast.PropertyNode; +import org.codehaus.groovy.ast.RecordComponentNode; +import org.codehaus.groovy.ast.expr.ClassExpression; +import org.codehaus.groovy.ast.expr.Expression; +import org.codehaus.groovy.ast.expr.PropertyExpression; import org.codehaus.groovy.ast.stmt.Statement; +import org.codehaus.groovy.classgen.asm.BytecodeHelper; import org.codehaus.groovy.control.CompilationUnit; import org.codehaus.groovy.control.CompilePhase; +import org.codehaus.groovy.control.CompilerConfiguration; import org.codehaus.groovy.control.SourceUnit; +import org.objectweb.asm.Handle; +import org.objectweb.asm.Opcodes; +import org.objectweb.asm.Type; import java.lang.annotation.Annotation; import java.util.ArrayList; +import java.util.LinkedList; import java.util.List; +import java.util.stream.Collectors; +import static org.apache.groovy.ast.tools.ClassNodeUtils.addGeneratedMethod; import static org.codehaus.groovy.ast.ClassHelper.makeWithoutCaching; +import static org.codehaus.groovy.ast.tools.GeneralUtils.bytecodeX; import static org.codehaus.groovy.ast.tools.GeneralUtils.constX; import static org.codehaus.groovy.ast.tools.GeneralUtils.getInstanceProperties; +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.stmt; import static org.objectweb.asm.Opcodes.ACC_ABSTRACT; import static org.objectweb.asm.Opcodes.ACC_FINAL; import static org.objectweb.asm.Opcodes.ACC_PRIVATE; import static org.objectweb.asm.Opcodes.ACC_PUBLIC; import static org.objectweb.asm.Opcodes.ACC_STATIC; import static org.objectweb.asm.Opcodes.ACC_SYNTHETIC; +import static org.objectweb.asm.Opcodes.ALOAD; +import static org.objectweb.asm.Opcodes.ARETURN; +import static org.objectweb.asm.Opcodes.IRETURN; /** * Handles generation of code for the @RecordType annotation. */ -@GroovyASTTransformation(phase = CompilePhase.CANONICALIZATION) +@GroovyASTTransformation(phase = CompilePhase.SEMANTIC_ANALYSIS) public class RecordTypeASTTransformation extends AbstractASTTransformation implements CompilationUnitAware { private CompilationUnit compilationUnit; + private static final String RECORD_CLASS_NAME = "java.lang.Record"; private static final Class<? extends Annotation> MY_CLASS = RecordBase.class; public static final ClassNode MY_TYPE = makeWithoutCaching(MY_CLASS, false); @@ -77,12 +99,42 @@ public class RecordTypeASTTransformation extends AbstractASTTransformation imple final PropertyHandler handler = PropertyHandler.createPropertyHandler(this, classLoader, (ClassNode) parent); if (handler == null) return; if (!handler.validateAttributes(this, anno)) return; - doMakeImmutable((ClassNode) parent, anno, handler); + doProcessRecordType((ClassNode) parent, anno, handler); } } - private void doMakeImmutable(ClassNode cNode, AnnotationNode node, PropertyHandler handler) { - List<PropertyNode> newProperties = new ArrayList<PropertyNode>(); + private void doProcessRecordType(ClassNode cNode, AnnotationNode node, PropertyHandler handler) { + RecordTypeMode mode = getMode(node, "mode"); + boolean isPostJDK16 = false; + String message = "Expecting JDK16+ but unable to determine target bytecode"; + if (sourceUnit != null) { + CompilerConfiguration config = sourceUnit.getConfiguration(); + String targetBytecode = config.getTargetBytecode(); + isPostJDK16 = CompilerConfiguration.isPostJDK16(targetBytecode); + message = "Expecting JDK16+ but found " + targetBytecode; + } + boolean isNative = isPostJDK16 && mode != RecordTypeMode.EMULATE; + if (isNative) { + String sName = cNode.getUnresolvedSuperClass().getName(); + // don't expect any parent to be set at this point but we only check at grammar + // level when using the record keyword so do a few more sanity checks here + if (!sName.equals("java.lang.Object") && !sName.equals(RECORD_CLASS_NAME)) { + addError("Invalid superclass for native record found: " + sName, cNode); + } + cNode.setSuperClass(ClassHelper.makeWithoutCaching(RECORD_CLASS_NAME)); + cNode.setModifiers(cNode.getModifiers() | Opcodes.ACC_RECORD); + final List<PropertyNode> pList = getInstanceProperties(cNode); + if (!pList.isEmpty()) { + cNode.setRecordComponentNodes(new ArrayList<>()); + } + for (PropertyNode pNode : pList) { + cNode.getRecordComponentNodes().add(new RecordComponentNode(cNode, pNode.getName(), pNode.getOriginType(), pNode.getAnnotations())); + } + } else if (mode == RecordTypeMode.NATIVE) { + 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; @@ -109,6 +161,25 @@ public class RecordTypeASTTransformation extends AbstractASTTransformation imple if (cNode.getDeclaredField("serialVersionUID") == null) { cNode.addField("serialVersionUID", ACC_PRIVATE | ACC_STATIC | ACC_FINAL, ClassHelper.long_TYPE, constX(0L)); } + + if (!hasAnnotation(cNode, ToStringASTTransformation.MY_TYPE)) { + if (isNative) { + createRecordToString(cNode); + } else { + ToStringASTTransformation.createToString(cNode, false, false, null, null, true, false, true, true); + } + } + + if (!hasAnnotation(cNode, EqualsAndHashCodeASTTransformation.MY_TYPE)) { + if (isNative) { + createRecordEquals(cNode); + createRecordHashCode(cNode); + } else { + EqualsAndHashCodeASTTransformation.createEquals(cNode, false, false, false, null, null); + EqualsAndHashCodeASTTransformation.createHashCode(cNode, false, false, false, null, null); + } + } + if (hasAnnotation(cNode, TupleConstructorASTTransformation.MY_TYPE)) { AnnotationNode tupleCons = cNode.getAnnotations(TupleConstructorASTTransformation.MY_TYPE).get(0); if (unsupportedTupleAttribute(tupleCons, "excludes")) return; @@ -116,8 +187,93 @@ public class RecordTypeASTTransformation extends AbstractASTTransformation imple if (unsupportedTupleAttribute(tupleCons, "includeProperties")) return; if (unsupportedTupleAttribute(tupleCons, "includeSuperFields")) return; if (unsupportedTupleAttribute(tupleCons, "callSuper")) return; - if (unsupportedTupleAttribute(tupleCons, "force")) return; + unsupportedTupleAttribute(tupleCons, "force"); + } + } + + private void createRecordToString(ClassNode cNode) { + String desc = BytecodeHelper.getMethodDescriptor(ClassHelper.STRING_TYPE, new ClassNode[]{cNode}); + Statement body = stmt(bytecodeX(ClassHelper.STRING_TYPE, mv -> { + mv.visitVarInsn(ALOAD, 0); + mv.visitInvokeDynamicInsn("toString", desc, createBootstrapMethod(), createBootstrapMethodArguments(cNode)); + mv.visitInsn(ARETURN); + mv.visitMaxs(0, 0); + mv.visitEnd(); + }) + ); + addGeneratedMethod(cNode, "toString", ACC_PUBLIC | ACC_FINAL, ClassHelper.STRING_TYPE, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, body); + } + + private void createRecordEquals(ClassNode cNode) { + String desc = BytecodeHelper.getMethodDescriptor(ClassHelper.boolean_TYPE, new ClassNode[]{cNode, ClassHelper.OBJECT_TYPE}); + Statement body = stmt(bytecodeX(ClassHelper.boolean_TYPE, mv -> { + mv.visitVarInsn(ALOAD, 0); + mv.visitVarInsn(ALOAD, 1); + mv.visitInvokeDynamicInsn("equals", desc, createBootstrapMethod(), createBootstrapMethodArguments(cNode)); + mv.visitInsn(IRETURN); + mv.visitMaxs(0, 0); + mv.visitEnd(); + }) + ); + addGeneratedMethod(cNode, "equals", ACC_PUBLIC | ACC_FINAL, ClassHelper.boolean_TYPE, params(param(ClassHelper.OBJECT_TYPE, "other")), ClassNode.EMPTY_ARRAY, body); + } + + private void createRecordHashCode(ClassNode cNode) { + String desc = BytecodeHelper.getMethodDescriptor(ClassHelper.int_TYPE, new ClassNode[]{cNode}); + Statement body = stmt(bytecodeX(ClassHelper.int_TYPE, mv -> { + mv.visitVarInsn(ALOAD, 0); + mv.visitInvokeDynamicInsn("hashCode", desc, createBootstrapMethod(), createBootstrapMethodArguments(cNode)); + mv.visitInsn(IRETURN); + mv.visitMaxs(0, 0); + mv.visitEnd(); + }) + ); + addGeneratedMethod(cNode, "hashCode", ACC_PUBLIC | ACC_FINAL, ClassHelper.int_TYPE, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, body); + } + + private Object[] createBootstrapMethodArguments(ClassNode cNode) { + String internalName = cNode.getName().replace('.', '/'); + String names = cNode.getRecordComponentNodes().stream().map(RecordComponentNode::getName).collect(Collectors.joining(";")); + List<Object> args = new LinkedList<>(); + args.add(Type.getType(BytecodeHelper.getTypeDescription(cNode))); + args.add(names); + cNode.getRecordComponentNodes().stream().forEach(rcn -> args.add(createFieldHandle(rcn, internalName))); + return args.toArray(); + } + + private Object createFieldHandle(RecordComponentNode rcn, String cName) { + return new Handle( + Opcodes.H_GETFIELD, + cName, + rcn.getName(), + BytecodeHelper.getTypeDescription(rcn.getType()), + false + ); + } + + private Handle createBootstrapMethod() { + return new Handle( + Opcodes.H_INVOKESTATIC, + "java/lang/runtime/ObjectMethods", + "bootstrap", + "(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/TypeDescriptor;Ljava/lang/Class;Ljava/lang/String;[Ljava/lang/invoke/MethodHandle;)Ljava/lang/Object;", + false + ); + } + + private static RecordTypeMode getMode(AnnotationNode node, String name) { + final Expression member = node.getMember(name); + if (member instanceof PropertyExpression) { + PropertyExpression prop = (PropertyExpression) member; + Expression oe = prop.getObjectExpression(); + if (oe instanceof ClassExpression) { + ClassExpression ce = (ClassExpression) oe; + if (ce.getType().getName().equals("groovy.transform.RecordTypeMode")) { + return RecordTypeMode.valueOf(prop.getPropertyAsString()); + } + } } + return null; } private boolean unsupportedTupleAttribute(AnnotationNode anno, String memberName) { diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java index b0e15ba..1cef134 100644 --- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java +++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java @@ -4716,16 +4716,6 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport { if (receiver.isInterface()) { methods.addAll(OBJECT_TYPE.getMethods(name)); - } else if (receiver.isRecord()) { - if (methods.stream().noneMatch(MethodNodeUtils::isGetterCandidate)) { - PropertyNode p = receiver.getProperty(name); - if (null != p) { - MethodNode getter = new MethodNode(p.getGetterName(), Modifier.PUBLIC, p.getType(), Parameter.EMPTY_ARRAY, - ClassNode.EMPTY_ARRAY, block(returnS(varX(p.getName())))); - getter.setDeclaringClass(receiver); - methods.add(getter); - } - } } if (methods.isEmpty() || receiver.isResolved()) { @@ -4837,9 +4827,9 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport { if (pname == null) { pname = extractPropertyNameFromMethodName("is", name); } + PropertyNode property = null; if (pname != null) { // we don't use property exists there because findMethod is called on super clases recursively - PropertyNode property = null; ClassNode curNode = receiver; while (property == null && curNode != null) { property = curNode.getProperty(pname); @@ -4854,13 +4844,26 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport { } curNode = curNode.getSuperClass(); } - if (property != null) { - int mods = Opcodes.ACC_PUBLIC | (property.isStatic() ? Opcodes.ACC_STATIC : 0); - MethodNode node = new MethodNode(name, mods, property.getType(), Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, GENERATED_EMPTY_STATEMENT); - node.setDeclaringClass(property.getDeclaringClass()); - return Collections.singletonList(node); + } else { + // look for a property with the getterName set since it may not match above + ClassNode curNode = receiver; + while (property == null && curNode != null && !curNode.isStaticClass()) { + for (PropertyNode p : curNode.getProperties()) { + if (name.equals(p.getGetterName())) { + property = p; + receiver = curNode; + break; + } + } + curNode = curNode.getSuperClass(); } } + if (property != null) { + int mods = Opcodes.ACC_PUBLIC | (property.isStatic() ? Opcodes.ACC_STATIC : 0); + MethodNode node = new MethodNode(name, mods, property.getType(), Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, GENERATED_EMPTY_STATEMENT); + node.setDeclaringClass(property.getDeclaringClass()); + return Collections.singletonList(node); + } } else if (methods.isEmpty() && args != null && args.length == 1) { // maybe we are looking for a setter ? String pname = extractPropertyNameFromMethodName("set", name); diff --git a/src/test-resources/core/RecordDeclaration_03x.groovy b/src/test-resources/core/RecordDeclaration_03x.groovy index 99d036d..0c29706 100644 --- a/src/test-resources/core/RecordDeclaration_03x.groovy +++ b/src/test-resources/core/RecordDeclaration_03x.groovy @@ -18,7 +18,6 @@ */ package core [email protected] record Fruit(String name, double price) {} @groovy.transform.CompileStatic diff --git a/src/test-resources/core/RecordDeclaration_08x.groovy b/src/test-resources/core/RecordDeclaration_08x.groovy index d1ced5d..d96a8f5 100644 --- a/src/test-resources/core/RecordDeclaration_08x.groovy +++ b/src/test-resources/core/RecordDeclaration_08x.groovy @@ -25,7 +25,7 @@ record Person(String name, int age) { } } -assert 'core.Person(name:Daniel, age:37)' == new Person('Daniel', 37).toString() +assert 'Person[name=Daniel, age=37]' == new Person('Daniel', 37).toString() try { new Person('Peter', 3) assert false, 'should failed because of invalid age' diff --git a/src/test-resources/core/RecordDeclaration_09x.groovy b/src/test-resources/core/RecordDeclaration_09x.groovy index 53027b1..bb54cde 100644 --- a/src/test-resources/core/RecordDeclaration_09x.groovy +++ b/src/test-resources/core/RecordDeclaration_09x.groovy @@ -26,7 +26,7 @@ record Person(String name, int age) { } } -assert 'core.Person(name:Daniel, age:37)' == new Person('Daniel', 37).toString() +assert 'Person[name=Daniel, age=37]' == new Person('Daniel', 37).toString() try { new Person('Peter', 3) assert false, 'should failed because of invalid age' diff --git a/src/test/groovy/org/codehaus/groovy/classgen/RecordTest.groovy b/src/test/groovy/org/codehaus/groovy/classgen/RecordTest.groovy index a3e2160..6a3200d 100644 --- a/src/test/groovy/org/codehaus/groovy/classgen/RecordTest.groovy +++ b/src/test/groovy/org/codehaus/groovy/classgen/RecordTest.groovy @@ -34,12 +34,13 @@ import org.junit.Test import static groovy.test.GroovyAssert.assertScript import static groovy.test.GroovyAssert.isAtLeastJdk +import static groovy.test.GroovyAssert.shouldFail import static org.junit.Assume.assumeTrue @CompileStatic class RecordTest { @Test - void testRecordOnJDK16plus() { + void testNativeRecordOnJDK16ByDefault() { assumeTrue(isAtLeastJdk('16.0')) assertScript ''' record RecordJDK16plus(String name) {} @@ -48,9 +49,9 @@ class RecordTest { } @Test - void testRecordOnJDK16plusWhenDisabled() { + void testRecordLikeOnJDK16withTargetBytecode15() { assumeTrue(isAtLeastJdk('16.0')) - def configuration = new CompilerConfiguration(recordsNative: false) + def configuration = new CompilerConfiguration(targetBytecode: '15') assertScript(new GroovyShell(configuration), ''' record RecordJDK16plus2(String name) {} assert java.lang.Record != RecordJDK16plus2.class.getSuperclass() @@ -58,6 +59,46 @@ class RecordTest { } @Test + void testAttemptedNativeRecordWithTargetBytecode15ShouldFail() { + assumeTrue(isAtLeastJdk('16.0')) + def configuration = new CompilerConfiguration(targetBytecode: '15') + def ex = shouldFail(new GroovyShell(configuration), ''' + import groovy.transform.* + @RecordType(mode=RecordTypeMode.NATIVE) + class RecordJDK16plus2 { + String name + } + assert java.lang.Record != RecordJDK16plus2.class.getSuperclass() + ''') + assert ex.message.contains('Expecting JDK16+ but found 15 when attempting to create a native record') + } + + @Test + void testNativeRecordWithSuperClassShouldFail() { + assumeTrue(isAtLeastJdk('16.0')) + def ex = shouldFail(''' + import groovy.transform.* + @RecordType + class RecordJDK16plus2 extends ArrayList { + String name + } + assert java.lang.Record != RecordJDK16plus2.class.getSuperclass() + ''') + assert ex.message.contains('Invalid superclass for native record found: java.util.ArrayList') + } + + @Test + void testNoNativeRecordOnJDK16WhenEmulating() { + assumeTrue(isAtLeastJdk('16.0')) + assertScript ''' + import groovy.transform.* + @RecordBase(mode=RecordTypeMode.EMULATE) + record RecordJDK16plus2(String name) {} + assert java.lang.Record != RecordJDK16plus2.class.getSuperclass() + ''' + } + + @Test void testInnerRecordIsImplicitlyStatic() { assertScript ''' class Test { @@ -261,6 +302,28 @@ 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.* + @CompileStatic + @ToString(includeNames=true) + record Record(String name, int x0, int x1, int x2, int x3, int x4, + int x5, int x6, int x7, int x8, int x9, int x10, int x11, int x12, int x13, int x14, + int x15, int x16, int x17, int x18, int x19, int x20) { + public Record { + x1 = -x1 + } + } + + 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()
