Skip to content

Commit eb00300

Browse files
eamonnmcmanuscushon
eamonnmcmanus
authored and
cushon
committed
Add hidden option to sort imports
The new options are --sort-imports=also to trigger import sorting, and --sort-imports=only to sort imports and do nothing else. Eventually, g-j-f should just sort imports always, but it can't do that until the new import order is fully established. (More accurately, it should sort imports if all the imports are inside the text to be formatted.) ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=117755463
1 parent c0f147a commit eb00300

File tree

8 files changed

+211
-21
lines changed

8 files changed

+211
-21
lines changed

‎core/src/main/java/com/google/googlejavaformat/java/FileToFormatPath.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
import com.google.common.collect.RangeSet;
1818

19+
import java.io.BufferedInputStream;
1920
import java.io.IOException;
2021
import java.io.InputStream;
2122
import java.nio.file.Files;
@@ -45,6 +46,6 @@ public String fileName() {
4546

4647
@Override
4748
public InputStream inputStream() throws IOException {
48-
return Files.newInputStream(path);
49+
return new BufferedInputStream(Files.newInputStream(path));
4950
}
5051
}

‎core/src/main/java/com/google/googlejavaformat/java/FormatFileCallable.java‎

Lines changed: 79 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import com.google.common.io.CharStreams;
2424
import com.google.googlejavaformat.FormatterDiagnostic;
2525

26+
import java.io.BufferedOutputStream;
2627
import java.io.File;
2728
import java.io.FileOutputStream;
2829
import java.io.IOException;
@@ -49,6 +50,7 @@ class FormatFileCallable implements Callable<Boolean> {
4950
private final Object outputLock;
5051
private final int indentMultiplier;
5152
private final boolean inPlace;
53+
private final SortImports sortImports;
5254
private final PrintWriter outWriter;
5355
private final PrintWriter errWriter;
5456

@@ -57,33 +59,58 @@ class FormatFileCallable implements Callable<Boolean> {
5759
Object outputLock,
5860
int indentMultiplier,
5961
boolean inPlace,
62+
SortImports sortImports,
6063
PrintWriter outWriter,
6164
PrintWriter errWriter) {
6265
Preconditions.checkArgument(
63-
!(inPlace && fileToFormat instanceof FileToFormatStdin), "Cannot format stdin in place");
66+
!(inPlace && fileToFormat instanceof FileToFormatStdin),
67+
"Cannot format stdin in place");
6468

6569
this.fileToFormat = Preconditions.checkNotNull(fileToFormat);
6670
this.outputLock = Preconditions.checkNotNull(outputLock);
6771
this.indentMultiplier = indentMultiplier;
6872
this.inPlace = inPlace;
73+
this.sortImports = sortImports;
6974
this.outWriter = Preconditions.checkNotNull(outWriter);
7075
this.errWriter = Preconditions.checkNotNull(errWriter);
7176
}
7277

