Issue #13038: handled inner class method definition in VariableDeclarationUsageDistanceCheck #14635

jxr98 · 2024-03-10T03:40:37Z

Issue: #13038
Diff Regression config: https://gist.githubusercontent.com/jxr98/8f42477fea778a2b62873c9af6ad8493/raw/b6b8b98a74810181a66fc2cdb04dfc7f45109f85/VariableDeclarationUsageDistance.xml

jxr98 · 2024-03-10T04:25:36Z

GitHub, generate report

github-actions · 2024-03-10T06:18:42Z

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/49133d8_2024061816/reports/diff/index.html

jxr98 · 2024-03-10T18:35:16Z

I've read the pitest wiki and the pitest mutation overview. I understand removed conditional - replaced equality check with true mutation, but don't see how it applies to the code. I have tried to replace the first == with equal and remove it. It doesn't seem right. Can anyone help me? Thanks.

nrmancuso · 2024-03-28T16:11:30Z

I've read the pitest wiki and the pitest mutation overview. I understand removed conditional - replaced equality check with true mutation, but don't see how it applies to the code. I have tried to replace the first == with equal and remove it. It doesn't seem right. Can anyone help me? Thanks.

I am fairly certain this means that now the ONLY tokens that are passed to this method are in these conditions, so we might as well return true from the isZeroDistanceToken method, which means that where ever this method is used could probably be mutated to just true as well. Try to remove this method completely and see what happens.

jxr98 · 2024-04-01T02:13:02Z

I've read the pitest wiki and the pitest mutation overview. I understand removed conditional - replaced equality check with true mutation, but don't see how it applies to the code. I have tried to replace the first == with equal and remove it. It doesn't seem right. Can anyone help me? Thanks.

I am fairly certain this means that now the ONLY tokens that are passed to this method are in these conditions, so we might as well return true from the isZeroDistanceToken method, which means that where ever this method is used could probably be mutated to just true as well. Try to remove this method completely and see what happens.

Thanks. I've tried to return true or false in the isZeroDistanceToken method and remove it completely, but some of the tests we have failed after that. Still can't figure out why.

romani · 2024-04-01T12:41:34Z

as a hack, please make method as public and write junit test for this method to clearly show that survival can gone if we do have explicit test.

jxr98 · 2024-04-02T01:34:26Z

as a hack, please make method as public and write junit test for this method to clearly show that survival can gone if we do have explicit test.

