Skip to content

Compiler classpath and DiagnosticCollector #2

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.

Sign up for GitHub

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

Open
rlsutton1 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Compiler classpath and DiagnosticCollector #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@
<artifactId>maven-compiler-plugin</artifactId>
<version>3.0</version>
<configuration>
<source>1.8</source>
<target>1.8</target>
<source>1.7</source>
<target>1.7</target>
</configuration>
</plugin>
<plugin>
Expand Down
168 changes: 90 additions & 78 deletions src/main/java/com/javax0/jscc/Compiler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@
import java.util.List;
import java.util.Map;

import javax.tools.DiagnosticCollector;
import javax.tools.JavaCompiler;
import javax.tools.JavaFileObject;
import javax.tools.ToolProvider;

/**
Expand All @@ -26,89 +28,99 @@
*/
public class Compiler {

private ClassLoader classLoader = Compiler.class.getClassLoader();

private String compilerErrorOutput = null;

/**
* Setting the parent class loader for the {@see ByteClassLoader} used to
* load the compiled byte code. This is needed when there are different
* class loaders in the application and the code compiling the class uses
* different class loader than the code wanting to use the compiled class.
* If no class loader is specified via this setter then the actual class
* loader will be used, which may work for most of the cases.
*
* @param classLoader
*/
public void setClassLoader(ClassLoader classLoader) {
this.classLoader = classLoader;
}
private ClassLoader classLoader = Compiler.class.getClassLoader();

@verhas verhas Jun 28, 2016 •
edited
Loading

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

General recommendation: when you make a change in a project do not reformat the code, keep the tabbing. Otherwise the 'diff' will show a lot of lines deleted and reinserted and it is hard to assess the true nature of the modification.


private String calculateSimpleClassName(String canonicalClassName) {
return canonicalClassName
.substring(canonicalClassName.lastIndexOf('.') + 1);
}
private String compilerErrorOutput = null;

DiagnosticCollector<JavaFileObject> diagnostics = null;

private Iterable<String> optionList = null;

/**
* Setting the parent class loader for the {@see ByteClassLoader} used to
* load the compiled byte code. This is needed when there are different
* class loaders in the application and the code compiling the class uses
* different class loader than the code wanting to use the compiled class.
* If no class loader is specified via this setter then the actual class
* loader will be used, which may work for most of the cases.
*
* @param classLoader
*/
public void setClassLoader(ClassLoader classLoader) {
this.classLoader = classLoader;
}

private String calculateSimpleClassName(String canonicalClassName) {
return canonicalClassName.substring(canonicalClassName.lastIndexOf('.') + 1);
}

/**
* Compile the Java code provided as a string and if the compilation was
* successful then load the class.
*
* @param sourceCode
* @param canonicalClassName
* the fully qualified name of the class
* @return the loaded class or null if the compilation was not successful
* @throws Exception
* exceptions loading the class are not caught by the method
*/
public Class<?> compile(String sourceCode, String canonicalClassName) throws Exception {
JavaCompiler compiler = ToolProvider.getSystemJavaCompiler();
List<JavaSourceFromString> sources = new LinkedList<>();
String className = calculateSimpleClassName(canonicalClassName);
sources.add(new JavaSourceFromString(className, sourceCode));

/**
* Compile the Java code provided as a string and if the compilation was
* successful then load the class.
*
* @param sourceCode
* @param canonicalClassName
* the fully qualified name of the class
* @return the loaded class or null if the compilation was not successful
* @throws Exception
* exceptions loading the class are not caught by the method
*/
public Class<?> compile(String sourceCode, String canonicalClassName)
throws Exception {
JavaCompiler compiler = ToolProvider.getSystemJavaCompiler();
List<JavaSourceFromString> sources = new LinkedList<>();
String className = calculateSimpleClassName(canonicalClassName);
sources.add(new JavaSourceFromString(className, sourceCode));

StringWriter sw = new StringWriter();
MemoryJavaFileManager fm = new MemoryJavaFileManager(
compiler.getStandardFileManager(null, null, null));
JavaCompiler.CompilationTask task = compiler.getTask(sw, fm, null,
null, null, sources);

Boolean compilationWasSuccessful = task.call();
if (compilationWasSuccessful) {
ByteClassLoader byteClassLoader = new ByteClassLoader(new URL[0],
classLoader, classesByteArraysMap(fm));

Class<?> klass = byteClassLoader.loadClass(canonicalClassName);
byteClassLoader.close();
return klass;
} else {
compilerErrorOutput = sw.toString();
return null;
}
StringWriter sw = null;

@verhas verhas Jun 28, 2016 •
edited
Loading

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

make it

final StringWritre sw;
if( diagnostics == null ){
  sw = new StringWriter();
  } else {
  sw = null;
  }

Much cleaner idiom.

if (diagnostics == null) {
sw = new StringWriter();
}
MemoryJavaFileManager fm = new MemoryJavaFileManager(compiler.getStandardFileManager(diagnostics, null, null));
JavaCompiler.CompilationTask task = compiler.getTask(sw, fm, diagnostics, optionList, null, sources);

/**
* Get the map of class name / class byte array from the file manager.
*
* @param fileManager
* @return
*/
private Map<String, byte[]> classesByteArraysMap(
MemoryJavaFileManager fileManager) {
Map<String, byte[]> map = new HashMap<>();
for (String name : fileManager.getClassFileObjectsMap().keySet()) {
map.put(name, fileManager.getClassFileObjectsMap().get(name)
.getByteArray());
}
return map;
Boolean compilationWasSuccessful = task.call();
if (compilationWasSuccessful) {
ByteClassLoader byteClassLoader = new ByteClassLoader(new URL[0], classLoader, classesByteArraysMap(fm));

Class<?> klass = byteClassLoader.loadClass(canonicalClassName);
byteClassLoader.close();
return klass;
}
if (sw != null) {

@verhas verhas Jun 28, 2016

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Deleting else online 88 makes the code less elegant and less readable. Usually there has to be only one single return statement in a method. In some cases there can be more, like

  1. a success return
  2. an error return

In this case the structure is

   ..... something to calculate

  if( success condition ){
    return success
    }else{
    return error
   }
}```

compilerErrorOutput = sw.toString();
}
return null;

}

/**
*
* @return null if the source was compiled fine or the textual output of the
* compiler when there was some error.
*/
public String getCompilerErrorOutput() {
return compilerErrorOutput;
/**
* Get the map of class name / class byte array from the file manager.
*
* @param fileManager
* @return
*/
private Map<String, byte[]> classesByteArraysMap(MemoryJavaFileManager fileManager) {
Map<String, byte[]> map = new HashMap<>();
for (String name : fileManager.getClassFileObjectsMap().keySet()) {
map.put(name, fileManager.getClassFileObjectsMap().get(name).getByteArray());
}
return map;
}

/**
*
* @return null if the source was compiled fine or the textual output of the
* compiler when there was some error.
*/
public String getCompilerErrorOutput() {
return compilerErrorOutput;
}

public void setOptionList(Iterable<String> optionList) {
this.optionList = optionList;
}

public void setDiagnosticCollector(DiagnosticCollector<JavaFileObject> diagnostics) {
this.diagnostics = diagnostics;
}
}
46 changes: 21 additions & 25 deletions src/main/java/com/javax0/jscc/MemoryJavaFileManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,30 +13,26 @@
import org.slf4j.Logger;

@verhas verhas Jun 28, 2016

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I simply can not see what has changed in this file in addition to formatting.

import org.slf4j.LoggerFactory;

public class MemoryJavaFileManager extends
ForwardingJavaFileManager<StandardJavaFileManager> {
private static final Logger LOG = LoggerFactory
.getLogger(MemoryJavaFileManager.class);
private final Map<String, MemoryFileObject> classFilesMap;

protected MemoryJavaFileManager(final StandardJavaFileManager fileManager) {
super(fileManager);
classFilesMap = new HashMap<>();
}

public Map<String, MemoryFileObject> getClassFileObjectsMap() {
return classFilesMap;
}

@Override
public JavaFileObject getJavaFileForOutput(final Location location,
final String className, final Kind kind, final FileObject sibling)
throws IOException {
LOG.debug("getJavaFileForOutput({},{},{},{}", location, className,
kind, sibling);
MemoryFileObject classFile = new MemoryFileObject(className);
classFilesMap.put(className, classFile);
return classFile;
}
public class MemoryJavaFileManager extends ForwardingJavaFileManager<StandardJavaFileManager> {
private static final Logger LOG = LoggerFactory.getLogger(MemoryJavaFileManager.class);
private final Map<String, MemoryFileObject> classFilesMap;

protected MemoryJavaFileManager(final StandardJavaFileManager fileManager) {
super(fileManager);
classFilesMap = new HashMap<>();
}

public Map<String, MemoryFileObject> getClassFileObjectsMap() {
return classFilesMap;
}

@Override
public JavaFileObject getJavaFileForOutput(final Location location, final String className, final Kind kind,
final FileObject sibling) throws IOException {
LOG.debug("getJavaFileForOutput({},{},{},{}", location, className, kind, sibling);
MemoryFileObject classFile = new MemoryFileObject(className);
classFilesMap.put(className, classFile);
return classFile;
}

}