Skip to content

Commit

Permalink
Fix import reorderer to handle single-line-comments in imports
Browse files Browse the repository at this point in the history 
MOE_MIGRATED_REVID=240781176
  • Loading branch information
@TimvdLippe @cushon
TimvdLippe authored and cushon committed Apr 3, 2019
1 parent 5e114ca commit f246638
Show file tree
Hide file tree
Showing 2 changed files with 144 additions and 22 deletions.
31 changes: 22 additions & 9 deletions core/src/main/java/com/google/googlejavaformat/java/ImportOrderer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,15 +70,20 @@ private class Import implements Comparable<Import> {
/** The name being imported, for example {@code java.util.List}. */
final String imported;

/** The characters after the final {@code ;}, up to and including the line terminator. */
/**
* The {@code //} comment lines after the final {@code ;}, up to and including the line
* terminator of the last one. Note: In case two imports were separated by a space (which is
* disallowed by the style guide), the trailing whitespace of the first import does not include
* a line terminator.
*/
final String trailing;

/** True if this is {@code import static}. */
final boolean isStatic;

Import(String imported, String trailing, boolean isStatic) {
this.imported = imported;
this.trailing = trailing.trim();
this.trailing = trailing;
this.isStatic = isStatic;
}

Expand All @@ -91,7 +96,8 @@ public int compareTo(Import that) {
return this.imported.compareTo(that.imported);
}

// This is a complete line to be output for this import, including the line terminator.
// One or multiple lines, the import itself and following comments, including the line
// terminator.
@Override
public String toString() {
StringBuilder sb = new StringBuilder();
Expand All @@ -100,10 +106,11 @@ public String toString() {
sb.append("static ");
}
sb.append(imported).append(';');
if (!trailing.isEmpty()) {
sb.append(' ').append(trailing);
if (trailing.trim().isEmpty()) {
sb.append(lineSeparator);
} else {
sb.append(trailing);
}
sb.append(lineSeparator);
return sb.toString();
}
}
Expand Down Expand Up @@ -175,7 +182,7 @@ private static class ImportsAndIndex {
* <imports> -> (<end-of-line> | <import>)*
* <import> -> "import" <whitespace> ("static" <whitespace>)?
* <identifier> ("." <identifier>)* ("." "*")? <whitespace>? ";"
* <whitespace>? <line-comment>? <end-of-line>
* <whitespace>? <end-of-line>? (<line-comment> <end-of-line>)*
* }</pre>
*
* @param i the index to start parsing at.
Expand Down Expand Up @@ -221,13 +228,19 @@ private ImportsAndIndex scanImports(int i) throws FormatterException {
trailing.append(tokenAt(i));
i++;
}
if (isSlashSlashCommentToken(i)) {
if (isNewlineToken(i)) {
trailing.append(tokenAt(i));
i++;
}
if (isNewlineToken(i)) {
// Gather (if any) all single line comments and accompanied line terminators following this
// import
while (isSlashSlashCommentToken(i)) {
trailing.append(tokenAt(i));
i++;
if (isNewlineToken(i)) {
trailing.append(tokenAt(i));
i++;
}
}
imports.add(new Import(importedName, trailing.toString(), isStatic));
// Remember the position just after the import we just saw, before skipping blank lines.
Expand Down
135 changes: 122 additions & 13 deletions core/src/test/java/com/google/googlejavaformat/java/ImportOrdererTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,128 @@ public static Collection<Object[]> parameters() {
"public class Blim {}",
},
},
{
{
"import java.util.Collection;",
"// BUG: diagnostic contains",
"import java.util.List;",
"",
"class B74235047 {}"
},
{
"import java.util.Collection;",
"// BUG: diagnostic contains",
"import java.util.List;",
"",
"class B74235047 {}"
}
},
{
{
"import java.util.Set;",
"import java.util.Collection;",
"// BUG: diagnostic contains",
"import java.util.List;",
"",
"class B74235047 {}"
},
{
"import java.util.Collection;",
"// BUG: diagnostic contains",
"import java.util.List;",
"import java.util.Set;",
"",
"class B74235047 {}"
}
},
{
{
"import java.util.List;",
"// BUG: diagnostic contains",
"import java.util.Set;",
"import java.util.Collection;",
"",
"class B74235047 {}"
},
{
"import java.util.Collection;",
"import java.util.List;",
"// BUG: diagnostic contains",
"import java.util.Set;",
"",
"class B74235047 {}"
}
},
{
{
"// BEGIN-STRIP",
"import com.google.testing.testsize.MediumTest;",
"import com.google.testing.testsize.MediumTestAttribute;",
"// END-STRIP",
"",
"class B74235047 {}"
},
{
"// BEGIN-STRIP",
"import com.google.testing.testsize.MediumTest;",
"import com.google.testing.testsize.MediumTestAttribute;",
"// END-STRIP",
"",
"class B74235047 {}"
}
},
{
{
"import com.google.testing.testsize.MediumTest; // Keep this import",
"import com.google.testing.testsize.MediumTestAttribute; // Keep this import",
"",
"class B74235047 {}"
},
{
"import com.google.testing.testsize.MediumTest; // Keep this import",
"import com.google.testing.testsize.MediumTestAttribute; // Keep this import",
"",
"class B74235047 {}"
}
},
{
{
"import java.util.Set;",
"import java.util.List;",
"",
"// This comment doesn't get moved because of the blank line.",
"",
"class B74235047 {}"
},
{
"import java.util.List;",
"import java.util.Set;",
"",
"// This comment doesn't get moved because of the blank line.",
"",
"class B74235047 {}"
}
},
{
{
"import b.B;",
"// MOE: end_strip",
"import c.C;",
"// MOE: begin_strip",
"import a.A;",
"",
"class B74235047 {}"
},
{
"import a.A;",
"import b.B;",
"// MOE: end_strip",
"import c.C;",
"// MOE: begin_strip",
"",
"class B74235047 {}"
}
},
{
{
// Check double semicolons
Expand Down Expand Up @@ -256,19 +378,6 @@ public static Collection<Object[]> parameters() {
"!!Could not parse imported name, at: ",
}
},
{
{
"package com.google.example;",
"",
"import com.foo.Second;",
"import com.foo.First;",
"// we don't support comments between imports",
"import com.foo.Third;",
},
{
"!!Imports not contiguous (perhaps a comment separates them?)",
}
},
{
{
"import com.foo.Second;",
Expand Down

0 comments on commit f246638

Please sign in to comment.