Pass owner to navigator when it can be inferred from the target#2618
Open
Bawnorton wants to merge 2 commits intominecraft-dev:devfrom
Open
Pass owner to navigator when it can be inferred from the target#2618Bawnorton wants to merge 2 commits intominecraft-dev:devfrom
Bawnorton wants to merge 2 commits intominecraft-dev:devfrom
Conversation
fix source file lookup when referencing same-project / sibling-project class
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Following on from #2616
As it turns out simply transforming the result of
getCustomOwnerwas almost enough by itself, I just did it wrong in testing before 😅, however, when it comes to navigating to the target there were a couple of issues. Notably injection points would rely on the untransformed class passed from theAtResolver(providing the selector with enough context to transform that per #2616 would be a bit of a pain).MinecraftDev/src/main/kotlin/platform/mixin/handlers/injectionPoint/AtResolver.kt
Line 230 in 66f57a6
This results in the injection points looking for the target in the wrong class, so instead, where possible we trust the qualifier of the
target(forINVOKE/INVOKE_ASSIGN/FIELD) to find the element, which ultimately seems to have done the trick for most injection points.Notably
HEADwould wind up getting stuck in the target's injector annotation itself, resulting in a target resolution failure, so I opted to skip annotation visiting on that injection point navigator as there isn't any reasonHEADshould point there anyway.The injection points
CONSTANT/NEW/CTOR_HEADworked out the box which was convenient, howeverMIXINEXTRAS:EXPRESSIONhas a an issue that any definitions that reference a member "belonging" to the mixin will fail to navigate to from the gutter icon as eventually the qualifier will be queried on said member by ME's matching algorithm which will resolve as the mixin rather than the qualifier in the definition. If I were to make the same change to ME's matching it'd be to trust the definition qualifier when looking for the member rather than the AST's qualifier, however, I don't know the best way to implement that as there's a lot going on there.This fails:

This succeeds:

Also fixes source file lookup when referencing same-project / sibling-project class, as sometimes the file is already decompiled when you get it via
stubClass.containingFile