[ 
https://issues.apache.org/jira/browse/GROOVY-12106?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18092075#comment-18092075
 ] 

ASF GitHub Bot commented on GROOVY-12106:
-----------------------------------------

Copilot commented on code in PR #2635:
URL: https://github.com/apache/groovy/pull/2635#discussion_r3486973642


##########
src/main/java/org/codehaus/groovy/transform/stc/TraitTypeCheckingExtension.java:
##########
@@ -82,17 +82,21 @@ public List<MethodNode> handleMissingMethod(final ClassNode 
receiver, final Stri
             ClassNode returnType = 
mce.getNodeMetaData(TraitASTTransformation.DO_DYNAMIC);
             if (returnType != null) return 
Collections.singletonList(makeDynamic(call, returnType));
 
-            // GROOVY-7322, GROOVY-8272, GROOVY-8587, GROOVY-8854, 
GROOVY-10312: trait: this.m($static$self)
+            // GROOVY-7322, GROOVY-8272, GROOVY-8587, GROOVY-8854, 
GROOVY-10312, GROOVY-12106: trait: this.m($static$self)
             ClassNode targetClass = 
isClassClassNodeWrappingConcreteType(receiver)? 
receiver.getGenericsTypes()[0].getType(): receiver;
             if (Traits.isTrait(targetClass.getOuterClass()) && 
argumentTypes.length > 0 && ClassHelper.isClassType(argumentTypes[0])) {
-                Parameter[] signature = 
java.util.Arrays.stream(argumentTypes).map(t -> new 
Parameter(t,"")).toArray(Parameter[]::new);
                 List<ClassNode> traits = 
Traits.findTraits(targetClass.getOuterClass());
                 traits.remove(targetClass.getOuterClass());
 
                 for (ClassNode trait : traits) { // check super trait for 
static method
-                    MethodNode method = 
Traits.findHelper(trait).getDeclaredMethod(name, signature);
-                    if (method != null && method.isStatic()) {
-                        return Collections.singletonList(makeDynamic(call, 
method.getReturnType()));
+                    // GROOVY-12106: resolve via the type checker rather than 
an exact
+                    // parameter-type match, so an argument whose static type 
is a subtype
+                    // of the declared parameter still resolves the inherited 
helper static
+                    // (matching how plain-class static inheritance behaves).
+                    for (MethodNode method : 
typeCheckingVisitor.findMethod(Traits.findHelper(trait), name, argumentTypes)) {
+                        if (method.isStatic()) {
+                            return Collections.singletonList(makeDynamic(call, 
method.getReturnType()));
+                        }
                     }
                 }

Review Comment:
   `typeCheckingVisitor.findMethod` performs full STC method lookup (including 
DGM/extension methods) and returns a best-match set with non-deterministic 
iteration order when there are multiple best candidates. In this trait-helper 
resolution path, that can (a) accidentally match an unrelated extension method 
on the helper class and/or (b) pick an arbitrary overload’s return type when 
multiple candidates tie, which can break downstream type-checking that relies 
on `DYNAMIC_RESOLUTION`’s inferred return type. It would be safer and more 
precise to restrict the search to the helper’s declared methods and then apply 
subtype-aware overload selection (e.g. 
`StaticTypeCheckingSupport.chooseBestMethod`) over just those static 
candidates, falling back to `Object` if resolution is ambiguous.





> STC cannot resolve inherited static trait method from sub-trait body
> --------------------------------------------------------------------
>
>                 Key: GROOVY-12106
>                 URL: https://issues.apache.org/jira/browse/GROOVY-12106
>             Project: Groovy
>          Issue Type: Bug
>          Components: Static Type Checker
>    Affects Versions: 5.0.7
>            Reporter: James Fredley
>            Assignee: Paul King
>            Priority: Major
>              Labels: static-method, sts, traits
>
> h2. Summary
> Groovy 5 static type checking does not resolve a static method declared by a 
> parent trait when the method is called from a sub-trait body.
> This is independent of Grails and can be reproduced with plain Groovy traits 
> under @CompileStatic.
> This was re-tested on Groovy 5.0.7-SNAPSHOT. Both an unqualified call and a 
> qualified call fail static type checking:
> {code:groovy}
> inheritedStaticMethod(value)
> {code}
> {code:groovy}
> ParentTrait.inheritedStaticMethod(value)
> {code}
> The failure is distinct from GROOVY-11985, which fixed 
> static-override-via-this. This issue is about resolving a parent trait's 
> static method from a sub-trait body.
> h2. Standalone reproducer
> {code:groovy}
> import groovy.transform.CompileStatic
> @CompileStatic
> trait ParentTrait {
>     static void configure(Closure closure, Object target) {
>         if (closure != null) {
>             closure.delegate = target
>             closure.resolveStrategy = Closure.DELEGATE_ONLY
>             closure.call()
>         }
>     }
> }
> @CompileStatic
> trait ChildTrait extends ParentTrait {
>     void applyConfig(Closure closure, Object target) {
>         configure(closure, target)
>     }
> }
> @CompileStatic
> class Example implements ChildTrait {
> }
> {code}
> h2. Expected result
> The code should pass static type checking. A sub-trait should be able to 
> resolve a static method declared by a parent trait.
> h2. Actual result
> Static type checking fails on the call from the sub-trait body:
> {code}
> Cannot find matching method ChildTrait#configure(groovy.lang.Closure, 
> java.lang.Object)
> {code}
> Using a qualified call also fails:
> {code:groovy}
> ParentTrait.configure(closure, target)
> {code}
> h2. Real-world use case
> Apache Grails has the same pattern in the GraphQL DSL helper traits.
> Parent trait method:
> {code}
> grails-data-graphql/core/src/main/groovy/org/grails/gorm/graphql/entity/dsl/helpers/ExecutesClosures.groovy
> line 34
> {code}
> {code:groovy}
> @CompileStatic
> trait ExecutesClosures {
>     static void withDelegate(@DelegatesTo(strategy = Closure.DELEGATE_ONLY) 
> Closure closure, Object delegate) {
>         ...
>     }
> }
> {code}
> Sub-traits that should be able to call the inherited helper:
> {code}
> grails-data-graphql/core/src/main/groovy/org/grails/gorm/graphql/entity/dsl/helpers/Arguable.groovy
> grails-data-graphql/core/src/main/groovy/org/grails/gorm/graphql/entity/dsl/helpers/ComplexTyped.groovy
> {code}
> Grails currently works around this by inlining the parent trait helper body 
> into each sub-trait. Once this Groovy STC issue is fixed, that inline 
> workaround can be removed.
> h2. Workaround
> Inline the static helper body in each sub-trait, or avoid calling the 
> inherited static trait method from the sub-trait.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to