From 51243db06b81f3dcedaaa30d30ddce463883b159 Mon Sep 17 00:00:00 2001 From: Stephan Herrmann Date: Thu, 13 Jun 2024 14:14:36 +0200 Subject: [PATCH] [batch compiler] Unrecognized option : --patch-module detect some errors + promote module not found to regular compile error --- .../ast/CompilationUnitDeclaration.java | 15 +- .../compiler/batch/CompilationUnit.java | 5 +- .../jdt/internal/compiler/batch/Main.java | 25 ++-- .../compiler/batch/messages.properties | 1 + .../compiler/problem/ProblemReporter.java | 5 + .../regression/ModuleCompilationTests.java | 132 ++++++++++++++++++ 6 files changed, 169 insertions(+), 14 deletions(-) diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/CompilationUnitDeclaration.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/CompilationUnitDeclaration.java index fc8f1c1666c..3d99670b175 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/CompilationUnitDeclaration.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/CompilationUnitDeclaration.java @@ -838,8 +838,19 @@ public ModuleBinding module(LookupEnvironment environment) { } if (this.compilationResult != null) { ICompilationUnit compilationUnit = this.compilationResult.compilationUnit; - if (compilationUnit != null) - return compilationUnit.module(environment); + if (compilationUnit != null) { + ModuleBinding module = compilationUnit.module(environment); + if (module == null) { + ReferenceContext save = environment.problemReporter.referenceContext; + try { + environment.problemReporter.referenceContext = this; + environment.problemReporter.moduleNotFound(this, compilationUnit.getModuleName()); + } finally { + environment.problemReporter.referenceContext = save; + } + } + return module; + } } return environment.module; } diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/batch/CompilationUnit.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/batch/CompilationUnit.java index 3a9c945f4bf..c875a3ced80 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/batch/CompilationUnit.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/batch/CompilationUnit.java @@ -133,10 +133,7 @@ public char[] getModuleName() { public ModuleBinding module(LookupEnvironment rootEnvironment) { if (this.moduleBinding != null) return this.moduleBinding; - this.moduleBinding = rootEnvironment.getModule(this.module); - if (this.moduleBinding == null) - throw new IllegalStateException("Module should be known"); //$NON-NLS-1$ - return this.moduleBinding; + return this.moduleBinding = rootEnvironment.getModule(this.module); } @Override public String getDestinationPath() { diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/batch/Main.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/batch/Main.java index d5c465fbb38..b57726553ab 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/batch/Main.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/batch/Main.java @@ -3382,6 +3382,18 @@ public CompilationUnit[] getCompilationUnits() { int[] orderedIndex = IntStream.range(0, fileCount).boxed().sorted((i1, i2) -> { return this.filenames[i1].compareTo(this.filenames[i2]); }).mapToInt(i -> i).toArray(); + Map location2patchedModule = new HashMap<>(); + if (this.patchModules != null) { + for (Entry entry : this.patchModules.entrySet()) { + StringTokenizer tokenizer = new StringTokenizer(entry.getValue(), File.pathSeparator); + while (tokenizer.hasMoreTokens()) { + String location = tokenizer.nextToken(); + if (location2patchedModule.put(location, entry.getKey()) != null) { + throw new IllegalArgumentException(this.bind("configure.duplicateLocationPatchModule", location)); //$NON-NLS-1$ + } + } + } + } for (int round = 0; round < 2; round++) { for (int i : orderedIndex) { char[] charName = this.filenames[i].toCharArray(); @@ -3423,15 +3435,12 @@ public CompilationUnit[] getCompilationUnits() { }; } String modName = this.modNames[i]; - if (modName == null && this.patchModules != null) { + if (modName == null) { // does this source file patch an existing module? - patchEntries: for (Entry entry : this.patchModules.entrySet()) { - StringTokenizer tokenizer = new StringTokenizer(entry.getValue(), File.pathSeparator); - while (tokenizer.hasMoreTokens()) { - if (fileName.startsWith(tokenizer.nextToken()+File.separator)) { - modName = entry.getKey(); - break patchEntries; - } + patchEntries: for (Entry entry : location2patchedModule.entrySet()) { + if (fileName.startsWith(entry.getKey()+File.separator)) { + modName = entry.getValue(); + break patchEntries; } } } diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/batch/messages.properties b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/batch/messages.properties index 6047ccdf18f..aa74509bb36 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/batch/messages.properties +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/batch/messages.properties @@ -82,6 +82,7 @@ configure.invalidModuleDescriptor = cannot open the module descriptor from {0} configure.invalidModuleOption = incorrectly formatted option: {0} configure.duplicateExport = can specify a package in a module only once with --add-export configure.duplicatePatchModule = duplicate module in --patch-module: {0} +configure.duplicateLocationPatchModule = location {0} is specified more than once in --patch-module configure.invalidSyntaxPatchModule = invalid syntax for --patch-module: {0} configure.OneOfModuleOrSourcePath = cannot specify both -source-path and --module-source-path configure.duplicateBootClasspath = duplicate bootclasspath specification: {0} diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/problem/ProblemReporter.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/problem/ProblemReporter.java index 76a37691cef..84265a3f95c 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/problem/ProblemReporter.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/problem/ProblemReporter.java @@ -11591,6 +11591,11 @@ public void invalidModule(ModuleReference ref) { String[] args = new String[] { CharOperation.charToString(ref.moduleName) }; this.handle(IProblem.UndefinedModule, args, args, ref.sourceStart, ref.sourceEnd); } +public void moduleNotFound(CompilationUnitDeclaration compilationUnitDeclaration, char[] moduleName) { + String[] args = new String[] { String.valueOf(moduleName) }; + ASTNode location = (compilationUnitDeclaration.currentPackage != null) ? compilationUnitDeclaration.currentPackage : compilationUnitDeclaration; + handle(IProblem.UndefinedModule, args, args, location.sourceStart, location.sourceEnd); +} public void missingModuleAddReads(char[] requiredModuleName) { String[] args = new String[] { new String(requiredModuleName) }; this.handle(IProblem.UndefinedModuleAddReads, args, args, 0, 0); diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/ModuleCompilationTests.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/ModuleCompilationTests.java index 848dbe9c7c8..32cd934e3ca 100644 --- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/ModuleCompilationTests.java +++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/ModuleCompilationTests.java @@ -6022,4 +6022,136 @@ public void testPatchModuleSingle() { "", false); } + + public void testPatchModuleSingle_duplicateModule() { + File outputDirectory = new File(OUTPUT_DIR); + Util.flushDirectoryContent(outputDirectory); + String src1 = OUTPUT_DIR + File.separator + "src1"; + String src2 = OUTPUT_DIR + File.separator + "src2"; + List files = new ArrayList<>(); + writeFileCollecting(files, src1 + File.separator + "test1", + "A.java", + "package test1;\n" + + "public class A {}"); + StringBuilder buffer = new StringBuilder(); + buffer.append(" -9 ") + .append(" -proc:none ") + .append(" --patch-module mod.one=\"").append(src1).append("\" ") + .append(" --patch-module mod.one=\"").append(src2).append("\" ") + .append(" --module-path \"").append(Util.getJavaClassLibsAsString()).append("\" "); + runNegativeModuleTest( + files, + buffer, + "", + "duplicate module in --patch-module: mod.one\n", + false, + "--patch-module specified more than once"); + } + + public void testPatchModuleSingle_duplicateLocation1() { + File outputDirectory = new File(OUTPUT_DIR); + Util.flushDirectoryContent(outputDirectory); + String src1 = OUTPUT_DIR + File.separator + "src1"; + List files = new ArrayList<>(); + writeFileCollecting(files, src1 + File.separator + "test1", + "A.java", + "package test1;\n" + + "public class A {}"); + StringBuilder buffer = new StringBuilder(); + buffer.append(" -9 ") + .append(" -proc:none ") + .append(" --patch-module mod.one=\"").append(src1).append("\"") + .append(File.pathSeparatorChar).append("\"").append(src1).append("\" ") + .append(" --module-path \"").append(Util.getJavaClassLibsAsString()).append("\" "); + runNegativeModuleTest( + files, + buffer, + "", + "location ---OUTPUT_DIR_PLACEHOLDER---/src1 is specified more than once in --patch-module\n", + false, + "", + "", + JavacTestOptions.SKIP); + } + + public void testPatchModuleSingle_duplicateLocation2() { + File outputDirectory = new File(OUTPUT_DIR); + Util.flushDirectoryContent(outputDirectory); + String src1 = OUTPUT_DIR + File.separator + "src1"; + List files = new ArrayList<>(); + writeFileCollecting(files, src1 + File.separator + "test1", + "A.java", + "package test1;\n" + + "public class A {}"); + StringBuilder buffer = new StringBuilder(); + buffer.append(" -9 ") + .append(" -proc:none ") + .append(" --patch-module mod.one=\"").append(src1).append("\" ") + .append(" --patch-module mod.two=\"").append(src1).append("\" ") + .append(" --module-path \"").append(Util.getJavaClassLibsAsString()).append("\" "); + runNegativeModuleTest( + files, + buffer, + "", + "location ---OUTPUT_DIR_PLACEHOLDER---/src1 is specified more than once in --patch-module\n", + false, + "", + "", + JavacTestOptions.SKIP); + } + + public void testPatchModuleSingle_syntaxErr() { + File outputDirectory = new File(OUTPUT_DIR); + Util.flushDirectoryContent(outputDirectory); + String src1 = OUTPUT_DIR + File.separator + "src1"; + String src2 = OUTPUT_DIR + File.separator + "src2"; + List files = new ArrayList<>(); + writeFileCollecting(files, src1 + File.separator + "test1", + "A.java", + "package test1;\n" + + "public class A {}"); + StringBuilder buffer = new StringBuilder(); + buffer.append(" -9 ") + .append(" -proc:none ") + .append(" --patch-module mod.one \"").append(src1).append("\" ") + .append(" --module-path \"").append(Util.getJavaClassLibsAsString()).append("\" "); + runNegativeModuleTest( + files, + buffer, + "", + "invalid syntax for --patch-module: mod.one\n", + false, + "bad value for --patch-module option"); + } + + public void testPatchModuleSingle_noSuchModule() { + File outputDirectory = new File(OUTPUT_DIR); + Util.flushDirectoryContent(outputDirectory); + String src1 = OUTPUT_DIR + File.separator + "src1"; + List files = new ArrayList<>(); + writeFileCollecting(files, src1 + File.separator + "test1", + "A.java", + "package test1;\n" + + "public class A {}"); + StringBuilder buffer = new StringBuilder(); + buffer.append(" -9 ") + .append(" -proc:none ") + .append(" --patch-module mod.one=\"").append(src1).append("\" ") + .append(" --module-path \"").append(Util.getJavaClassLibsAsString()).append("\" "); + runNegativeModuleTest( + files, + buffer, + "", + """ + ---------- + 1. ERROR in ---OUTPUT_DIR_PLACEHOLDER---/src1/test1/A.java (at line 1) + package test1; + ^^^^^ + mod.one cannot be resolved to a module + ---------- + 1 problem (1 error) + """, + false, + "module not found"); + } }