-
-
Notifications
You must be signed in to change notification settings - Fork 349
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
feature(runtime): allow to instantiate any class at any time #884
Conversation
monperrus
commented
Oct 5, 2016
•
edited
Loading
edited
@@ -794,6 +794,7 @@ public void setBinaryOutputDirectory(String path) { | |||
@Override | |||
public void setBinaryOutputDirectory(File outputDirectory) { | |||
modelBuilder.setBinaryOutputDirectory(outputDirectory); | |||
getFactory().getEnvironment().setBinaryOutputDirectory(outputDirectory.getAbsolutePath()); |
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.
Do not duplicate the information.
You can use the environment in the in the model builder.
JDTBatchCompiler.this.jdtCompiler.reportProblem(problem); | ||
} else { | ||
new Exception().printStackTrace(); | ||
System.err.println(problem + " " + new String(problem.getOriginatingFileName())); |
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.
We have a logger please use it.
} | ||
} catch (ParentNotInitializedException ignore) { | ||
// no constructor in parent |
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.
You have this exception only because you set the parent of the CtClass to null when you use the snippet to create the class.
if (ret instanceof CtClass) { | ||
CtClass klass = (CtClass) ret; | ||
klass.setSimpleName(klass.getSimpleName().replaceAll("^[0-9]*", "")); | ||
klass.setParent(null); |
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.
Use the rootPackage instead of null.
// cleaning afterwards | ||
new File(getFactory().getEnvironment().getBinaryOutputDirectory() + "/" + getQualifiedName().replace('.', '/') + ".class").delete(); | ||
|
||
return klass.newInstance(); |
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 think it is a good idea to force the compilation the complete project because if the user did not set shouldCompile to true, all reference to other classes of the project will produce compilation error.
Same error if the klass contains a reference to a new class created with spoon.
Factory factory = new Launcher().getFactory(); | ||
CtClass c = factory.Code().createCodeSnippetStatement( | ||
"class X implements spoon.test.compilation.Ifoo { public int foo() {int i=0; return i;} }").compile(); | ||
c.addModifier(ModifierKind.PUBLIC); // required otherwise java.lang.IllegalAccessException at runtime when instantiating |
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 should be nice the handle this workaround in newInstance method
82f0737
to
6ad7f82
Compare
/** | ||
* Generates the bytecode associated to the classes stored in this | ||
* compiler's factory. The bytecode is generated in the directory given by | ||
* {@link #getBinaryOutputDirectory()}. | ||
* | ||
* @see #getSourceClasspath() | ||
*/ | ||
boolean compile(); |
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.
SpoonModelBuilder
is an interface (and very exposed). Think to deprecate methods rather than remove them like this.
* | ||
* @see #getSourceClasspath() | ||
*/ | ||
boolean compileInputSources(); |
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.
Same here.
import spoon.reflect.declaration.CtType; | ||
import spoon.reflect.visitor.DefaultJavaPrettyPrinter; | ||
|
||
import java.util.ArrayList; | ||
import java.util.List; | ||
|
||
class CompilationUnitWrapper extends CompilationUnit { | ||
public class CompilationUnitWrapper extends CompilationUnit { |
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.
This is really necessary to expose this class?
} | ||
} catch (ParentNotInitializedException ignore) { | ||
// no parent set somewhere |
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.
That smells bad...
6ad7f82
to
71211bc
Compare