-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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.
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
Issue #14825: fix false positive for WhitespaceAroundCheck #14858
Conversation
|| isArrayInitialization(currentType, parentType); | ||
} | ||
} | ||
return result; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to refactor this method since complexity was >10 and caused violation
Hi @strkkk :) please generate a regression report when you get a chance, you can assign me to this PR when you are ready. |
Github, generate report |
No violations on this by this fix:
Requested in issue: Something is weird, I think we mixing switch as statement and like expression. In statement, spaces around might make sense, in expression form we should skip it completely for preceded and following spaces. So there should be two violations disappear https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/e6cdb69_2024152818/reports/diff/openjdk17/index.html#A4 . If we agree on this we should just mention in doc that we don't handle switch expressions at all. Thought? |
@romani it makes sense imo. There is WhitespaceAfter check if user wants to force whitespace after switch. |
Yes, I am in agreement, @strkkk let's update the docs to reflect this. |
ok, done. I added a note |
3a49e02
to
e92ef06
Compare
fixed |
3 more CI failures , restarted |
@romani build is green |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need a bit more test inputs to get all kind of ok
code that previously was violated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Items
...ls/checkstyle/checks/whitespace/whitespacearound/InputWhitespaceAroundSwitchExpressions.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok to merge
@strkkk , if will be awesome if you fix your account in CircleCI, this CI is not running for your account. This created a leak. |
Closes #14825
Diff Regression config: https://gist.githubusercontent.com/strkkk/26466254f89c32d8f1a6bd53b6f5b251/raw/3dfd508a98b976b267532ea4d4c0986d7b78c11c/config_single.xml