[
https://issues.apache.org/jira/browse/GROOVY-11974?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18077202#comment-18077202
]
ASF GitHub Bot commented on GROOVY-11974:
-----------------------------------------
Copilot commented on code in PR #2502:
URL: https://github.com/apache/groovy/pull/2502#discussion_r3165511139
##########
src/main/java/org/codehaus/groovy/transform/RecordTypeASTTransformation.java:
##########
@@ -183,6 +183,31 @@ public static boolean recordNative(final ClassNode node) {
return node.getUnresolvedSuperClass() != null &&
RECORD_CLASS_NAME.equals(node.getUnresolvedSuperClass().getName());
}
+ /**
+ * Predicts whether {@code cNode} will be compiled as a native JVM record.
+ * Mirrors the decision logic in {@link #doProcessRecordType} (presence of
+ * {@code @RecordBase}, target bytecode {@code >= 16}, and
+ * {@code @RecordOptions(mode != EMULATE)}) so callers running before the
+ * transform itself fires can act on the same outcome.
+ * <p>
+ * The joint-compilation stub generator uses this at {@code
Phases.CONVERSION}
+ * to decide whether to emit {@code record Foo(...)} syntax. Unlike
+ * {@link #recordNative(ClassNode)}, which inspects the post-transform
+ * super class, this predicate inspects only the source-level annotations.
+ *
+ * @param cNode the candidate class node
+ * @param targetBytecode the target bytecode level
+ * (see {@link
CompilerConfiguration#getTargetBytecode()})
+ */
+ @Incubating
+ public static boolean wouldBeNativeRecord(final ClassNode cNode, final
String targetBytecode) {
+ if (cNode == null || cNode.getAnnotations(MY_TYPE).isEmpty()) return
false;
Review Comment:
`wouldBeNativeRecord` calls `StringGroovyMethods.isAtLeast(targetBytecode,
...)`, which throws if `targetBytecode` is null (it constructs `new
BigDecimal(left)`). Since the method is public API and callers may not always
have a target bytecode (and this class already has messaging for the 'unable to
determine target bytecode' case), add a null/blank guard (return false) before
calling `isAtLeast`.
```suggestion
if (cNode == null || cNode.getAnnotations(MY_TYPE).isEmpty()) return
false;
if (targetBytecode == null || targetBytecode.trim().isEmpty())
return false;
```
##########
src/test/groovy/org/codehaus/groovy/tools/stubgenerator/RecordTypeJointCompilationStubTest.groovy:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.codehaus.groovy.tools.stubgenerator
+
+/**
+ * GROOVY-11974: native records in joint compilation must produce a stub
+ * that javac compiles as a record, so that Java callers see the canonical
+ * constructor and the {@code componentName()} accessors.
+ */
+class RecordTypeJointCompilationStubTest extends StringSourcesStubTestCase {
+
+ @Override
+ protected void configure() {
Review Comment:
This stub test uses Java record syntax and `Class::isRecord`, which requires
running on JDK 16+. To keep the test suite green when executed on older JDKs
(where `javac` can't compile records), add a JUnit assumption (e.g.,
`assumeTrue(isAtLeastJdk('16.0'))`) early in the test (such as in `init()` or
`configure()`) so it is skipped when records aren't supported.
```suggestion
private static boolean isAtLeastJdk(final String version) {
List<Integer> current =
System.getProperty('java.specification.version')
.tokenize('.')
.collect { it as Integer }
List<Integer> required = version.tokenize('.')
.collect { it as Integer }
int max = Math.max(current.size(), required.size())
for (int i = 0; i < max; i += 1) {
int currentPart = (i < current.size() ? current[i] : 0)
int requiredPart = (i < required.size() ? required[i] : 0)
if (currentPart != requiredPart) {
return currentPart > requiredPart
}
}
true
}
@Override
protected void configure() {
org.junit.Assume.assumeTrue(isAtLeastJdk('16.0'))
```
##########
src/test/groovy/org/codehaus/groovy/tools/stubgenerator/RecordTypeJointCompilationStubTest.groovy:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.codehaus.groovy.tools.stubgenerator
+
+/**
+ * GROOVY-11974: native records in joint compilation must produce a stub
+ * that javac compiles as a record, so that Java callers see the canonical
+ * constructor and the {@code componentName()} accessors.
+ */
+class RecordTypeJointCompilationStubTest extends StringSourcesStubTestCase {
+
+ @Override
+ protected void configure() {
+ super.configure()
+ config.targetBytecode = '17'
+ }
+
+ Map<String, String> provideSources() {
+ [
+ 'JavaCaller.java': '''
+ public class JavaCaller {
+ public static int sumViaRecordAccessors() {
+ Point p = new Point(3, 4);
+ return p.x() + p.y();
+ }
+ public static String describe() {
+ return new Point(1, 2).getClass().isRecord()
+ ? "record" : "class";
+ }
+ }
+ ''',
+ 'Point.groovy': '''
+ record Point(int x, int y) {}
+ '''
+ ]
+ }
+
+ void verifyStubs() {
+ def stub = stubJavaSourceFor('Point')
+ // header is rendered as record syntax, not class
+ assert stub =~ /\brecord\s+Point\s*\(\s*int\s+x\s*,\s*int\s+y\s*\)/
+ // no `class Point`, no `extends`
+ assert !(stub =~ /\bclass\s+Point\b/)
+ assert !(stub =~ /\bextends\s+java\.lang\.Object\b/)
+ // no instance-field declarations
+ assert !(stub =~ /(?m)^\s*(?:private|protected|public)?\s*int\s+x\s*;/)
Review Comment:
The 'no instance-field declarations' assertion only checks for an `int x;`
field and doesn't cover `y` (or modifiers like `final`). If the stub
accidentally emitted `private final int x;` or `int y;`, this test would still
pass. Consider broadening the regex (e.g., allow optional `final` and check
both component names) to make the test reliably detect any instance field
emission for record components.
```suggestion
// no instance-field declarations for record components
assert !(stub =~
/(?m)^\s*(?:private|protected|public)?\s*(?:final\s+)?int\s+(?:x|y)\s*;/)
```
> stub generator could emit records into stubs when in native records mode
> ------------------------------------------------------------------------
>
> Key: GROOVY-11974
> URL: https://issues.apache.org/jira/browse/GROOVY-11974
> Project: Groovy
> Issue Type: Improvement
> Reporter: Paul King
> Assignee: Paul King
> Priority: Major
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
