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/01/14 23:06:41 UTC
[groovy] branch master updated: GROOVY-10904: run verification on all classes before bytecode generation
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 f8d78e6eb5 GROOVY-10904: run verification on all classes before bytecode generation
f8d78e6eb5 is described below
commit f8d78e6eb56e5d966d157c7d0b89e21eee2130b3
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sat Jan 14 13:23:10 2023 -0600
GROOVY-10904: run verification on all classes before bytecode generation
---
.../java/org/codehaus/groovy/ast/ModuleNode.java | 16 ++---
.../codehaus/groovy/control/CompilationUnit.java | 33 ++++++----
.../transform/stc/StaticTypeCheckingVisitor.java | 2 +
.../transform/stc/MethodReferenceTest.groovy | 21 +++++++
.../post/OldVariablePostconditionTests.groovy | 73 ++++++++++------------
5 files changed, 86 insertions(+), 59 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/ast/ModuleNode.java b/src/main/java/org/codehaus/groovy/ast/ModuleNode.java
index b56c28fd32..5e22024b79 100644
--- a/src/main/java/org/codehaus/groovy/ast/ModuleNode.java
+++ b/src/main/java/org/codehaus/groovy/ast/ModuleNode.java
@@ -464,20 +464,22 @@ public class ModuleNode extends ASTNode {
public void sortClasses() {
if (isEmpty()) return;
- List<ClassNode> sorted = new LinkedList<>(), todo = getClasses();
+ List<ClassNode> classes = getClasses();
+ if (classes.size() == 1) return;
+ List<ClassNode> ordered = new LinkedList<>();
int level = 1;
- while (!todo.isEmpty()) {
- for (Iterator<ClassNode> it = todo.iterator(); it.hasNext(); ) {
- ClassNode cn = it.next(), sc = cn;
+ while (!classes.isEmpty()) {
+ for (Iterator<ClassNode> it = classes.iterator(); it.hasNext(); ) {
+ ClassNode cn = it.next(), sc = cn.getSuperClass();
- for (int i = 0; sc != null && i < level; i += 1) sc = sc.getSuperClass();
+ for (int i = 1; i < level && sc != null; i += 1) sc = sc.getSuperClass();
if (sc != null && sc.isPrimaryClassNode()) continue;
- sorted.add(cn);
+ ordered.add(cn);
it.remove();
}
level += 1;
}
- this.classes = sorted;
+ this.classes = ordered;
}
public boolean hasImportsResolved() {
diff --git a/src/main/java/org/codehaus/groovy/control/CompilationUnit.java b/src/main/java/org/codehaus/groovy/control/CompilationUnit.java
index 974a4f8db8..e695367126 100644
--- a/src/main/java/org/codehaus/groovy/control/CompilationUnit.java
+++ b/src/main/java/org/codehaus/groovy/control/CompilationUnit.java
@@ -302,6 +302,8 @@ public class CompilationUnit extends ProcessingUnit {
}
}, Phases.INSTRUCTION_SELECTION);
+ addPhaseOperation(verification, Phases.CLASS_GENERATION);
+
addPhaseOperation(classgen, Phases.CLASS_GENERATION);
addPhaseOperation(groovyClass -> {
@@ -739,17 +741,11 @@ public class CompilationUnit extends ProcessingUnit {
return false;
}
- /**
- * Runs the class generation phase on a single {@code ClassNode}.
- */
- private final IPrimaryClassNodeOperation classgen = new IPrimaryClassNodeOperation() {
+ private final IPrimaryClassNodeOperation verification = new IPrimaryClassNodeOperation() {
@Override
public void call(final SourceUnit source, final GeneratorContext context, final ClassNode classNode) throws CompilationFailedException {
new OptimizerVisitor(CompilationUnit.this).visitClass(classNode, source); // GROOVY-4272: repositioned from static import visitor
- //
- // Run the Verifier on the outer class
- //
GroovyClassVisitor visitor = new Verifier();
try {
visitor.visitClass(classNode);
@@ -777,7 +773,20 @@ public class CompilationUnit extends ProcessingUnit {
// because the class may be generated even if an error was found
// and that class may have an invalid format we fail here if needed
getErrorCollector().failIfErrors();
+ }
+
+ @Override
+ public boolean needSortedInput() {
+ return true;
+ }
+ };
+ /**
+ * Generates bytecode for a single {@code ClassNode}.
+ */
+ private final IPrimaryClassNodeOperation classgen = new IPrimaryClassNodeOperation() {
+ @Override
+ public void call(final SourceUnit source, final GeneratorContext context, final ClassNode classNode) throws CompilationFailedException {
//
// Prep the generator machinery
//
@@ -793,7 +802,7 @@ public class CompilationUnit extends ProcessingUnit {
//
// Run the generation and create the class (if required)
//
- visitor = new AsmClassGenerator(source, context, classVisitor, sourceName);
+ AsmClassGenerator visitor = new AsmClassGenerator(source, context, classVisitor, sourceName);
visitor.visitClass(classNode);
byte[] bytes = ((ClassWriter) classVisitor).toByteArray();
@@ -807,11 +816,13 @@ public class CompilationUnit extends ProcessingUnit {
}
//
- // Recurse for inner classes
+ // Recurse for generated classes
//
- LinkedList<ClassNode> innerClasses = ((AsmClassGenerator) visitor).getInnerClasses();
+ Deque<ClassNode> innerClasses = visitor.getInnerClasses();
while (!innerClasses.isEmpty()) {
- classgen.call(source, context, innerClasses.removeFirst());
+ ClassNode innerClass = innerClasses.removeFirst();
+ verification.call(source, context, innerClass);
+ classgen.call(source, context, innerClass);
}
}
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 0fdfb2db34..9e0d5b9351 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -2506,10 +2506,12 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
if (name.equals(pn.getGetterNameOrDefault())) {
MethodNode node = new MethodNode(name, Opcodes.ACC_PUBLIC | (pn.isStatic() ? Opcodes.ACC_STATIC : 0), pn.getType(), Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null);
node.setDeclaringClass(pn.getDeclaringClass());
+ node.setSynthetic(true);
return node;
} else if (name.equals(pn.getSetterNameOrDefault()) && !Modifier.isFinal(pn.getModifiers())) {
MethodNode node = new MethodNode(name, Opcodes.ACC_PUBLIC | (pn.isStatic() ? Opcodes.ACC_STATIC : 0), VOID_TYPE, new Parameter[]{new Parameter(pn.getType(), pn.getName())}, ClassNode.EMPTY_ARRAY, null);
node.setDeclaringClass(pn.getDeclaringClass());
+ node.setSynthetic(true);
return node;
}
}
diff --git a/src/test/groovy/transform/stc/MethodReferenceTest.groovy b/src/test/groovy/transform/stc/MethodReferenceTest.groovy
index 1cd2b11912..a95da8d5b8 100644
--- a/src/test/groovy/transform/stc/MethodReferenceTest.groovy
+++ b/src/test/groovy/transform/stc/MethodReferenceTest.groovy
@@ -966,6 +966,27 @@ final class MethodReferenceTest {
}
}
+ @Test // GROOVY-10904
+ void testPropertyMethodLocation() {
+ for (tag in ['@TypeChecked', '@CompileStatic', '@CompileDynamic']) {
+ assertScript shell, """
+ $tag
+ class Test {
+ static class Profile {
+ String foo, bar
+ }
+
+ Map<String, Profile> profiles = [new Profile()].stream()
+ .collect(Collectors.toMap(Profile::getFoo, Function.identity()))
+
+ static main(args) {
+ assert this.newInstance().getProfiles().size() == 1
+ }
+ }
+ """
+ }
+ }
+
@Test // GROOVY-10742, GROOVY-10858
void testIncompatibleReturnType() {
def err = shouldFail shell, '''
diff --git a/subprojects/groovy-contracts/src/test/groovy/org/apache/groovy/contracts/tests/post/OldVariablePostconditionTests.groovy b/subprojects/groovy-contracts/src/test/groovy/org/apache/groovy/contracts/tests/post/OldVariablePostconditionTests.groovy
index b40cbe119d..c81d9ad6dc 100644
--- a/subprojects/groovy-contracts/src/test/groovy/org/apache/groovy/contracts/tests/post/OldVariablePostconditionTests.groovy
+++ b/subprojects/groovy-contracts/src/test/groovy/org/apache/groovy/contracts/tests/post/OldVariablePostconditionTests.groovy
@@ -26,8 +26,7 @@ import org.junit.Test
*
* @see groovy.contracts.Ensures
*/
-
-class OldVariablePostconditionTests extends BaseTestClass {
+final class OldVariablePostconditionTests extends BaseTestClass {
def templateSourceCode = '''
package tests
@@ -112,58 +111,50 @@ class OldVariablePostconditionTests extends BaseTestClass {
@Test
void generate_old_variables_for_super_class() {
+ add_class_to_classpath '''
+ package tests
- def baseClassSource = '''
- package tests
-
- import groovy.contracts.*
-
- class Account {
- protected BigDecimal balance
+ import groovy.contracts.*
- def Account( BigDecimal amount = 0.0 ) {
- balance = amount
- }
+ class Account {
+ private BigDecimal balance
- void deposit( BigDecimal amount ) {
- balance += amount
- }
+ Account(BigDecimal balance = 0.0) {
+ this.balance = balance
+ }
- @Requires({ amount >= 0.0 })
- BigDecimal withdraw( BigDecimal amount ) {
- if (balance < amount) return 0.0
+ BigDecimal getBalance() {
+ return balance
+ }
- balance -= amount
- return amount
- }
+ @Requires({ amount > 0.0 })
+ void deposit(BigDecimal amount) {
+ balance += amount
+ }
- BigDecimal getBalance() {
- return balance
+ @Requires({ amount > 0.0 })
+ void withdraw(BigDecimal amount) {
+ if (balance >= amount) {
+ balance -= amount
+ }
+ }
}
- }
'''
- def descendantClassSource = '''
- package tests
-
- import groovy.contracts.*
+ def betterAccount = create_instance_of '''
+ package tests
- class BetterAccount extends Account {
- @Ensures({ balance == old.balance - (amount * 0.5) })
- BigDecimal withdraw( BigDecimal amount ) {
- if (balance < amount) return 0.0
+ import groovy.contracts.*
- balance -= amount * 0.5
- return amount
+ class BetterAccount extends Account {
+ @Ensures({ balance == old.balance - (amount * 0.5) })
+ void withdraw(BigDecimal amount) {
+ super.withdraw(amount * 0.5)
+ }
}
- }
'''
-
- add_class_to_classpath baseClassSource
-
- def betterAccount = create_instance_of(descendantClassSource)
betterAccount.deposit(30.0)
betterAccount.withdraw(10.0)
-
+ assert betterAccount.balance == 25.0
}
-}
\ No newline at end of file
+}