intellij-plugin-save-actions icon indicating copy to clipboard operation
intellij-plugin-save-actions copied to clipboard

Remove unused suppress warning annotations removes USED supress warning annotations.

Open Delfic opened this issue 8 years ago • 30 comments
trafficstars

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.

Delfic avatar Jun 27 '17 18:06 Delfic

Thanks @Delfic I'll look into it.

This is somewhat similar to #79: I don't actually control what the processor does, I'm just calling Intellij code. We'll have two choices either (1) open a bug a Jetbrains (2) re implement the processor myself

dubreuia avatar Jun 28 '17 08:06 dubreuia

Have you opened a bug with Jetbrains?

NikolayMetchev avatar Oct 18 '17 11:10 NikolayMetchev

I didn't open a ticked. I've searched and did not find one. https://youtrack.jetbrains.com/issues?q=supressWarnings

Delfic avatar Oct 18 '17 15:10 Delfic

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?

NikolayMetchev avatar Oct 18 '17 16:10 NikolayMetchev

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...

Delfic avatar Oct 18 '17 16:10 Delfic

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.

NikolayMetchev avatar Oct 18 '17 16:10 NikolayMetchev

@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"

Delfic avatar Oct 18 '17 17:10 Delfic

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.

dubreuia avatar Oct 22 '17 12:10 dubreuia

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?

Delfic avatar Oct 22 '17 15:10 Delfic

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 avatar Oct 22 '17 19:10 dubreuia

Here: https://github.com/dubreuia/intellij-plugin-save-actions#development-environment

dubreuia avatar Oct 22 '17 19:10 dubreuia

@dubreuia Thank for the documentation. I think I've found the bug.

You're calling a class that implements the fixes for this inspections: inspectionsuppresion

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) redundantsuppression

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.

Delfic avatar Oct 22 '17 22:10 Delfic

@dubreuia I've created a PR. https://github.com/dubreuia/intellij-plugin-save-actions/pull/115

If anything is amiss, let me know. Thanks

Delfic avatar Oct 22 '17 23:10 Delfic

Thanks @Delfic I'll look into it.

dubreuia avatar Oct 23 '17 08:10 dubreuia

Thank you for the opportunity :)

Delfic avatar Oct 23 '17 08:10 Delfic

NPE on save https://github.com/dubreuia/intellij-plugin-save-actions/pull/115

dubreuia avatar Oct 23 '17 09:10 dubreuia

@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

dubreuia avatar Oct 26 '17 11:10 dubreuia

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"

nikowitt avatar Jan 11 '18 09:01 nikowitt

Yup, Still Open since we haven't figured out yet why the NPE occurs.

Delfic avatar Jan 11 '18 09:01 Delfic

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.

dubreuia avatar Jan 11 '18 12:01 dubreuia

Is there any workaround for this issue?

pablomusumeci avatar Mar 19 '18 12:03 pablomusumeci

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.

dubreuia avatar Mar 19 '18 12:03 dubreuia

I stumbled on this one again today: this annotation got deleted com/dubreuia/processors/java/InspectionProcessor.java:90

dubreuia avatar Aug 08 '18 10:08 dubreuia

+1 IntelliJ Ultimate 2019.1 Save Actions v1.4.0

(Had it last year too.)

MartinX3 avatar Apr 05 '19 13:04 MartinX3

+1 IntelliJ Ultime 2019.1.3 Save Action 1.5.0

A workaround would be necessary asap :)

koflerdavidgrawe avatar Jul 17 '19 07:07 koflerdavidgrawe

+1 IntelliJ Ultime 2019.1.3 Save Action 1.5.0

A workaround would be necessary asap :)

As or a workaround you can run the inspection by name "Redundant suppression"

Delfic avatar May 24 '20 00:05 Delfic

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.

Delfic avatar May 24 '20 01:05 Delfic

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?

Delfic avatar May 24 '20 15:05 Delfic

Move to https://github.com/dubreuia/intellij-plugin-save-actions/projects/2 (ideas)

dubreuia avatar Sep 11 '20 21:09 dubreuia

I can confirm this issue. See attached if this helps:

2020-11-26_15-08-19.zip My Application.zip

AndroidDeveloperLB avatar Nov 26 '20 13:11 AndroidDeveloperLB