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
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import com.google.googlejavaformat.Doc;
import com.google.googlejavaformat.DocBuilder;
import com.google.googlejavaformat.FormattingError;
import com.google.googlejavaformat.java.JavaFormatterOptions.Style;
import com.google.googlejavaformat.Newlines;
import com.google.googlejavaformat.Op;
import com.google.googlejavaformat.OpsBuilder;
Expand Down Expand Up @@ -156,7 +157,7 @@ public CharSequence getCharContent(boolean ignoreEncodingErrors) throws IOExcept
createVisitor(
"com.google.googlejavaformat.java.java21.Java21InputAstVisitor", builder, options);
} else {
visitor = new JavaInputAstVisitor(builder, options.indentationMultiplier());
visitor = new JavaInputAstVisitor(builder, options.indentationMultiplier(), options.style());
}
visitor.scan(unit, null);
builder.sync(javaInput.getText().length());
Expand All @@ -172,8 +173,8 @@ private static JavaInputAstVisitor createVisitor(
try {
return Class.forName(className)
.asSubclass(JavaInputAstVisitor.class)
.getConstructor(OpsBuilder.class, int.class)
.newInstance(builder, options.indentationMultiplier());
.getConstructor(OpsBuilder.class, int.class, Style.class)
.newInstance(builder, options.indentationMultiplier(), options.style());
} catch (ReflectiveOperationException e) {
throw new LinkageError(e.getMessage(), e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
import com.google.googlejavaformat.Output.BreakTag;
import com.google.googlejavaformat.java.DimensionHelpers.SortedDims;
import com.google.googlejavaformat.java.DimensionHelpers.TypeWithDims;
import com.google.googlejavaformat.java.JavaFormatterOptions.Style;
import com.sun.source.tree.AnnotatedTypeTree;
import com.sun.source.tree.AnnotationTree;
import com.sun.source.tree.ArrayAccessTree;
Expand Down Expand Up @@ -304,10 +305,15 @@ private static ImmutableSetMultimap<String, String> typeAnnotations() {
return result.build();
}

// Used to ensure correct indentation of subexpressions in aosp style
private boolean inBinaryExpression = false;
private int dotExpressionLevel = 0;

protected final OpsBuilder builder;

protected static final Indent.Const ZERO = Indent.Const.ZERO;
protected final int indentMultiplier;
protected final Style style;
protected final Indent.Const minusTwo;
protected final Indent.Const minusFour;
protected final Indent.Const plusTwo;
Expand Down Expand Up @@ -341,9 +347,10 @@ private static final ImmutableList<Op> forceBreakList(Optional<BreakTag> breakTa
*
* @param builder the {@link OpsBuilder}
*/
public JavaInputAstVisitor(OpsBuilder builder, int indentMultiplier) {
public JavaInputAstVisitor(OpsBuilder builder, int indentMultiplier, Style style) {
this.builder = builder;
this.indentMultiplier = indentMultiplier;
this.style = style;
minusTwo = Indent.Const.make(-2, indentMultiplier);
minusFour = Indent.Const.make(-4, indentMultiplier);
plusTwo = Indent.Const.make(+2, indentMultiplier);
Expand All @@ -357,6 +364,10 @@ private boolean inExpression() {
return inExpression.peekLast();
}

private boolean useAospStyle() {
return style.equals(Style.AOSP);
}

@Override
public Void scan(Tree tree, Void unused) {
inExpression.addLast(tree instanceof ExpressionTree || inExpression.peekLast());
Expand Down Expand Up @@ -515,7 +526,7 @@ public Void visitNewArray(NewArrayTree node, Void unused) {
builder.close();
}
if (node.getInitializers() != null) {
if (node.getType() != null) {
if (!useAospStyle() && node.getType() != null) {
builder.space();
}
visitArrayInitializer(node.getInitializers());
Expand All @@ -525,14 +536,16 @@ public Void visitNewArray(NewArrayTree node, Void unused) {

public boolean visitArrayInitializer(List<? extends ExpressionTree> expressions) {
int cols;
final Indent.Const initializerIndent = useAospStyle() ? plusFour : plusTwo;
final Indent.Const initializerUnindent = useAospStyle() ? minusFour : minusTwo;
if (expressions.isEmpty()) {
tokenBreakTrailingComment("{", plusTwo);
if (builder.peekToken().equals(Optional.of(","))) {
token(",");
}
token("}", plusTwo);
} else if ((cols = argumentsAreTabular(expressions)) != -1) {
builder.open(plusTwo);
builder.open(initializerIndent);
token("{");
builder.forcedBreak();
boolean afterFirstToken = false;
Expand All @@ -554,9 +567,9 @@ public boolean visitArrayInitializer(List<? extends ExpressionTree> expressions)
builder.close();
afterFirstToken = true;
}
builder.breakOp(minusTwo);
builder.breakOp(initializerUnindent);
builder.close();
token("}", plusTwo);
token("}", initializerIndent);
} else {
// Special-case the formatting of array initializers inside annotations
// to more eagerly use a one-per-line layout.
Expand All @@ -576,8 +589,8 @@ public boolean visitArrayInitializer(List<? extends ExpressionTree> expressions)
boolean shortItems = hasOnlyShortItems(expressions);
boolean allowFilledElementsOnOwnLine = shortItems || !inMemberValuePair;

builder.open(plusTwo);
tokenBreakTrailingComment("{", plusTwo);
builder.open(initializerIndent);
tokenBreakTrailingComment("{", initializerIndent);
boolean hasTrailingComma = hasTrailingToken(builder.getInput(), expressions, ",");
builder.breakOp(hasTrailingComma ? FillMode.FORCED : FillMode.UNIFIED, "", ZERO);
if (allowFilledElementsOnOwnLine) {
Expand All @@ -597,9 +610,9 @@ public boolean visitArrayInitializer(List<? extends ExpressionTree> expressions)
if (allowFilledElementsOnOwnLine) {
builder.close();
}
builder.breakOp(minusTwo);
builder.breakOp(initializerUnindent);
builder.close();
token("}", plusTwo);
token("}", initializerIndent);
}
return false;
}
Expand Down Expand Up @@ -731,7 +744,11 @@ public Void visitNewClass(NewClassTree node, Void unused) {
visitAnnotations(annotations, BreakOrNot.NO, BreakOrNot.YES);
}
scan(node.getIdentifier(), null);
addArguments(node.getArguments(), plusFour);
if (useAospStyle()) {
addArguments(node.getArguments(), this.dotExpressionLevel > 0 ? ZERO : plusFour);
} else {
addArguments(node.getArguments(), plusFour);
}
builder.close();
if (node.getClassBody() != null) {
addBodyDeclarations(
Expand Down Expand Up @@ -777,7 +794,7 @@ public Void visitDoWhileLoop(DoWhileLoopTree node, Void unused) {
token("do");
visitStatement(
node.getStatement(),
CollapseEmptyOrNot.YES,
useAospStyle() ? CollapseEmptyOrNot.NO : CollapseEmptyOrNot.YES,
AllowLeadingBlankLine.YES,
AllowTrailingBlankLine.YES);
if (node.getStatement().getKind() == BLOCK) {
Expand Down Expand Up @@ -1261,7 +1278,20 @@ public Void visitBinary(BinaryTree node, Void unused) {
List<String> operators = new ArrayList<>();
walkInfix(precedence(node), node, operands, operators);
FillMode fillMode = hasOnlyShortItems(operands) ? INDEPENDENT : UNIFIED;
builder.open(plusFour);

// Do not double indent subexpressions of a binary operator in aosp style
boolean isParentExpression = !this.inBinaryExpression;
if (useAospStyle()) {
if (isParentExpression) {
builder.open(operators.get(0).equals("+") ? plusTwo : plusFour);
} else {
builder.open(ZERO);
}
this.inBinaryExpression = true;
} else {
builder.open(plusFour);
}

scan(operands.get(0), null);
int operatorsN = operators.size();
for (int i = 0; i < operatorsN; i++) {
Expand All @@ -1270,6 +1300,11 @@ public Void visitBinary(BinaryTree node, Void unused) {
builder.space();
scan(operands.get(i + 1), null);
}

if (isParentExpression) {
this.inBinaryExpression = false;
}

builder.close();
return null;
}
Expand Down Expand Up @@ -1643,7 +1678,11 @@ public Void visitMethod(MethodTree node, Void unused) {

private void methodBody(MethodTree node) {
if (node.getBody().getStatements().isEmpty()) {
builder.blankLineWanted(BlankLineWanted.NO);
if (useAospStyle()) {
builder.forcedBreak();
} else {
builder.blankLineWanted(BlankLineWanted.NO);
}
} else {
builder.open(plusTwo);
builder.forcedBreak();
Expand Down Expand Up @@ -3062,13 +3101,15 @@ void visitDot(ExpressionTree node0) {
token(".");
} else {
builder.open(plusFour);
this.dotExpressionLevel++;
scan(getArrayBase(node), null);
builder.breakOp();
needDot = true;
}
formatArrayIndices(getArrayIndices(node));
if (stack.isEmpty()) {
builder.close();
this.dotExpressionLevel--;
return;
}
}
Expand Down Expand Up @@ -3139,6 +3180,7 @@ void visitDot(ExpressionTree node0) {

if (node != null) {
builder.close();
this.dotExpressionLevel--;
}
}

Expand Down Expand Up @@ -3235,7 +3277,7 @@ private void visitDotWithPrefix(
// Are there method invocations or field accesses after the prefix?
boolean trailingDereferences = !prefixes.isEmpty() && getLast(prefixes) < items.size() - 1;

builder.open(plusFour);
builder.open(useAospStyle() && inBinaryExpression ? ZERO : plusFour);
for (int times = 0; times < prefixes.size(); times++) {
builder.open(ZERO);
}
Expand Down Expand Up @@ -3898,7 +3940,11 @@ protected void addBodyDeclarations(
if (braces.isYes()) {
builder.space();
tokenBreakTrailingComment("{", plusTwo);
builder.blankLineWanted(BlankLineWanted.NO);
if (useAospStyle()) {
builder.forcedBreak();
} else {
builder.blankLineWanted(BlankLineWanted.NO);
}
builder.open(ZERO);
if (builder.peekToken().equals(Optional.of(";"))) {
builder.open(plusTwo);
Expand Down Expand Up @@ -3961,7 +4007,11 @@ private void classDeclarationTypeList(String token, List<? extends Tree> types)
return;
}
builder.breakToFill(" ");
builder.open(types.size() > 1 ? plusFour : ZERO);
if (useAospStyle()) {
builder.open(ZERO);
} else {
builder.open(types.size() > 1 ? plusFour : ZERO);
}
token(token);
builder.space();
boolean afterFirstToken = false;
Expand Down Expand Up @@ -4049,15 +4099,16 @@ private static Direction canLocalHaveHorizontalAnnotations(ModifiersTree modifie

/**
* Should a field with a set of modifiers be declared with horizontal annotations? This is
* currently true if all annotations are parameterless annotations.
* currently true if all annotations are parameterless annotations and we are not using
* AOSP style.
*/
private static Direction fieldAnnotationDirection(ModifiersTree modifiers) {
private Direction fieldAnnotationDirection(ModifiersTree modifiers) {
for (AnnotationTree annotation : modifiers.getAnnotations()) {
if (!annotation.getArguments().isEmpty()) {
return Direction.VERTICAL;
}
}
return Direction.HORIZONTAL;
return useAospStyle() ? Direction.VERTICAL : Direction.HORIZONTAL;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package com.google.googlejavaformat.java.java21;

import com.google.googlejavaformat.OpsBuilder;
import com.google.googlejavaformat.java.JavaFormatterOptions.Style;
import com.google.googlejavaformat.java.JavaInputAstVisitor;
import com.sun.source.tree.CaseTree;
import com.sun.source.tree.ConstantCaseLabelTree;
Expand All @@ -33,8 +34,8 @@
*/
public class Java21InputAstVisitor extends JavaInputAstVisitor {

public Java21InputAstVisitor(OpsBuilder builder, int indentMultiplier) {
super(builder, indentMultiplier);
public Java21InputAstVisitor(OpsBuilder builder, int indentMultiplier, Style style) {
super(builder, indentMultiplier, style);
}

@Override
Expand Down