Issue #13999: Resolve pitest suppression for getTagId() method of TagParser

[suniti0804]

Issue #13999

Dependency Tree

TagParser.parseTags(String[], int)  (com.puppycrawl.tools.checkstyle.checks.javadoc)
    TagParser.TagParser(String[], int)  (com.puppycrawl.tools.checkstyle.checks.javadoc)
        JavadocStyleCheck.checkHtmlTags(DetailAST, TextBlock)  (com.puppycrawl.tools.checkstyle.checks.javadoc)
            JavadocStyleCheck.checkComment(DetailAST, TextBlock)  (com.puppycrawl.tools.checkstyle.checks.javadoc)
                JavadocStyleCheck.visitToken(DetailAST)  (com.puppycrawl.tools.checkstyle.checks.javadoc)
                    TreeWalker.notifyVisit(DetailAST, AstState)  (com.puppycrawl.tools.checkstyle)
                    TestUtil.isStatefulFieldClearedDuringBeginTree(AbstractCheck, DetailAST, String, Predicate<Object>)  (com.puppycrawl.tools.checkstyle.internal.utils)

Module

1. JavadocStyleCheck

Diff Regression config: https://gist.githubusercontent.com/suniti0804/466108cc17f588f2311aa2d4f40b60bb/raw/f1a067202e11a4639696c0779f36574e4a81000d/pull-14033-regerssion-config

Diff Regression projects: https://gist.githubusercontent.com/suniti0804/1d7bd7fbc50bf6c93896fbe2bbbf2c4a/raw/f71611ef97d0737b6d8bb1c1253cab18b9342429/projects-to-test-on-for-github-action.properties

Report label: Issue#14214-Report

[suniti0804]

Github, generate report

[github-actions[bot]]

Issue#14214-Report: https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/0452b7e_2024134856/reports/diff/index.html

[suniti0804]

There is no verbiage on why this change is ok and it is impossible to create a test to kill it.

@rnveach doesn't look like we found a test in diff report, or any regressions

[rnveach]

That is not an explanation of the code and why this change is ok. It just shows that we couldn't find anything in our pre-sets that this change had an effect. We need some type of analysis on why we should accept such a change and try to avoid damage to our users.

[suniti0804]

@rnveach Explanation for why I believe this change is okay

To validate my findings, I created a new test case testGetTagIdWithSpaces() where javadocText contains spaces after the column index. The test passed both before and after the removal of the trim() method.

Test Method

    @Test
    public void testGetTagIdWithSpaces() {
        String[] javadocTextWithSpaces = {"@tag   "}; // Add leading or trailing spaces after the tag
        TagParser.Point tagStart = new TagParser.Point(0, 0); // Adjust this based on your code

        // Act
        String result = TagParser.getTagId(javadocTextWithSpaces, tagStart);

        // Assert
        assertEquals("tag", result); // The expected result is "tag" without the spaces
    }

Case 1: Existing code: text = text.substring(column).trim(); Test Result: Passed

Case 2: Proposed code: text = text.substring(column); Test Result: Passed

Based on this analysis, I conclude that the removal of trim() doesn't affect the functionality of the code, and the mutation is safe.

[rnveach]

This doesn't tell me why we can remove the code and not have an issue. Adding a test case with no insight into the changed code is just another form of regression (black box testing). It is like you have no understanding of what you did and how it impacted the rest of the code, but you just know it didn't produce a difference.

I am looking for a white box answer. For examples, are we doing a duplicate trim in separate code location and it doesn't make sense to trim the string twice? Do we do a scan on the string (like indexof) and thus it doesn't matter what comes before or after? Does this string never have spaces, making a trim pointless? Etc...

[romani]

@suniti0804, please look at https://github.com/checkstyle/checkstyle/pull/14253 as example

[nrmancuso]

We have lost contact with author, anyone is welcome to reuse changes here to close the issue.

[romani]

this fix was done in some other PR, nothing here to merge.