Remove unused suppress warning annotations removes USED supress warning annotations.
Delfic opened this issue · 30 comments
Example
public Button getActionButton(final String actionButtonId) {
Button button = null;
final ActionToolbar actionToolbar = this.getActionToolbar();
if (actionToolbar != null && CollectionUtils.isNotEmpty(actionToolbar.getChildren())) {
for (final Component component : new ArrayList<Component>(actionToolbar.getChildren())) {
if ((component instanceof Button) && (StringUtils.equals(actionButtonId, component.getId()))) {
button = (Button) component;
break;
}
}
}
return button;
}
actionToolbar.getChildren() is unchecked but the @SuppressWarnings("unchecked") was removed upon save even thought it was being used.
Have you opened a bug with Jetbrains?
I didn't open a ticked. I've searched and did not find one. https://youtrack.jetbrains.com/issues?q=supressWarnings
I didn't see an inspection that corresponded to the functionality you are referring to. The unused code inspection has properties for fields, methods, etc... but no annotations. It makes me think that whatever code you are invoking is unused. Can you reproduce the bug in Idea?
Actually I cannot, I've never found such functionality on IDEA. The closest I've found was an inspection for suppression that suggests to remove the annotations. Maybe it's invoking such inspection with the suggested resolution...
So is it possible that it is not a bug in the intellij code but there is some extra context that needs to be set up or passed in somehow? As it stands this feature in the save actions plugin cannot be enabled and therefore is useless.
@dubreuia What's your say in this? Is there any documentation on the API for the IDEA code you used? Is it the feature that I think it is?
@NikolayMetchev Not quite useless. One might want to not use any suppress warnings and might want to go through code and remove it. Not useless but might need a rebranding to "remove suppressWarnings annotations"
Hey @Delfic I'm using the quick fix class https://github.com/JetBrains/intellij-community/blob/306d705e1829bd3c74afc2489bfb7ed59d686b84/plugins/InspectionGadgets/testsrc/com/siyeh/ig/maturity/SuppressionAnnotationInspectionTest.java
I have no idea why it doesn't work the intended way, and honestly this is really low in the task list right now. You can look into it if you want! My best guess would be I'm not using the quick fix properly.
Hello @dubreuia. I would gladly help out on this one. Quick question: is the link you provided the right one? That seems like an almost empty test case and not a "quick fix class".
I've tried looking for the LightInspectionTestCase to check on the doTest method but to no avail :(
Help me out here: where, in save-actions, is the call to this particular fix?
Sorry, this is the good link: https://github.com/JetBrains/intellij-community/blob/d19918722bcc19d449f0fdd762027db9cec3bf10/plugins/InspectionGadgets/src/com/siyeh/ig/maturity/SuppressionAnnotationInspection.java
- it is instancited here: com/dubreuia/processors/java/ProcessorFactory.java:58
- called here: com/dubreuia/processors/java/InspectionProcessor.java:96
I'll add some documentation to help you setup the environment
@dubreuia Thank for the documentation. I think I've found the bug.
You're calling a class that implements the fixes for this inspections:
Regarding the description of the action and what started this bug report in the first place, I believe what you wanted to achieve was using this code analysis (not direct inspection, only available through Analyze)
I've made tests and this is the one we want.
Which is implemented here
com.intellij.codeInspection.RedundantSuppressInspection
Could not find this one on the git project you linked but it's present in the intellij-core-analysis.jar in the intellij-core-163.7743.44.zip which is found on the link you provided in the documentation.
This is a GlobalInspectionTool rather than a LocalInspection so some tweaking will be necessary.
Thank you for the opportunity :)
@Delfic I can't find why it doesn't work, I've reverted the change on master, you'll need to do a new PR
From what I read from the thread, this issue is still completely open, right? I've just stumbled across this one with activated "Remove unused suppress warning annotation"
Yup, Still Open since we haven't figured out yet why the NPE occurs.
Not sure either, you need to debug it and see with it occurs in intellij code. I'm pretty sure it has to do with the fact that instead of using com.intellij.codeInspection.ex.LocalInspectionToolWrapper like the other inspections, you're using the GlobalToolWrapper. There's probably a context variable to setup somewhere.
Is there any workaround for this issue?
Hey @pablomusumeci, not for now no. But @Delfic submited a PR that was a step in the right direction, if you want to look at it and resubmit, I gladly take PRs.
I stumbled on this one again today: this annotation got deleted com/dubreuia/processors/java/InspectionProcessor.java:90
+1
IntelliJ Ultimate 2019.1
Save Actions v1.4.0
(Had it last year too.)
+1
IntelliJ Ultime 2019.1.3
Save Action 1.5.0
A workaround would be necessary asap :)
+1
IntelliJ Ultime 2019.1.3
Save Action 1.5.0A workaround would be necessary asap :)
As or a workaround you can run the inspection by name "Redundant suppression"
I did a couple of attempts, one of which I didn't get far and opened a draft PR about it.
The other one, I applied the same strategy of doing a refactor on the processor and doing a refactor and I've hit two bumps:
One it doesn't even find @SuppressWarnings it only finds @java.lang.SuppressWarnings
I changed the unit test files to have the fully qualified version and continued with the debugging and got to a point where it finds the scopes but now it needs some inspection tools available in the inspection manager so that they actually check if the scope is being wrongfully suppressed or not. So maybe the way the inspection is called it just won't work because it needs all the inspections of all the possible suppressed scopes to be there to confirm if those scopes can be safely removed from the SuppressWarnings annotation.
Are those inspections missing from the overall save-actions context or just from the unit test context?
Tomorrow I'll try building and installing the plug-in from my branch to see if it works in spite of the unit tests failing.
So it was easier to build and install, just install the zip inside the distribution folder. So... It doesn't work installing the distributed plugin after the changes I'm testing. Happy thought is that the unit test works :)
So to solve this we would need to have all the tools that inspect for the suppressed scopes loaded.
Is that feasible?
I can confirm this issue. See attached if this helps: