Skip to content
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

Should the formatter also add missing braces #51

Open
aozarov opened this issue May 24, 2016 · 9 comments
Open

Should the formatter also add missing braces #51

aozarov opened this issue May 24, 2016 · 9 comments

Comments

@aozarov
Copy link

aozarov commented May 24, 2016

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
}
aozarov added a commit to aozarov/appengine-java-vm-runtime that referenced this issue May 24, 2016
…rmat -i

Also, fix missing braces (as formatter didn't do that - see
google/google-java-format#51)
@kevinb9n
Copy link
Contributor

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
Copy link

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
Copy link

Balaji94 commented Mar 7, 2021

Will this be fixed anytime soon?

@lazystone
Copy link

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
Copy link
Contributor

kevinb9n commented Jun 12, 2021

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
Copy link
Collaborator

cushon commented Jun 12, 2021

+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
Copy link

romani commented Jul 20, 2024

@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
Copy link
Collaborator

cushon commented Jul 20, 2024

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
Copy link

romani commented Jul 20, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants