Skip to content

Commit

Permalink
Change error reporting methods to use format strings.
Browse files Browse the repository at this point in the history 
RELNOTES=n/a

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=315322251
  • Loading branch information
@eamonnmcmanus @cpovirk
eamonnmcmanus authored and cpovirk committed Jun 9, 2020
1 parent b22f969 commit 2f437b5
Show file tree
Hide file tree
Showing 7 changed files with 203 additions and 201 deletions.
31 changes: 16 additions & 15 deletions value/src/main/java/com/google/auto/value/processor/AutoOneOfProcessor.java
View file
Expand Up @@ -72,7 +72,7 @@ boolean propertiesCanBeVoid() {
void processType(TypeElement autoOneOfType) {
if (autoOneOfType.getKind() != ElementKind.CLASS) {
errorReporter()
.abortWithError("@" + AUTO_ONE_OF_NAME + " only applies to classes", autoOneOfType);
.abortWithError(autoOneOfType, "@" + AUTO_ONE_OF_NAME + " only applies to classes");
}
checkModifiersIfNested(autoOneOfType);
DeclaredType kindMirror = mirrorForKindType(autoOneOfType);
Expand Down Expand Up @@ -128,9 +128,9 @@ private DeclaredType mirrorForKindType(TypeElement autoOneOfType) {
// but it has happened in the past and can crash the compiler.
errorReporter()
.abortWithError(
autoOneOfType,
"annotation processor for @AutoOneOf was invoked with a type"
+ " that does not have that annotation; this is probably a compiler bug",
autoOneOfType);
+ " that does not have that annotation; this is probably a compiler bug");
}
AnnotationValue kindValue =
AnnotationMirrors.getAnnotationValue(oneOfAnnotation.get(), "value");
Expand Down Expand Up @@ -183,8 +183,9 @@ private ImmutableMap<String, String> propertyToKindMap(
if (!transformedEnumConstants.containsKey(transformed)) {
errorReporter()
.reportError(
"Enum has no constant with name corresponding to property '" + property + "'",
kindElement);
kindElement,
"Enum has no constant with name corresponding to property '%s'",
property);
}
});
// Enum constants that have no property
Expand All @@ -193,10 +194,9 @@ private ImmutableMap<String, String> propertyToKindMap(
if (!transformedPropertyNames.containsKey(transformed)) {
errorReporter()
.reportError(
"Name of enum constant '"
+ constant.getSimpleName()
+ "' does not correspond to any property name",
constant);
constant,
"Name of enum constant '%s' does not correspond to any property name",
constant.getSimpleName());
}
});
throw new AbortProcessingException();
Expand All @@ -220,15 +220,17 @@ private ExecutableElement findKindGetterOrAbort(
case 0:
errorReporter()
.reportError(
autoOneOfType + " must have a no-arg abstract method returning " + kindMirror,
autoOneOfType);
autoOneOfType,
"%s must have a no-arg abstract method returning %s",
autoOneOfType,
kindMirror);
break;
case 1:
return Iterables.getOnlyElement(kindGetters);
default:
for (ExecutableElement getter : kindGetters) {
errorReporter()
.reportError("More than one abstract method returns " + kindMirror, getter);
.reportError(getter, "More than one abstract method returns %s", kindMirror);
}
}
throw new AbortProcessingException();
Expand All @@ -252,8 +254,7 @@ && objectMethodToOverride(method) == ObjectMethod.NONE) {
// implement this alien method.
errorReporter()
.reportWarning(
"Abstract methods in @AutoOneOf classes must have no parameters",
method);
method, "Abstract methods in @AutoOneOf classes must have no parameters");
}
}
errorReporter().abortIfAnyError();
Expand All @@ -277,7 +278,7 @@ private void defineVarsForType(
@Override
Optional<String> nullableAnnotationForMethod(ExecutableElement propertyMethod) {
if (nullableAnnotationFor(propertyMethod, propertyMethod.getReturnType()).isPresent()) {
errorReporter().reportError("@AutoOneOf properties cannot be @Nullable", propertyMethod);
errorReporter().reportError(propertyMethod, "@AutoOneOf properties cannot be @Nullable");
}
return Optional.empty();
}
Expand Down
63 changes: 31 additions & 32 deletions value/src/main/java/com/google/auto/value/processor/AutoValueOrOneOfProcessor.java
View file
Expand Up @@ -300,12 +300,10 @@ public final boolean process(Set<? extends TypeElement> annotations, RoundEnviro
// was in deferredTypes.
for (TypeElement type : deferredTypes) {
errorReporter.reportError(
"Did not generate @"
+ simpleAnnotationName
+ " class for "
+ type.getQualifiedName()
+ " because it references undefined types",
type);
type,
"Did not generate @%s class for %s because it references undefined types",
simpleAnnotationName,
type.getQualifiedName());
}
return false;
}
Expand Down Expand Up @@ -333,7 +331,7 @@ public final boolean process(Set<? extends TypeElement> annotations, RoundEnviro
} catch (RuntimeException e) {
String trace = Throwables.getStackTraceAsString(e);
errorReporter.reportError(
"@" + simpleAnnotationName + " processor threw an exception: " + trace, type);
type, "@%s processor threw an exception: %s", simpleAnnotationName, trace);
throw e;
}
}
Expand Down Expand Up @@ -401,7 +399,7 @@ final ImmutableSet<Property> propertySet(
nullableAnnotation);
props.add(p);
if (p.isNullable() && returnType.getKind().isPrimitive()) {
errorReporter().reportError("Primitive types cannot be @Nullable", propertyMethod);
errorReporter().reportError(propertyMethod, "Primitive types cannot be @Nullable");
}
});
return props.build();
Expand Down Expand Up @@ -511,10 +509,15 @@ final ImmutableBiMap<String, ExecutableElement> propertyNameToMethodMap(
String name = allPrefixed ? nameWithoutPrefix(methodName) : methodName;
ExecutableElement old = map.put(name, method);
if (old != null) {
String message = "More than one @" + simpleAnnotationName + " property called " + name;
errorReporter.reportError(message, method);
List<ExecutableElement> contexts = new ArrayList<>(Arrays.asList(method));
if (reportedDups.add(name)) {
errorReporter.reportError(message, old);
contexts.add(old);
}
// Report the error for both of the methods. If this is a third or further occurrence,
// reportedDups prevents us from reporting more than one error for the same method.
for (ExecutableElement context : contexts) {
errorReporter.reportError(
context, "More than one @%s property called %s", simpleAnnotationName, name);
}
}
}
Expand Down Expand Up @@ -619,16 +622,16 @@ final void checkModifiersIfNested(TypeElement type) {
if (enclosingKind.isClass() || enclosingKind.isInterface()) {
if (type.getModifiers().contains(Modifier.PRIVATE)) {
errorReporter.abortWithError(
"@" + simpleAnnotationName + " class must not be private", type);
type, "@%s class must not be private", simpleAnnotationName);
} else if (Visibility.effectiveVisibilityOfElement(type).equals(Visibility.PRIVATE)) {
// The previous case, where the class itself is private, is much commoner so it deserves
// its own error message, even though it would be caught by the test here too.
errorReporter.abortWithError(
"@" + simpleAnnotationName + " class must not be nested in a private class", type);
type, "@%s class must not be nested in a private class", simpleAnnotationName);
}
if (!type.getModifiers().contains(Modifier.STATIC)) {
errorReporter.abortWithError(
"Nested @" + simpleAnnotationName + " class must be static", type);
type, "Nested @%s class must be static", simpleAnnotationName);
}
}
// In principle type.getEnclosingElement() could be an ExecutableElement (for a class
Expand Down Expand Up @@ -768,10 +771,9 @@ final void checkReturnType(TypeElement autoValueClass, ExecutableElement getter)
warnAboutPrimitiveArrays(autoValueClass, getter);
} else {
errorReporter.reportError(
"An @"
+ simpleAnnotationName
+ " class cannot define an array-valued property unless it is a primitive array",
getter);
getter,
"An @%s class cannot define an array-valued property unless it is a primitive array",
simpleAnnotationName);
}
}
}
Expand All @@ -792,20 +794,17 @@ private void warnAboutPrimitiveArrays(TypeElement autoValueClass, ExecutableElem
// inherited class is not being recompiled. Instead, in this case we point to the @AutoValue
// class itself, and we include extra text in the error message that shows the full name of
// the inherited method.
String warning =
"An @"
+ simpleAnnotationName
+ " property that is a primitive array returns the original array, which can"
boolean sameClass = getter.getEnclosingElement().equals(autoValueClass);
Element element = sameClass ? getter : autoValueClass;
String context = sameClass ? "" : (" Method: " + getter.getEnclosingElement() + "." + getter);
errorReporter.reportWarning(
element,
"An @%s property that is a primitive array returns the original array, which can"
+ " therefore be modified by the caller. If this is OK, you can suppress this warning"
+ " with @SuppressWarnings(\"mutable\"). Otherwise, you should replace the property"
+ " with an immutable type, perhaps a simple wrapper around the original array.";
boolean sameClass = getter.getEnclosingElement().equals(autoValueClass);
if (sameClass) {
errorReporter.reportWarning(warning, getter);
} else {
errorReporter.reportWarning(
warning + " Method: " + getter.getEnclosingElement() + "." + getter, autoValueClass);
}
+ " with an immutable type, perhaps a simple wrapper around the original array.%s",
simpleAnnotationName,
context);
}
}

