Copilot commented on code in PR #2605:
URL: https://github.com/apache/groovy/pull/2605#discussion_r3400443795
##########
subprojects/groovy-contracts/src/test/groovy/org/apache/groovy/contracts/compability/SynchronizedTests.groovy:
##########
@@ -41,4 +41,52 @@ class SynchronizedTests extends GroovyShellTestCase {
evaluate source
}
+
+ // GROOVY-12084: @Synchronized wraps the method body in a
SynchronizedStatement
+ // (at CANONICALIZATION) before contracts run; an @Ensures with no
@Requires used
+ // to throw ClassCastException because the postcondition generator assumed
the body
+ // was still a BlockStatement
+ void test_Synchronized_with_Ensures_but_no_Requires() {
+
+ def source = """
+ import groovy.contracts.*
+
+ class A {
+
+ @groovy.transform.Synchronized
+ @Ensures({ result >= 0 })
+ def m(int a) { return a}
+
+ }
+
+ def a = new A()
+ assert a.m(12) == 12
+ """
+
+ evaluate source
+ }
+
+ // GROOVY-12084: same wrapping scenario combined with a class invariant
+ void test_Synchronized_with_Ensures_and_Invariant_but_no_Requires() {
+
+ def source = """
+ import groovy.contracts.*
+
+ @Invariant({ count >= 0 })
+ class A {
+
+ int count = 0
+
+ @groovy.transform.Synchronized
+ @Ensures({ result >= 0 })
+ int m(int a) { count = a; return a }
+
+ }
+
+ def a = new A()
+ assert a.m(12) == 12
+ """
+
+ evaluate source
Review Comment:
This test currently passes as long as compilation/evaluation succeeds; it
doesn't verify that the class invariant is actually evaluated for a
synchronized method. Consider making the method violate the invariant while
still satisfying the postcondition, and assert that `ClassInvariantViolation`
is thrown.
##########
subprojects/groovy-contracts/src/test/groovy/org/apache/groovy/contracts/compability/SynchronizedTests.groovy:
##########
@@ -41,4 +41,52 @@ class SynchronizedTests extends GroovyShellTestCase {
evaluate source
}
+
+ // GROOVY-12084: @Synchronized wraps the method body in a
SynchronizedStatement
+ // (at CANONICALIZATION) before contracts run; an @Ensures with no
@Requires used
+ // to throw ClassCastException because the postcondition generator assumed
the body
+ // was still a BlockStatement
+ void test_Synchronized_with_Ensures_but_no_Requires() {
+
+ def source = """
+ import groovy.contracts.*
+
+ class A {
+
+ @groovy.transform.Synchronized
+ @Ensures({ result >= 0 })
+ def m(int a) { return a}
+
+ }
+
+ def a = new A()
+ assert a.m(12) == 12
+ """
+
+ evaluate source
Review Comment:
This regression test only asserts the happy path (`a.m(12)`), so it would
still pass if the `@Ensures` weaving was accidentally skipped. Adding an
assertion that a failing call throws `PostconditionViolation` would better
verify that the postcondition is actually executed for `@Synchronized` methods
(and not just that compilation succeeds).
##########
subprojects/groovy-contracts/src/main/java/org/apache/groovy/contracts/generation/PostconditionGenerator.java:
##########
@@ -92,6 +92,10 @@ public void addOldVariablesMethod(final ClassNode classNode)
{
*/
public void generatePostconditionAssertionStatement(MethodNode method,
org.apache.groovy.contracts.domain.Postcondition postcondition) {
+ // GROOVY-12084: normalize a non-block body (e.g. a @Synchronized
method whose body was wrapped
+ // in a SynchronizedStatement at CANONICALIZATION) before weaving, so
the casts below are safe
+ ensureBlockBody(method);
Review Comment:
`ensureBlockBody(method)` is added for explicit postconditions, but
`generateDefaultPostconditionStatement(...)` still calls
`addPostcondition(...)` without normalizing the method body. If a method is
transformed to a non-`BlockStatement` body (e.g. `@Synchronized`) and only
inherits postconditions from a superclass,
`addPostcondition`/`getReturnStatements` can still throw `ClassCastException`
when casting `method.getCode()` to `BlockStatement`. Consider also normalizing
in the default-postcondition path (or inside `addPostcondition`) so all
postcondition weaving is safe.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