Thanks. The method is very straightforward containing several or operator as follows
, and the code is here. From debugging and experience, the survivals are Expr, if, for, return, {, etc. The conditions in the method can't be complete I guess. 🤔

nrmancuso · 2024-04-02T02:59:24Z

but some of the tests we have failed after that. Still can't figure out why

If this is true, then we are missing tests from the pitest profile in pom.xml for this check. Please push up your changes and show this failure so that we can help debug.

…ableDeclarationUsageDistanceCheck

jxr98 · 2024-04-02T21:00:31Z

but some of the tests we have failed after that. Still can't figure out why

If this is true, then we are missing tests from the pitest profile in pom.xml for this check. Please push up your changes and show this failure so that we can help debug.

I've pushed the changes I made. If the method just return true directly, the failed tests are shown as in the file below.
com.puppycrawl.tools.checkstyle.checks.coding.VariableDeclarationUsageDistanceCheckTest.txt

It looks like the pom file includes the target class and target test of this check correctly. I'd really appreciate it if you could help me debug. I guess I might make some mistakes where I'm not aware of right now.

nrmancuso · 2024-04-03T03:51:35Z

It looks like the pom file includes the target class and target test of this check correctly. I'd really appreciate it if you could help me debug. I guess I might make some mistakes where I'm not aware of right now.

There is definitely something weird going on here. I've checked this code out locally, and tried to hardcode the mutation in a number of ways, but the tests are always failing. The only thing I can think of is that the bytecode inlines this method and the actual mutation isn't what is shown in the suppression.

I suggest that you check the bytecode for this file, you can see how to do this at https://github.com/checkstyle/checkstyle/wiki/How-to-Generate-and-Analyze-Pitest-Reports#decompiler-usage-to-see-bytecode.

@Vyom-Yadav have you seen anything like this before?

rnveach · 2024-04-03T03:20:20Z

...s/coding/variabledeclarationusagedistance/InputVariableDeclarationUsageDistanceGeneral4.java

@@ -22,8 +22,7 @@ void mm() {
        <T> void xx(List<T> m){}
    }
    public void method9() {
-        // until https://github.com/checkstyle/checkstyle/issues/13011


First I would confirm that these 2 issues are exactly the same and explain it so we understand. I find it odd that a method issue would fix a class issue, but maybe no one noticed the cases were extremely similar.

Also, apparently you reported both issues. :)

Oo right they r almost the same.

rnveach · 2024-04-03T03:59:17Z

...ava/com/puppycrawl/tools/checkstyle/checks/coding/VariableDeclarationUsageDistanceCheck.java

@@ -784,7 +787,8 @@ private static boolean isZeroDistanceToken(int type) {
                || type == TokenTypes.MODIFIERS
                || type == TokenTypes.RESOURCE
                || type == TokenTypes.EXTENDS_CLAUSE
-                || type == TokenTypes.IMPLEMENTS_CLAUSE;
+                || type == TokenTypes.IMPLEMENTS_CLAUSE
+                || type == TokenTypes.CLASS_DEF;


removed conditional - replaced equality check with true

This is my local when I run the profile.

I removed one of these lines completely, or something similar to changing || type == TokenTypes.EXTENDS_CLAUSE to || false, and all the tests passed. This shows the mutation. The mutation in this case is reversed from the text that says true.

At a glance this sort of makes sense. extends and implements both have class as a parent.

So now we must identify how to kill these mutations, or understand why they can't be killed. I would look over https://github.com/checkstyle/checkstyle/wiki/How-to-Generate-and-Analyze-Pitest-Reports#how-to-find-java-code-that-can-kill-mutation and https://github.com/checkstyle/contribution/tree/master/checkstyle-tester#checkstyle-pitest-regression for the next steps. You can also look over other pitest issues that describe the more complex process. We do not normally accept new code with new surviving mutations.

At a glance this sort of makes sense. extends and implements both have class as a parent.

Yes, this sounds like what is happening here. We should probably add some test cases for local enums, records, and interfaces as well.

Thank u so much!!! I see. Trying to solve it.

jxr98 marked this pull request as draft March 10, 2024 03:40

jxr98 force-pushed the ff branch from 53326f1 to 49133d8 Compare March 10, 2024 03:41

jxr98 force-pushed the ff branch from 49133d8 to 9810e8a Compare March 17, 2024 17:17

jxr98 force-pushed the ff branch 2 times, most recently from 0425b43 to d762894 Compare April 1, 2024 02:07

Issue checkstyle#13038: handled inner class method definition in Vari…

ac1ab87

…ableDeclarationUsageDistanceCheck

jxr98 force-pushed the ff branch from d762894 to ac1ab87 Compare April 2, 2024 20:52

rnveach reviewed Apr 3, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue #13038: handled inner class method definition in VariableDeclarationUsageDistanceCheck #14635

Issue #13038: handled inner class method definition in VariableDeclarationUsageDistanceCheck #14635

jxr98 commented Mar 10, 2024

jxr98 commented Mar 10, 2024

github-actions bot commented Mar 10, 2024

jxr98 commented Mar 10, 2024 •

edited

nrmancuso commented Mar 28, 2024

jxr98 commented Apr 1, 2024 •

edited

romani commented Apr 1, 2024

jxr98 commented Apr 2, 2024

nrmancuso commented Apr 2, 2024

jxr98 commented Apr 2, 2024

nrmancuso commented Apr 3, 2024

rnveach Apr 3, 2024

jxr98 Apr 10, 2024 •

edited

rnveach Apr 3, 2024 •

edited

nrmancuso Apr 3, 2024

jxr98 Apr 10, 2024

Issue #13038: handled inner class method definition in VariableDeclarationUsageDistanceCheck #14635

Are you sure you want to change the base?

Issue #13038: handled inner class method definition in VariableDeclarationUsageDistanceCheck #14635

Conversation

jxr98 commented Mar 10, 2024

jxr98 commented Mar 10, 2024

github-actions bot commented Mar 10, 2024

jxr98 commented Mar 10, 2024 • edited

nrmancuso commented Mar 28, 2024

jxr98 commented Apr 1, 2024 • edited

romani commented Apr 1, 2024

jxr98 commented Apr 2, 2024

nrmancuso commented Apr 2, 2024

jxr98 commented Apr 2, 2024

nrmancuso commented Apr 3, 2024

rnveach Apr 3, 2024

Choose a reason for hiding this comment

jxr98 Apr 10, 2024 • edited

Choose a reason for hiding this comment

rnveach Apr 3, 2024 • edited

Choose a reason for hiding this comment

nrmancuso Apr 3, 2024

Choose a reason for hiding this comment

jxr98 Apr 10, 2024

Choose a reason for hiding this comment

jxr98 commented Mar 10, 2024 •

edited

jxr98 commented Apr 1, 2024 •

edited

jxr98 Apr 10, 2024 •

edited

rnveach Apr 3, 2024 •

edited