Add reporting of unneeded inline suppressions

[focusshifter]

Rubocop's Lint/UnneededDisable is performing a very useful task - it runs the disabled code smell checks nevertheless and reports if they are no longer needed.

It would be nice of reek to do the same - report if some inline suppressions are no longer needed and could be removed.

[troessner]

It would be nice of reek to do the same - report if some inline suppressions are no longer needed and could be removed.

Yes, yes and double yes :) Couldn't agree more, this is definitely a feature Reek should have.

I'm blocked for the next weeks but if nobody looks into this feature in the meantime (looking at you @mvz or @waldyr) I'll definitely get this implemented.

[waldyr]

@troessner @mvz I look forward to implement this feature.

How can it be achieved and how would you do it?

[mvz]

How can it be achieved and how would you do it?

I think the idea would be to run the detectors both with and without the suppressions. The question is where do you put this duplication?

If you put it at the top, Reek would do a lot of duplicate work, since most detectors will have no suppressions for most of the code.

Therefore, it makes more sense to do the duplication at the bottom, i.e., when running one detector on one context: For a given detector and context,

• First run the detector on the context as usual.
• Then, if there is no smell,
• And there is context-dependent configuration,
• Run the detector again.
• If there is still no smell, flag the context-dependent configuration as unneeded.

I would also take a look at how RuboCop handles this.

[troessner]

Sounds like a plan @mvz !

[waldyr]

Thanks @mvz :)

[waldyr]

They have a cop to do it UnneededDisable Cop.

Based on how they do it, Is it possible with today's Reek?

• Read the reek's magic comments
• Discover the Detector being modified
• Call the detector in question without the comments' configuration
• If there is no smell, flag it as unneeded.

[mvz]

Well, like RuboCop this would be a slightly different detector. It needs to know about other detectors for example.

I think RuboCop's method works best if we only look at disabling, since then you only have to run the detector enabled. If we also want to look at other configuration (max_something, etc.), then we have to run the detector both with and without the inline config, which means extra work since we already ran the detector with the config when we were detecting its smell.

[mvz]

So, one question that needs answering here is: Do we check:

• Only suppression?
• Other configuration?

[troessner]

I think I'm going to tackle this next ;)