Skip to content
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.

Sign up for GitHub

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

Jump to bottom

baseMethod.overrides(childMethod) returns true #123

Closed
yigit opened this issue Oct 12, 2020 · 1 comment · Fixed by #124
Closed

baseMethod.overrides(childMethod) returns true #123

yigit opened this issue Oct 12, 2020 · 1 comment · Fixed by #124
Assignees
@neetopia
Labels
bug Something isn't working

Comments

@yigit
Copy link
Collaborator

yigit commented Oct 12, 2020

Given:

interface MyInterface {
  fun foo():Unit
}

class MyClass : MyInterface {
   override fun foo():Unit {}
}

calling resolver.overrides(myInterface.foo, myClass.foo) returns true instead of false

This started failing after I moved room from old package name to the new one, I'm guessing it might be related to this change:
a7085af

repo case: yigit@aa08209
@neetopia neetopia self-assigned this Oct 12, 2020
@neetopia neetopia added the bug Something isn't working label Oct 12, 2020
@neetopia
Copy link
Contributor

a temporary workaround can be manually checking class hierarchy before calling overrides

@yigit yigit mentioned this issue Oct 12, 2020
Merged
copybara-service bot pushed a commit to androidx/androidx that referenced this issue Oct 13, 2020
@yigit
update ksp to the new package
a172ed3 
This CL updates KSP to latest version.
It is rather large CL due to package name change in KSP.

Unfortunately, the new version has a regression where
override check does not check for type inheritance.
I implemented a workaround for it in Room for now.
google/ksp#123

Another notable change is that KSTypeReference's type
is not null anymore so I updated the code for it and removed
requireType.

As the CL is already confusing, didn't go through closed
issues to remove their workarounds yet, will do followup
CLs for them.

Bug: 160322705
Test: existing tests
Change-Id: Ie4902a4d23bb3448d81273a508775cbd8c3b0bd4
yigit added a commit to yigit/ksp that referenced this issue Oct 18, 2020
@yigit
Check class hierarchy when checking for overrides
b9ceceb 
There was a bug in overrides check where superClass.foo overrides
subClass.foo would return true even though super class cannot override
a subClass. This PR fixes that issue by checking for class hiearchy
in addition to the method signature.

Fixes: google#123
Test: CheckOverrideProcessor
neetopia pushed a commit that referenced this issue Oct 18, 2020
@yigit @neetopia
Check class hierarchy when checking for overrides
eae30cf 
There was a bug in overrides check where superClass.foo overrides
subClass.foo would return true even though super class cannot override
a subClass. This PR fixes that issue by checking for class hiearchy
in addition to the method signature.

Fixes: #123
Test: CheckOverrideProcessor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@neetopia neetopia

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Check for type hierarchy while checking overrides yigit/ksp
2 participants
@yigit @neetopia