Issue #13501: Kill mutation for AnnotationUtil by Kevin222004 · Pull Request #13415 · checkstyle/checkstyle

Kevin222004 · 2023-07-21T19:52:12Z

Issue #13501: Kill mutation for AnnotationUtil

Mutations

checkstyle/config/pitest-suppressions/pitest-utils-suppressions.xml

Lines 3 to 10 in 8804126

    
           <mutation unstable="false"> 
        
             <sourceFile>AnnotationUtil.java</sourceFile> 
        
             <mutatedClass>com.puppycrawl.tools.checkstyle.utils.AnnotationUtil</mutatedClass> 
        
             <mutatedMethod>containsAnnotation</mutatedMethod> 
        
             <mutator>org.pitest.mutationtest.engine.gregor.mutators.RemoveConditionalMutator_EQUAL_ELSE</mutator> 
        
             <description>removed conditional - replaced equality check with false</description> 
        
             <lineContent>if (ast == null) {</lineContent> 
        
           </mutation>

Explaination

Method 1

checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/utils/AnnotationUtil.java

Lines 90 to 96 in 8804126

    
           public static boolean containsAnnotation(final DetailAST ast) { 
        
               if (ast == null) { 
        
                   throw new IllegalArgumentException(THE_AST_IS_NULL); 
        
               } 
        
               final DetailAST holder = getAnnotationHolder(ast); 
        
               return holder != null && holder.findFirstToken(TokenTypes.ANNOTATION) != null; 
        
           }

This method

checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/utils/AnnotationUtil.java

Line 91 in 8804126

if (ast == null) {

is mutated and the usage of this method is only in the check PackageAnnotationCheck

checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/checks/annotation/PackageAnnotationCheck.java

Lines 107 to 109 in 8804126

    
           public void visitToken(final DetailAST ast) { 
        
               final boolean containsAnnotation = 
        
                   AnnotationUtil.containsAnnotation(ast);

at line no 109 as this is visit token and no other such operation is perfoming on ast it will only be the acceptaed token which is PACKAGE_DEF so

checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/utils/AnnotationUtil.java

Lines 91 to 93 in 8804126

    
           if (ast == null) { 
        
               throw new IllegalArgumentException(THE_AST_IS_NULL); 
        
           }

this removal will not make any affect

Second reason
may be stronger then above one
lets suppose above we have not condition ast == null and by chance ast is null then while calling

checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/utils/AnnotationUtil.java

Line 94 in 8804126

final DetailAST holder = getAnnotationHolder(ast);

getAnnotationHolder already have this condition

checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/utils/AnnotationUtil.java

Lines 178 to 181 in 8804126

    
           public static DetailAST getAnnotationHolder(DetailAST ast) { 
        
               if (ast == null) { 
        
                   throw new IllegalArgumentException(THE_AST_IS_NULL); 
        
               }

.

Method 2

checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/utils/AnnotationUtil.java

Lines 113 to 116 in 8804126

    
           public static boolean containsAnnotation(DetailAST ast, Set<String> annotations) { 
        
               if (ast == null) { 
        
                   throw new IllegalArgumentException(THE_AST_IS_NULL); 
        
               }

in which similar mutation at line 114

know this method is called by one method in same class at

checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/utils/AnnotationUtil.java

Lines 165 to 167 in 8804126

    
           public static boolean hasOverrideAnnotation(DetailAST ast) { 
        
               return containsAnnotation(ast, OVERRIDE_ANNOTATIONS); 
        
           }

and this method is used by 5 Checks

ParameterNumberCheck :- https://checkstyle.org/checks/sizes/parameternumber.html#ParameterNumber
MissingOverrideCheck :- https://checkstyle.org/checks/annotation/missingoverride.html#MissingOverride
MethodNameCheck :- https://checkstyle.org/checks/naming/methodname.html#MethodName
IllegalTypeCheck :- https://checkstyle.org/checks/coding/illegaltype.html
IllegalThrowsCheck :- https://checkstyle.org/checks/coding/illegalthrows.html

1) ParameterNumberCheck :-
usage of method is at
https://github.com/checkstyle/checkstyle/blob/
8804126/src/main/java/com/puppycrawl/tools/checkstyle/checks/sizes/ParameterNumberCheck.java#L177-L181
line 180 know this method in ast is called only at visit token at line 164

checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/checks/sizes/ParameterNumberCheck.java

Lines 161 to 164 in 8804126

    
           public void visitToken(DetailAST ast) { 
        
               final DetailAST params = ast.findFirstToken(TokenTypes.PARAMETERS); 
        
               final int count = params.getChildCount(TokenTypes.PARAMETER_DEF); 
        
               if (count > max && !shouldIgnoreNumberOfParameters(ast)) {

over here directly ast is used which is in parameter so it is also bever going to null

2) MissingOverrideCheck :-
Similar reason as above the usage is at

checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/checks/annotation/MissingOverrideCheck.java

Lines 227 to 253 in 8804126

    
           public void visitToken(final DetailAST ast) { 
        
               final boolean containsTag = containsInheritDocTag(ast); 
        
               if (containsTag && !JavadocTagInfo.INHERIT_DOC.isValidOn(ast)) { 
        
                   log(ast, MSG_KEY_TAG_NOT_VALID_ON, 
        
                       JavadocTagInfo.INHERIT_DOC.getText()); 
        
               } 
        
               else { 
        
                   boolean check = true; 
        
                   if (javaFiveCompatibility) { 
        
                       final DetailAST defOrNew = ast.getParent().getParent(); 
        
                       if (defOrNew.findFirstToken(TokenTypes.EXTENDS_CLAUSE) != null 
        
                           || defOrNew.findFirstToken(TokenTypes.IMPLEMENTS_CLAUSE) != null 
        
                           || defOrNew.getType() == TokenTypes.LITERAL_NEW) { 
        
                           check = false; 
        
                       } 
        
                   } 
        
                   if (check 
        
                       && containsTag 
        
                       && !AnnotationUtil.hasOverrideAnnotation(ast)) { 
        
                       log(ast, MSG_KEY_ANNOTATION_MISSING_OVERRIDE); 
        
                   } 
        
               } 
        
           }

the method is used at line 248 and similar ast is direclty passed from parameter.

3) MethodNameCheck :-
Similar reason for method name check

checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/checks/naming/MethodNameCheck.java

Lines 231 to 235 in 8804126

    
           public void visitToken(DetailAST ast) { 
        
               if (!AnnotationUtil.hasOverrideAnnotation(ast)) { 
        
                   // Will check the name against the format. 
        
                   super.visitToken(ast); 
        
               }

4) IllegalThrowsCheck :-
Similar reason

checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/IllegalThrowsCheck.java

Lines 228 to 232 in 8804126

    
           private boolean isIgnorableMethod(DetailAST methodDef) { 
        
               return shouldIgnoreMethod(methodDef.findFirstToken(TokenTypes.IDENT).getText()) 
        
                   || ignoreOverriddenMethods 
        
                      && AnnotationUtil.hasOverrideAnnotation(methodDef); 
        
           }

5) IllegalTypeCheck :-
Similar reasons

checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/IllegalTypeCheck.java

Lines 807 to 812 in 8804126

    
           private boolean isCheckedMethod(DetailAST ast) { 
        
               final String methodName = 
        
                   ast.findFirstToken(TokenTypes.IDENT).getText(); 
        
               return isVerifiable(ast) && !ignoredMethodNames.contains(methodName) 
        
                       && !AnnotationUtil.hasOverrideAnnotation(ast); 
        
           }

Second reason

checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/utils/AnnotationUtil.java

Line 125 in 8804126

    
           final DetailAST firstMatchingAnnotation = findFirstAnnotation(ast, annotationNode -> {

call

checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/utils/AnnotationUtil.java

Lines 255 to 257 in 8804126

    
           private static DetailAST findFirstAnnotation(final DetailAST ast, 
        
                                                        Predicate<DetailAST> predicate) { 
        
               final DetailAST holder = getAnnotationHolder(ast);

and it is calling

checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/utils/AnnotationUtil.java

Lines 178 to 181 in 8804126

    
           public static DetailAST getAnnotationHolder(DetailAST ast) { 
        
               if (ast == null) { 
        
                   throw new IllegalArgumentException(THE_AST_IS_NULL); 
        
               }

so here ast is checked again

Kevin222004 · 2023-07-21T20:12:10Z

https://kevin222004.github.io/PitestReport/com.puppycrawl.tools.checkstyle.utils/AnnotationUtil.java.html
This contains 2 mutation may be this is issue.

Kevin222004 · 2023-07-21T20:32:49Z

@romani
https://checkstyle.semaphoreci.com/jobs/01ed458d-ea27-4b03-9c02-06e28c81f883

[ERROR] Errors: 03:03
[ERROR]   ParenPadCheckTest.testNoStackoverflowError:466->AbstractModuleTestSupport.verifyWithLimitedResources:390 » Execution java.lang.Error: Error was thrown while processing /home/semaphore/checkstyle/src/test/resources/com/puppycrawl/tools/checkstyle/checks/whitespace/parenpad/InputParenPadNoStackoverflowError.java03:03
[INFO]

Again NoStackoverFlowError

As far I don't remember we have done any changes in favour of pitest which create this type of issue

romani

I ok to remove such null validation.
I see no value in them, it is still exception, absolutely sae details user will get from NPE.

throw new IllegalArgumentException("annotations cannot be null"); also can be removed and all similar

Vyom-Yadav

LGTM!

rdiachenko

lgtm

rdiachenko · 2023-08-01T20:46:59Z

@Kevin222004 please resolve conflicts

romani · 2023-08-02T16:46:46Z

@Kevin222004 , conflict

romani · 2023-08-02T17:00:04Z

@Kevin222004 , CI is failing

Kevin222004 · 2023-08-02T17:11:12Z

Can you please re run circle ci

Vyom-Yadav · 2023-08-02T18:27:12Z

Can you please re run circle ci

@Kevin222004 Just add some change, push the commit, remove that change, push again, that will re trigger the CI with current changes. You can use this if no one is around.

romani · 2023-08-03T00:47:55Z

CIs were restarted

romani · 2023-08-04T01:29:13Z

@Kevin222004 , please resolve conflict

Kevin222004 · 2023-08-04T04:38:16Z

conflict resolved

Kevin222004 force-pushed the ut2 branch from 6d58498 to 4e8e97d Compare July 21, 2023 19:54

Kevin222004 mentioned this pull request Jul 21, 2023

Resolve pitest survivals in modules - 2 #13321

Closed

41 tasks

romani approved these changes Jul 22, 2023

View reviewed changes

romani requested review from Vyom-Yadav and rdiachenko July 22, 2023 00:40

romani assigned Vyom-Yadav Jul 22, 2023

Vyom-Yadav approved these changes Aug 1, 2023

View reviewed changes

Vyom-Yadav assigned rdiachenko and unassigned Vyom-Yadav Aug 1, 2023

rdiachenko approved these changes Aug 1, 2023

View reviewed changes

This was referenced Aug 2, 2023

storage Kevin222004/GitHub-action#3

Closed

Resolve pitest survivals in modules - 3 #13501

Closed

Kevin222004 changed the title ~~Issue #13321: Kill mutation for AnnotationUtil~~ Issue #13501: Kill mutation for AnnotationUtil Aug 2, 2023

Kevin222004 force-pushed the ut2 branch 2 times, most recently from 802a877 to c5aa526 Compare August 2, 2023 16:56

Kevin222004 force-pushed the ut2 branch from c5aa526 to 5a6dfa1 Compare August 2, 2023 18:52

Kevin222004 force-pushed the ut2 branch from 5a6dfa1 to af0aec5 Compare August 3, 2023 16:13

Issue checkstyle#13501: Kill mutation for AnnotationUtil

c698b0f

Kevin222004 force-pushed the ut2 branch from af0aec5 to c698b0f Compare August 4, 2023 04:37

romani merged commit 1cef660 into checkstyle:master Aug 4, 2023

	<mutation unstable="false">
	<sourceFile>AnnotationUtil.java</sourceFile>
	<mutatedClass>com.puppycrawl.tools.checkstyle.utils.AnnotationUtil</mutatedClass>
	<mutatedMethod>containsAnnotation</mutatedMethod>
	<mutator>org.pitest.mutationtest.engine.gregor.mutators.RemoveConditionalMutator_EQUAL_ELSE</mutator>
	<description>removed conditional - replaced equality check with false</description>
	<lineContent>if (ast == null) {</lineContent>
	</mutation>

	public static boolean containsAnnotation(final DetailAST ast) {
	if (ast == null) {
	throw new IllegalArgumentException(THE_AST_IS_NULL);
	}
	final DetailAST holder = getAnnotationHolder(ast);
	return holder != null && holder.findFirstToken(TokenTypes.ANNOTATION) != null;
	}

	public void visitToken(final DetailAST ast) {
	final boolean containsAnnotation =
	AnnotationUtil.containsAnnotation(ast);

	public static DetailAST getAnnotationHolder(DetailAST ast) {
	if (ast == null) {
	throw new IllegalArgumentException(THE_AST_IS_NULL);
	}

	public static boolean hasOverrideAnnotation(DetailAST ast) {
	return containsAnnotation(ast, OVERRIDE_ANNOTATIONS);
	}

	public void visitToken(DetailAST ast) {
	final DetailAST params = ast.findFirstToken(TokenTypes.PARAMETERS);
	final int count = params.getChildCount(TokenTypes.PARAMETER_DEF);
	if (count > max && !shouldIgnoreNumberOfParameters(ast)) {

	public void visitToken(final DetailAST ast) {
	final boolean containsTag = containsInheritDocTag(ast);
	if (containsTag && !JavadocTagInfo.INHERIT_DOC.isValidOn(ast)) {
	log(ast, MSG_KEY_TAG_NOT_VALID_ON,
	JavadocTagInfo.INHERIT_DOC.getText());
	}
	else {
	boolean check = true;

	if (javaFiveCompatibility) {
	final DetailAST defOrNew = ast.getParent().getParent();

	if (defOrNew.findFirstToken(TokenTypes.EXTENDS_CLAUSE) != null
	\|\| defOrNew.findFirstToken(TokenTypes.IMPLEMENTS_CLAUSE) != null
	\|\| defOrNew.getType() == TokenTypes.LITERAL_NEW) {
	check = false;
	}
	}

	if (check
	&& containsTag
	&& !AnnotationUtil.hasOverrideAnnotation(ast)) {
	log(ast, MSG_KEY_ANNOTATION_MISSING_OVERRIDE);
	}
	}
	}

	public void visitToken(DetailAST ast) {
	if (!AnnotationUtil.hasOverrideAnnotation(ast)) {
	// Will check the name against the format.
	super.visitToken(ast);
	}

	private boolean isIgnorableMethod(DetailAST methodDef) {
	return shouldIgnoreMethod(methodDef.findFirstToken(TokenTypes.IDENT).getText())
	\|\| ignoreOverriddenMethods
	&& AnnotationUtil.hasOverrideAnnotation(methodDef);
	}

	private boolean isCheckedMethod(DetailAST ast) {
	final String methodName =
	ast.findFirstToken(TokenTypes.IDENT).getText();
	return isVerifiable(ast) && !ignoredMethodNames.contains(methodName)
	&& !AnnotationUtil.hasOverrideAnnotation(ast);
	}

	private static DetailAST findFirstAnnotation(final DetailAST ast,
	Predicate<DetailAST> predicate) {
	final DetailAST holder = getAnnotationHolder(ast);

Uh oh!

Conversation

Kevin222004 commented Jul 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Mutations

Explaination

Uh oh!

Kevin222004 commented Jul 21, 2023

Uh oh!

Kevin222004 commented Jul 21, 2023

Uh oh!

romani left a comment

Choose a reason for hiding this comment

Uh oh!

Vyom-Yadav left a comment

Choose a reason for hiding this comment

Uh oh!

rdiachenko left a comment

Choose a reason for hiding this comment

Uh oh!

rdiachenko commented Aug 1, 2023

Uh oh!

romani commented Aug 2, 2023

Uh oh!

romani commented Aug 2, 2023

Uh oh!

Kevin222004 commented Aug 2, 2023

Uh oh!

Vyom-Yadav commented Aug 2, 2023

Uh oh!

romani commented Aug 3, 2023

Uh oh!

romani commented Aug 4, 2023

Uh oh!

Kevin222004 commented Aug 4, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Kevin222004 commented Jul 21, 2023 •

edited

Loading