# HG changeset patch # User jlahoda # Date 1489485079 -3600 # Node ID e1b620ac6c98d2bcc60fdfd26a84ccda9c907fcf # Parent 202973b2d1ae073366fb9fdd3bad05856c8154a9 8175119: Need to specify module of types created by Filer.createSourceFile/Filer.createClassFile? Summary: Clarifications and improvements to jx.a.processing.Filer for creating and reading files in and from modules. Reviewed-by: darcy, jjg diff -r 202973b2d1ae -r e1b620ac6c98 langtools/src/java.compiler/share/classes/javax/annotation/processing/Filer.java --- a/langtools/src/java.compiler/share/classes/javax/annotation/processing/Filer.java Tue Mar 14 08:19:41 2017 +0100 +++ b/langtools/src/java.compiler/share/classes/javax/annotation/processing/Filer.java Tue Mar 14 10:51:19 2017 +0100 @@ -28,6 +28,7 @@ import javax.tools.JavaFileManager; import javax.tools.*; import javax.lang.model.element.Element; +import javax.lang.model.util.Elements; import java.io.IOException; /** @@ -157,6 +158,12 @@ * example, to create a source file for type {@code a.B} in module * {@code foo}, use a {@code name} argument of {@code "foo/a.B"}. * + *

If no explicit module prefix is given and modules are supported + * in the environment, a suitable module is inferred. If a suitable + * module cannot be inferred {@link FilerException} is thrown. + * An implementation may use information about the configuration of + * the annotation processing tool as part of the inference. + * *

Creating a source file in or for an unnamed package in a named * module is not supported. * @@ -176,6 +183,17 @@ * ProcessingEnvironment#getSourceVersion source version} being used * for this run. * + * @implNote In the reference implementation, if the annotation + * processing tool is processing a single module M, + * then M is used as the module for files created without + * an explicit module prefix. If the tool is processing multiple + * modules, and {@link + * Elements#getPackageElement(java.lang.CharSequence) + * Elements.getPackageElement(package-of(name))} + * returns a package, the module that owns the returned package is used + * as the target module. A separate option may be used to provide the target + * module if it cannot be determined using the above rules. + * * @param name canonical (fully qualified) name of the principal type * being declared in this file or a package name followed by * {@code ".package-info"} for a package information file @@ -184,8 +202,11 @@ * {@code null} * @return a {@code JavaFileObject} to write the new source file * @throws FilerException if the same pathname has already been - * created, the same type has already been created, or the name is - * otherwise not valid for the entity requested to being created + * created, the same type has already been created, the name is + * otherwise not valid for the entity requested to being created, + * if the target module cannot be determined, if the target + * module is not writable, or a module is specified when the environment + * doesn't support modules. * @throws IOException if the file cannot be created * @jls 7.3 Compilation Units */ @@ -213,6 +234,12 @@ * example, to create a class file for type {@code a.B} in module * {@code foo}, use a {@code name} argument of {@code "foo/a.B"}. * + *

If no explicit module prefix is given and modules are supported + * in the environment, a suitable module is inferred. If a suitable + * module cannot be inferred {@link FilerException} is thrown. + * An implementation may use information about the configuration of + * the annotation processing tool as part of the inference. + * *

Creating a class file in or for an unnamed package in a named * module is not supported. * @@ -221,6 +248,17 @@ * ProcessingEnvironment#getSourceVersion source version} being * used for this run. * + * @implNote In the reference implementation, if the annotation + * processing tool is processing a single module M, + * then M is used as the module for files created without + * an explicit module prefix. If the tool is processing multiple + * modules, and {@link + * Elements#getPackageElement(java.lang.CharSequence) + * Elements.getPackageElement(package-of(name))} + * returns a package, the module that owns the returned package is used + * as the target module. A separate option may be used to provide the target + * module if it cannot be determined using the above rules. + * * @param name binary name of the type being written or a package name followed by * {@code ".package-info"} for a package information file * @param originatingElements type or package or module elements causally @@ -228,8 +266,10 @@ * {@code null} * @return a {@code JavaFileObject} to write the new class file * @throws FilerException if the same pathname has already been - * created, the same type has already been created, or the name is - * not valid for a type + * created, the same type has already been created, the name is + * not valid for a type, if the target module cannot be determined, + * if the target module is not writable, or a module is specified when + * the environment doesn't support modules. * @throws IOException if the file cannot be created */ JavaFileObject createClassFile(CharSequence name, @@ -255,11 +295,37 @@ * does not contain a "{@code /}" character, the entire argument * is interpreted as a package name. * + *

