Issue checkstyle#11460: Fixed the default behavior of SuppressoinXpat…

…hSingleFilter
cs304-2022 · May 25, 2022 · 3e596b6 · 3e596b6
1 parent 8b03a0a
commit 3e596b6
Show file tree

Hide file tree

Showing 8 changed files with 199 additions and 41 deletions.
diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/filters/XpathFilterElement.java b/src/main/java/com/puppycrawl/tools/checkstyle/filters/XpathFilterElement.java
@@ -21,6 +21,7 @@
 
 import java.util.List;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
@@ -70,6 +71,9 @@ public class XpathFilterElement implements TreeWalkerFilter {
     /** Xpath query. */
     private final String xpathQuery;
 
+    /** Indicates if all properties are set to null. */
+    private final boolean isEmptyConfig;
+
     /**
      * Creates a {@code XpathElement} instance.
      *
@@ -82,42 +86,11 @@ public class XpathFilterElement implements TreeWalkerFilter {
      */
     public XpathFilterElement(String files, String checks,
                        String message, String moduleId, String query) {
-        filePattern = files;
-        if (files == null) {
-            fileRegexp = null;
-        }
-        else {
-            fileRegexp = Pattern.compile(files);
-        }
-        checkPattern = checks;
-        if (checks == null) {
-            checkRegexp = null;
-        }
-        else {
-            checkRegexp = CommonUtil.createPattern(checks);
-        }
-        messagePattern = message;
-        if (message == null) {
-            messageRegexp = null;
-        }
-        else {
-            messageRegexp = Pattern.compile(message);
-        }
-        this.moduleId = moduleId;
-        xpathQuery = query;
-        if (xpathQuery == null) {
-            xpathExpression = null;
-        }
-        else {
-            final XPathEvaluator xpathEvaluator = new XPathEvaluator(
-                    Configuration.newConfiguration());
-            try {
-                xpathExpression = xpathEvaluator.createExpression(xpathQuery);
-            }
-            catch (XPathException ex) {
-                throw new IllegalArgumentException("Unexpected xpath query: " + xpathQuery, ex);
-            }
-        }
+        this(Optional.ofNullable(files).map(Pattern::compile).orElse(null),
+             Optional.ofNullable(checks).map(CommonUtil::createPattern).orElse(null),
+             Optional.ofNullable(message).map(Pattern::compile).orElse(null),
+             moduleId,
+             query);
     }
 
     /**
@@ -171,11 +144,17 @@ public XpathFilterElement(Pattern files, Pattern checks, Pattern message,
                 throw new IllegalArgumentException("Incorrect xpath query: " + xpathQuery, ex);
             }
         }
+        isEmptyConfig = fileRegexp == null
+                             && checkRegexp == null
+                             && messageRegexp == null
+                             && moduleId == null
+                             && xpathExpression == null;
     }
 
     @Override
     public boolean accept(TreeWalkerAuditEvent event) {
-        return !isFileNameAndModuleAndModuleNameMatching(event)
+        return isEmptyConfig
+                || !isFileNameAndModuleAndModuleNameMatching(event)
                 || !isMessageNameMatching(event)
                 || !isXpathQueryMatching(event);
     }
@@ -261,7 +240,8 @@ private List<Item> getItems(TreeWalkerAuditEvent event) {
 
     @Override
     public int hashCode() {
-        return Objects.hash(filePattern, checkPattern, messagePattern, moduleId, xpathQuery);
+        return Objects.hash(filePattern, checkPattern, messagePattern,
+            moduleId, xpathQuery, isEmptyConfig);
     }
 
     @Override
@@ -277,7 +257,8 @@ public boolean equals(Object other) {
                 && Objects.equals(checkPattern, xpathFilter.checkPattern)
                 && Objects.equals(messagePattern, xpathFilter.messagePattern)
                 && Objects.equals(moduleId, xpathFilter.moduleId)
-                && Objects.equals(xpathQuery, xpathFilter.xpathQuery);
+                && Objects.equals(xpathQuery, xpathFilter.xpathQuery)
+                && Objects.equals(isEmptyConfig, xpathFilter.isEmptyConfig);
     }
 
 }
diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/filters/SuppressionXpathSingleFilterTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/filters/SuppressionXpathSingleFilterTest.java
@@ -327,6 +327,79 @@ public void testThrowException() throws Exception {
         }
     }
 
+    @Test
+    public void testAllNullConfiguration() throws Exception {
+        final String[] expected = {
+            "18:1: " + getCheckMessage(MissingJavadocTypeCheck.class, MSG_JAVADOC_MISSING),
+        };
+
+        final String[] suppressed = CommonUtil.EMPTY_STRING_ARRAY;
+
+        verifyFilterWithInlineConfigParser(
+                getPath("InputSuppressionXpathSingleFilterAllNullConfiguration.java"),
+                expected, removeSuppressed(expected, suppressed));
+    }
+
+    @Test
+    public void testDecideByIdAndExpression() throws Exception {
+        final String[] expected = {
+            "20:1: " + getCheckMessage(MissingJavadocTypeCheck.class, MSG_JAVADOC_MISSING),
+        };
+
+        final String[] suppressed = {
+            "20:1: " + getCheckMessage(MissingJavadocTypeCheck.class, MSG_JAVADOC_MISSING),
+        };
+
+        verifyFilterWithInlineConfigParser(
+                getPath("InputSuppressionXpathSingleFilterDecideByIdAndExpression.java"),
+                expected, removeSuppressed(expected, suppressed));
+    }
+
+    @Test
+    public void testDefaultFileProperty() throws Exception {
+        final String[] expected = {
+            "20:1: " + getCheckMessage(MissingJavadocTypeCheck.class, MSG_JAVADOC_MISSING),
+        };
+
+        final String[] suppressed = {
+            "20:1: " + getCheckMessage(MissingJavadocTypeCheck.class, MSG_JAVADOC_MISSING),
+        };
+
+        verifyFilterWithInlineConfigParser(
+                getPath("InputSuppressionXpathSingleFilterDefaultFileProperty.java"),
+                expected, removeSuppressed(expected, suppressed));
+    }
+
+    @Test
+    public void testDecideByCheck() throws Exception {
+        final String[] expected = {
+            "18:1: " + getCheckMessage(MissingJavadocTypeCheck.class, MSG_JAVADOC_MISSING),
+        };
+
+        final String[] suppressed = {
+            "18:1: " + getCheckMessage(MissingJavadocTypeCheck.class, MSG_JAVADOC_MISSING),
+        };
+
+        verifyFilterWithInlineConfigParser(
+                getPath("InputSuppressionXpathSingleFilterDecideByCheck.java"),
+                expected, removeSuppressed(expected, suppressed));
+    }
+
+    @Test
+    public void testDecideById() throws Exception {
+        final String[] expected = {
+            "19:1: " + getCheckMessage(MissingJavadocTypeCheck.class, MSG_JAVADOC_MISSING),
+        };
+
+        final String[] suppressed = {
+            "19:1: " + getCheckMessage(MissingJavadocTypeCheck.class, MSG_JAVADOC_MISSING),
+        };
+
+        verifyFilterWithInlineConfigParser(
+                getPath("InputSuppressionXpathSingleFilterDecideById.java"),
+                expected, removeSuppressed(expected, suppressed));
+    }
+
     private static SuppressionXpathSingleFilter createSuppressionXpathSingleFilter(
             String files, String checks, String message, String moduleId, String query) {
         final SuppressionXpathSingleFilter filter = new SuppressionXpathSingleFilter();

diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/filters/XpathFilterElementTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/filters/XpathFilterElementTest.java
@@ -153,8 +153,8 @@ public void testIncorrectQuery() {
             assertWithMessage("Exception is expected but got " + test).fail();
         }
         catch (IllegalArgumentException ex) {
-            assertWithMessage("Message should be: Unexpected xpath query")
-                    .that(ex.getMessage().contains("Unexpected xpath query"))
+            assertWithMessage("Message should be: Incorrect xpath query")
+                    .that(ex.getMessage().contains("Incorrect xpath query"))
                     .isTrue();
         }
     }

diff --git a/...s/suppressionxpathsinglefilter/InputSuppressionXpathSingleFilterAllNullConfiguration.java b/...s/suppressionxpathsinglefilter/InputSuppressionXpathSingleFilterAllNullConfiguration.java
@@ -0,0 +1,19 @@
+/*
+SuppressionXpathSingleFilter
+files = (default)(null)
+checks = (default)(null)
+message = (default)(null)
+id = (default)(null)
+query = (default)(null)
+
+com.puppycrawl.tools.checkstyle.checks.javadoc.MissingJavadocTypeCheck
+scope = (default)public
+excludeScope = (default)(null)
+skipAnnotations = (default)Generated
+tokens = CLASS_DEF
+
+*/
+package com.puppycrawl.tools.checkstyle.filters.suppressionxpathsinglefilter;
+
+public class InputSuppressionXpathSingleFilterAllNullConfiguration { // violation
+}
diff --git a/.../filters/suppressionxpathsinglefilter/InputSuppressionXpathSingleFilterDecideByCheck.java b/.../filters/suppressionxpathsinglefilter/InputSuppressionXpathSingleFilterDecideByCheck.java
@@ -0,0 +1,20 @@
+/*
+SuppressionXpathSingleFilter
+files = (default)(null)
+checks = MissingJavadocTypeCheck
+message = (default)(null)
+id = (default)(null)
+query = (default)(null)
+
+com.puppycrawl.tools.checkstyle.checks.javadoc.MissingJavadocTypeCheck
+scope = (default)public
+excludeScope = (default)(null)
+skipAnnotations = (default)Generated
+tokens = CLASS_DEF
+
+*/
+package com.puppycrawl.tools.checkstyle.filters.suppressionxpathsinglefilter;
+
+public class InputSuppressionXpathSingleFilterDecideByCheck { // filtered violation
+
+}
diff --git a/...yle/filters/suppressionxpathsinglefilter/InputSuppressionXpathSingleFilterDecideById.java b/...yle/filters/suppressionxpathsinglefilter/InputSuppressionXpathSingleFilterDecideById.java
@@ -0,0 +1,21 @@
+/*
+SuppressionXpathSingleFilter
+files = (default)(null)
+checks = (default)(null)
+message = (default)(null)
+id = 007
+query = (default)(null)
+
+com.puppycrawl.tools.checkstyle.checks.javadoc.MissingJavadocTypeCheck
+id = 007
+scope = (default)public
+excludeScope = (default)(null)
+skipAnnotations = (default)Generated
+tokens = CLASS_DEF
+
+*/
+package com.puppycrawl.tools.checkstyle.filters.suppressionxpathsinglefilter;
+
+public class InputSuppressionXpathSingleFilterDecideById { // filtered violation
+
+}
diff --git a/...uppressionxpathsinglefilter/InputSuppressionXpathSingleFilterDecideByIdAndExpression.java b/...uppressionxpathsinglefilter/InputSuppressionXpathSingleFilterDecideByIdAndExpression.java
@@ -0,0 +1,22 @@
+/*
+SuppressionXpathSingleFilter
+files = (default)(null)
+checks = (default)(null)
+message = (default)(null)
+id = 007
+query = /COMPILATION_UNIT/CLASS_DEF \
+        [./IDENT[@text='InputSuppressionXpathSingleFilterDecideByIdAndExpression']]
+
+com.puppycrawl.tools.checkstyle.checks.javadoc.MissingJavadocTypeCheck
+id = 007
+scope = (default)public
+excludeScope = (default)(null)
+skipAnnotations = (default)Generated
+tokens = CLASS_DEF
+
+*/
+package com.puppycrawl.tools.checkstyle.filters.suppressionxpathsinglefilter;
+
+public class InputSuppressionXpathSingleFilterDecideByIdAndExpression { // filtered violation
+
+}
diff --git a/...rs/suppressionxpathsinglefilter/InputSuppressionXpathSingleFilterDefaultFileProperty.java b/...rs/suppressionxpathsinglefilter/InputSuppressionXpathSingleFilterDefaultFileProperty.java
@@ -0,0 +1,22 @@
+/*
+SuppressionXpathSingleFilter
+files = (default)(null)
+checks = MissingJavadocTypeCheck
+message = Missing a Javadoc comment
+id = 007
+query = /COMPILATION_UNIT/CLASS_DEF \
+        [./IDENT[@text='InputSuppressionXpathSingleFilterDefaultFileProperty']]
+
+com.puppycrawl.tools.checkstyle.checks.javadoc.MissingJavadocTypeCheck
+id = 007
+scope = (default)public
+excludeScope = (default)(null)
+skipAnnotations = (default)Generated
+tokens = CLASS_DEF
+
+*/
+package com.puppycrawl.tools.checkstyle.filters.suppressionxpathsinglefilter;
+
+public class InputSuppressionXpathSingleFilterDefaultFileProperty { // filtered violation
+
+}