Skip to content

Issue #13328: Kill mutation for StringLiteralEqualityCheck#13277

Merged
romani merged 1 commit intocheckstyle:masterfrom
Kevin222004:String
Jul 8, 2023
Merged

Issue #13328: Kill mutation for StringLiteralEqualityCheck#13277
romani merged 1 commit intocheckstyle:masterfrom
Kevin222004:String

Conversation

@Kevin222004
Copy link
Copy Markdown
Contributor

@Kevin222004 Kevin222004 commented Jun 20, 2023

Issue #13328: Kill mutation for StringLiteralEqualityCheck


Check :-
https://checkstyle.org/config_coding.html#StringLiteralEquality


Mutation :-

<mutation unstable="false">
<sourceFile>StringLiteralEqualityCheck.java</sourceFile>
<mutatedClass>com.puppycrawl.tools.checkstyle.checks.coding.StringLiteralEqualityCheck</mutatedClass>
<mutatedMethod>areStringsConcatenated</mutatedMethod>
<mutator>org.pitest.mutationtest.engine.gregor.mutators.experimental.NakedReceiverMutator</mutator>
<description>replaced call to com/puppycrawl/tools/checkstyle/api/DetailAST::findFirstToken with receiver</description>
<lineContent>DetailAST plusAst = ast.findFirstToken(TokenTypes.PLUS);</lineContent>
</mutation>


Explaination

Reomval of this will not make any issue

the ast

private static boolean areStringsConcatenated(DetailAST ast) {

is always either == or !=

so while entering in the while loop ast is never going to be null so it will alwyas execute.
know lets suppose we have code like

if (s == "a") {}

so the ast look like

    |       |--LITERAL_IF -> if [14:8]
    |       |   |--LPAREN -> ( [14:11]
    |       |   |--EXPR -> EXPR [14:14]
    |       |   |   `--EQUAL -> == [14:14]
    |       |   |       |--IDENT -> s [14:12]
    |       |   |       `--STRING_LITERAL -> "a" [14:17]
    |       |   |--RPAREN -> ) [14:20]
    |       |   `--SLIST -> { [14:22]
    |       |       `--RCURLY -> } [15:8]

our ast would be EQUAL -> == [14:14] and when it enetr the while loop the if will not execute

if (plusAst.findFirstToken(TokenTypes.STRING_LITERAL) == null

and result become true which is already set true by

so output will not effect and in the case if

if (s == "a" + "bc") {}
the ast

    |       |   |--EXPR -> EXPR [14:14]
    |       |   |   `--EQUAL -> == [14:14]
    |       |   |       |--IDENT -> s [14:12]
    |       |   |       `--PLUS -> + [14:21]
    |       |   |           |--STRING_LITERAL -> "a" [14:17]
    |       |   |           `--STRING_LITERAL -> "bc" [14:23]
    |       |   |--RPAREN -> ) [14:27]

here plus always become parent if it is used between string literal so if will execute

if (plusAst.findFirstToken(TokenTypes.STRING_LITERAL) == null
&& plusAst.findFirstToken(TokenTypes.TEXT_BLOCK_LITERAL_BEGIN) == null) {
plusAst = plusAst.findFirstToken(TokenTypes.PLUS);
}

and it will change ast to TokenTypes.Plus

Know as above explaintaion

if (s == "a") {}

this is also going to be the same as

ast.findFirstToken(TokenTypes.STRING_LITERAL) != null
|| ast.findFirstToken(TokenTypes.TEXT_BLOCK_LITERAL_BEGIN) != null

so removal of this both make not issue.


Regression :-

Report 1 :- https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/04dd886_2023175346/reports/diff/index.html

Report 2:- https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/04dd886_2023063730/reports/diff/index.html


Diff Regression config: https://gist.githubusercontent.com/Kevin222004/694cadb00ab976e27fd978d7bc8841a1/raw/8f4e4d48b33f7ccefab4cca495181c0236846380/StringLiteralEquality.xml
Diff Regression projects: https://gist.githubusercontent.com/Kevin222004/9600f179b602d4c971bdb0a050099005/raw/360a95ed7bb60d7a0956e531199d484c4d6f6617/test-projects.properties
Report label: R2

@Kevin222004
Copy link
Copy Markdown
Contributor Author

Github, generate report

@github-actions
Copy link
Copy Markdown
Contributor

@Kevin222004
Copy link
Copy Markdown
Contributor Author

Github, generate report

@github-actions
Copy link
Copy Markdown
Contributor

@Kevin222004 Kevin222004 marked this pull request as ready for review June 24, 2023 16:11
@Kevin222004
Copy link
Copy Markdown
Contributor Author

Github, generate report

@github-actions
Copy link
Copy Markdown
Contributor

@romani
Copy link
Copy Markdown
Member

romani commented Jun 25, 2023

Pitest CI is not happy

@Kevin222004 Kevin222004 force-pushed the String branch 3 times, most recently from 6c37a9d to b0db8b7 Compare June 26, 2023 07:47
@Kevin222004
Copy link
Copy Markdown
Contributor Author

@romani I have make a slice change and added test case which are not to killed any mutation but to ensure that it only log violation to String literal if you think that it is not require tell me i will remove it

@romani
Copy link
Copy Markdown
Member

romani commented Jun 26, 2023

Extra test is ok, keep it.
I just point you that CI is not happy.

@Kevin222004
Copy link
Copy Markdown
Contributor Author

@romani https://app.circleci.com/pipelines/github/checkstyle/checkstyle/19239/workflows/39d08198-14cf-42d2-ae2c-3659d268d821/jobs/321748 link check is not related to this pr should I cover that in this or create a minor one

@romani
Copy link
Copy Markdown
Member

romani commented Jun 26, 2023

Please rebase, we released new layout of website, this is reason of failure

@romani
Copy link
Copy Markdown
Member

romani commented Jun 27, 2023

Rebased

@Kevin222004
Copy link
Copy Markdown
Contributor Author

Thnaks I was just going to do :)

@Kevin222004
Copy link
Copy Markdown
Contributor Author

Copy link
Copy Markdown
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.

Last item.

Items

@Kevin222004
Copy link
Copy Markdown
Contributor Author

@Kevin222004 Kevin222004 force-pushed the String branch 2 times, most recently from ac72ee3 to 73d5b6b Compare June 27, 2023 03:38
Copy link
Copy Markdown
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

@Kevin222004 Kevin222004 changed the title Issue #13109: Kill mutation for StringLiteralEqualityCheck Issue #13328: Kill mutation for StringLiteralEqualityCheck Jul 1, 2023
@romani
Copy link
Copy Markdown
Member

romani commented Jul 1, 2023

@Kevin222004 , you need to fix commit message to reference same issue as in description.
Or use old issue in description.

Copy link
Copy Markdown
Member

@rdiachenko rdiachenko left a comment

Choose a reason for hiding this comment

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

lgtm

@rdiachenko rdiachenko assigned Vyom-Yadav and unassigned rdiachenko Jul 3, 2023
Copy link
Copy Markdown
Member

@Vyom-Yadav Vyom-Yadav left a comment

Choose a reason for hiding this comment

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

LGTM!

@Vyom-Yadav Vyom-Yadav assigned romani and unassigned Vyom-Yadav Jul 8, 2023
@romani romani merged commit 55bcab7 into checkstyle:master Jul 8, 2023
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.

4 participants