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.

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

[Decompiler] Fix short name imports shadowed by super inner classes (IDEA-196315) #854

Closed
wants to merge 1 commit into from

Conversation

JDLogic
Copy link
Contributor

@JDLogic JDLogic commented Sep 11, 2018

Currently, decompiling the modified test class will result in the short names Builder and Entry incorrectly being used as both these imports will be shadowed. This PR adds a check to the import collector to see if a short name will be shadowed a super inner class and adjusts the import accordingly.

@JDLogic
Copy link
Contributor Author

JDLogic commented Sep 14, 2018

I just thought of a way to slightly improve this. I'll update the PR this weekend when I have time.

@JDLogic
Copy link
Contributor Author

JDLogic commented Sep 16, 2018

Cleaned up this PR. Should be much better.

@@ -158,4 +161,24 @@ public int writeImports(TextBuffer buffer) {
.map(ent -> ent.getValue() + "." + ent.getKey())
.collect(Collectors.toList());
}

private boolean isSuperInnerClass(StructClass cl, String className) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little bit worried about performance here, wouldn't it be better to populate all classes names set in the ImportCollector constructor like we have already for fields (see org.jetbrains.java.decompiler.main.collectors.ImportCollector#setFieldNames)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just got a chance to look this over, and you are correct. I'll update this PR soon.

@JDLogic
Copy link
Contributor Author

JDLogic commented Sep 26, 2018

Updated with the requested changes made.

@gorrus
Copy link
Member

gorrus commented Sep 27, 2018

merged into master, thanks for contribution!

@gorrus gorrus closed this Sep 27, 2018
@gorrus gorrus added the Merged label Sep 27, 2018
SergeyZh pushed a commit that referenced this pull request Oct 9, 2020
)

* Consider navigation element in PSI equality checks

This seems to solve issues related to psi becoming outdated.
Extension of comment by ajchun:
mplushnikov/lombok-intellij-plugin#840 (comment)

Covers issues: #821, #827, #829, #840, #842, #844, #846, #850, #853, #854, #855, #857

* Remove debug logging related to equality of PsiElements

GitOrigin-RevId: e9ad9801b15dc91b872b14add012a9261033fbc7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants