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.

Sign up for GitHub

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

Jump to bottom

This adds an AOSP formatter to the Eclipse plugin. #251

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

This adds an AOSP formatter to the Eclipse plugin. #251

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
002651f
This adds an AOSP formatter to the Eclipse plugin. This is achieved
Feb 6, 2018
478e67c
Removed unnecessary formatting changes caused by Eclipse.
Feb 6, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion core/pom.xml
View file
Open in desktop
Expand Up @@ -22,7 +22,7 @@
<parent>
<groupId>com.google.googlejavaformat</groupId>
<artifactId>google-java-format-parent</artifactId>
<version>1.6-SNAPSHOT</version>
<version>1.6</version>
</parent>

<artifactId>google-java-format</artifactId>
Expand Down
68 changes: 40 additions & 28 deletions core/src/main/java/com/google/googlejavaformat/java/Formatter.java
View file
Open in desktop
Expand Up @@ -16,27 +16,13 @@

import static java.nio.charset.StandardCharsets.UTF_8;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.common.collect.Iterators;
import com.google.common.collect.Range;
import com.google.common.collect.RangeSet;
import com.google.common.collect.TreeRangeSet;
import com.google.common.io.CharSink;
import com.google.common.io.CharSource;
import com.google.errorprone.annotations.Immutable;
import com.google.googlejavaformat.Doc;
import com.google.googlejavaformat.DocBuilder;
import com.google.googlejavaformat.FormattingError;
import com.google.googlejavaformat.Newlines;
import com.google.googlejavaformat.Op;
import com.google.googlejavaformat.OpsBuilder;
import java.io.IOError;
import java.io.IOException;
import java.net.URI;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;

import org.openjdk.javax.tools.Diagnostic;
import org.openjdk.javax.tools.DiagnosticCollector;
import org.openjdk.javax.tools.DiagnosticListener;
Expand All @@ -52,6 +38,22 @@
import org.openjdk.tools.javac.util.Log;
import org.openjdk.tools.javac.util.Options;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.common.collect.Iterators;
import com.google.common.collect.Range;
import com.google.common.collect.RangeSet;
import com.google.common.collect.TreeRangeSet;
import com.google.common.io.CharSink;
import com.google.common.io.CharSource;
import com.google.errorprone.annotations.Immutable;
import com.google.googlejavaformat.Doc;
import com.google.googlejavaformat.DocBuilder;
import com.google.googlejavaformat.FormattingError;
import com.google.googlejavaformat.Newlines;
import com.google.googlejavaformat.Op;
import com.google.googlejavaformat.OpsBuilder;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems the imports were reordered according to Eclipse's default ordering, instead of retaining the order specified by the style guide.


/**
* This is google-java-format, a new Java formatter that follows the Google Java Style Guide quite
* precisely---to the letter and to the spirit.
Expand All @@ -65,8 +67,8 @@
* is a final EOF token to hold final comments.
*
* <p>The formatter walks the AST to generate a Greg Nelson/Derek Oppen-style list of formatting
* {@link Op}s [1--2] that then generates a structured {@link Doc}. Each AST node type has a visitor
* to emit a sequence of {@link Op}s for the node.
* {@link Op}s [1--2] that then generates a structured {@link Doc} . Each AST node type has a
* visitor to emit a sequence of {@link Op}s for the node.
*
* <p>Some data-structure operations are easier in the list of {@link Op}s, while others become
* easier in the {@link Doc}. The {@link Op}s are walked to attach the comments. As the {@link Op}s
Expand Down Expand Up @@ -100,9 +102,14 @@ public Formatter(JavaFormatterOptions options) {
this.options = options;
}

/** @return The options used for this formatter. */
public JavaFormatterOptions options() {
return options;
}

/**
* Construct a {@code Formatter} given a Java compilation unit. Parses the code; builds a {@link
* JavaInput} and the corresponding {@link JavaOutput}.
* JavaInput} and the corresponding {@link JavaOutput} .
*
* @param javaInput the input, a Java compilation unit
* @param javaOutput the {@link JavaOutput}
Expand All @@ -114,7 +121,8 @@ static void format(final JavaInput javaInput, JavaOutput javaOutput, JavaFormatt
DiagnosticCollector<JavaFileObject> diagnostics = new DiagnosticCollector<>();
context.put(DiagnosticListener.class, diagnostics);
Options.instance(context).put("allowStringFolding", "false");
// TODO(cushon): this should default to the latest supported source level, remove this after
// TODO(cushon): this should default to the latest supported source
// level, remove this after
// backing out
// https://github.com/google/error-prone-javac/commit/c97f34ddd2308302587ce2de6d0c984836ea5b9f
Options.instance(context).put(Option.SOURCE, "9");
Expand All @@ -138,9 +146,9 @@ public CharSequence getCharContent(boolean ignoreEncodingErrors) throws IOExcept
JavacParser parser =
parserFactory.newParser(
javaInput.getText(),
/*keepDocComments=*/ true,
/*keepEndPos=*/ true,
/*keepLineMap=*/ true);
/* keepDocComments= */ true,
/* keepEndPos= */ true,
/* keepLineMap= */ true);
unit = parser.parseCompilationUnit();
unit.sourcefile = source;