If the given location is neither a {@linkplain + * JavaFileManager.Location#isModuleOrientedLocation() + * module oriented location}, nor an {@linkplain + * JavaFileManager.Location#isOutputLocation() + * output location containing multiple modules}, and the explicit + * module prefix is given, {@link FilerException} is thrown. + * + *

If the given location is either a module oriented location, + * or an output location containing multiple modules, and no explicit + * modules prefix is given, a suitable module is + * inferred. If a suitable module cannot be inferred {@link + * FilerException} is thrown. An implementation may use information + * about the configuration of the annotation processing tool + * as part of the inference. + * *

Files created via this method are not registered for * annotation processing, even if the full pathname of the file * would correspond to the full pathname of a new source file * or new class file. * + * @implNote In the reference implementation, if the annotation + * processing tool is processing a single module M, + * then M is used as the module for files created without + * an explicit module prefix. If the tool is processing multiple + * modules, and {@link + * Elements#getPackageElement(java.lang.CharSequence) + * Elements.getPackageElement(package-of(name))} + * returns a package, the module that owns the returned package is used + * as the target module. A separate option may be used to provide the target + * module if it cannot be determined using the above rules. + * * @param location location of the new file * @param moduleAndPkg module and/or package relative to which the file * should be named, or the empty string if none @@ -270,7 +336,9 @@ * @return a {@code FileObject} to write the new resource * @throws IOException if the file cannot be created * @throws FilerException if the same pathname has already been - * created + * created, if the target module cannot be determined, + * or if the target module is not writable, or if an explicit + * target module is specified and the location does not support it. * @throws IllegalArgumentException for an unsupported location * @throws IllegalArgumentException if {@code moduleAndPkg} is ill-formed * @throws IllegalArgumentException if {@code relativeName} is not relative @@ -294,13 +362,41 @@ * does not contain a "{@code /}" character, the entire argument * is interpreted as a package name. * + *

If the given location is neither a {@linkplain + * JavaFileManager.Location#isModuleOrientedLocation() + * module oriented location}, nor an {@linkplain + * JavaFileManager.Location#isOutputLocation() + * output location containing multiple modules}, and the explicit + * module prefix is given, {@link FilerException} is thrown. + * + *

If the given location is either a module oriented location, + * or an output location containing multiple modules, and no explicit + * modules prefix is given, a suitable module is + * inferred. If a suitable module cannot be inferred {@link + * FilerException} is thrown. An implementation may use information + * about the configuration of the annotation processing tool + * as part of the inference. + * + * @implNote In the reference implementation, if the annotation + * processing tool is processing a single module M, + * then M is used as the module for files read without + * an explicit module prefix. If the tool is processing multiple + * modules, and {@link + * Elements#getPackageElement(java.lang.CharSequence) + * Elements.getPackageElement(package-of(name))} + * returns a package, the module that owns the returned package is used + * as the source module. A separate option may be used to provide the target + * module if it cannot be determined using the above rules. + * * @param location location of the file * @param moduleAndPkg module and/or package relative to which the file * should be searched for, or the empty string if none * @param relativeName final pathname components of the file * @return an object to read the file * @throws FilerException if the same pathname has already been - * opened for writing + * opened for writing, if the source module cannot be determined, + * or if the target module is not writable, or if an explicit target + * module is specified and the location does not support it. * @throws IOException if the file cannot be opened * @throws IllegalArgumentException for an unsupported location * @throws IllegalArgumentException if {@code moduleAndPkg} is ill-formed diff -r 202973b2d1ae -r e1b620ac6c98 langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Modules.java --- a/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Modules.java Tue Mar 14 08:19:41 2017 +0100 +++ b/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Modules.java Tue Mar 14 10:51:19 2017 +0100 @@ -714,6 +714,11 @@ return rootModules.contains(module); } + public Set getRootModules() { + Assert.checkNonNull(rootModules); + return rootModules; + } + class ModuleVisitor extends JCTree.Visitor { private ModuleSymbol sym; private final Set allRequires = new HashSet<>(); diff -r 202973b2d1ae -r e1b620ac6c98 langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/main/Arguments.java --- a/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/main/Arguments.java Tue Mar 14 08:19:41 2017 +0100 +++ b/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/main/Arguments.java Tue Mar 14 10:51:19 2017 +0100 @@ -614,6 +614,7 @@ validateAddModules(sv); validateAddReads(sv); validateLimitModules(sv); + validateDefaultModuleForCreatedFiles(sv); if (lintOptions && options.isSet(Option.ADD_OPENS)) { log.warning(LintCategory.OPTIONS, Warnings.AddopensIgnored); @@ -751,6 +752,17 @@ } } + private void validateDefaultModuleForCreatedFiles(SourceVersion sv) { + String moduleName = options.get(Option.DEFAULT_MODULE_FOR_CREATED_FILES); + if (moduleName != null) { + if (!SourceVersion.isName(moduleName, sv)) { + // syntactically invalid module name: e.g. --default-module-for-created-files m! + log.error(Errors.BadNameForOption(Option.DEFAULT_MODULE_FOR_CREATED_FILES, + moduleName)); + } + } + } + /** * Returns true if there are no files or classes specified for use. * @return true if there are no files or classes specified for use diff -r 202973b2d1ae -r e1b620ac6c98 langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/main/Option.java --- a/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/main/Option.java Tue Mar 14 08:19:41 2017 +0100 +++ b/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/main/Option.java Tue Mar 14 10:51:19 2017 +0100 @@ -399,6 +399,30 @@ } }, + DEFAULT_MODULE_FOR_CREATED_FILES("--default-module-for-created-files", + "opt.arg.default.module.for.created.files", + "opt.default.module.for.created.files", EXTENDED, BASIC) { + @Override + public void process(OptionHelper helper, String option, String arg) throws InvalidValueException { + String prev = helper.get(DEFAULT_MODULE_FOR_CREATED_FILES); + if (prev != null) { + throw helper.newInvalidValueException("err.option.too.many", + DEFAULT_MODULE_FOR_CREATED_FILES.primaryName); + } else if (arg.isEmpty()) { + throw helper.newInvalidValueException("err.no.value.for.option", option); + } else if (getPattern().matcher(arg).matches()) { + helper.put(DEFAULT_MODULE_FOR_CREATED_FILES.primaryName, arg); + } else { + throw helper.newInvalidValueException("err.bad.value.for.option", option, arg); + } + } + + @Override + public Pattern getPattern() { + return Pattern.compile("[^,].*"); + } + }, + X("--help-extra -X", "opt.X", STANDARD, INFO) { @Override public void process(OptionHelper helper, String option) throws InvalidValueException { diff -r 202973b2d1ae -r e1b620ac6c98 langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/processing/JavacFiler.java --- a/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/processing/JavacFiler.java Tue Mar 14 08:19:41 2017 +0100 +++ b/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/processing/JavacFiler.java Tue Mar 14 10:51:19 2017 +0100 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2005, 2016, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2005, 2017, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -60,6 +60,8 @@ import com.sun.tools.javac.util.DefinedBy.Api; import static com.sun.tools.javac.code.Lint.LintCategory.PROCESSING; +import com.sun.tools.javac.code.Symbol.PackageSymbol; +import com.sun.tools.javac.main.Option; /** * The FilerImplementation class must maintain a number of @@ -384,6 +386,8 @@ private final Set initialClassNames; + private final String defaultTargetModule; + JavacFiler(Context context) { this.context = context; fileManager = context.get(JavaFileManager.class); @@ -408,6 +412,10 @@ initialClassNames = new LinkedHashSet<>(); lint = (Lint.instance(context)).isEnabled(PROCESSING); + + Options options = Options.instance(context); + + defaultTargetModule = options.get(Option.DEFAULT_MODULE_FOR_CREATED_FILES); } @Override @DefinedBy(Api.ANNOTATION_PROCESSING) @@ -427,29 +435,42 @@ private Pair checkOrInferModule(CharSequence moduleAndPkg) throws FilerException { String moduleAndPkgString = moduleAndPkg.toString(); int slash = moduleAndPkgString.indexOf('/'); - - if (slash != (-1)) { - //module name specified: - String module = moduleAndPkgString.substring(0, slash); + String module; + String pkg; - ModuleSymbol explicitModule = syms.getModule(names.fromString(module)); + if (slash == (-1)) { + //module name not specified: + int lastDot = moduleAndPkgString.lastIndexOf('.'); + String pack = lastDot != (-1) ? moduleAndPkgString.substring(0, lastDot) : ""; + ModuleSymbol msym = inferModule(pack); - if (explicitModule == null) { - throw new FilerException("Module: " + module + " does not exist."); + if (msym != null) { + return Pair.of(msym, moduleAndPkgString); + } + + if (defaultTargetModule == null) { + throw new FilerException("Cannot determine target module."); } - if (!modules.isRootModule(explicitModule)) { - throw new FilerException("Cannot write to the given module!"); - } - - return Pair.of(explicitModule, moduleAndPkgString.substring(slash + 1)); + module = defaultTargetModule; + pkg = moduleAndPkgString; } else { - if (modules.multiModuleMode) { - throw new FilerException("No module to write to specified!"); - } + //module name specified: + module = moduleAndPkgString.substring(0, slash); + pkg = moduleAndPkgString.substring(slash + 1); + } + + ModuleSymbol explicitModule = syms.getModule(names.fromString(module)); - return Pair.of(modules.getDefaultModule(), moduleAndPkgString); + if (explicitModule == null) { + throw new FilerException("Module: " + module + " does not exist."); } + + if (!modules.isRootModule(explicitModule)) { + throw new FilerException("Cannot write to the given module."); + } + + return Pair.of(explicitModule, pkg); } private JavaFileObject createSourceOrClassFile(ModuleSymbol mod, boolean isSourceFile, String name) throws IOException { @@ -495,17 +516,13 @@ CharSequence moduleAndPkg, CharSequence relativeName, Element... originatingElements) throws IOException { - Pair moduleAndPackage = checkOrInferModule(moduleAndPkg); - ModuleSymbol msym = moduleAndPackage.fst; - String pkg = moduleAndPackage.snd; + Tuple3 locationModuleAndPackage = checkOrInferModule(location, moduleAndPkg, true); + location = locationModuleAndPackage.a; + ModuleSymbol msym = locationModuleAndPackage.b; + String pkg = locationModuleAndPackage.c; locationCheck(location); - if (modules.multiModuleMode) { - Assert.checkNonNull(msym); - location = this.fileManager.getLocationForModule(location, msym.name.toString()); - } - String strPkg = pkg.toString(); if (strPkg.length() > 0) checkName(strPkg); @@ -534,14 +551,9 @@ public FileObject getResource(JavaFileManager.Location location, CharSequence moduleAndPkg, CharSequence relativeName) throws IOException { - Pair moduleAndPackage = checkOrInferModule(moduleAndPkg); - ModuleSymbol msym = moduleAndPackage.fst; - String pkg = moduleAndPackage.snd; - - if (modules.multiModuleMode) { - Assert.checkNonNull(msym); - location = this.fileManager.getLocationForModule(location, msym.name.toString()); - } + Tuple3 locationModuleAndPackage = checkOrInferModule(location, moduleAndPkg, false); + location = locationModuleAndPackage.a; + String pkg = locationModuleAndPackage.c; if (pkg.length() > 0) checkName(pkg); @@ -578,6 +590,99 @@ return new FilerInputFileObject(fileObject); } + private Tuple3 checkOrInferModule(JavaFileManager.Location location, + CharSequence moduleAndPkg, + boolean write) throws IOException { + String moduleAndPkgString = moduleAndPkg.toString(); + int slash = moduleAndPkgString.indexOf('/'); + boolean multiModuleLocation = location.isModuleOrientedLocation() || + (modules.multiModuleMode && location.isOutputLocation()); + String module; + String pkg; + + if (slash == (-1)) { + //module name not specified: + if (!multiModuleLocation) { + //package oriented location: + return new Tuple3<>(location, modules.getDefaultModule(), moduleAndPkgString); + } + + if (location.isOutputLocation()) { + ModuleSymbol msym = inferModule(moduleAndPkgString); + + if (msym != null) { + Location moduleLoc = + fileManager.getLocationForModule(location, msym.name.toString()); + return new Tuple3<>(moduleLoc, msym, moduleAndPkgString); + } + } + + if (defaultTargetModule == null) { + throw new FilerException("No module specified and the location is either " + + "a module-oriented location, or a multi-module " + + "output location."); + } + + module = defaultTargetModule; + pkg = moduleAndPkgString; + } else { + //module name specified: + module = moduleAndPkgString.substring(0, slash); + pkg = moduleAndPkgString.substring(slash + 1); + } + + if (multiModuleLocation) { + ModuleSymbol explicitModule = syms.getModule(names.fromString(module)); + + if (explicitModule == null) { + throw new FilerException("Module: " + module + " does not exist."); + } + + if (write && !modules.isRootModule(explicitModule)) { + throw new FilerException("Cannot write to the given module."); + } + + Location moduleLoc = fileManager.getLocationForModule(location, module); + + return new Tuple3<>(moduleLoc, explicitModule, pkg); + } else { + throw new FilerException("Module specified but the location is neither " + + "a module-oriented location, nor a multi-module " + + "output location."); + } + } + + static final class Tuple3 { + final A a; + final B b; + final C c; + + public Tuple3(A a, B b, C c) { + this.a = a; + this.b = b; + this.c = c; + } + } + + private ModuleSymbol inferModule(String pkg) { + if (modules.getDefaultModule() == syms.noModule) + return modules.getDefaultModule(); + + Set rootModules = modules.getRootModules(); + + if (rootModules.size() == 1) { + return rootModules.iterator().next(); + } + + PackageSymbol pack = elementUtils.getPackageElement(pkg); + + if (pack != null && pack.modle != syms.unnamedModule) { + return pack.modle; + } + + return null; + } + private void checkName(String name) throws FilerException { checkName(name, false); } diff -r 202973b2d1ae -r e1b620ac6c98 langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java --- a/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java Tue Mar 14 08:19:41 2017 +0100 +++ b/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java Tue Mar 14 10:51:19 2017 +0100 @@ -1059,8 +1059,10 @@ roots = prev.roots.appendList(parsedFiles); // Check for errors after parsing - if (unrecoverableError()) + if (unrecoverableError()) { + compiler.initModules(List.nil()); return; + } roots = compiler.initModules(roots); diff -r 202973b2d1ae -r e1b620ac6c98 langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/javac.properties --- a/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/javac.properties Tue Mar 14 08:19:41 2017 +0100 +++ b/langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/javac.properties Tue Mar 14 10:51:19 2017 +0100 @@ -131,6 +131,8 @@ "name args" javac.opt.arg.multi-release=\ +javac.opt.arg.default.module.for.created.files=\ + ## extended options @@ -323,6 +325,8 @@ javac.opt.inherit_runtime_environment=\ Inherit module system configuration options from the runtime environment. +javac.opt.default.module.for.created.files=\ + Fallback target module for files created by annotation processors, if none specified or inferred. ## errors diff -r 202973b2d1ae -r e1b620ac6c98 langtools/test/tools/javac/modules/AnnotationProcessing.java --- a/langtools/test/tools/javac/modules/AnnotationProcessing.java Tue Mar 14 08:19:41 2017 +0100 +++ b/langtools/test/tools/javac/modules/AnnotationProcessing.java Tue Mar 14 10:51:19 2017 +0100 @@ -23,7 +23,7 @@ /** * @test - * @bug 8133884 8162711 8133896 8172158 8172262 8173636 + * @bug 8133884 8162711 8133896 8172158 8172262 8173636 8175119 * @summary Verify that annotation processing works. * @library /tools/lib * @modules @@ -37,10 +37,12 @@ import java.io.IOException; import java.io.OutputStream; import java.io.Reader; +import java.io.UncheckedIOException; import java.io.Writer; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; import java.util.HashSet; @@ -49,6 +51,7 @@ import java.util.Objects; import java.util.Set; import java.util.concurrent.Callable; +import java.util.function.Consumer; import java.util.function.Function; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -527,53 +530,173 @@ tb.writeJavaFiles(m1, "module m1x { exports api1; }", - "package api1; public class Api { GenApi ga; impl.Impl i; }"); + "package api1; public class Api { }", + "package clash; public class C { }"); writeFile("1", m1, "api1", "api"); - writeFile("1", m1, "impl", "impl"); + writeFile("2", m1, "clash", "clash"); Path m2 = moduleSrc.resolve("m2x"); tb.writeJavaFiles(m2, "module m2x { requires m1x; exports api2; }", - "package api2; public class Api { api1.GenApi ga1; GenApi qa2; impl.Impl i;}"); + "package api2; public class Api { }", + "package clash; public class C { }"); + + writeFile("3", m2, "api2", "api"); + writeFile("4", m2, "clash", "api"); + + //passing testcases: + for (String module : Arrays.asList("", "m1x/")) { + for (String originating : Arrays.asList("", ", jlObject")) { + tb.writeJavaFiles(m1, + "package test; class Test { api1.Impl i; }"); - writeFile("2", m2, "api2", "api"); - writeFile("2", m2, "impl", "impl"); + //source: + runCompiler(base, + moduleSrc, + classes, + "createSource(() -> filer.createSourceFile(\"" + module + "api1.Impl\"" + originating + "), \"api1.Impl\", \"package api1; public class Impl {}\")", + "--module-source-path", moduleSrc.toString()); + assertFileExists(classes, "m1x", "api1", "Impl.class"); + + //class: + runCompiler(base, + moduleSrc, + classes, + "createClass(() -> filer.createClassFile(\"" + module + "api1.Impl\"" + originating + "), \"api1.Impl\", \"package api1; public class Impl {}\")", + "--module-source-path", moduleSrc.toString()); + assertFileExists(classes, "m1x", "api1", "Impl.class"); + + Files.delete(m1.resolve("test").resolve("Test.java")); - for (FileType fileType : FileType.values()) { - if (Files.isDirectory(classes)) { - tb.cleanDirectory(classes); - } else { - Files.createDirectories(classes); + //resource class output: + runCompiler(base, + moduleSrc, + classes, + "createSource(() -> filer.createResource(StandardLocation.CLASS_OUTPUT, \"" + module + "api1\", \"impl\"" + originating + "), \"impl\", \"impl\")", + "--module-source-path", moduleSrc.toString()); + assertFileExists(classes, "m1x", "api1", "impl"); } + } + + //get resource module source path: + runCompiler(base, + m1, + classes, + "doReadResource(() -> filer.getResource(StandardLocation.MODULE_SOURCE_PATH, \"m1x/api1\", \"api\"), \"1\")", + "--module-source-path", moduleSrc.toString()); + + //can generate resources to the single root module: + runCompiler(base, + m1, + classes, + "createSource(() -> filer.createResource(StandardLocation.CLASS_OUTPUT, \"m1x/impl\", \"impl\"), \"impl\", \"impl\")", + "--module-source-path", moduleSrc.toString()); + assertFileExists(classes, "m1x", "impl", "impl"); + + //check --default-module-for-created-files option: + for (String pack : Arrays.asList("clash", "doesnotexist")) { + tb.writeJavaFiles(m1, + "package test; class Test { " + pack + ".Pass i; }"); + runCompiler(base, + moduleSrc, + classes, + "createSource(() -> filer.createSourceFile(\"" + pack + ".Pass\")," + + " \"" + pack + ".Pass\"," + + " \"package " + pack + ";" + + " public class Pass { }\")", + "--module-source-path", moduleSrc.toString(), + "--default-module-for-created-files=m1x"); + assertFileExists(classes, "m1x", pack, "Pass.class"); + assertFileNotExists(classes, "m2x", pack, "Pass.class"); - new JavacTask(tb) - .options("-processor", MultiModeAPITestAP.class.getName(), - "--module-source-path", moduleSrc.toString(), - "-Afiletype=" + fileType.name()) - .outdir(classes) - .files(findJavaFiles(moduleSrc)) - .run() - .writeAll(); + runCompiler(base, + moduleSrc, + classes, + "createClass(() -> filer.createClassFile(\"" + pack + ".Pass\")," + + " \"" + pack + ".Pass\"," + + " \"package " + pack + ";" + + " public class Pass { }\")", + "--module-source-path", moduleSrc.toString(), + "--default-module-for-created-files=m1x"); + assertFileExists(classes, "m1x", pack, "Pass.class"); + assertFileNotExists(classes, "m2x", pack, "Pass.class"); + + Files.delete(m1.resolve("test").resolve("Test.java")); + + runCompiler(base, + moduleSrc, + classes, + "createSource(() -> filer.createResource(StandardLocation.CLASS_OUTPUT," + + " \"" + pack + "\", \"impl\"), \"impl\", \"impl\")", + "--module-source-path", moduleSrc.toString(), + "--default-module-for-created-files=m1x"); + assertFileExists(classes, "m1x", pack, "impl"); + assertFileNotExists(classes, "m2x", pack, "impl"); + + runCompiler(base, + moduleSrc, + classes, + "doReadResource(() -> filer.getResource(StandardLocation.CLASS_OUTPUT," + + " \"" + pack + "\", \"resource\"), \"1\")", + p -> writeFile("1", p.resolve("m1x"), pack, "resource"), + "--module-source-path", moduleSrc.toString(), + "--default-module-for-created-files=m1x"); + } - assertFileExists(classes, "m1x", "api1", "GenApi.class"); - assertFileExists(classes, "m1x", "impl", "Impl.class"); - assertFileExists(classes, "m1x", "api1", "gen1"); - assertFileExists(classes, "m2x", "api2", "GenApi.class"); - assertFileExists(classes, "m2x", "impl", "Impl.class"); - assertFileExists(classes, "m2x", "api2", "gen1"); + //wrong default module: + runCompiler(base, + moduleSrc, + classes, + "expectFilerException(() -> filer.createResource(StandardLocation.CLASS_OUTPUT," + + " \"clash\", \"impl\"))", + "--module-source-path", moduleSrc.toString(), + "--default-module-for-created-files=doesnotexist"); + + String[] failingCases = { + //must not generate to unnamed package: + "expectFilerException(() -> filer.createSourceFile(\"Fail\"))", + "expectFilerException(() -> filer.createClassFile(\"Fail\"))", + "expectFilerException(() -> filer.createSourceFile(\"m1x/Fail\"))", + "expectFilerException(() -> filer.createClassFile(\"m1x/Fail\"))", + + //cannot infer module name, package clash: + "expectFilerException(() -> filer.createSourceFile(\"clash.Fail\"))", + "expectFilerException(() -> filer.createClassFile(\"clash.Fail\"))", + "expectFilerException(() -> filer.createResource(StandardLocation.CLASS_OUTPUT, \"clash\", \"impl\"))", + "expectFilerException(() -> filer.getResource(StandardLocation.CLASS_OUTPUT, \"clash\", \"impl\"))", + + //cannot infer module name, package does not exist: + "expectFilerException(() -> filer.createSourceFile(\"doesnotexist.Fail\"))", + "expectFilerException(() -> filer.createClassFile(\"doesnotexist.Fail\"))", + "expectFilerException(() -> filer.createResource(StandardLocation.CLASS_OUTPUT, \"doesnotexist\", \"impl\"))", + "expectFilerException(() -> filer.getResource(StandardLocation.CLASS_OUTPUT, \"doesnotexist\", \"impl\"))", + + //cannot generate sources/classes to modules that are not root modules: + "expectFilerException(() -> filer.createSourceFile(\"java.base/fail.Fail\"))", + "expectFilerException(() -> filer.createClassFile(\"java.base/fail.Fail\"))", + + //cannot read from module locations if module not given and not inferable: + "expectFilerException(() -> filer.getResource(StandardLocation.SYSTEM_MODULES, \"fail\", \"Fail\"))", + + //wrong module given: + "expectException(() -> filer.getResource(StandardLocation.SYSTEM_MODULES, \"java.compiler/java.lang\", \"Object.class\"))", + }; + + for (String failingCode : failingCases) { + System.err.println("failing code: " + failingCode); + runCompiler(base, + moduleSrc, + classes, + failingCode, + "--module-source-path", moduleSrc.toString()); } } - enum FileType { - SOURCE, - CLASS; - } - public static abstract class GeneratingAP extends AbstractProcessor { - void createSource(CreateFileObject file, String name, String content) { + public void createSource(CreateFileObject file, String name, String content) { try (Writer out = file.create().openWriter()) { out.write(content); } catch (IOException ex) { @@ -581,7 +704,7 @@ } } - void createClass(CreateFileObject file, String name, String content) { + public void createClass(CreateFileObject file, String name, String content) { String fileNameStub = name.replace(".", File.separator); try (OutputStream out = file.create().openOutputStream()) { @@ -617,7 +740,7 @@ } } - void doReadResource(CreateFileObject file, String expectedContent) { + public void doReadResource(CreateFileObject file, String expectedContent) { try { StringBuilder actualContent = new StringBuilder(); @@ -636,11 +759,19 @@ } } + public void checkResourceExists(CreateFileObject file) { + try { + file.create().openInputStream().close(); + } catch (IOException ex) { + throw new IllegalStateException(ex); + } + } + public interface CreateFileObject { public FileObject create() throws IOException; } - void expectFilerException(Callable c) { + public void expectFilerException(Callable c) { try { c.call(); throw new AssertionError("Expected exception not thrown"); @@ -651,6 +782,17 @@ } } + public void expectException(Callable c) { + try { + c.call(); + throw new AssertionError("Expected exception not thrown"); + } catch (IOException ex) { + //expected + } catch (Exception ex) { + throw new IllegalStateException(ex); + } + } + @Override public SourceVersion getSupportedSourceVersion() { return SourceVersion.latest(); @@ -658,167 +800,220 @@ } - @SupportedAnnotationTypes("*") - @SupportedOptions({"filetype", "modulename"}) - public static final class MultiModeAPITestAP extends GeneratingAP { - - int round; - - @Override - public boolean process(Set annotations, RoundEnvironment roundEnv) { - if (round++ != 0) - return false; - - createClass("m1x", "api1.GenApi", "package api1; public class GenApi {}"); - createClass("m1x", "impl.Impl", "package impl; public class Impl {}"); - createClass("m2x", "api2.GenApi", "package api2; public class GenApi {}"); - createClass("m2x", "impl.Impl", "package impl; public class Impl {}"); - - createResource("m1x", "api1", "gen1"); - createResource("m2x", "api2", "gen1"); - - readResource("m1x", "api1", "api", "1"); - readResource("m1x", "impl", "impl", "1"); - readResource("m2x", "api2", "api", "2"); - readResource("m2x", "impl", "impl", "2"); - - Filer filer = processingEnv.getFiler(); - - expectFilerException(() -> filer.createSourceFile("fail.Fail")); - expectFilerException(() -> filer.createClassFile("fail.Fail")); - expectFilerException(() -> filer.createResource(StandardLocation.CLASS_OUTPUT, "fail", "fail")); - expectFilerException(() -> filer.getResource(StandardLocation.MODULE_SOURCE_PATH, "fail", "fail")); - - //must not generate to unnamed package: - expectFilerException(() -> filer.createSourceFile("m1/Fail")); - expectFilerException(() -> filer.createClassFile("m1/Fail")); - - //cannot generate resources to modules that are not root modules: - expectFilerException(() -> filer.createSourceFile("java.base/fail.Fail")); - expectFilerException(() -> filer.createClassFile("java.base/fail.Fail")); - expectFilerException(() -> filer.createResource(StandardLocation.CLASS_OUTPUT, "java.base/fail", "Fail")); - - return false; - } - - void createClass(String expectedModule, String name, String content) { - Filer filer = processingEnv.getFiler(); - FileType filetype = FileType.valueOf(processingEnv.getOptions().getOrDefault("filetype", "")); - - switch (filetype) { - case SOURCE: - createSource(() -> filer.createSourceFile(expectedModule + "/" + name), name, content); - break; - case CLASS: - createClass(() -> filer.createClassFile(expectedModule + "/" + name), name, content); - break; - default: - throw new AssertionError("Unexpected filetype: " + filetype); - } - } - - void createResource(String expectedModule, String pkg, String relName) { - try { - Filer filer = processingEnv.getFiler(); - - filer.createResource(StandardLocation.CLASS_OUTPUT, expectedModule + "/" + pkg, relName) - .openOutputStream() - .close(); - } catch (IOException ex) { - throw new IllegalStateException(ex); - } - } - - void readResource(String expectedModule, String pkg, String relName, String expectedContent) { - Filer filer = processingEnv.getFiler(); - - doReadResource(() -> filer.getResource(StandardLocation.MODULE_SOURCE_PATH, expectedModule + "/" + pkg, relName), - expectedContent); - } - - } - @Test - public void testGenerateInSingleNameModeAPI(Path base) throws Exception { + public void testGenerateSingleModule(Path base) throws Exception { Path classes = base.resolve("classes"); Files.createDirectories(classes); - Path m1 = base.resolve("module-src"); + Path src = base.resolve("module-src"); + Path m1 = src.resolve("m1x"); tb.writeJavaFiles(m1, - "module m1x { }"); + "module m1x { }", + "package test; class Test { impl.Impl i; }"); + Path m2 = src.resolve("m2x"); - writeFile("3", m1, "impl", "resource"); + tb.writeJavaFiles(m2, + "module m2x { }"); - new JavacTask(tb) - .options("-processor", SingleNameModeAPITestAP.class.getName(), - "-sourcepath", m1.toString()) - .outdir(classes) - .files(findJavaFiles(m1)) - .run() - .writeAll(); + for (String[] options : new String[][] {new String[] {"-sourcepath", m1.toString()}, + new String[] {"--module-source-path", src.toString()}}) { + String modulePath = options[0].equals("--module-source-path") ? "m1x" : ""; + //passing testcases: + for (String module : Arrays.asList("", "m1x/")) { + for (String originating : Arrays.asList("", ", jlObject")) { + tb.writeJavaFiles(m1, + "package test; class Test { impl.Impl i; }"); - assertFileExists(classes, "impl", "Impl1.class"); - assertFileExists(classes, "impl", "Impl2.class"); - assertFileExists(classes, "impl", "Impl3"); - assertFileExists(classes, "impl", "Impl4.class"); - assertFileExists(classes, "impl", "Impl5.class"); - assertFileExists(classes, "impl", "Impl6"); - assertFileExists(classes, "impl", "Impl7.class"); - assertFileExists(classes, "impl", "Impl8.class"); - assertFileExists(classes, "impl", "Impl9"); - } + //source: + runCompiler(base, + m1, + classes, + "createSource(() -> filer.createSourceFile(\"" + module + "impl.Impl\"" + originating + "), \"impl.Impl\", \"package impl; public class Impl {}\")", + options); + assertFileExists(classes, modulePath, "impl", "Impl.class"); + //class: + runCompiler(base, + m1, + classes, + "createClass(() -> filer.createClassFile(\"" + module + "impl.Impl\"" + originating + "), \"impl.Impl\", \"package impl; public class Impl {}\")", + options); + assertFileExists(classes, modulePath, "impl", "Impl.class"); - @SupportedAnnotationTypes("*") - public static final class SingleNameModeAPITestAP extends GeneratingAP { + Files.delete(m1.resolve("test").resolve("Test.java")); - int round; - - @Override - public synchronized void init(ProcessingEnvironment processingEnv) { - super.init(processingEnv); + //resource class output: + runCompiler(base, + m1, + classes, + "createSource(() -> filer.createResource(StandardLocation.CLASS_OUTPUT, \"impl\", \"impl\"" + originating + "), \"impl\", \"impl\")", + options); + assertFileExists(classes, modulePath, "impl", "impl"); + } + } } - @Override - public boolean process(Set annotations, RoundEnvironment roundEnv) { - if (round++ != 0) - return false; + //get resource source path: + writeFile("1", m1, "impl", "resource"); + runCompiler(base, + m1, + classes, + "doReadResource(() -> filer.getResource(StandardLocation.SOURCE_PATH, \"impl\", \"resource\"), \"1\")", + "-sourcepath", m1.toString()); + //must not specify module when reading non-module oriented locations: + runCompiler(base, + m1, + classes, + "expectFilerException(() -> filer.getResource(StandardLocation.SOURCE_PATH, \"m1x/impl\", \"resource\"))", + "-sourcepath", m1.toString()); - Filer filer = processingEnv.getFiler(); + Files.delete(m1.resolve("impl").resolve("resource")); - createSource(() -> filer.createSourceFile("impl.Impl1"), "impl.Impl1", "package impl; class Impl1 {}"); - createClass(() -> filer.createClassFile("impl.Impl2"), "impl.Impl2", "package impl; class Impl2 {}"); - createSource(() -> filer.createResource(StandardLocation.CLASS_OUTPUT, "impl", "Impl3"), "impl.Impl3", ""); - doReadResource(() -> filer.getResource(StandardLocation.SOURCE_PATH, "impl", "resource"), "3"); + //can read resources from the system module path if module name given: + runCompiler(base, + m1, + classes, + "checkResourceExists(() -> filer.getResource(StandardLocation.SYSTEM_MODULES, \"java.base/java.lang\", \"Object.class\"))", + "-sourcepath", m1.toString()); - createSource(() -> filer.createSourceFile("m1x/impl.Impl4"), "impl.Impl4", "package impl; class Impl4 {}"); - createClass(() -> filer.createClassFile("m1x/impl.Impl5"), "impl.Impl5", "package impl; class Impl5 {}"); - createSource(() -> filer.createResource(StandardLocation.CLASS_OUTPUT, "m1x/impl", "Impl6"), "impl.Impl6", ""); - doReadResource(() -> filer.getResource(StandardLocation.SOURCE_PATH, "m1x/impl", "resource"), "3"); + //can read resources from the system module path if module inferable: + runCompiler(base, + m1, + classes, + "expectFilerException(() -> filer.getResource(StandardLocation.SYSTEM_MODULES, \"java.lang\", \"Object.class\"))", + "-sourcepath", m1.toString()); + + //cannot generate resources to modules that are not root modules: + runCompiler(base, + m1, + classes, + "expectFilerException(() -> filer.createResource(StandardLocation.CLASS_OUTPUT, \"java.base/fail\", \"Fail\"))", + "--module-source-path", src.toString()); - TypeElement jlObject = processingEnv.getElementUtils().getTypeElement("java.lang.Object"); + //can generate resources to the single root module: + runCompiler(base, + m1, + classes, + "createSource(() -> filer.createResource(StandardLocation.CLASS_OUTPUT, \"impl\", \"impl\"), \"impl\", \"impl\")", + "--module-source-path", src.toString()); + assertFileExists(classes, "m1x", "impl", "impl"); - //"broken" originating element: - createSource(() -> filer.createSourceFile("impl.Impl7", jlObject), "impl.Impl7", "package impl; class Impl7 {}"); - createClass(() -> filer.createClassFile("impl.Impl8", jlObject), "impl.Impl8", "package impl; class Impl8 {}"); - createSource(() -> filer.createResource(StandardLocation.CLASS_OUTPUT, "impl", "Impl9", jlObject), "impl.Impl9", ""); - + String[] failingCases = { //must not generate to unnamed package: - expectFilerException(() -> filer.createSourceFile("Fail")); - expectFilerException(() -> filer.createClassFile("Fail")); - expectFilerException(() -> filer.createSourceFile("m1x/Fail")); - expectFilerException(() -> filer.createClassFile("m1x/Fail")); + "expectFilerException(() -> filer.createSourceFile(\"Fail\"))", + "expectFilerException(() -> filer.createClassFile(\"Fail\"))", + "expectFilerException(() -> filer.createSourceFile(\"m1x/Fail\"))", + "expectFilerException(() -> filer.createClassFile(\"m1x/Fail\"))", + + //cannot generate sources/classes to modules that are not root modules: + "expectFilerException(() -> filer.createSourceFile(\"java.base/fail.Fail\"))", + "expectFilerException(() -> filer.createClassFile(\"java.base/fail.Fail\"))", + + //cannot specify module name for class output when not in the multi-module mode: + "expectFilerException(() -> filer.createResource(StandardLocation.CLASS_OUTPUT, \"m1x/fail\", \"Fail\"))", - //cannot generate resources to modules that are not root modules: - expectFilerException(() -> filer.createSourceFile("java.base/fail.Fail")); - expectFilerException(() -> filer.createClassFile("java.base/fail.Fail")); - expectFilerException(() -> filer.createResource(StandardLocation.CLASS_OUTPUT, "java.base/fail", "Fail")); + //cannot read from module locations if module not given: + "expectFilerException(() -> filer.getResource(StandardLocation.SYSTEM_MODULES, \"fail\", \"Fail\"))", + + //wrong module given: + "expectException(() -> filer.getResource(StandardLocation.SYSTEM_MODULES, \"java.compiler/java.lang\", \"Object.class\"))", + }; - return false; + for (String failingCode : failingCases) { + System.err.println("failing code: " + failingCode); + runCompiler(base, + m1, + classes, + failingCode, + "-sourcepath", m1.toString()); } + Files.delete(m1.resolve("module-info.java")); + tb.writeJavaFiles(m1, + "package test; class Test { }"); + + runCompiler(base, + m1, + classes, + "expectFilerException(() -> filer.createSourceFile(\"m1x/impl.Impl\"))", + "-sourcepath", m1.toString(), + "-source", "8"); + + runCompiler(base, + m1, + classes, + "expectFilerException(() -> filer.createClassFile(\"m1x/impl.Impl\"))", + "-sourcepath", m1.toString(), + "-source", "8"); + } + + private void runCompiler(Path base, Path src, Path classes, + String code, String... options) throws IOException { + runCompiler(base, src, classes, code, p -> {}, options); + } + + private void runCompiler(Path base, Path src, Path classes, + String code, Consumer generateToClasses, + String... options) throws IOException { + Path apClasses = base.resolve("ap-classes"); + if (Files.exists(apClasses)) { + tb.cleanDirectory(apClasses); + } else { + Files.createDirectories(apClasses); + } + compileAP(apClasses, code); + if (Files.exists(classes)) { + tb.cleanDirectory(classes); + } else { + Files.createDirectories(classes); + } + generateToClasses.accept(classes); + List opts = new ArrayList<>(); + opts.addAll(Arrays.asList(options)); + opts.add("-processorpath"); + opts.add(System.getProperty("test.class.path") + File.pathSeparator + apClasses.toString()); + opts.add("-processor"); + opts.add("AP"); + new JavacTask(tb) + .options(opts) + .outdir(classes) + .files(findJavaFiles(src)) + .run() + .writeAll(); + } + + private void compileAP(Path target, String code) { + String processorCode = + "import java.util.*;\n" + + "import javax.annotation.processing.*;\n" + + "import javax.lang.model.*;\n" + + "import javax.lang.model.element.*;\n" + + "import javax.lang.model.type.*;\n" + + "import javax.lang.model.util.*;\n" + + "import javax.tools.*;\n" + + "@SupportedAnnotationTypes(\"*\")\n" + + "public final class AP extends AnnotationProcessing.GeneratingAP {\n" + + "\n" + + " int round;\n" + + "\n" + + " @Override\n" + + " public boolean process(Set annotations, RoundEnvironment roundEnv) {\n" + + " if (round++ != 0)\n" + + " return false;\n" + + " Filer filer = processingEnv.getFiler();\n" + + " TypeElement jlObject = processingEnv.getElementUtils().getTypeElement(\"java.lang.Object\");\n" + + code + ";\n" + + " return false;\n" + + " }\n" + + " }\n"; + new JavacTask(tb) + .options("-classpath", System.getProperty("test.class.path")) + .sources(processorCode) + .outdir(target) + .run() + .writeAll(); } @Test @@ -1089,13 +1284,17 @@ } - private static void writeFile(String content, Path base, String... pathElements) throws IOException { - Path file = resolveFile(base, pathElements); + private static void writeFile(String content, Path base, String... pathElements) { + try { + Path file = resolveFile(base, pathElements); + + Files.createDirectories(file.getParent()); - Files.createDirectories(file.getParent()); - - try (Writer out = Files.newBufferedWriter(file)) { - out.append(content); + try (Writer out = Files.newBufferedWriter(file)) { + out.append(content); + } + } catch (IOException ex) { + throw new UncheckedIOException(ex); } } @@ -1286,6 +1485,35 @@ } + @Test + public void testWrongDefaultTargetModule(Path base) throws Exception { + Path src = base.resolve("src"); + + tb.writeJavaFiles(src, + "package test; public class Test { }"); + + Path classes = base.resolve("classes"); + + Files.createDirectories(classes); + + List log = new JavacTask(tb) + .options("--default-module-for-created-files=m!", + "-XDrawDiagnostics") + .outdir(classes) + .files(findJavaFiles(src)) + .run(Task.Expect.FAIL) + .writeAll() + .getOutputLines(OutputKind.DIRECT); + + List expected = Arrays.asList( + "- compiler.err.bad.name.for.option: --default-module-for-created-files, m!" + ); + + if (!log.equals(expected)) { + throw new AssertionError("Expected output not found."); + } + } + private static void assertNonNull(String msg, Object val) { if (val == null) { throw new AssertionError(msg); @@ -1312,6 +1540,14 @@ } } + private static void assertFileNotExists(Path base, String... pathElements) { + Path file = resolveFile(base, pathElements); + + if (Files.exists(file)) { + throw new AssertionError("Expected file: " + file + " exist, but it does not."); + } + } + static Path resolveFile(Path base, String... pathElements) { Path file = base;