Daniel Sun created GROOVY-12726:
-----------------------------------
Summary: 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
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)