Expand Down Expand Up @@ -168,7 +176,8 @@ static boolean errorDiagnostic(Diagnostic<?> input) {
}
switch (input.getCode()) {
case "compiler.err.invalid.meth.decl.ret.type.req":
// accept constructor-like method declarations that don't match the name of their
// accept constructor-like method declarations that don't match the
// name of their
// enclosing class
return false;
default:
Expand Down Expand Up @@ -210,8 +219,8 @@ public String formatSource(String input) throws FormatterException {
* @param input the input string
* @return the output string
* @throws FormatterException if the input string cannot be parsed
* @see <a
* href="https://google.github.io/styleguide/javaguide.html#s3.3.3-import-ordering-and-spacing">
* @see <a href=
* "https://google.github.io/styleguide/javaguide.html#s3.3.3-import-ordering-and-spacing">
* Google Java Style Guide - 3.3.3 Import ordering and spacing</a>
*/
public String formatSourceAndFixImports(String input) throws FormatterException {
Expand Down Expand Up @@ -246,8 +255,10 @@ public ImmutableList<Replacement> getFormatReplacements(
String input, Collection<Range<Integer>> characterRanges) throws FormatterException {
JavaInput javaInput = new JavaInput(input);

// TODO(cushon): this is only safe because the modifier ordering doesn't affect whitespace,
// and doesn't change the replacements that are output. This is not true in general for
// TODO(cushon): this is only safe because the modifier ordering doesn't
// affect whitespace,
// and doesn't change the replacements that are output. This is not true
// in general for
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you reformatted the code, maybe manually fix this section?

// 'de-linting' changes (e.g. import ordering).
javaInput = ModifierOrderer.reorderModifiers(javaInput, characterRanges);

Expand Down Expand Up @@ -276,7 +287,8 @@ public static RangeSet<Integer> lineRangesToCharRanges(
for (Range<Integer> lineRange :
lineRanges.subRangeSet(Range.closedOpen(0, lines.size() - 1)).asRanges()) {
int lineStart = lines.get(lineRange.lowerEndpoint());
// Exclude the trailing newline. This isn't strictly necessary, but handling blank lines
// Exclude the trailing newline. This isn't strictly necessary, but
// handling blank lines
// as empty ranges is convenient.
int lineEnd = lines.get(lineRange.upperEndpoint()) - 1;
Range<Integer> range = Range.closedOpen(lineStart, lineEnd);
Expand Down
36 changes: 26 additions & 10 deletions core/src/main/java/com/google/googlejavaformat/java/SnippetFormatter.java
View file
Open in desktop
Expand Up @@ -14,14 +14,15 @@

package com.google.googlejavaformat.java;

import java.util.ArrayList;
import java.util.List;

import com.google.common.base.CharMatcher;
import com.google.common.base.Preconditions;
import com.google.common.collect.DiscreteDomain;
import com.google.common.collect.Range;
import com.google.common.collect.RangeSet;
import com.google.common.collect.TreeRangeSet;
import java.util.ArrayList;
import java.util.List;

/** Formats a subset of a compilation unit. */
public class SnippetFormatter {
Expand Down Expand Up @@ -56,16 +57,25 @@ public void closeBraces(int initialIndent) {
}
}

private static final int INDENTATION_SIZE = 2;
private final Formatter formatter = new Formatter();
private static final int SPACES_PER_INDENT_LEVEL = 2;
private final Formatter formatter;
private static final CharMatcher NOT_WHITESPACE = CharMatcher.whitespace().negate();

public SnippetFormatter() {
this(JavaFormatterOptions.defaultOptions());
}

public SnippetFormatter(JavaFormatterOptions options) {
Preconditions.checkNotNull(options);
this.formatter = new Formatter(options);
}

public String createIndentationString(int indentationLevel) {
Preconditions.checkArgument(
indentationLevel >= 0,
"Indentation level cannot be less than zero. Given: %s",
indentationLevel);
int spaces = indentationLevel * INDENTATION_SIZE;
int spaces = indentationLevel * indentationSize();
StringBuilder buf = new StringBuilder(spaces);
for (int i = 0; i < spaces; i++) {
buf.append(' ');
Expand Down Expand Up @@ -134,9 +144,10 @@ private static List<Replacement> toReplacements(String source, String replacemen
"source = \"" + source + "\", replacement = \"" + replacement + "\"");
}
/*
* In the past we seemed to have problems touching non-whitespace text in the formatter, even
* just replacing some code with itself. Retrospective attempts to reproduce this have failed,
* but this may be an issue for future changes.
* In the past we seemed to have problems touching non-whitespace text
* in the formatter, even just replacing some code with itself.
* Retrospective attempts to reproduce this have failed, but this may be
* an issue for future changes.
*/
List<Replacement> replacements = new ArrayList<>();
int i = NOT_WHITESPACE.indexIn(source);
Expand All @@ -163,8 +174,9 @@ private static List<Replacement> toReplacements(String source, String replacemen

private SnippetWrapper snippetWrapper(SnippetKind kind, String source, int initialIndent) {
/*
* Synthesize a dummy class around the code snippet provided by Eclipse. The dummy class is
* correctly formatted -- the blocks use correct indentation, etc.
* Synthesize a dummy class around the code snippet provided by Eclipse.
* The dummy class is correctly formatted -- the blocks use correct
* indentation, etc.
*/
switch (kind) {
case COMPILATION_UNIT:
Expand Down Expand Up @@ -215,4 +227,8 @@ private SnippetWrapper snippetWrapper(SnippetKind kind, String source, int initi
throw new IllegalArgumentException("Unknown snippet kind: " + kind);
}
}

private int indentationSize() {
return SPACES_PER_INDENT_LEVEL * formatter.options().indentationMultiplier();
}
}
6 changes: 3 additions & 3 deletions eclipse_plugin/META-INF/MANIFEST.MF
View file
Open in desktop
Expand Up @@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: google-java-format
Bundle-SymbolicName: google-java-format-eclipse-plugin;singleton:=true
Bundle-Version: 1.5.0
Bundle-Version: 1.6.0
Bundle-RequiredExecutionEnvironment: JavaSE-1.8
Require-Bundle: org.eclipse.jdt.core;bundle-version="3.10.0",
org.eclipse.jface,
Expand All @@ -11,5 +11,5 @@ Require-Bundle: org.eclipse.jdt.core;bundle-version="3.10.0",
org.eclipse.equinox.common
Bundle-ClassPath: .,
lib/guava-22.0.jar,
lib/javac-shaded-9-dev-r4023-3.jar,
lib/google-java-format-1.5.jar
lib/javac-shaded-9+181-r4173-1.jar,
lib/google-java-format-1.6.jar
4 changes: 2 additions & 2 deletions eclipse_plugin/build.properties
View file
Open in desktop
Expand Up @@ -3,6 +3,6 @@ output.. = target/classes
bin.includes = META-INF/,\
.,\
plugin.xml,\
lib/javac-shaded-9-dev-r4023-3.jar,\
lib/javac-shaded-9+181-r4173-1.jar,\
lib/guava-22.0.jar,\
lib/google-java-format-1.5.jar
lib/google-java-format-1.6.jar
7 changes: 6 additions & 1 deletion eclipse_plugin/plugin.xml
View file
Open in desktop
Expand Up @@ -5,7 +5,12 @@
<javaFormatter
class="com.google.googlejavaformat.java.GoogleJavaFormatter"
id="com.google.googlejavaformat.java.GoogleJavaFormatter"
name="google-java-format 1.4">
name="google-java-format 1.6">
</javaFormatter>
<javaFormatter
class="com.google.googlejavaformat.java.AospJavaFormatter"
id="com.google.googlejavaformat.java.AospJavaFormatter"
name="aosp-java-format 1.6">
</javaFormatter>
</extension>
</plugin>
6 changes: 3 additions & 3 deletions eclipse_plugin/pom.xml
View file
Open in desktop
Expand Up @@ -20,11 +20,11 @@
<parent>
<groupId>com.google.googlejavaformat</groupId>
<artifactId>google-java-format-parent</artifactId>
<version>1.5</version>
<version>1.6</version>
</parent>

<artifactId>google-java-format-eclipse-plugin</artifactId>
<version>1.5.0</version>
<version>1.6.0</version>
<packaging>eclipse-plugin</packaging>
<name>google-java-format Plugin for Eclipse 4.5+</name>

Expand All @@ -48,7 +48,7 @@
<dependency>
<groupId>com.google.googlejavaformat</groupId>
<artifactId>google-java-format</artifactId>
<version>1.5</version>
<version>${project.parent.version}</version>
</dependency>
</dependencies>

Expand Down