diff options
Diffstat (limited to 'value/src/main/java/com/google/auto/value')
3 files changed, 53 insertions, 68 deletions
diff --git a/value/src/main/java/com/google/auto/value/processor/AutoValueishProcessor.java b/value/src/main/java/com/google/auto/value/processor/AutoValueishProcessor.java index b7bee429..82d31bf8 100644 --- a/value/src/main/java/com/google/auto/value/processor/AutoValueishProcessor.java +++ b/value/src/main/java/com/google/auto/value/processor/AutoValueishProcessor.java @@ -17,6 +17,7 @@ package com.google.auto.value.processor; import static com.google.auto.common.AnnotationMirrors.getAnnotationValue; import static com.google.auto.common.GeneratedAnnotations.generatedAnnotation; +import static com.google.auto.common.MoreElements.asType; import static com.google.auto.common.MoreElements.getPackage; import static com.google.auto.common.MoreElements.isAnnotationPresent; import static com.google.auto.common.MoreStreams.toImmutableList; @@ -1215,8 +1216,24 @@ abstract class AutoValueishProcessor extends AbstractProcessor { private ImmutableList<AnnotationMirror> propertyFieldAnnotations( TypeElement type, ExecutableElement method) { + // We need to exclude type annotations from the ones being output on the method, since + // they will be output as part of the field's type. + Set<String> returnTypeAnnotations = + getReturnTypeAnnotations(method, this::annotationAppliesToFields); if (!hasAnnotationMirror(method, COPY_ANNOTATIONS_NAME)) { - return ImmutableList.of(); + // If there's no @CopyAnnotations, we will still copy a @Nullable annotation, if (1) it is not + // a TYPE_USE annotation (those appear as part of the type in the generated code) and (2) it + // applies to fields. All known non-TYPE_USE @Nullable annotations do apply to fields, but we + // check just in case. + return method.getAnnotationMirrors().stream() + .filter( + a -> { + TypeElement annotationType = asType(a.getAnnotationType().asElement()); + return isNullable(a) + && !returnTypeAnnotations.contains(annotationType.getQualifiedName().toString()) + && annotationAppliesToFields(annotationType); + }) + .collect(toImmutableList()); } ImmutableSet<String> excludedAnnotations = ImmutableSet.<String>builder() @@ -1224,10 +1241,6 @@ abstract class AutoValueishProcessor extends AbstractProcessor { .add(Override.class.getCanonicalName()) .build(); - // We need to exclude type annotations from the ones being output on the method, since - // they will be output as part of the field's type. - Set<String> returnTypeAnnotations = - getReturnTypeAnnotations(method, this::annotationAppliesToFields); Set<String> nonFieldAnnotations = method.getAnnotationMirrors().stream() .map(a -> a.getAnnotationType().asElement()) diff --git a/value/src/main/java/com/google/auto/value/processor/BuilderMethodClassifier.java b/value/src/main/java/com/google/auto/value/processor/BuilderMethodClassifier.java index b0872009..591b0cc8 100644 --- a/value/src/main/java/com/google/auto/value/processor/BuilderMethodClassifier.java +++ b/value/src/main/java/com/google/auto/value/processor/BuilderMethodClassifier.java @@ -403,6 +403,13 @@ abstract class BuilderMethodClassifier<E extends Element> { TypeMirror returnType = methodMirror.getReturnType(); if (typeUtils.isSubtype(builderType.asType(), returnType) && !MoreTypes.isTypeOf(Object.class, returnType)) { + if (nullableAnnotationFor(method, returnType).isPresent()) { + errorReporter. + reportWarning( + method, + "[%sBuilderSetterNullable] Setter methods always return the Builder so @Nullable" + + " is not appropriate", autoWhat()); + } // We allow the return type to be a supertype (other than Object), to support step builders. TypeMirror parameterType = Iterables.getOnlyElement(methodMirror.getParameterTypes()); propertyNameToSetters.put( diff --git a/value/src/main/java/com/google/auto/value/processor/TemplateVars.java b/value/src/main/java/com/google/auto/value/processor/TemplateVars.java index d9e3337b..73253629 100644 --- a/value/src/main/java/com/google/auto/value/processor/TemplateVars.java +++ b/value/src/main/java/com/google/auto/value/processor/TemplateVars.java @@ -15,27 +15,28 @@ */ package com.google.auto.value.processor; -import com.google.common.base.Throwables; +import static java.nio.charset.StandardCharsets.UTF_8; + +import com.google.common.base.Ascii; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.io.ByteStreams; import com.google.escapevelocity.Template; import java.io.File; import java.io.FileInputStream; -import java.io.FilterInputStream; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; import java.io.Reader; -import java.io.UnsupportedEncodingException; +import java.io.StringReader; +import java.io.UncheckedIOException; import java.lang.reflect.Field; import java.lang.reflect.Modifier; import java.net.URI; import java.net.URISyntaxException; import java.net.URL; -import java.nio.charset.StandardCharsets; import java.util.Map; import java.util.TreeMap; -import java.util.jar.JarEntry; import java.util.jar.JarFile; /** @@ -95,7 +96,7 @@ abstract class TemplateVars { * concrete subclass of TemplateVars) into the template returned by {@link #parsedTemplate()}. */ String toText() { - Map<String, Object> vars = toVars(); + ImmutableMap<String, Object> vars = toVars(); return parsedTemplate().evaluate(vars); } @@ -121,69 +122,42 @@ abstract class TemplateVars { static Template parsedTemplateForResource(String resourceName) { try { - return Template.parseFrom(resourceName, TemplateVars::readerFromResource); - } catch (UnsupportedEncodingException e) { - throw new AssertionError(e); - } catch (IOException | NullPointerException | IllegalStateException e) { - // https://github.com/google/auto/pull/439 says that we can get NullPointerException. - // https://github.com/google/auto/issues/715 says that we can get IllegalStateException - return retryParseAfterException(resourceName, e); - } - } - - private static Template retryParseAfterException(String resourceName, Exception exception) { - try { return Template.parseFrom(resourceName, TemplateVars::readerFromUrl); - } catch (IOException t) { - // Chain the original exception so we can see both problems. - Throwables.getRootCause(exception).initCause(t); - throw new AssertionError(exception); + } catch (IOException e) { + throw new UncheckedIOException(e); } } - private static Reader readerFromResource(String resourceName) { - InputStream in = TemplateVars.class.getResourceAsStream(resourceName); - if (in == null) { - throw new IllegalArgumentException("Could not find resource: " + resourceName); - } - return new InputStreamReader(in, StandardCharsets.UTF_8); - } - // This is an ugly workaround for https://bugs.openjdk.java.net/browse/JDK-6947916, as // reported in https://github.com/google/auto/issues/365. // The issue is that sometimes the InputStream returned by JarURLCollection.getInputStream() // can be closed prematurely, which leads to an IOException saying "Stream closed". - // We catch all IOExceptions, and fall back on logic that opens the jar file directly and - // loads the resource from it. Since that doesn't use JarURLConnection, it shouldn't be - // susceptible to the same bug. We only use this as fallback logic rather than doing it always, - // because jars are memory-mapped by URLClassLoader, so loading a resource in the usual way - // through the getResourceAsStream should be a lot more efficient than reopening the jar. + // To avoid that issue, we open the jar file directly and load the resource from it. Since that + // doesn't use JarURLConnection, it shouldn't be susceptible to the same bug. private static Reader readerFromUrl(String resourceName) throws IOException { URL resourceUrl = TemplateVars.class.getResource(resourceName); if (resourceUrl == null) { - // This is unlikely, since getResourceAsStream has already succeeded for the same resource. throw new IllegalArgumentException("Could not find resource: " + resourceName); } - InputStream in; try { - if (resourceUrl.getProtocol().equalsIgnoreCase("file")) { - in = inputStreamFromFile(resourceUrl); - } else if (resourceUrl.getProtocol().equalsIgnoreCase("jar")) { - in = inputStreamFromJar(resourceUrl); + if (Ascii.equalsIgnoreCase(resourceUrl.getProtocol(), "file")) { + return readerFromFile(resourceUrl); + } else if (Ascii.equalsIgnoreCase(resourceUrl.getProtocol(), "jar")) { + return readerFromJar(resourceUrl); } else { - throw new AssertionError("Template fallback logic fails for: " + resourceUrl); + throw new AssertionError("Template search logic fails for: " + resourceUrl); } } catch (URISyntaxException e) { throw new IOException(e); } - return new InputStreamReader(in, StandardCharsets.UTF_8); } - private static InputStream inputStreamFromJar(URL resourceUrl) - throws URISyntaxException, IOException { + private static Reader readerFromJar(URL resourceUrl) throws URISyntaxException, IOException { // Jar URLs look like this: jar:file:/path/to/file.jar!/entry/within/jar // So take apart the URL to open the jar /path/to/file.jar and read the entry // entry/within/jar from it. + // We could use the methods from JarURLConnection here, but that would risk provoking the same + // problem that prompted this workaround in the first place. String resourceUrlString = resourceUrl.toString().substring("jar:".length()); int bang = resourceUrlString.lastIndexOf('!'); String entryName = resourceUrlString.substring(bang + 1); @@ -191,28 +165,19 @@ abstract class TemplateVars { entryName = entryName.substring(1); } URI jarUri = new URI(resourceUrlString.substring(0, bang)); - JarFile jar = new JarFile(new File(jarUri)); - JarEntry entry = jar.getJarEntry(entryName); - InputStream in = jar.getInputStream(entry); - // We have to be careful not to close the JarFile before the stream has been read, because - // that would also close the stream. So we defer closing the JarFile until the stream is closed. - return new FilterInputStream(in) { - @Override - public void close() throws IOException { - super.close(); - jar.close(); - } - }; + try (JarFile jar = new JarFile(new File(jarUri)); + InputStream in = jar.getInputStream(jar.getJarEntry(entryName))) { + String contents = new String(ByteStreams.toByteArray(in), UTF_8); + return new StringReader(contents); + } } - // We don't really expect this case to arise, since the bug we're working around concerns jars - // not individual files. However, when running the test for this workaround from Maven, we do - // have files. That does mean the test is basically useless there, but Google's internal build - // system does run it using a jar, so we do have coverage. - private static InputStream inputStreamFromFile(URL resourceUrl) + // In most execution environments, we'll be dealing with a jar, but we handle individual files + // just for cases like running our tests with Maven. + private static Reader readerFromFile(URL resourceUrl) throws IOException, URISyntaxException { File resourceFile = new File(resourceUrl.toURI()); - return new FileInputStream(resourceFile); + return new InputStreamReader(new FileInputStream(resourceFile), UTF_8); } private static Object fieldValue(Field field, Object container) { |