-
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. |
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.5957% 68.8510% +0.2553%
- Complexity 29233 29430 +197
==================================================
Files 1426 1419 -7
Lines 113494 113076 -418
Branches 19548 19532 -16
==================================================
+ Hits 77852 77854 +2
+ Misses 29088 28681 -407
+ Partials 6554 6541 -13
|
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