-
Notifications
You must be signed in to change notification settings - Fork 145
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
Support providing DiagnosticListener
for compiling
#141
base: ea
Are you sure you want to change the base?
Conversation
} | ||
|
||
@NotNull | ||
Map<String, byte[]> compileFromJava(@NotNull String className, |
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.
I have moved this method down a few lines so that all loadFromJava
methods are grouped together.
public Class<?> loadFromJava(@NotNull ClassLoader classLoader, | ||
@NotNull String className, | ||
@NotNull String javaCode, | ||
@Nullable PrintWriter writer) throws ClassNotFoundException { | ||
@Nullable PrintWriter writer, | ||
@Nullable DiagnosticListener<? super JavaFileObject> diagnosticListener) throws ClassNotFoundException { |
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.
Not completely sure about the ? super JavaFileObject
; maybe users make assumptions then about the JavaFileObject
of the diagnostics, which are actually implementation details of Java-Runtime-Compiler, e.g. the file paths.
compiler.loadFromJava( | ||
classLoader, "TestClass", | ||
// definition with a mandatory warning for deprecated `Date.getDay()` | ||
"import java.util.Date; class TestClass { int i = new Date().getDay(); }", |
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.
In this line there is one of the reasons why I changed this test class to create a new CachedCompiler
instance for every test method, instead of using CompilerUtils.CACHED_COMPILER
and getClass().getClassLoader()
:
It seems this source code was missing the import java.util.Date
, which should normally cause a compiler error. However, previously no compilation error occurred, probably because one of the other test methods had already successfully loaded TestClass
and therefore the CachedCompiler#loadFromJava
call just returned that class instead of compiling the code.
cc106f4
to
e3c49a0
Compare
Resolves #93
Adds a new
CachedCompiler#loadFromJava
overload withjavax.tools.DiagnosticListener
parameter. The behavior of the existingloadFromJava
methods remains the same.Also adds Javadoc to some of the methods and constructors.
Edit: Sorry I overlooked that there is already #140 for adding Javadoc. Feel free to merge that first and then I can try to resolve conflicts here and maybe make some small Javadoc adjustments.
Any feedback is appreciated!