78+
enum SortImports {
79+
NO,
80+
ONLY,
81+
ALSO
82+
}
83+
7384
/**
7485
* Formats a file and returns whether the operation succeeded.
7586
*/
7687
@Override
7788
public Boolean call() {
78-
String stringFromStream = readInput();
79-
if (stringFromStream == null) {
89+
String inputString = readInput();
90+
if (inputString == null) {
8091
return false;
8192
}
8293

94+
if (sortImports != SortImports.NO) {
95+
String reordered = reorderImports(inputString);
96+
if (reordered == null) {
97+
return false;
98+
}
99+
100+
if (sortImports == SortImports.ONLY) {
101+
if (reordered.equals(inputString)) {
102+
return true;
103+
}
104+
return writeString(reordered);
105+
}
106+
107+
inputString = reordered;
108+
}
109+
83110
JavaInput javaInput;
84-
RangeSet<Integer> tokens;
111+
final RangeSet<Integer> tokens;
85112
try {
86-
javaInput = new JavaInput(fileToFormat.fileName(), stringFromStream);
113+
javaInput = new JavaInput(fileToFormat.fileName(), inputString);
87114
tokens = TreeRangeSet.create();
88115
for (Range<Integer> lineRange : fileToFormat.lineRanges().asRanges()) {
89116
tokens.add(javaInput.lineRangeToTokenRange(lineRange));
@@ -111,7 +138,7 @@ public Boolean call() {
111138
}
112139
}
113140

114-
JavaOutput javaOutput = new JavaOutput(javaInput, new JavaCommentsHelper());
141+
final JavaOutput javaOutput = new JavaOutput(javaInput, new JavaCommentsHelper());
115142
List<FormatterDiagnostic> errors = new ArrayList<>();
116143
Formatter.format(javaInput, javaOutput, Formatter.MAX_WIDTH, errors, indentMultiplier);
117144
if (!errors.isEmpty()) {
@@ -123,7 +150,14 @@ public Boolean call() {
123150
return false;
124151
}
125152

126-
return writeOutput(javaOutput, tokens);
153+
Write writeTokens =
154+
new Write() {
155+
@Override
156+
public void write(Writer writer) throws IOException {
157+
javaOutput.writeMerged(writer, tokens);
158+
}
159+
};
160+
return writeOutput(writeTokens);
127161
}
128162

129163
@Nullable
@@ -145,11 +179,42 @@ private String readInput() {
145179
}
146180
}
147181

148-
private boolean writeOutput(JavaOutput javaOutput, RangeSet<Integer> tokens) {
182+
@Nullable
183+
private String reorderImports(String inputString) {
184+
try {
185+
return ImportOrderer.reorderImports(fileToFormat.fileName(), inputString);
186+
} catch (FormatterException e) {
187+
synchronized (outputLock) {
188+
errWriter
189+
.append(fileToFormat.fileName())
190+
.append(": error sorting imports: ")
191+
.append(e.getMessage())
192+
.append('\n')
193+
.flush();
194+
}
195+
return null;
196+
}
197+
}
198+
199+
interface Write {
200+
void write(Writer writer) throws IOException;
201+
}
202+
203+
private boolean writeString(final String s) {
204+
return writeOutput(
205+
new Write() {
206+
@Override
207+
public void write(Writer writer) throws IOException {
208+
writer.write(s);
209+
}
210+
});
211+
}
212+
213+
private boolean writeOutput(Write write) {
149214
if (!inPlace) {
150215
synchronized (outputLock) {
151216
try {
152-
javaOutput.writeMerged(outWriter, tokens);
217+
write.write(outWriter);
153218
} catch (IOException e) {
154219
errWriter.append("cannot write output: " + e.getMessage()).flush();
155220
}
@@ -158,8 +223,11 @@ private boolean writeOutput(JavaOutput javaOutput, RangeSet<Integer> tokens) {
158223
}
159224
} else {
160225
String tempFileName = fileToFormat.fileName() + '#';
161-
try (Writer writer = new OutputStreamWriter(new FileOutputStream(tempFileName), UTF_8)) {
162-
javaOutput.writeMerged(writer, tokens);
226+
try (Writer writer =
227+
new OutputStreamWriter(
228+
new BufferedOutputStream(new FileOutputStream(tempFileName)),
229+
UTF_8)) {
230+
write.write(writer);
163231
outWriter.flush();
164232
} catch (IOException e) {
165233
synchronized (outputLock) {

‎core/src/main/java/com/google/googlejavaformat/java/Main.java‎

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import com.google.common.collect.Range;
2323
import com.google.common.collect.RangeSet;
2424
import com.google.common.collect.TreeRangeSet;
25+
import com.google.googlejavaformat.java.FormatFileCallable.SortImports;
2526

2627
import com.beust.jcommander.JCommander;
2728
import com.beust.jcommander.Parameter;
@@ -83,6 +84,14 @@ private static final class FormatterParameters {
8384
names = {"--help", "-help", "-h"}, description = "Print an extended usage statement.")
8485
boolean helpFlag = false;
8586

87+
@Parameter(
88+
names = {"--sort-imports", "-sort-imports"},
89+
description = "Sort import statements. "
90+
+ "--sort-imports=only to sort imports but do no other formatting. "
91+
+ "--sort-imports=also to sort imports and do other formatting.",
92+
hidden = true)
93+
String sortImportsFlag = "";
94+
8695
// TODO(eaftan): clang-format formats stdin -> stdout when no options are passed. We should
8796
// match that behavior.
8897
@Parameter(names = "-", description = "Format stdin -> stdout.")
@@ -156,6 +165,25 @@ public int format(String... args) throws UsageException {
156165
if (argInfo.parameters.helpFlag) {
157166
argInfo.throwUsage();
158167
}
168+
SortImports sortImports;
169+
switch (argInfo.parameters.sortImportsFlag) {
170+
case "":
171+
sortImports = SortImports.NO;
172+
break;
173+
case "only":
174+
sortImports = SortImports.ONLY;
175+
break;
176+
case "also":
177+
sortImports = SortImports.ALSO;
178+
break;
179+
default:
180+
errWriter.println("Invalid value for --sort-imports. Should be \"only\" or \"also\".");
181+
return 1;
182+
}
183+
if (sortImports != SortImports.NO && argInfo.isSelection()) {
184+
errWriter.println("--sort-imports can currently only apply to the whole file");
185+
return 1;
186+
}
159187

160188
ConstructFilesToFormatResult constructFilesToFormatResult = constructFilesToFormat(argInfo);
161189
boolean allOkay = constructFilesToFormatResult.allOkay;
@@ -177,6 +205,7 @@ public int format(String... args) throws UsageException {
177205
outputLock,
178206
indentMultiplier,
179207
argInfo.parameters.iFlag,
208+
sortImports,
180209
outWriter,
181210
errWriter)));
182211
}
@@ -277,10 +306,7 @@ public static ArgInfo processArgs(String... args) throws UsageException {
277306
if (parameters.iFlag && parameters.fileNamesFlag.isEmpty()) {
278307
argInfo.throwUsage();
279308
}
280-
if (!(parameters.linesFlags.isEmpty()
281-
&& parameters.offsetFlags.isEmpty()
282-
&& parameters.lengthFlags.isEmpty()
283-
|| filesToFormat == 1)) {
309+
if (argInfo.isSelection() && filesToFormat != 1) {
284310
argInfo.throwUsage();
285311
}
286312
if (parameters.offsetFlags.size() != parameters.lengthFlags.size()) {
@@ -293,6 +319,11 @@ public static ArgInfo processArgs(String... args) throws UsageException {
293319
return argInfo;
294320
}
295321

322+
boolean isSelection() {
323+
return !parameters.linesFlags.isEmpty() || !parameters.offsetFlags.isEmpty()
324+
|| !parameters.lengthFlags.isEmpty();
325+
}
326+
296327
private ArgInfo(FormatterParameters parameters, JCommander jCommander) {
297328
this.parameters = parameters;
298329
this.jCommander = jCommander;

‎core/src/test/java/com/google/googlejavaformat/java/FormatterTest.java‎

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import static com.google.common.io.Files.getFileExtension;
1818
import static com.google.common.io.Files.getNameWithoutExtension;
1919
import static com.google.common.truth.Truth.assertThat;
20+
import static com.google.common.truth.Truth.assertWithMessage;
2021
import static org.junit.Assert.assertEquals;
2122
import static org.junit.Assert.assertTrue;
2223
import static org.junit.Assert.fail;
@@ -33,6 +34,7 @@
3334
import org.junit.runners.JUnit4;
3435

3536
import java.io.ByteArrayInputStream;
37+
import java.io.IOException;
3638
import java.io.InputStream;
3739
import java.io.InputStreamReader;
3840
import java.io.PrintWriter;
@@ -71,11 +73,7 @@ public void testFormatter() throws Exception {
7173
assertEquals("bad testdata file names", 1, subPath.getNameCount());
7274
String baseName = getNameWithoutExtension(subPath.getFileName().toString());
7375
String extension = getFileExtension(subPath.getFileName().toString());
74-
final String stringFromStream;
75-
try (InputStream stream = classLoader.getResourceAsStream(resourceName)) {
76-
stringFromStream =
77-
CharStreams.toString(new InputStreamReader(stream, StandardCharsets.UTF_8));
78-
}
76+
String stringFromStream = getResource(resourceName);
7977
switch (extension) {
8078
case "input":
8179
inputs.put(baseName, stringFromStream);
@@ -290,4 +288,46 @@ public void importsNotReorderedByDefault() throws FormatterException {
290288
+ "\n\npublic class ExampleTest {}\n";
291289
assertThat(output).isEqualTo(expect);
292290
}
291+
292+
@Test
293+
public void importOrderingWithoutFormatting() throws IOException, UsageException {
294+
importOrdering(
295+
"--sort-imports=only",
296+
"com/google/googlejavaformat/java/testimports/A.imports-only");
297+
}
298+
299+
@Test
300+
public void importOrderingAndFormatting() throws IOException, UsageException {
301+
importOrdering(
302+
"--sort-imports=also",
303+
"com/google/googlejavaformat/java/testimports/A.imports-and-formatting");
304+
}
305+
306+
private void importOrdering(String sortArg, String outputResourceName)
307+
throws IOException, UsageException {
308+
Path tmpdir = testFolder.newFolder().toPath();
309+
Path path = tmpdir.resolve("Foo.java");
310+
311+
String inputResourceName = "com/google/googlejavaformat/java/testimports/A.input";
312+
String input = getResource(inputResourceName);
313+
String expectedOutput = getResource(outputResourceName);
314+
Files.write(path, input.getBytes(StandardCharsets.UTF_8));
315+
316+
StringWriter out = new StringWriter();
317+
StringWriter err = new StringWriter();
318+
Main main = new Main(new PrintWriter(out, true), new PrintWriter(err, true), System.in);
319+
main.format(new String[] {sortArg, "-i", path.toString()});
320+
321+
assertThat(err.toString()).isEmpty();
322+
assertThat(out.toString()).isEmpty();
323+
String output = new String(Files.readAllBytes(path), StandardCharsets.UTF_8);
324+
assertThat(output).isEqualTo(expectedOutput);
325+
}
326+
327+
private String getResource(String resourceName) throws IOException {
328+
try (InputStream stream = getClass().getClassLoader().getResourceAsStream(resourceName)) {
329+
assertWithMessage("Missing resource: " + resourceName).that(stream).isNotNull();
330+
return CharStreams.toString(new InputStreamReader(stream, StandardCharsets.UTF_8));
331+
}
332+
}
293333
}

‎core/src/test/java/com/google/googlejavaformat/java/MainTest.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,4 +93,14 @@ public void testUsageOutput() {
9393
assertThat(usage).contains("the result is sent to stdout");
9494
}
9595
}
96+
97+
@Test
98+
public void cantImportSortRangeYet() throws UsageException {
99+
StringWriter out = new StringWriter();
100+
StringWriter err = new StringWriter();
101+
Main main = new Main(new PrintWriter(out, true), new PrintWriter(err, true), System.in);
102+
assertThat(main.format("-sort-imports=only", "-lines=5:10", "-")).isEqualTo(1);
103+
assertThat(err.toString()).contains(
104+
"--sort-imports can currently only apply to the whole file");
105+
}
96106
}

‎core/src/test/resources/com/google/googlejavaformat/java/testimports/A.imports-and-formatting‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package com.google.example;
2+
3+
import static com.google.truth.Truth.assertThat;
4+
import static org.junit.Assert.fail;
5+
6+
import com.google.common.base.Preconditions;
7+
import java.util.List;
8+
import javax.annotations.Nullable;
9+
import org.junit.runner.RunWith;
10+
import org.junit.runners.JUnit4;
11+
12+
@RunWith(JUnit4.class)
13+
public class SomeTest {}

‎core/src/test/resources/com/google/googlejavaformat/java/testimports/A.imports-only‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
package com.google.example;
2+
3+
import static com.google.truth.Truth.assertThat;
4+
import static org.junit.Assert.fail;
5+
6+
import com.google.common.base.Preconditions;
7+
import java.util.List;
8+
import javax.annotations.Nullable;
9+
import org.junit.runner.RunWith;
10+
import org.junit.runners.JUnit4;
11+
12+
@RunWith( JUnit4.class ) public class SomeTest {}

‎core/src/test/resources/com/google/googlejavaformat/java/testimports/A.input‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
package com.google.example;
2+
3+
import com.google.common.base.Preconditions;
4+
5+
import org.junit.runner.RunWith;
6+
import org.junit.runners.JUnit4;
7+
8+
import java.util.List;
9+
10+
import javax.annotations.Nullable;
11+
12+
import static org.junit.Assert.fail;
13+
import static com.google.truth.Truth.assertThat;
14+
15+
@RunWith( JUnit4.class ) public class SomeTest {}

0 commit comments

Comments
 (0)