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

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
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
2 changes: 1 addition & 1 deletion core/pom.xml
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,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,6 +100,11 @@ 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}.
Expand Down
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
/*
* Copyright 2017 Google Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
* or implied. See the License for the specific language governing permissions and limitations under
* the License.
*/

package com.google.googlejavaformat.java;

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

import org.eclipse.jdt.core.dom.ASTParser;
import org.eclipse.jdt.core.formatter.CodeFormatter;
import org.eclipse.jface.text.IRegion;
import org.eclipse.jface.text.Region;
import org.eclipse.text.edits.MultiTextEdit;
import org.eclipse.text.edits.ReplaceEdit;
import org.eclipse.text.edits.TextEdit;

import com.google.common.base.Preconditions;
import com.google.common.collect.Range;
import com.google.googlejavaformat.java.SnippetFormatter.SnippetKind;

/**
* This class provides the bulk of the logic to run java formatter within Eclipse. It provides the
* ability to formatting options to be specified.
*/
public abstract class AbstractJavaFormatter extends CodeFormatter {

@Override
public TextEdit format(
int kind, String source, int offset, int length, int indentationLevel, String lineSeparator) {
IRegion[] regions = new IRegion[] {new Region(offset, length)};
return formatInternal(kind, source, regions, indentationLevel);
}

@Override
public TextEdit format(
int kind, String source, IRegion[] regions, int indentationLevel, String lineSeparator) {
return formatInternal(kind, source, regions, indentationLevel);
}

@Override
public String createIndentationString(int indentationLevel) {
Preconditions.checkArgument(
indentationLevel >= 0,
"Indentation level cannot be less than zero. Given: %s",
indentationLevel);
int spaces = indentationLevel * indentationSize();
StringBuilder buf = new StringBuilder(spaces);
for (int i = 0; i < spaces; i++) {
buf.append(' ');
}
return buf.toString();
}

/** Runs the Google Java formatter on the given source, with only the given ranges specified. */
private TextEdit formatInternal(int kind, String source, IRegion[] regions, int initialIndent) {
try {
boolean includeComments =
(kind & CodeFormatter.F_INCLUDE_COMMENTS) == CodeFormatter.F_INCLUDE_COMMENTS;
kind &= ~CodeFormatter.F_INCLUDE_COMMENTS;
SnippetKind snippetKind;
switch (kind) {
case ASTParser.K_EXPRESSION:
snippetKind = SnippetKind.EXPRESSION;
break;
case ASTParser.K_STATEMENTS:
snippetKind = SnippetKind.STATEMENTS;
break;
case ASTParser.K_CLASS_BODY_DECLARATIONS:
snippetKind = SnippetKind.CLASS_BODY_DECLARATIONS;
break;
case ASTParser.K_COMPILATION_UNIT:
snippetKind = SnippetKind.COMPILATION_UNIT;
break;
default:
throw new IllegalArgumentException(String.format("Unknown snippet kind: %d", kind));
}
List<Replacement> replacements =
createSnippetFormatter()
.format(
snippetKind, source, rangesFromRegions(regions), initialIndent, includeComments);
if (idempotent(source, regions, replacements)) {
// Do not create edits if there's no diff.
return null;
}
// Convert replacements to text edits.
return editFromReplacements(replacements);
} catch (IllegalArgumentException | FormatterException exception) {
// Do not format on errors.
return null;
}
}

private List<Range<Integer>> rangesFromRegions(IRegion[] regions) {
List<Range<Integer>> ranges = new ArrayList<>();
for (IRegion region : regions) {
ranges.add(Range.closedOpen(region.getOffset(), region.getOffset() + region.getLength()));
}
return ranges;
}

/** @return {@code true} if input and output texts are equal, else {@code false}. */
private boolean idempotent(String source, IRegion[] regions, List<Replacement> replacements) {
// This implementation only checks for single replacement.
if (replacements.size() == 1) {
Replacement replacement = replacements.get(0);
String output = replacement.getReplacementString();
// Entire source case: input = output, nothing changed.
if (output.equals(source)) {
return true;
}
// Single region and single replacement case: if they are equal,
// nothing changed.
if (regions.length == 1) {
Range<Integer> range = replacement.getReplaceRange();
String snippet = source.substring(range.lowerEndpoint(), range.upperEndpoint());
if (output.equals(snippet)) {
return true;
}
}
}
return false;
}

private TextEdit editFromReplacements(List<Replacement> replacements) {
// Split the replacements that cross line boundaries.
TextEdit edit = new MultiTextEdit();
for (Replacement replacement : replacements) {
Range<Integer> replaceRange = replacement.getReplaceRange();
edit.addChild(
new ReplaceEdit(
replaceRange.lowerEndpoint(),
replaceRange.upperEndpoint() - replaceRange.lowerEndpoint(),
replacement.getReplacementString()));
}
return edit;
}

/**
* Create an instance of the formatter that will be used. This method should be aligned with
* {@link #indentationSize()} such that the indentation size corresponds with the indent level of
* the format options used to create the snippet formatter.
*/
protected abstract SnippetFormatter createSnippetFormatter();

/**
* The number of spaces per indent level. This method should be aligned with {@link
* #createSnippetFormatter()} such that the indentation size corresponds with the indent level of
* the format options used to create the snippet formatter.
*/
protected abstract int indentationSize();
Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 maybe SnippetFormatter#indentationSize() can be made public instead? That does mean a modification to its public API, but since this caller could use it, maybe so can others.

Copy link
Author

Choose a reason for hiding this comment

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

I can make it public, though I tend to prefer more restrictive access until a need arises. That said, I'll make change if you prefer. Please advise.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, it seems a need arose ;).

But in all seriousness: I'm not a maintainer of this project, so my advice carries little weight. Let's see what the maintainers say.

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*
* Copyright 2018 Google Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
* or implied. See the License for the specific language governing permissions and limitations under
* the License.
*/
package com.google.googlejavaformat.java;

import com.google.googlejavaformat.java.JavaFormatterOptions.Style;

/** Runs the AOSP Java formatter on the given code. */
public class AospJavaFormatter extends AbstractJavaFormatter {

@Override
protected SnippetFormatter createSnippetFormatter() {
return new SnippetFormatter(JavaFormatterOptions.builder().style(Style.AOSP).build());
}

@Override
protected int indentationSize() {
return 4;
}
}
Loading