-
Notifications
You must be signed in to change notification settings - Fork 27
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -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); | ||
|
@@ -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 { | ||
|
||
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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd check here for these elements to not be null? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok 😃, so maybe something like this ? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 😃