indentation mismatch with Google formatter over code blocks under switch #14294
Comments
Please change your description example to have single config and just reference what is not correct violations to your mind.
Please share link to description of this tool. Keep in mind there are bunch interpretations of Google Style, different tools might treat Google style wording differently. |
There is no non-logical violation, it's just that I cannot configure checkstyle to have no violation, as the options are contradictory
I added the link to download the formatter |
|
So all you try to do is to make checkstyle produce no violations on code that is formatted by another tool? |
Sort of, but I would expect to be able to configure checkstyle, such as if the formatter is the google implementation of google-java-format, no error is generated. For example, if I take the official file of this repo available here https://github.com/checkstyle/checkstyle/blob/master/src/main/resources/google_checks.xml, and ran the formatter, I still get the following checkstyle errors: Starting audit...
[WARN] /home/path/mymodule/src/main/java/com/example/checkstyle/CheckstyleScope.java:10:1: Missing a Javadoc comment. [MissingJavadocType]
[WARN] /home/path/mymodule/src/main/java/com/example/checkstyle/CheckstyleScope.java:14:9: '{' at column 9 should be on the previous line. [LeftCurly]
[WARN] /home/path/mymodule/src/main/java/com/example/checkstyle/CheckstyleScope.java:19:9: '{' at column 9 should be on the previous line. [LeftCurly]
[WARN] /home/path/mymodule/src/main/java/com/example/checkstyle/CheckstyleScope.java:27:5: 'block lcurly' has incorrect indentation level 4, expected level should be 6. [Indentation]
[WARN] /home/path/mymodule/src/main/java/com/example/checkstyle/CheckstyleScope.java:29:5: 'block rcurly' has incorrect indentation level 4, expected level should be 6. [Indentation]
Audit done. |
|
We never tested our Check against this tool, there are numerous interpretations of vague Google style wording. If you be able make input java file (after auto formatting) to run against checkstyle I can not approve issue by fact that we are not compliant in behavior with some other tool. |
|
I am not claiming that This issue I am opening is two fold:
|
google-java-format appears year after checkstyle provided style validation. But now a day better to be starting looking on each other. Did you consider google-java-format to have a defect? Let's focus on checkstyle Check requirements and problems. Let's make statement of problem like: In such wording it be easier to accept issue. |
I did consider it but ruled it out, as https://google.github.io/styleguide/javaguide.html#s4.1-braces does not mention anonymous scope, so implementations are free do to whatever they want, so one my want to configure a validation tool such as the code snippet provided does not raise any error. |
|
Please create issue on Google style to clarify it, if they update doc as google-java-format doing in some version (latest for today) we can change checkstyle config and Check to match their rules. And I hope new version of formatter will change behavior. |
|
This is a bug in checkstyle's implementation of Google Java Style.
package com.example.checkstyle;
public class CheckstyleScope {
private static void indentationIssue(Condition condition) {
switch (condition) {
case TRUE:
{
System.out.println("True");
break;
}
case FALSE:
{
System.out.println("False");
break;
}
default:
System.out.println("Other");
}
{
System.out.println("True");
}
}
private enum Condition {
TRUE,
FALSE
}
}
The line breaks between e.g. The 'Indentation' findings above are also incorrect. Checkstyle accepts the indentation below, which is disallowed by https://google.github.io/styleguide/javaguide.html#s4.2-block-indentation. *** CheckstyleScope.java.bak 2024-01-22 08:34:18.473151354 -0800
--- CheckstyleScope.java 2024-01-22 08:34:26.561141138 -0800
***************
*** 16,26 ****
default:
System.out.println("Other");
}
! {
! System.out.println("True");
! }
}
private enum Condition {
TRUE,
--- 16,26 ----
default:
System.out.println("Other");
}
! {
! System.out.println("True");
! }
}
private enum Condition {
TRUE, |
|
@cushon , thanks a lot for detailed report. |
romani
added
the
false positive
|
@romani @rdiachenko after removing <module name="Indentation">
<property name="basicOffset" value="2"/>
<property name="braceAdjustment" value="2"/>
<property name="caseIndent" value="2"/>
<property name="throwsIndent" value="4"/>
<property name="lineWrappingIndentation" value="4"/>
<property name="arrayInitIndent" value="2"/>
</module>public class Test { { // false-negative
System.out.println("Testing");
}
{
System.out.println("Testing");
}
int bbb = 9; { // should be separated by a blank line
System.out.println("Testing");
}
int aaa = 9;
{ // should be separated by a blank line
System.out.println("Testing");
}
{
System.out.println("Testing");
} int testtttt = 00;
int ccc = 9;
{ // good
System.out.println("Testing");
}
void test() {
int b = 9; { // false-negative
System.out.println("Testing");
}
Test t = new Test(); { // false-negative
System.out.println("Testing");
}
}
Test(int x) {
int b = 9; { // false-negative
System.out.println("Testing");
}
Test t = new Test(); { // false-negative
System.out.println("Testing");
}
}
static {
}
static
{ // violations (LeftCurlyEol), false-positive ( #14294 )
} // false-positive ( #14294 )
void test2() {
switch (a == 2) {
case true:
{ // violations (LeftCurlyNl), false-positive ( #14294 )
} // false-positive ( #14294 )
break;
default:
{ // violations (LeftCurlyNl), false-positive ( #14294 )
} // false-positive ( #14294 )
break;
}
}
Runnable runnable = () -> {
while (condition()) {
method();
}
};
Runnable runnable2 = () ->
{ // violations (LeftCurlyEol), false-positive ( #14294 )
while (condition()) {
method();
}
}; // false-positive ( #14294 )
private boolean condition() {
// TODO Auto-generated method stub
throw new UnsupportedOperationException("Unimplemented method 'condition'");
}
}without removing after removing it: <module name="Indentation">
<property name="basicOffset" value="2"/>
<!-- <property name="braceAdjustment" value="2"/> -->
<property name="caseIndent" value="2"/>
<property name="throwsIndent" value="4"/>
<property name="lineWrappingIndentation" value="4"/>
<property name="arrayInitIndent" value="2"/>
</module>diff of both outputs: $ diff -u withProperty.txt withoutProperty.txt
--- withProperty.txt 2024-09-24 00:00:54.612087500 +0530
+++ withoutProperty.txt 2024-09-24 00:01:31.217213700 +0530
@@ -5,16 +5,8 @@
[WARN] C:\checkstyle testing\.\Test.java:14:3: 'INSTANCE_INIT' should be separated from previous line. [EmptyLineSeparator]
[WARN] C:\checkstyle testing\.\Test.java:20:3: '}' at column 3 should be alone on a line. [RightCurlyAlone]
[WARN] C:\checkstyle testing\.\Test.java:20:5: 'VARIABLE_DEF' should be separated from previous line. [EmptyLineSeparator]
-[WARN] C:\checkstyle testing\.\Test.java:52:3: 'static initialization lcurly' has incorrect indentation level 2, expected level should be 4. [Indentation]
[WARN] C:\checkstyle testing\.\Test.java:52:3: '{' at column 3 should be on the previous line. [LeftCurlyEol]
-[WARN] C:\checkstyle testing\.\Test.java:53:3: 'static initialization rcurly' has incorrect indentation level 2, expected level should be 4. [Indentation]
-[WARN] C:\checkstyle testing\.\Test.java:58:7: 'block lcurly' has incorrect indentation level 6, expected level should be 8. [Indentation]
-[WARN] C:\checkstyle testing\.\Test.java:59:7: 'block rcurly' has incorrect indentation level 6, expected level should be 8. [Indentation]
[WARN] C:\checkstyle testing\.\Test.java:61:7: 'block' child has incorrect indentation level 6, expected level should be 8. [Indentation]
-[WARN] C:\checkstyle testing\.\Test.java:64:7: 'block lcurly' has incorrect indentation level 6, expected level should be 8. [Indentation]
-[WARN] C:\checkstyle testing\.\Test.java:65:7: 'block rcurly' has incorrect indentation level 6, expected level should be 8. [Indentation]
[WARN] C:\checkstyle testing\.\Test.java:67:7: 'block' child has incorrect indentation level 6, expected level should be 8. [Indentation]
-[WARN] C:\checkstyle testing\.\Test.java:78:3: 'block lcurly' has incorrect indentation level 2, expected level should be 4. [Indentation]
[WARN] C:\checkstyle testing\.\Test.java:78:3: '{' at column 3 should be on the previous line. [LeftCurlyEol]
-[WARN] C:\checkstyle testing\.\Test.java:82:3: 'block rcurly' has incorrect indentation level 2, expected level should be 4. [Indentation]
Audit done.Most false positives gets removed after removing the property. I haven't checked the other cases yet so Im not so sure if this can negatively impact in other places Following false positives are there still there: PS: ignore the trailing comments and config file name, I used same example file and config file mentioned at #15324 (comment) |
|
@Zopsss, please share checkstyle snapshot behavior on this code. |
|
latest behavior: so we still violate scoped blocks: Indention violations are not expected. I think we can suppress Indentation under pattern: until this issue in Indention is fixed. So: if above xpath works to suppress, we should use them. |
|
this works on such example, but we need bigger regression testing we should use in testing this project, it uses blocks: https://github.com/oliviercailloux/JARiS/blob/5f35a4d772f5b48851430ddc4d3763462187560c/src/main/java/io/github/oliviercailloux/jaris/credentials/CredentialsReader.java#L211-L213 |
…d updated documentation
romani
changed the title
github-project-automation
bot
moved this from In Progress
to Done
in Refine Google Style Guide
I have read check documentation:
I have downloaded the latest checkstyle from: https://checkstyle.org/cmdline.html#Download_and_Run
I have executed the cli and showed it below, as cli describes the problem better than 1,000 words
If I modify the rules to change to
<property name="braceAdjustment" value="0"/>,I manage to get ride of some errors but new ones appears:
The file was previously formatted with google-java-format (https://github.com/google/google-java-format/releases/tag/v1.19.2):
/var/tmp $ java -jar ~/Downloads/google-java-format-1.19.2-all-deps.jar --replace mymodule/src/main/java/com/example/checkstyle/CheckstyleScope.javaMight be linked with #3612, #3899 and #9326
The text was updated successfully, but these errors were encountered: