[
https://issues.apache.org/jira/browse/GROOVY-12726?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18140505#comment-18140505
]
ASF GitHub Bot commented on GROOVY-12726:
-----------------------------------------
testlens-app[bot] commented on PR #3262:
URL: https://github.com/apache/groovy/pull/3262#issuecomment-6716953759
## 🚨 All tests passed but jobs failed 🚨
### Failed Jobs without Test Failures
❌ [CodeQL / Analyze \(java,
21\)](https://github.com/apache/groovy/actions/runs/42444534834/job/128620576187?pr=3262)
🏷️ Commit: 0431d5b4a5987605c61cbeef93224872c806f97c
▶️ Tests: 5 executed
🟡 Checks: 4/24 completed
### Rerun Controls
Click the checkbox to trigger a rerun:
🔲 **Rerun jobs**
---
_Learn more about TestLens at
[testlens.app/docs](https://testlens.app/docs/features/pr-comment/)._
> Fix joint-compilation stubber placeholder cleanup and field-level
> @Bindable/@Vetoable stubbing
> ----------------------------------------------------------------------------------------------
>
> Key: GROOVY-12726
> URL: https://issues.apache.org/jira/browse/GROOVY-12726
> Project: Groovy
> Issue Type: Bug
> Reporter: Daniel Sun
> Assignee: Daniel Sun
> Priority: Major
>
> h2. Overview
> During joint compilation, stubber AST transformations
> ({{ToStringASTStubber}}, {{EqualsAndHashCodeASTStubber}},
> {{IndexedPropertyASTStubber}}, {{BindableASTStubber}}, and
> {{VetoableASTStubber}}) run in the {{CONVERSION}} phase to insert placeholder
> {{MethodNode}} stubs (marked via {{StubberSupport}}) so that the Java stub
> generator can emit method signatures before the full AST transformations run
> in later phases. Several edge cases in the handoff between
> {{CONVERSION}}-phase stubbers and their corresponding full AST
> transformations can leave dummy stub methods in final bytecode or omit
> required methods from Java stubs.
> h2. Problem Description
> h3. 1. {{@ToString}} and {{@EqualsAndHashCode}} leak dummy stub methods when
> {{_toString}}, {{_hashCode}}, or {{_equals}} is declared
> In {{ToStringASTTransformation.createToString}} and
> {{EqualsAndHashCodeASTTransformation}} ({{createHashCode}} and
> {{createEquals}}), the presence of a {{CONVERSION}}-phase stub sets
> {{hasExistingToString}}, {{hasExistingHashCode}}, or {{hasExistingEquals}} to
> {{true}}. However, the transformations checked {{hasDeclaredMethod(cNode,
> UNDER_TO_STRING, 0)}} (and {{UNDER_HASH_CODE}} / {{UNDER_EQUALS}}) and
> returned early *before* checking {{StubberSupport.isStub(...)}}:
> {code:java}
> if (hasExistingToString) {
> if (hasDeclaredMethod(cNode, UNDER_TO_STRING, 0)) return;
> MethodNode toString = cNode.getDeclaredMethod(TO_STRING,
> Parameter.EMPTY_ARRAY);
> if (StubberSupport.isStub(toString)) {
> stubberPlaceholder = toString;
> hasExistingToString = false;
> }
> ...
> }
> {code}
> As a result, if a class annotated with {{@ToString}} or
> {{@EqualsAndHashCode}} also declared a helper method ({{_toString()}},
> {{_hashCode()}}, or {{_equals(Object)}}), the transformation returned early
> during {{CANONICALIZATION}} without replacing the stubber placeholder,
> leaving the dummy stub implementation ({{return ""}}, {{return 0}}, or
> {{return false}}) in the compiled class.
> h3. 2. {{@IndexedProperty}} leaks setter stubs on immutable classes
> {{IndexedPropertyASTStubber}} runs in {{CONVERSION}}, before meta-annotations
> (such as a custom {{@AnnotationCollector}} expanding to {{@ImmutableBase}})
> are expanded in {{SEMANTIC_ANALYSIS}}, and therefore emits both {{getX(int)}}
> and {{setX(int, V)}} placeholders. Later, in {{CANONICALIZATION}},
> {{IndexedPropertyASTTransformation}} detects {{IMMUTABLE_BREADCRUMB}} on the
> field and skips {{addArraySetter}} / {{addListSetter}}, but did not remove
> the {{CONVERSION}}-phase setter placeholder, leaving a no-op {{setX(int, V)}}
> method in the bytecode of the immutable class. In addition,
> {{IndexedPropertyASTStubber.isClassImmutable}} did not check for
> {{@RecordType}} or {{@RecordBase}}.
> h3. 3. {{BindableASTTransformation}} and {{VetoableASTTransformation}} do not
> strip stub placeholders in {{needsPropertyChangeSupport}} /
> {{needsVetoableChangeSupport}}
> {{BindableASTTransformation.needsPropertyChangeSupport}} and
> {{VetoableASTTransformation.needsVetoableChangeSupport}} scan
> {{declaringClass.getMethods()}} for existing listener registration and firing
> methods without first removing {{StubberSupport.isStub(m)}} placeholders,
> which can cause them to treat stubber placeholders as user-supplied
> implementations.
> h3. 4. {{BindableASTStubber}} and {{VetoableASTStubber}} ignore field-level
> {{@Bindable}} and {{@Vetoable}} annotations
> {{@Bindable}} and {{@Vetoable}} can be applied to individual fields as well
> as classes, and both forms generate class-level listener methods
> ({{addPropertyChangeListener}}, {{addVetoableChangeListener}}, etc.).
> However, {{BindableASTStubber}} and {{VetoableASTStubber}} returned early
> whenever {{!(parent instanceof ClassNode)}}, so Java sources in a joint
> compilation unit could not call listener methods on Groovy classes that only
> used field-level {{@Bindable}} or {{@Vetoable}}.
> h2. Solution
> * Check {{StubberSupport.isStub(...)}} before checking for {{_toString}},
> {{_hashCode}}, and {{_equals}} in {{ToStringASTTransformation}} and
> {{EqualsAndHashCodeASTTransformation}} so stubber placeholders are always
> replaced with the generated implementation.
> * Remove any stubber-generated indexed setter placeholder in
> {{IndexedPropertyASTTransformation}} when {{immutable}} is {{true}}, and
> include {{@RecordType}} and {{@RecordBase}} in
> {{IndexedPropertyASTStubber.isClassImmutable}}.
> * Remove stubber placeholders at the start of
> {{BindableASTTransformation.needsPropertyChangeSupport}} and
> {{VetoableASTTransformation.needsVetoableChangeSupport}}.
> * Update {{BindableASTStubber}} and {{VetoableASTStubber}} to also emit
> listener method stubs on the declaring class when {{@Bindable}} or
> {{@Vetoable}} annotates a non-static, non-final property field.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
