Skip to content

Issue #17120: fix false positive for Lambda child in indentation check #17138

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
May 28, 2025
Merged

Issue #17120: fix false positive for Lambda child in indentation check #17138

merged 1 commit into from
May 28, 2025

Conversation

Aziz-755
Copy link
Contributor

@Aziz-755 Aziz-755 commented May 25, 2025 •
edited
Loading

@Aziz-755
Copy link
Contributor Author

GitHub, generate report

@github-actions GitHub Actions
Copy link
Contributor

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

@Aziz-755
Copy link
Contributor Author

Github, generate report for Indentation/all-examples-in-one

@github-actions GitHub Actions
Copy link
Contributor

Report for Indentation/all-examples-in-one:
https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/b006df6a52dade90260802b6dafad0583deaa0c6_2025132112/reports/diff/index.html

@Aziz-755 Aziz-755 mentioned this pull request May 25, 2025
Closed
@Aziz-755
Copy link
Contributor Author

@romani PR is ready for review

romani
romani requested changes May 25, 2025
View reviewed changes
Copy link
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.

thanks a lot.

just test extension to be sure on just another edge case.
Looks like no much usage of this new code pattern still in web

@romani
Copy link
Member

romani commented May 25, 2025

@christophpre , can you test this update on your code base ?

git clone https://github.com/Aziz-755/checkstyle/tree/fp_for_indentation_check
cd checkstyle
mvn clean install -Pno-validations

and you can use snapshot version from your maven repository.
if you need -all jar, please run mvn clean package -Passembly

@christophpre
Copy link

@romani I tested this PR fix with my example gist, but unfortunately it doesn't fix my issue.

But maybe as @Aziz-755 pointed out here: #17120 (comment) I might have to check my checkstyle settings again.

@Aziz-755 Aziz-755 force-pushed the fp_for_indentation_check branch from b006df6 to 90d1bc6 Compare May 25, 2025 21:30
@Aziz-755
Copy link
Contributor Author

Github, generate report for Indentation/all-examples-in-one

@Aziz-755 Aziz-755 force-pushed the fp_for_indentation_check branch 2 times, most recently from ae69202 to 6badc86 Compare May 25, 2025 21:58
@github-actions GitHub Actions
Copy link
Contributor

Report for Indentation/all-examples-in-one:
https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/90d1bc6d4e8d8d8e25e5cf7a93cccf7c79905689_2025223535/reports/diff/index.html

@Aziz-755 Aziz-755 requested a review from romani May 26, 2025 08:09
romani
romani requested changes May 26, 2025
View reviewed changes
Copy link
Member

@romani romani left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Please add to Inputs https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/90d1bc6d4e8d8d8e25e5cf7a93cccf7c79905689_2025223535/reports/diff/sevntu-checkstyle/index.html#A1

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/90d1bc6d4e8d8d8e25e5cf7a93cccf7c79905689_2025223535/reports/diff/openjdk21/index.html#A98

Minor

@romani
Copy link
Member

romani commented May 26, 2025

@Aziz-755 , please do few minor above and we are good to merge

@Aziz-755 Aziz-755 force-pushed the fp_for_indentation_check branch from 6badc86 to 9c1440e Compare May 27, 2025 20:54
@Aziz-755 Aziz-755 force-pushed the fp_for_indentation_check branch from 9c1440e to 3a128b5 Compare May 27, 2025 21:17
@Aziz-755 Aziz-755 requested a review from romani May 27, 2025 23:05
romani
romani approved these changes May 28, 2025
View reviewed changes
Copy link
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.

thanks a lot.

@romani romani merged commit 36aeb47 into checkstyle:master May 28, 2025
117 checks passed
@wendigo
Copy link

wendigo commented Jun 2, 2025

This breaks a lot of existing code. In Trino we have nested switch expressions like this: https://github.com/trinodb/trino/blob/3da377228dd6f7a05eda7e9216407bff59099919/core/trino-spi/src/main/java/io/trino/spi/function/ScalarFunctionAdapter.java#L166

This now fails with:

[ERROR] src/main/java/io/trino/spi/function/ScalarFunctionAdapter.java:[168,17] (indentation) Indentation: 'case' child has incorrect indentation level 16, expected level should be 24.
[ERROR] src/main/java/io/trino/spi/function/ScalarFunctionAdapter.java:[169,17] (indentation) Indentation: 'case' child has incorrect indentation level 16, expected level should be 24.
[ERROR] src/main/java/io/trino/spi/function/ScalarFunctionAdapter.java:[170,17] (indentation) Indentation: 'case' child has incorrect indentation level 16, expected level should be 24.
[ERROR] src/main/java/io/trino/spi/function/ScalarFunctionAdapter.java:[171,17] (indentation) Indentation: 'case' child has incorrect indentation level 16, expected level should be 24.
[ERROR] src/main/java/io/trino/spi/function/ScalarFunctionAdapter.java:[175,17] (indentation) Indentation: 'case' child has incorrect indentation level 16, expected level should be 24.
[ERROR] src/main/java/io/trino/spi/function/ScalarFunctionAdapter.java:[176,13] (indentation) Indentation: 'switch rcurly' has incorrect indentation level 12, expected level should be 20.
[ERROR] src/main/java/io/trino/spi/function/ScalarFunctionAdapter.java:[178,17] (indentation) Indentation: 'case' child has incorrect indentation level 16, expected level should be 24.
[ERROR] src/main/java/io/trino/spi/function/ScalarFunctionAdapter.java:[179,17] (indentation) Indentation: 'case' child has incorrect indentation level 16, expected level should be 24.
[ERROR] src/main/java/io/trino/spi/function/ScalarFunctionAdapter.java:[180,17] (indentation) Indentation: 'case' child has incorrect indentation level 16, expected level should be 24.
[ERROR] src/main/java/io/trino/spi/function/ScalarFunctionAdapter.java:[181,17] (indentation) Indentation: 'case' child has incorrect indentation level 16, expected level should be 24.
[ERROR] src/main/java/io/trino/spi/function/ScalarFunctionAdapter.java:[182,13] (indentation) Indentation: 'switch rcurly' has incorrect indentation level 12, expected level should be 20.
[ERROR] src/main/java/io/trino/spi/function/ScalarFunctionAdapter.java:[184,17] (indentation) Indentation: 'case' child has incorrect indentation level 16, expected level should be 24.
[ERROR] src/main/java/io/trino/spi/function/ScalarFunctionAdapter.java:[185,17] (indentation) Indentation: 'case' child has incorrect indentation level 16, expected level should be 24.
[ERROR] src/main/java/io/trino/spi/function/ScalarFunctionAdapter.java:[186,17] (indentation) Indentation: 'case' child has incorrect indentation level 16, expected level should be 24.
[ERROR] src/main/java/io/trino/spi/function/ScalarFunctionAdapter.java:[187,13] (indentation) Indentation: 'switch rcurly' has incorrect indentation level 12, expected level should be 20.

@romani
Copy link
Member

romani commented Jun 2, 2025

@wendigo , please report new issue, to let us see all details

@wendigo
Copy link

wendigo commented Jun 2, 2025

@romani #17163

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@romani romani romani approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

False positives for google_checks when using switch expression in lambda
4 participants
@Aziz-755 @romani @christophpre @wendigo