Skip to content

Commit 637988f

Browse files
sir-buckyballcushon
authored andcommitted
Suppress formatting exceptions in FormattingFiler
When developing an annotation processor which uses this code, throwing an exception hides the problematic code from the developer. This change emits the unformatted code so it can be debugged. MOE_MIGRATED_REVID=146717858
1 parent e44b58a commit 637988f

File tree

3 files changed

+72
-15
lines changed

3 files changed

+72
-15
lines changed

core/src/main/java/com/google/googlejavaformat/java/filer/FormattingFiler.java

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@
1818

1919
import com.google.googlejavaformat.java.Formatter;
2020
import java.io.IOException;
21+
import javax.annotation.Nullable;
2122
import javax.annotation.processing.Filer;
23+
import javax.annotation.processing.Messager;
2224
import javax.lang.model.element.Element;
2325
import javax.tools.FileObject;
2426
import javax.tools.JavaFileManager;
@@ -33,17 +35,30 @@ public final class FormattingFiler implements Filer {
3335
private final Filer delegate;
3436
// TODO(ronshapiro): consider allowing users to create their own Formatter instance
3537
private final Formatter formatter = new Formatter();
38+
private final Messager messager;
3639

3740
/** @param delegate filer to decorate */
3841
public FormattingFiler(Filer delegate) {
42+
this(delegate, null);
43+
}
44+
45+
/**
46+
* Create a new {@link FormattingFiler}. An optional {@link Messager} may be specified to make
47+
* logs more visible.
48+
*
49+
* @param delegate filer to decorate
50+
* @param messager to log warnings to
51+
*/
52+
public FormattingFiler(Filer delegate, @Nullable Messager messager) {
3953
this.delegate = checkNotNull(delegate);
54+
this.messager = messager;
4055
}
4156

4257
@Override
4358
public JavaFileObject createSourceFile(CharSequence name, Element... originatingElements)
4459
throws IOException {
4560
return new FormattingJavaFileObject(
46-
delegate.createSourceFile(name, originatingElements), formatter);
61+
delegate.createSourceFile(name, originatingElements), formatter, messager);
4762
}
4863

4964
@Override

core/src/main/java/com/google/googlejavaformat/java/filer/FormattingJavaFileObject.java

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@
2222
import com.google.googlejavaformat.java.FormatterException;
2323
import java.io.IOException;
2424
import java.io.Writer;
25+
import javax.annotation.Nullable;
26+
import javax.annotation.processing.Messager;
27+
import javax.tools.Diagnostic;
2528
import javax.tools.ForwardingJavaFileObject;
2629
import javax.tools.JavaFileObject;
2730

@@ -31,11 +34,19 @@ final class FormattingJavaFileObject extends ForwardingJavaFileObject<JavaFileOb
3134
private static final int DEFAULT_FILE_SIZE = 80 * 500;
3235

3336
private final Formatter formatter;
37+
private final Messager messager;
3438

35-
/** @param delegate {@link JavaFileObject} to decorate */
36-
FormattingJavaFileObject(JavaFileObject delegate, Formatter formatter) {
39+
/**
40+
* Create a new {@link FormattingJavaFileObject}.
41+
*
42+
* @param delegate {@link JavaFileObject} to decorate
43+
* @param messager to log messages with.
44+
*/
45+
FormattingJavaFileObject(
46+
JavaFileObject delegate, Formatter formatter, @Nullable Messager messager) {
3747
super(checkNotNull(delegate));
3848
this.formatter = checkNotNull(formatter);
49+
this.messager = messager;
3950
}
4051

4152
@Override
@@ -62,7 +73,15 @@ public Writer openStream() throws IOException {
6273
}
6374
});
6475
} catch (FormatterException e) {
65-
throw new IOException("Error formatting " + getName(), e);
76+
// An exception will happen when the code being formatted has an error. It's better to
77+
// log the exception and emit unformatted code so the developer can view the code which
78+
// caused a problem.
79+
try (Writer writer = fileObject.openWriter()) {
80+
writer.append(stringBuilder.toString());
81+
}
82+
if (messager != null) {
83+
messager.printMessage(Diagnostic.Kind.NOTE, "Error formatting " + getName());
84+
}
6685
}
6786
}
6887
};

core/src/test/java/com/google/googlejavaformat/java/filer/FormattingFilerTest.java

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,19 @@
1515
package com.google.googlejavaformat.java.filer;
1616

1717
import static com.google.common.truth.Truth.assertThat;
18-
import static org.junit.Assert.fail;
1918

2019
import com.google.common.base.Joiner;
21-
import com.google.googlejavaformat.java.FormatterException;
2220
import com.google.testing.compile.CompilationRule;
2321
import java.io.IOException;
2422
import java.io.StringWriter;
2523
import java.io.Writer;
2624
import java.net.URI;
25+
import java.util.ArrayList;
26+
import java.util.List;
2727
import javax.annotation.processing.Filer;
28+
import javax.annotation.processing.Messager;
29+
import javax.lang.model.element.AnnotationMirror;
30+
import javax.lang.model.element.AnnotationValue;
2831
import javax.lang.model.element.Element;
2932
import javax.tools.FileObject;
3033
import javax.tools.JavaFileManager.Location;
@@ -43,18 +46,38 @@ public class FormattingFilerTest {
4346
@Rule public CompilationRule compilationRule = new CompilationRule();
4447

4548
@Test
46-
public void invalidSyntaxThrowsError() throws IOException {
49+
public void invalidSyntaxDoesNotThrowError() throws IOException {
50+
List<String> logMessages = new ArrayList<>();
51+
Messager messager =
52+
new Messager() {
53+
@Override
54+
public void printMessage(javax.tools.Diagnostic.Kind kind, CharSequence msg) {
55+
logMessages.add(kind.toString() + ";" + msg);
56+
}
57+
58+
@Override
59+
public void printMessage(javax.tools.Diagnostic.Kind kind, CharSequence msg, Element e) {}
60+
61+
@Override
62+
public void printMessage(
63+
javax.tools.Diagnostic.Kind kind, CharSequence msg, Element e, AnnotationMirror a) {}
64+
65+
@Override
66+
public void printMessage(
67+
javax.tools.Diagnostic.Kind kind,
68+
CharSequence msg,
69+
Element e,
70+
AnnotationMirror a,
71+
AnnotationValue v) {}
72+
};
73+
4774
String file = Joiner.on('\n').join("package foo;", "public class Bar {");
48-
FormattingFiler formattingFiler = new FormattingFiler(new FakeFiler());
75+
FormattingFiler formattingFiler = new FormattingFiler(new FakeFiler(), messager);
4976
Writer writer = formattingFiler.createSourceFile("foo.Bar").openWriter();
5077
writer.write(file);
51-
try {
52-
writer.close();
53-
fail("An exception should be thrown if formatting fails");
54-
} catch (IOException expected) {
55-
assertThat(expected.getMessage()).contains("foo.Bar");
56-
assertThat(expected.getCause().getClass()).isAssignableTo(FormatterException.class);
57-
}
78+
writer.close();
79+
80+
assertThat(logMessages).containsExactly("NOTE;Error formatting foo.Bar");
5881
}
5982

6083
@Test

0 commit comments

Comments
 (0)