Expand Down Expand Up @@ -837,7 +836,7 @@ final String getSerialVersionUID(TypeElement type) {
return value + "L";
} else {
errorReporter.reportError(
"serialVersionUID must be a static final long compile-time constant", field);
field, "serialVersionUID must be a static final long compile-time constant");
break;
}
}
Expand Down Expand Up @@ -1103,7 +1102,7 @@ final void writeSourceFile(String className, String text, TypeElement originatin
// error later because user code will have a reference to the code we were supposed to
// generate (new AutoValue_Foo() or whatever) and that reference will be undefined.
errorReporter.reportWarning(
"Could not write generated class " + className + ": " + e, originatingType);
originatingType, "Could not write generated class %s: %s", className, e);
}
}
}
77 changes: 38 additions & 39 deletions value/src/main/java/com/google/auto/value/processor/AutoValueProcessor.java
View file
Expand Up @@ -114,15 +114,17 @@ public synchronized void init(ProcessingEnvironment processingEnv) {
try {
extensions = extensionsFromLoader(loaderForExtensions);
} catch (RuntimeException | Error e) {
StringBuilder warning = new StringBuilder();
warning.append(
"An exception occurred while looking for AutoValue extensions."
+ " No extensions will function.");
if (e instanceof ServiceConfigurationError) {
warning.append(" This may be due to a corrupt jar file in the compiler's classpath.");
}
warning.append("\n").append(Throwables.getStackTraceAsString(e));
errorReporter().reportWarning(warning.toString(), null);
String explain =
(e instanceof ServiceConfigurationError)
? " This may be due to a corrupt jar file in the compiler's classpath."
: "";
errorReporter()
.reportWarning(
null,
"An exception occurred while looking for AutoValue extensions."
+ " No extensions will function.%s\n%s",
explain,
Throwables.getStackTraceAsString(e));
extensions = ImmutableList.of();
}
}
Expand Down Expand Up @@ -167,22 +169,21 @@ void processType(TypeElement type) {
// but it has happened in the past and can crash the compiler.
errorReporter()
.abortWithError(
type,
"annotation processor for @AutoValue was invoked with a type"
+ " that does not have that annotation; this is probably a compiler bug",
type);
+ " that does not have that annotation; this is probably a compiler bug");
}
if (type.getKind() != ElementKind.CLASS) {
errorReporter().abortWithError("@AutoValue only applies to classes", type);
errorReporter().abortWithError(type, "@AutoValue only applies to classes");
}
if (ancestorIsAutoValue(type)) {
errorReporter().abortWithError("One @AutoValue class may not extend another", type);
errorReporter().abortWithError(type, "One @AutoValue class may not extend another");
}
if (implementsAnnotation(type)) {
errorReporter()
.abortWithError(
"@AutoValue may not be used to implement an annotation"
+ " interface; try using @AutoAnnotation instead",
type);
type, "@AutoValue may not be used to implement an annotation"
+ " interface; try using @AutoAnnotation instead");
}
checkModifiersIfNested(type);

