Skip to content

Fix google-java-format reformatting code formatted by Android Studio for Platform when running with AOSP style #1239

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

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

josh-gordon-fb
Copy link

Google-java-format when using AOSP style will reformat code that was already formatted by Google's Android Studio for Platform IDE (and vice versa). Both styles are compliant with the guidelines specified in AOSP Java code style for contributors. This prevents formatters from clobbering each other by updating google-java-format to be compliant with Android Studio for Platform's format when running with --aosp. No changes are made to the standard format.

Tested by formatting with Android Studio for Platform then google-java-format over multiple projects in AOSP and diffing to confirm no changes. All unit tests pass.

This will be needed for later commits on this branch. Passed the style with which to format into JavaInputAstVisitor when it is created to allow appropriate format selection when formatting with aosp format
…p style

Android Studio for Platform has a few differences in how it formats binary expressions. This change matches them when running with --aosp. In ASfP top level binary expressions are double indented. Subexpressions are single indented. Plus operators are always single indented.
…tyle

Android Studio for Platform doesn't indent lists of interfaces when breaking them up over multiple lines. This matches that behavior in google-java-format when running with --aosp.
…tyle

Android Studio for Platform doesn't add extra indentation to dotted expressions when they are decendents of binary expressions.
…sp style

Android Studio for Platform puts braces on different lines. Match that behavior in aosp style.
… style

Android Studio for Platform does not insert an extra space between array dimensions and the initializer body. This matches that behavior.
Android Studio for Platform puts newlines in empty classes. Match that behavior in aosp style.
…ng in aosp style

When a dot expression wraps to a new line, Android Studio for Platform doesn't wrap it. Add a stack counter for the number of dot expressions visited to be able to match this behavior in aosp style.
…tyle

Android Studio for Platform does not collapse empty do-while blocks. Match that behavior in aosp style.
…p style

Android Studio for Platform uses an extra level of indentation for array initializers. Match that behavior in aosp style.
Fix a typo lower in the branch that I already pushed :)
Copy link

google-cla bot commented Apr 7, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@cushon
Copy link
Collaborator

cushon commented Apr 11, 2025

Google-java-format when using AOSP style will reformat code that was already formatted by Google's Android Studio for Platform IDE (and vice versa). Both styles are compliant with the guidelines specified in AOSP Java code style for contributors.

Preserving the formatting of existing code is not a goal of this project, see: https://github.com/google/google-java-format/wiki/FAQ#why-didnt-it-leave-my-perfectly-valid-code-alone

The alternatives I'd recommend are to either standardize on one formatter for a particular codebase, or to run google-java-format on changed lines to avoid re-formatting unmodified code that isn't formatted in its style.

@josh-gordon-fb
Copy link
Author

Preserving the formatting of existing code is not a goal of this project, see: https://github.com/google/google-java-format/wiki/FAQ#why-didnt-it-leave-my-perfectly-valid-code-alone

My goal here isn't to make google-java-format preserve the formatting of existing code. My goal is to make google-java-format's existing --aosp flag to match the style enforced by Google's IDE for AOSP.

The alternatives I'd recommend are to either standardize on one formatter for a particular codebase, or to run google-java-format on changed lines to avoid re-formatting unmodified code that isn't formatted in its style.

Android Studio for Platform is "the official IDE for Android platform development", but its formatter can't be run in CI which makes standardizing on it difficult. I don't think any sort of compatibility with other formatters is necessary, but we should fix the unintentional behavior of google-java-format not matching Google's IDE for AOSP code.

@plumpy
Copy link
Collaborator

plumpy commented Apr 15, 2025

Maybe you want to use the google-java-format plugin for Android Studio? It will override the built-in formatter so you don't have two different formatters competing with each other.

@cushon
Copy link
Collaborator

cushon commented Apr 15, 2025

I think the default Android Studio behaviour is using IntelliJ's formatter with a configuration to follow AOSP style. This is related to the discussion of java-google-style.xml in #695. The source of truth for AOSP style is their style guide, which google-java-format follows. The IntelliJ formatter configuration is less authoritative than the style guide, and making google-java-format more similar to that behaviour isn't a goal.

@jpumford
Copy link

jpumford commented Apr 15, 2025

@plumpy

Maybe you want to use the google-java-format plugin for Android Studio? It will override the built-in formatter so you don't have two different formatters competing with each other.

It's my understanding from the style guide that users are expected to adhere to the configuration that Google generates (either through ASfP defaults or aidegen): We expect that as IDE users work on the code, imports in all packages will match this pattern without extra engineering effort. If we use GJF as a plugin, we diverge from the configuration enforced by Google's AOSP maintainers.

@cushon

The source of truth for AOSP style is their style guide, which google-java-format follows

I'm not sure this is correct. I don't think GJF complies with AOSP's style guide, even with the --aosp CLI flag. The style guide is a mix of semantic rules and syntactic ones, but of the very few syntactic rules listed I already see some divergence. Likely not exhaustive, but here's a few:

  • GJF import orderings and spacing don't match the style guide EDIT: this is user error on my part, I see where AOSP is handled in ImportOrderer (I seem to recall some issues last year with comments interspersed in the imports, but I can't reproduce the issue locally right now)
  • Annotation ordering doesn't match the style guide

@cushon
Copy link
Collaborator

cushon commented Apr 16, 2025

It's my understanding from the style guide that users are expected to adhere to the configuration that Google generates (either through ASfP defaults or aidegen): We expect that as IDE users work on the code, imports in all packages will match this pattern without extra engineering effort.

I read that section as specifically talking about a style for imports, not as saying that the source of truth for all style decisions is IntelliJ's formatter with the AOSP configuration.

GJF import orderings and spacing don't match the style guide (I seem to recall some issues last year with comments interspersed in the imports, but I can't reproduce the issue locally right now)

The formatter should be following AOSP import order when --aosp is configured (as long as --skip-sorting-imports is not also configured). If you're able to reproduce this, please file a bug.

Annotation ordering doesn't match the style guide

Do you have a specific example?

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

Successfully merging this pull request may close these issues.

4 participants