Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #13809: Kill mutation in SuppressionCommentFilter #13833

Merged
merged 1 commit into from
Nov 8, 2023

Conversation

Kevin222004
Copy link
Contributor

@Kevin222004 Kevin222004 commented Oct 5, 2023

Issue #13809: Kill mutation in SuppressionCommentFilter


Filters

https://checkstyle.sourceforge.io/filters/suppressioncommentfilter.html#SuppressionCommentFilter


Mutation

<mutation unstable="false">
<sourceFile>SuppressionCommentFilter.java</sourceFile>
<mutatedClass>com.puppycrawl.tools.checkstyle.filters.SuppressionCommentFilter</mutatedClass>
<mutatedMethod>accept</mutatedMethod>
<mutator>org.pitest.mutationtest.engine.gregor.mutators.NonVoidMethodCallMutator</mutator>
<description>removed call to com/puppycrawl/tools/checkstyle/filters/SuppressionCommentFilter::getFileContents</description>
<lineContent>if (getFileContents() != currentContents) {</lineContent>
</mutation>


Explaination

test added.
Modified test prooves that exception is happening:

✔ ~/java/github/romani/checkstyle [Kevin222004/SuppressionCommentFilter L|✚ 2…1⚑ 1] 
 $ git diff
diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/filters/SuppressionCommentFilter.java b/src/main/java/com/puppycrawl/tools/checkstyle/filters/SuppressionCommentFilter.java
index 5a82d0c..0a99106 100644
--- a/src/main/java/com/puppycrawl/tools/checkstyle/filters/SuppressionCommentFilter.java
+++ b/src/main/java/com/puppycrawl/tools/checkstyle/filters/SuppressionCommentFilter.java
@@ -285,7 +285,7 @@ public class SuppressionCommentFilter
             // contents and tag suppressions
             final FileContents currentContents = event.getFileContents();
 
-            if (getFileContents() != currentContents) {
+            if (true) {
                 setFileContents(currentContents);
                 tagSuppressions();
             }
diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/filters/SuppressionCommentFilterTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/filters/SuppressionCommentFilterTest.java
index 19ae666..65fcdaa 100644
--- a/src/test/java/com/puppycrawl/tools/checkstyle/filters/SuppressionCommentFilterTest.java
+++ b/src/test/java/com/puppycrawl/tools/checkstyle/filters/SuppressionCommentFilterTest.java
@@ -21,24 +21,33 @@ package com.puppycrawl.tools.checkstyle.filters;
 
 import static com.google.common.truth.Truth.assertWithMessage;
 import static com.puppycrawl.tools.checkstyle.checks.naming.AbstractNameCheck.MSG_INVALID_PATTERN;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 import java.io.File;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Map;
+import java.util.regex.Pattern;
 
 import org.junit.jupiter.api.Test;
 
 import com.puppycrawl.tools.checkstyle.AbstractModuleTestSupport;
 import com.puppycrawl.tools.checkstyle.DefaultConfiguration;
+import com.puppycrawl.tools.checkstyle.JavaParser;
 import com.puppycrawl.tools.checkstyle.TreeWalker;
 import com.puppycrawl.tools.checkstyle.TreeWalkerAuditEvent;
 import com.puppycrawl.tools.checkstyle.api.CheckstyleException;
+import com.puppycrawl.tools.checkstyle.api.DetailAST;
 import com.puppycrawl.tools.checkstyle.api.FileContents;
 import com.puppycrawl.tools.checkstyle.api.FileText;
+import com.puppycrawl.tools.checkstyle.api.SeverityLevel;
+import com.puppycrawl.tools.checkstyle.api.TextBlock;
 import com.puppycrawl.tools.checkstyle.api.Violation;
 import com.puppycrawl.tools.checkstyle.checks.coding.IllegalCatchCheck;
 import com.puppycrawl.tools.checkstyle.checks.naming.AbstractNameCheck;
 import com.puppycrawl.tools.checkstyle.checks.naming.ConstantNameCheck;
+import com.puppycrawl.tools.checkstyle.checks.naming.MemberNameCheck;
 import com.puppycrawl.tools.checkstyle.internal.utils.TestUtil;
 import com.puppycrawl.tools.checkstyle.utils.CommonUtil;
 import nl.jqno.equalsverifier.EqualsVerifier;
@@ -649,4 +658,57 @@ public class SuppressionCommentFilterTest
         return TestUtil.getInternalState(filter, "tags");
     }
 
+    @Test
+    public void testCachingByFilecontentInstance() throws Exception {
+
+        File file = new File(getPath("InputSuppressionCommentFilterSuppressById6.java"));
+        DetailAST rootast = JavaParser.parseFile(file, JavaParser.Options.WITH_COMMENTS);
+        String[] lines = {"//CSOFF", "//CSON"};
+        final FileContents fileContents = new FileContents(
+                new FileText(file, Arrays.asList(lines)));
+        for (int lineNo = 0; lineNo < lines.length; lineNo++) {
+            final int colNo = lines[lineNo].indexOf("//");
+            if (colNo >= 0) {
+                fileContents.reportSingleLineComment(lineNo + 1, colNo);}
+        }
+
+
+        final FileContents mockedContents = mock(FileContents.class);
+        Map<Integer, TextBlock> returnValue =
+                fileContents.getSingleLineComments();
+        when(mockedContents.getSingleLineComments())
+                .thenReturn(returnValue)
+                .thenThrow(new IllegalStateException("Second call is not allowed"));
+
+
+        String[] args = {"line_length", "^[a-z][a-zA-Z0-9]*$"};
+        Violation violation = new Violation(27, 9, 58,
+                "com.puppycrawl.tools.checkstyle.checks.naming.messages",
+                "name.invalidPattern",
+                args,
+                SeverityLevel.ERROR,
+                "ignore",
+                MemberNameCheck.class,
+                null);
+
+        TreeWalkerAuditEvent event = new TreeWalkerAuditEvent(
+                mockedContents, file.getAbsolutePath(), violation, rootast);
+
+        SuppressionCommentFilter ss = new SuppressionCommentFilter();
+        ss.setOffCommentFormat(Pattern.compile("CSOFF"));
+        ss.setOnCommentFormat(Pattern.compile("CSON"));
+        ss.setCheckFormat("MemberNameCheck");
+        ss.finishLocalSetup();
+
+        assertWithMessage("").that(ss.accept(event)).isTrue();
+        try {
+            ss.accept(event);
+            assertWithMessage("second execution expecting exception").fail();
+        } catch (IllegalStateException expected) {
+           assertWithMessage("exception is epected")
+                   .that(expected.getMessage())
+                   .isEqualTo("Second call is not allowed");
+        }
+    }
+
 }

@Vyom-Yadav Vyom-Yadav self-assigned this Oct 9, 2023
Copy link
Member

@romani romani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

direction is good.

items:

@romani romani assigned romani and unassigned Vyom-Yadav Oct 22, 2023
@romani
Copy link
Member

romani commented Oct 26, 2023

@Kevin222004 , let us know if you need more help to handle this PR .

@romani romani force-pushed the SuppressionCommentFilter branch 3 times, most recently from 0cdb756 to ec54435 Compare November 7, 2023 14:00
Copy link
Member

@romani romani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok to merge

@romani romani assigned rnveach and unassigned romani Nov 7, 2023
@rnveach rnveach assigned romani and unassigned rnveach Nov 8, 2023
@romani romani merged commit bbeef6d into checkstyle:master Nov 8, 2023
113 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants