[
https://issues.apache.org/jira/browse/GROOVY-12084?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18088407#comment-18088407
]
ASF GitHub Bot commented on GROOVY-12084:
-----------------------------------------
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.
> groovy-contracts: @Synchronized method with @Ensures/@Invariant but no
> @Requires throws ClassCastException (SynchronizedStatement → BlockStatement)
> ---------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12084
> URL: https://issues.apache.org/jira/browse/GROOVY-12084
> Project: Groovy
> Issue Type: Bug
> Components: groovy-contracts
> Reporter: Paul King
> Assignee: Paul King
> Priority: Major
>
> This code:
> {code:groovy}
> import groovy.contracts.*
> class A {
> @groovy.transform.Synchronized
> //@Requires({ a >= 0 })
> @Ensures({ a >= 0 })
> def m(int a) { return a}
> }
> def a = new A()
> a.m(12)
> {code}
> Gives this error:
> {noformat}
> Exception thrown
> BUG! exception in phase 'instruction selection' in source unit
> 'ConsoleScript6' class org.codehaus.groovy.ast.stmt.SynchronizedStatement
> cannot be cast to class org.codehaus.groovy.ast.stmt.BlockStatement
> (org.codehaus.groovy.ast.stmt.SynchronizedStatement and
> org.codehaus.groovy.ast.stmt.BlockStatement are in unnamed module of loader
> org.codehaus.groovy.tools.RootLoader @6e0be858)
> at
> org.codehaus.groovy.control.CompilationUnit$ISourceUnitOperation.doPhaseOperation(CompilationUnit.java:980)
> ...
> {noformat}
> It runs fine if the @Requires is present.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
