Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JBLOGGING-189 Include originating element when creating files with Filer #119

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@

import javax.annotation.processing.ProcessingEnvironment;
import javax.annotation.processing.SupportedOptions;
import javax.lang.model.element.Element;
import javax.lang.model.element.TypeElement;
import javax.tools.StandardLocation;

Expand Down Expand Up @@ -98,7 +99,8 @@ public void processTypeElement(final TypeElement annotation, final TypeElement e

final String fileName = messageInterface.simpleName() + reportType.getExtension();
try (
final BufferedWriter writer = createWriter(messageInterface.packageName(), fileName);
final BufferedWriter writer = createWriter(messageInterface.packageName(), fileName,
processingEnv.getElementUtils().getTypeElement(messageInterface.name()));
final ReportWriter reportWriter = ReportWriter.of(reportType, messageInterface, writer)) {
reportWriter.writeHeader(reportTitle);
// Process the methods
Expand All @@ -113,10 +115,11 @@ public void processTypeElement(final TypeElement annotation, final TypeElement e
}
}

private BufferedWriter createWriter(final String packageName, final String fileName) throws IOException {
private BufferedWriter createWriter(final String packageName, final String fileName, Element originatingElement)
throws IOException {
if (reportPath == null) {
return new BufferedWriter(processingEnv.getFiler()
.createResource(StandardLocation.SOURCE_OUTPUT, packageName, fileName).openWriter());
.createResource(StandardLocation.SOURCE_OUTPUT, packageName, fileName, originatingElement).openWriter());
}
final Path outputPath = Paths.get(reportPath, packageName.replace(".", FileSystems.getDefault().getSeparator()),
fileName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,8 @@ private void generateDefaultTranslationFile(final MessageInterface messageInterf
try {
if (generatedFilesPath == null) {
final FileObject fileObject = processingEnv.getFiler().createResource(StandardLocation.CLASS_OUTPUT,
messageInterface.packageName(), fileName);
messageInterface.packageName(), fileName,
processingEnv.getElementUtils().getTypeElement(messageInterface.name()));
// Note the FileObject#openWriter() is used here. The FileObject#openOutputStream() returns an output stream
// that writes each byte separately which results in poor performance.
writer = new BufferedWriter(fileObject.openWriter());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,16 @@
import static org.jboss.jdeparser.JTypes.typeOf;

import java.io.IOException;
import java.io.OutputStream;
import java.io.Serializable;
import java.util.HashMap;
import java.util.Locale;
import java.util.Map;
import java.util.Properties;

import javax.annotation.processing.Filer;
import javax.annotation.processing.ProcessingEnvironment;
import javax.lang.model.element.Element;
import javax.lang.model.element.TypeElement;

import org.jboss.jdeparser.FormatPreferences;
Expand Down Expand Up @@ -95,7 +98,10 @@ public abstract class ClassModel {
this.messageInterface = messageInterface;
this.className = messageInterface.packageName() + "." + className;
this.superClassName = superClassName;
sources = JDeparser.createSources(JFiler.newInstance(processingEnv.getFiler()),
sources = JDeparser.createSources(
new JFilerOriginatingElementAware(
processingEnv.getElementUtils().getTypeElement(messageInterface.name()),
processingEnv.getFiler()),
new FormatPreferences(new Properties()));
sourceFile = sources.createSourceFile(messageInterface.packageName(), className);
classDef = sourceFile._class(JMod.PUBLIC, className);
Expand Down Expand Up @@ -362,4 +368,43 @@ private JExpr determineLocale(final String locale, final JType localeType) {

return initializer;
}

/**
* This version of the {@link JFiler} passes an originating element to the underlying {@link Filer}.
* It allows building tools, like Gradle, to figure out a better incremental compilation plan.
* In contrast, a full recompilation will most likely be required without an originating element.
* <p>
* Other than passing the originating element this {@link JFiler} should behave exactly
* as the one created with {@link JFiler#newInstance(Filer)}
*/
private static class JFilerOriginatingElementAware extends JFiler {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be nice to add a little javadoc, or some comments, to explain to other maintainers why this is useful?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking a look and for the suggestions 😃🎉
Applied the changes and pushed an updated version 😃


private final Element originatingElement;
private final Filer filer;

private JFilerOriginatingElementAware(Element originatingElement, Filer filer) {
if (originatingElement == null) {
throw new ProcessingException(null,
"Creating an instance of a %s without an originating element is not allowed.", getClass().getName());
}
if (filer == null) {
throw new ProcessingException(originatingElement,
"Creating an instance of a %s without a non-null %s value is not allowed.", getClass().getName(),
Filer.class.getName());
}
this.originatingElement = originatingElement;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd check here for these elements to not be null?
Otherwise it fails later and it would get trickier to narrow down

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to propose this to happen slightly differently. I'd propose throwing a ProcessingException as it can take the element as an argument and potentially fail with a better message.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok 😃, so maybe something like this ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect. I know these are highly unlikely, but may as well have as much information as possible if they are thrown :)

this.filer = filer;
}

@Override
public OutputStream openStream(String packageName, String fileName) throws IOException {
// Create the FQCN
final StringBuilder sb = new StringBuilder(packageName);
if (sb.charAt(sb.length() - 1) != '.') {
sb.append('.');
}
sb.append(fileName);
return filer.createSourceFile(sb, originatingElement).openOutputStream();
}
}
}