-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
GROOVY-8283: field hides getter of super class (not interface) #1767
base: master
Are you sure you want to change the base?
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 |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1767 +/- ##
==================================================
+ Coverage 68.8221% 68.8289% +0.0068%
- Complexity 29437 29460 +23
==================================================
Files 1420 1420
Lines 113141 113156 +15
Branches 19543 19552 +9
==================================================
+ Hits 77866 77884 +18
+ Misses 28735 28731 -4
- Partials 6540 6541 +1
|
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