intellij-community
intellij-community copied to clipboard
IDEA-323330
Fixes IDEA-323330
Summary of changes:
- Made the affected classes and constructors public
- Also changed the visibility of methods from
privatetoprotectedso that they can be overriden
Why do you need these classes public? What functionality are you trying to achieve in your plugin?
The answers for these questions are already inside IDEA-323330.
Why do you need these classes public?
So that I don't get a warning when publishing to your marketplace. Also all other classes inside these packages are public...
What functionality are you trying to achieve in your plugin?
Code formatting when e.g. saving & much more. Please read the plugin description (in the issue) for more information.
Please point out the plugin description in the issue, I don't see it.
These classes have nothing to do with either code formatting or saving. And there are likely many ways to avoid the warning (including not using the classes). So what are you doing with these classes that you need them public? What changes do you make in their behaviour?
Making the classes public is acceptable, but making every member protected seems overkill. These members are not designed to be overridden. Perhaps there is some other way to achieve what you need.
Please point out the plugin description in the issue, I don't see it.
Here is our plugin and here the original one that we forked.
How to get there
or
(the Readme contains the same description as on the marketplace)
These classes have nothing to do with either code formatting or saving.
The classes are used for running inspections and fixing them. They are used inside the following actions of the plugin:
Add class qualifier to static member access outside declaring classusesUnqualifiedStaticCallVisitor- https://github.com/xdev-software/intellij-plugin-save-actions/blob/456210101ad7466aa8d6a0f25e224d397dbccea6/src/main/java/software/xdev/saveactions/processors/java/JavaProcessor.java#L74-L75
Change visibility of field or method to lower accessusesAccessCanBeTightenedInspection- https://github.com/xdev-software/intellij-plugin-save-actions/blob/456210101ad7466aa8d6a0f25e224d397dbccea6/src/main/java/software/xdev/saveactions/processors/java/JavaProcessor.java#L111-L112
And there are likely many ways to avoid the warning (including not using the classes).
If we shouldn't use these classes then what are alternatives?
So what are you doing with these classes that you need them public?
So that we can instantiate them and don't have to copy the code.
What changes do you make in their behavior?
As far as I have seen there were currently no changes made to the behavior by the original authors. We also didn't modify their behavior.
Making the classes public is acceptable, but making every member protected seems overkill. These members are not designed to be overridden. Perhaps there is some other way to achieve what you need.
I was thinking that maybe others might need to do changes to these members or we do somewhere in the future that's why I made them overrideable. Otherwise a new PR would be required then.
However I can revert making the protected if this is fine for you :)
I looked into this some more, and it seems to me you could just move your CustomUnqualifiedStaticUsageInspection to a different package without any problems. It doesn't use any package local members.
Your CustomAccessCanBeTightenedInspection uses just a single package local method which I now have public (will appear in the repo soon). Or you could just reimplement this one rather short method. Then this class will also be moveable to a different package.
Doing the changes above will avoid the verification warnings when publishing the plugin. Therefore I don't think the changes in this pull request are necessary. Do you disagree?
This could be a workaround however the code would still be duplicated, which has disadvantages like bloating the plugin and needing to update the duplicated code regularly.
I still think that the best solution would be if we can use the code that is also used inside IntelliJ and by that just simply making it public.
As far as I have seen there were currently no changes made to the behavior by the original authors. We also didn't modify their behavior.
I also rechecked the classes in detail and came to the following exact changes that are required so that we can remove the duplicated code:
UnqualifiedStaticCallVisitorneeds to be public (currentlyprivate) so that it can be instantiatedisUnqualifiedStaticAccessneeds to be overriden (currentlyprivate) -> it must at least beprotected
AccessCanBeTightenedInspectionneeds to be public (currentlypackage-private) so that it can be instantiated- Additionally I just found out that
AccessCanBeTightenedInspectionusespackage-privatemembers ofVisibilityInspectionwhich are not accessible due to different classloaders (as is the case when using plugins), they likely should also bepublic(see also https://github.com/xdev-software/intellij-plugin-save-actions/issues/14); Affected:int getMinVisibilityLevel(PsiMember)boolean containsReferenceTo(PsiElement, PsiElement)(static)
- Additionally I just found out that
I rebased and updated the PR accordingly.