-
Notifications
You must be signed in to change notification settings - Fork 1.9k
GROOVY-8283: field hides getter of super class (not interface) #1767
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
fb192a5
to
8324f8b
Compare
@blackdrag @paulk-asert If I refresh the branch, do you think this is viable? The test case should illustrate the new hiding behavior. |
fe14c7f
to
ec44e38
Compare
For circumstances that go through
package p
class A {}
class B {}
class C {
boolean setter
protected A foo = new A()
A getFooA() { return this.@foo }
void setFoo(A a) { setter = true; this.@foo = a }
}
class D extends C {
protected B foo = new B() // hides A#foo; should hide A#setFoo in subclasses
B getFooB() { return this.@foo }
} import p.*
class E extends D {}
def e = new E()
e.foo = null // not the field from this perspective
assert e.setter // fails |
8452e8a
to
b4325e4
Compare
1465b0c
to
d068ecd
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1767 +/- ##
==================================================
- Coverage 68.0868% 68.0836% -0.0032%
- Complexity 29190 29210 +20
==================================================
Files 1377 1377
Lines 114141 114164 +23
Branches 19796 19806 +10
==================================================
+ Hits 77715 77727 +12
- Misses 30093 30100 +7
- Partials 6333 6337 +4
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR addresses GROOVY-8283, implementing field hiding behavior where a field in a subclass can hide a getter method of its superclass (but not interface methods). The changes ensure consistent behavior across dynamic execution, static type checking, and static compilation contexts.
Key changes include:
- Enhanced field visibility logic in
MetaClassImpl
to properly handle field shadowing of superclass getters - Updated static type checking to consider field accessibility when selecting between fields and getter methods
- Modified bytecode generation to respect field hiding rules during property access
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
File | Description |
---|---|
src/test/groovy/bugs/Groovy8283.groovy |
Comprehensive test suite covering field/property shadowing scenarios across dynamic, type-checked, and compile-static contexts |
src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java |
Enhanced property resolution logic to handle field accessibility and getter method selection with bridge method filtering |
src/main/java/org/codehaus/groovy/runtime/ScriptBytecodeAdapter.java |
Updated property access methods to handle missing property exceptions and sender class context properly |
src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java |
Added field hiding checks in static compilation to prevent getter usage when field should take precedence |
src/main/java/groovy/lang/MetaClassImpl.java |
Core field visibility logic implementation with new isVisibleProperty method to determine when fields should hide superclass getters |
For the dynamic side of 8283, field can be selected in
MetaClassImpl
. If access method is declared by an interface, the interface method is the one indexed so must be considered pervasive in the type hierarchy (cannot be safely hidden). Not sure if this kind of name hiding should be extended to inner classes; hopefully the commit is enough of a first step to determine if this protocol should be changed or left alone.@blackdrag @paulk-asert
TODO:
StaticTypeCheckingVisitor
does vsMetaClassImpl
)https://issues.apache.org/jira/browse/GROOVY-8283