Issue #13086: InnerAssignmentCheck failed for one line code in Java 1… #13090

Andromeda227799 · 2023-05-25T08:49:06Z

Fixing Issue #13086 by allowing Expressions to have switch cases as parent AST

Diff Regression config: https://gist.githubusercontent.com/Andromeda227799/52c0c74faeb9ba9334955fc68b624e98/raw/8ff5e9484d8c48022d9817fd299a1c5b6b644e71/check.xml

romani

please generate regression diff report.
Example how to do this: #13085
full doc: https://github.com/checkstyle/contribution/tree/master/checkstyle-tester#report-generation

items:

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

Andromeda227799 · 2023-05-27T05:44:26Z

GitHub, generate report

nrmancuso · 2023-05-27T10:26:58Z

...rces/com/puppycrawl/tools/checkstyle/checks/coding/innerassignment/InputInnerAssignment.java

@@ -33,6 +33,20 @@ void innerAssignments()
        }
    }

+    public void expressionCanHaveParentSwitchCaseIssue13086()


Please do not extend existing inputs. We need to create a completely new test and test input using switch statements, switch expressions, with both switch rules and labeled block statements. You will need to create the new input in resources-noncompilable in order to support syntax greater than jdk11.

Yes, please create new Input file, this input file is already too big.

Please in addition add test case from user, to be absolutely clear that user reported case is covered

@nrmancuso @romani This issue also exists when I ran with Java 11, so should I simply create a new test file in src/test/resources/com/puppycrawl/tools/checkstyle/checks/coding/innerassignment instead of adding it in resources-noncompilable

No, we must include exact case from issue report.

@Andromeda227799 always allow reviewers to resolve their items, please just reply to each with “done” or discussion if necessary.

nrmancuso · 2023-05-27T11:24:14Z

@Andromeda227799 you have not supplied a configuration, so no report will be generated. Please read https://github.com/checkstyle/contribution/tree/master/checkstyle-tester#report-generation

Andromeda227799 · 2023-05-28T05:09:05Z

GitHub, generate report

github-actions · 2023-05-28T05:11:12Z

Report generation failed on phase "make_report",
step "Generate report".
Link: https://github.com/checkstyle/checkstyle/actions/runs/5102549215

Andromeda227799 · 2023-05-28T05:15:07Z

Report generation failed on phase "make_report", step "Generate report".Link: https://github.com/checkstyle/checkstyle/actions/runs/5102549215

@romani could you help me with why this failed?

nrmancuso · 2023-05-28T06:51:04Z

@Andromeda227799 you can click into link provided by GitHub action to see point of failure, looks like your configuration is not valid.

Andromeda227799 · 2023-06-05T08:14:24Z

GitHub, generate report

github-actions · 2023-06-05T08:16:37Z

Report generation failed on phase "make_report",
step "Generate report".
Link: https://github.com/checkstyle/checkstyle/actions/runs/5174846776

Andromeda227799 · 2023-06-05T08:22:19Z

GitHub, generate report

Andromeda227799 · 2023-06-05T10:15:01Z

GitHub, generate report

github-actions · 2023-06-05T10:17:10Z

Report generation failed on phase "make_report",
step "Generate report".
Link: https://github.com/checkstyle/checkstyle/actions/runs/5175951691

nrmancuso · 2023-06-05T13:18:43Z

@Andromeda227799 please follow https://github.com/checkstyle/contribution/tree/master/checkstyle-tester#report-generation:~:text=Report%20configuration%20template%20when%20both%20master%20branch%20and%20patch%20branch%20have%20same%20config%20(most%20cases%20like%20bugfixes%2C%20code%20enhancements%2C%20etc.). and use template config from link as a base.

Andromeda227799 · 2023-06-05T14:40:32Z

GitHub, generate report

github-actions · 2023-06-05T15:46:14Z

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

romani

Please add:
https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/dfbaccb_2023153903/reports/diff/openjdk17/index.html#A40
https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/dfbaccb_2023153903/reports/diff/openjdk17/index.html#A46
to our Inputs for this Check

Items:

romani · 2023-06-06T11:29:51Z

...l/tools/checkstyle/checks/coding/innerassignment/InputInnerAssignmentWithEnhancedSwitch.java

+    void method(final String operation) {
+        boolean flag = false;
+        switch (operation) {
+            case "Y" -> flag = true;  // ok


Please remove trailing comment // ok
I know there are a lot of examples where we have them, but it is our mistake of past.
No // violation means ok, so //ok is just a noise.

… in Java 14 switch expression

romani · 2023-06-06T11:40:09Z

@Andromeda227799 , please look at my request above to extend Inputs , thanks in advance.

nrmancuso · 2023-06-06T12:22:59Z

We lost some valid violations, example:
https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/dfbaccb_2023153903/reports/diff/openjdk17/xref/home/runner/work/checkstyle/checkstyle/.ci-temp/contribution/checkstyle-tester/repositories/openjdk17/test/langtools/tools/javac/switchexpr/DefiniteAssignment1.java.html#L404

We need to differentiate between assignment in switch expression (should be violation) and switch statements (should not be a violation).

This should be expected behavior:

public class Test {

    private boolean flag;

    void getOperationOk(final String operation) {
        switch (operation) {
            case "Y" -> flag = true;  // should not be violation
            case "N" -> {
                flag = false; // ok
            }
            default -> throw new UnsupportedOperationException();
        }
    }

    void getOperationViolation1(final String operation) {

        final boolean result = switch (operation) {
            case "Y" -> flag = true;  // should be violation
            case "N" -> {
                yield flag = false; // should be violation
            }
            case "Z" -> {
                flag = false;
                yield flag; // ok
            }
            default -> throw new UnsupportedOperationException();
        };
    }

        void getOperationViolation2(final String operation) {

        final boolean result = switch (operation) {
            case "Y" : flag = true;  // should be violation
            case "N" : {
                yield flag = false; // should be violation
            }
            case "Z" : {
                flag = false;
                yield flag; // ok
            }
            default : throw new UnsupportedOperationException();
        };
    }
}

pmensalt · 2023-08-07T12:51:56Z

Is there any update or advancement regarding this issue?

nrmancuso · 2023-08-08T16:24:06Z

@Andromeda227799 do you plan to continue to work on this issue?

Is there any update or advancement regarding this issue?

@pmensalt looks like we might have lost contact with the author of this PR, let's see if they respond.

romani · 2023-09-02T14:50:45Z

if no activity before 1 october , we will close PR and next contributor can reuse this code and complete update

nrmancuso · 2023-10-06T11:14:32Z

We have lost contact with contributor, anyone is welcome to reuse changes here to resolve issue.

AayushSaini101 · 2023-10-07T04:32:21Z

@nrmancuso "We need to differentiate between assignment in switch expression (should be violation) and switch statements (should not be a violation)." Regarding this can you guide me a little bit on how can we add these? We have added the Switch rule {TokenTypes.EXPR, TokenTypes.SWITCH_RULE}, but do i need to create new rule for the second condition that is final boolean result = switch (operation) {

nrmancuso · 2023-10-07T04:48:43Z

@AayushSaini101 let’s open a new PR, please leave comments in your PR with what you are struggling with

romani requested changes May 25, 2023

View reviewed changes

src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/InnerAssignmentCheck.java Show resolved Hide resolved

Andromeda227799 requested a review from romani May 27, 2023 05:44

Andromeda227799 force-pushed the shamith-fix/13086 branch 3 times, most recently from ea81f62 to 7965389 Compare May 27, 2023 10:07

nrmancuso reviewed May 27, 2023

View reviewed changes

Andromeda227799 force-pushed the shamith-fix/13086 branch 4 times, most recently from 8c32701 to dfbaccb Compare May 28, 2023 09:40

romani requested changes Jun 6, 2023

View reviewed changes

Issue checkstyle#13086: InnerAssignmentCheck failed for one line code…

5ee4d93

… in Java 14 switch expression

Andromeda227799 force-pushed the shamith-fix/13086 branch from dfbaccb to 5ee4d93 Compare June 6, 2023 11:38

nrmancuso added awaiting changes awaiting response labels Aug 8, 2023

rnveach assigned romani Aug 31, 2023

nrmancuso closed this Oct 6, 2023

nrmancuso mentioned this pull request Oct 6, 2023

InnerAssignmentCheck failed for one line code in Java 14 switch expression #13086

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue #13086: InnerAssignmentCheck failed for one line code in Java 1… #13090

Issue #13086: InnerAssignmentCheck failed for one line code in Java 1… #13090

Andromeda227799 commented May 25, 2023 •

edited

romani left a comment •

edited

Andromeda227799 commented May 27, 2023

nrmancuso May 27, 2023

romani May 27, 2023

romani May 27, 2023

Andromeda227799 May 28, 2023 •

edited

nrmancuso May 28, 2023

nrmancuso May 28, 2023

Andromeda227799 May 28, 2023

nrmancuso commented May 27, 2023

Andromeda227799 commented May 28, 2023

github-actions bot commented May 28, 2023

Andromeda227799 commented May 28, 2023

nrmancuso commented May 28, 2023

Andromeda227799 commented Jun 5, 2023

github-actions bot commented Jun 5, 2023

Andromeda227799 commented Jun 5, 2023

Andromeda227799 commented Jun 5, 2023

github-actions bot commented Jun 5, 2023

nrmancuso commented Jun 5, 2023

Andromeda227799 commented Jun 5, 2023

github-actions bot commented Jun 5, 2023

romani left a comment •

edited

romani Jun 6, 2023

romani commented Jun 6, 2023

nrmancuso commented Jun 6, 2023 •

edited

pmensalt commented Aug 7, 2023

nrmancuso commented Aug 8, 2023

romani commented Sep 2, 2023

nrmancuso commented Oct 6, 2023

AayushSaini101 commented Oct 7, 2023

nrmancuso commented Oct 7, 2023

Issue #13086: InnerAssignmentCheck failed for one line code in Java 1… #13090

Issue #13086: InnerAssignmentCheck failed for one line code in Java 1… #13090

Conversation

Andromeda227799 commented May 25, 2023 • edited

romani left a comment • edited

Choose a reason for hiding this comment

Andromeda227799 commented May 27, 2023

nrmancuso May 27, 2023

Choose a reason for hiding this comment

romani May 27, 2023

Choose a reason for hiding this comment

romani May 27, 2023

Choose a reason for hiding this comment

Andromeda227799 May 28, 2023 • edited

Choose a reason for hiding this comment

nrmancuso May 28, 2023

Choose a reason for hiding this comment

nrmancuso May 28, 2023

Choose a reason for hiding this comment

Andromeda227799 May 28, 2023

Choose a reason for hiding this comment

nrmancuso commented May 27, 2023

Andromeda227799 commented May 28, 2023

github-actions bot commented May 28, 2023

Andromeda227799 commented May 28, 2023

nrmancuso commented May 28, 2023

Andromeda227799 commented Jun 5, 2023

github-actions bot commented Jun 5, 2023

Andromeda227799 commented Jun 5, 2023

Andromeda227799 commented Jun 5, 2023

github-actions bot commented Jun 5, 2023

nrmancuso commented Jun 5, 2023

Andromeda227799 commented Jun 5, 2023

github-actions bot commented Jun 5, 2023

romani left a comment • edited

Choose a reason for hiding this comment

romani Jun 6, 2023

Choose a reason for hiding this comment

romani commented Jun 6, 2023

nrmancuso commented Jun 6, 2023 • edited

pmensalt commented Aug 7, 2023

nrmancuso commented Aug 8, 2023

romani commented Sep 2, 2023

nrmancuso commented Oct 6, 2023

AayushSaini101 commented Oct 7, 2023

nrmancuso commented Oct 7, 2023

Andromeda227799 commented May 25, 2023 •

edited

romani left a comment •

edited

Andromeda227799 May 28, 2023 •

edited

romani left a comment •

edited

nrmancuso commented Jun 6, 2023 •

edited