Expand Down Expand Up @@ -331,9 +332,9 @@ private ImmutableList<AutoValueExtension> applicableExtensions(
default:
errorReporter()
.reportError(
"More than one extension wants to generate the final class: "
+ finalExtensions.stream().map(this::extensionName).collect(joining(", ")),
type);
type,
"More than one extension wants to generate the final class: %s",
finalExtensions.stream().map(this::extensionName).collect(joining(", ")));
break;
}
return ImmutableList.copyOf(applicableExtensions);
Expand All @@ -353,11 +354,10 @@ private ImmutableSet<ExecutableElement> methodsConsumedByExtensions(
if (propertyMethod == null) {
errorReporter()
.reportError(
"Extension "
+ extensionName(extension)
+ " wants to consume a property that does not exist: "
+ consumedProperty,
type);
type,
"Extension %s wants to consume a property that does not exist: %s",
extensionName(extension),
consumedProperty);
} else {
consumedHere.add(propertyMethod);
}
Expand All @@ -366,23 +366,22 @@ private ImmutableSet<ExecutableElement> methodsConsumedByExtensions(
if (!abstractMethods.contains(consumedMethod)) {
errorReporter()
.reportError(
"Extension "
+ extensionName(extension)
+ " wants to consume a method that is not one of the abstract methods in this"
+ " class: "
+ consumedMethod,
type);
type,
"Extension %s wants to consume a method that is not one of the abstract methods"
+ " in this class: %s",
extensionName(extension),
consumedMethod);
} else {
consumedHere.add(consumedMethod);
}
}
for (ExecutableElement repeat : intersection(consumed, consumedHere)) {
errorReporter()
.reportError(
"Extension "
+ extensionName(extension)
+ " wants to consume a method that was already consumed by another extension",
repeat);
repeat,
"Extension %s wants to consume a method that was already consumed by another"
+ " extension",
extensionName(extension));
}
consumed.addAll(consumedHere);
}
Expand All @@ -404,11 +403,11 @@ && objectMethodToOverride(method) == ObjectMethod.NONE) {
// ElementUtils.override that sometimes fails to recognize that one method overrides
// another, and therefore leaves us with both an abstract method and the subclass method
// that overrides it. This shows up in AutoValueTest.LukesBase for example.
String message = "Abstract method is neither a property getter nor a Builder converter";
if (extensionsPresent) {
message += ", and no extension consumed it";
}
errorReporter().reportWarning(message, method);
String extensionMessage = extensionsPresent ? ", and no extension consumed it" : "";
errorReporter().reportWarning(
method,
"Abstract method is neither a property getter nor a Builder converter%s",
extensionMessage);
}
}
errorReporter().abortIfAnyError();
Expand Down

0 comments on commit 2f437b5

Please sign in to comment.