google / google-java-format

Reformats Java source code to comply with Google Java Style.
Other
5.54k stars 851 forks source link

Should the formatter also add missing braces #51

Open aozarov opened 8 years ago

aozarov commented 8 years ago

To correct any violation of https://google.github.io/styleguide/javaguide.html#s4.1.1-braces-always-used

Currently it looks like a code that is formatted like

if (condition)
  do_something

would be formatted (after running the tool) as:

if (condition) do_someting

where as I would expect it to be formatted as:

if (condition) {
  do_something
}
kevinb9n commented 8 years ago

Yes, adding braces is a request we're considering (there are some concerns unique to making non-whitespace changes that we'd have to sort through).

In the meantime, the current behavior is acceptable, because (a) it's exactly what users following AOSP style want, and (b) for Google Style the code was already incorrect anyway, and maybe the odd change will prompt a human to make the right fix.

rjeberhard commented 5 years ago

Any update on this? I keep having to resolve flame wars between those who want what the Google style guide says vs. what the tools actually enforce.

Balaji94 commented 3 years ago

Will this be fixed anytime soon?

lazystone commented 3 years ago

This feature would be really helpful. We have some legacy code where we have a lot of if conditions without braces. This makes it so error prone for a change. Just reformatting that code would improve readability and make it a bit friendly for changes.

kevinb9n commented 3 years ago

Pasting comment from similar issue:

So far, GJF has been focused on being a formatter, which is a somewhat narrower purpose than "style correcter". It does nothing but insert and remove whitespace characters, and sometimes reorder things. The more general tool would be useful, but I don't think we plan for this to become that. If cushon replies, take his word over mine. :-)

(EDIT: I was reminded that it does have a few other non-whitespace changes it's willing to make, so I made this point a little too strongly.)

cushon commented 3 years ago

+1 to what @kevinb9n said :)

FWIW we have an Error Prone check for missing braces that can be run as a refactoring, which we use internally to help enforce that style rule.

There are a handful of other cleanups like this that we may eventually be added to the formatter, but so far it hasn't been a priority, vs. focusing on pure whitespace reformatting changes that don't add or reorder tokens.

romani commented 1 month ago

@cushon ,

https://google.github.io/styleguide/javaguide.html#s4.1.1-braces-always-used

Braces are used with if, else, for, do and while statements, even when the body is empty or contains only a single statement. Other optional braces, such as those in a lambda expression, remain optional.

is it possible to get clarity on Other optional braces, such as those in a lambda expression, remain optional. is it applicable to if (condition) do_someting ?

if yes, it would be awesome to update style guide and close this holly war.

cushon commented 1 month ago

Braces are used with if ... statements, even when the body is empty or contains only a single statement.

Other optional braces, such as those in a lambda expression, remain optional.

Braces are required for if statement, including if (condition) do_someting, so it should be written as

if (condition) {
  do_someting
}

Braces are not required for lambda expressions, so both of the following are allowed:

Runnable r1 = () -> doSomething();
Runnable r2 = () -> {
  doSomething();
}

@romani is that helpful?

As far as this relates to google-java-format, the formatter output will always follow Style Guide rules about whitespace (including breaking lines and indentation). There are other style guide rules that are out of scope for the formatter to fix, including e.g. the rules about names.

romani commented 1 month ago

ok, thanks a lot !!

we just need to wait for sombody to fix this behavior in formatter.

Checkstyle is violating if (condition) do_someting , so if it is activated in project, there will be one more hint to user to add braces, manually for now.