SuppressWarningsHolder should not report violations

[romani]

Check documentation: https://checkstyle.org/config_annotation.html#SuppressWarningsHolder

$ cat config.xml 
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
          "-//Puppy Crawl//DTD Check Configuration 1.3//EN"
          "https://checkstyle.org/dtds/configuration_1_3.dtd">
<module name="Checker">
  <module name="TreeWalker">
    <module name="SuppressWarningsHolder"/>
  </module>
</module>

$ cat SuppressionXpathRegressionSuppressWarningsHolderOne.java 
package org.checkstyle.suppressionxpathfilter.suppresswarningsholder;

public class SuppressionXpathRegressionSuppressWarningsHolderOne {
    public static class MyResource implements AutoCloseable {
        @Override
        public void close() throws Exception { }
    }

    public static void main(String[] args) throws Exception {
        try (@SuppressWarnings("all") final MyResource r = new MyResource()) { } // warn
        try (@MyAnnotation("all") final MyResource resource = new MyResource()) { }
    }
}
@interface MyAnnotation {
    String[] value();
}

$ java -jar /var/tmp/checkstyle-8.29-all.jar -c config.xml \
     SuppressionXpathRegressionSuppressWarningsHolderOne.java 
Starting audit...
[ERROR] /var/tmp/SuppressionXpathRegressionSuppressWarningsHolderOne.java:10: 
Invalid target for @SuppressWarnings. [SuppressWarningsHolder]
Audit done.
Checkstyle ends with 1 errors.

---

SuppressWarningsHolder should support annotations in all possible places, fact that we probably can not distinguish what violation to suppress is bug of our implementation and should have github issue BUT not a violation in code.

issue is detected at #7841 (comment)
problem is already covered by UT - https://github.com/checkstyle/checkstyle/blob/master/src/it/resources/org/checkstyle/suppressionxpathfilter/suppresswarningsholder/SuppressionXpathRegressionSuppressWarningsHolderOne.java

[rnveach]

http://rveach.no-ip.org/checkstyle/regression/reports/290/
Regression shows the only violations produced by this check are in checkstyle.

src/it/resources/org/checkstyle/suppressionxpathfilter/suppresswarningsholder/SuppressionXpathRegressionSuppressWarningsHolderOne.java
src/test/resources/com/puppycrawl/tools/checkstyle/checks/suppresswarningsholder/InputSuppressWarningsHolder2.java
src/test/resources/com/puppycrawl/tools/checkstyle/grammar/InputJava7TryWithResources.java

[romani]

yes, and we have such tests only to have some violation.
Please approve removal of this validation, we should support annotations in all places.

[AmrDeveloper]

I'm on it

[AmrDeveloper]

@romani
I solved this issue. Then, the problem is what i should do with XpathRegressionSuppressWarningsHolderTest from this PR #7841 ?
This test expects violation but now the code will report no voilation so the test will fail

[rnveach]

The xpath test will have to be removed since it won't be possible to create violations. I think we can leave it off documentation too.

[AmrDeveloper]

@rnveach yup but after remove the xpath test and leave it off doc i will got error that i must add it

[rnveach]

You will need to change UT to not complain about this module.

[AmrDeveloper]

@rnveach I am already changed SuppressWarningsHolderTest to expect no violations in testAnnotationInTry method

[AmrDeveloper]

@rnveach I fixed the problem but I can't find UT that can cover switch default case I think we covered all possible case for parent type so any suggestion what should I do because I got this error

Rule violated for class com.puppycrawl.tools.checkstyle.checks.SuppressWarningsHolder: lines covered ratio is 0.98, but expected minimum is 1.00

[rnveach]

If you are ready, start a PR and we will discuss there.

[rnveach]

Fix was merged