Copilot commented on code in PR #2988:
URL: https://github.com/apache/groovy/pull/2988#discussion_r4317886360
##########
subprojects/groovy-contracts/src/main/java/org/apache/groovy/contracts/generation/AssertStatementCreationUtility.java:
##########
@@ -95,29 +95,22 @@ public static AssertStatement getAssertionStatement(final
BooleanExpression bool
* @return a {@link ReturnStatement} or <tt>null</tt>
*/
public static List<ReturnStatement> getReturnStatements(MethodNode method)
{
-
+ // GROOVY-12129: delegate implicit-return conversion to the compiler's
own ReturnAdder.
+ // The value-producing trailing statement may be a plain expression
statement, but also
+ // an if/else whose branches are expressions, a switch, or a try/catch
(e.g. after
+ // method-level @Decreases wrapping); converting only the first case
bound `result` to
+ // a synthesized default — and the synthesized `return <default>`
changed what the
+ // woven method returned. ReturnAdder performs exactly the conversion
the compiler
+ // would apply later anyway (including a default-value return for an
empty body,
+ // preserving GROOVY-12082 semantics), so weaving at its returns
cannot change what
+ // the method computes. Void methods are left untouched by ReturnAdder.
+ // GROOVY-12463: the conversion must run even when the method already
contains explicit
+ // returns: an early `return` does not make the trailing value
explicit, and skipping the
+ // conversion left that path unchecked. ReturnAdder leaves existing
returns unchanged.
+ new ReturnAdder().visitMethod(method);
Review Comment:
`getReturnStatements` now always mutates the passed `MethodNode` by running
`ReturnAdder`. If this utility is invoked more than once for the same
`MethodNode` during weaving/visiting, it can cause repeated AST transformation
work and makes call-order more fragile. Consider guarding with node metadata
(e.g., set a flag on the `MethodNode` after applying `ReturnAdder`) so the
conversion is applied at most once per method.
##########
subprojects/groovy-contracts/src/main/java/org/apache/groovy/contracts/generation/AssertStatementCreationUtility.java:
##########
@@ -95,29 +95,22 @@ public static AssertStatement getAssertionStatement(final
BooleanExpression bool
* @return a {@link ReturnStatement} or <tt>null</tt>
*/
public static List<ReturnStatement> getReturnStatements(MethodNode method)
{
Review Comment:
The Javadoc return description is incorrect: the method returns a
`List<ReturnStatement>` (likely empty), not a single `ReturnStatement` or
`null`. Please update `@return` to reflect the actual return type/semantics
(e.g., “list of return statements (possibly empty)”).
--
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]
