From c3fd232eb5dcf0e7b5f4262422155757bf5e35fb Mon Sep 17 00:00:00 2001 From: Gunnar Wagenknecht Date: Sat, 8 Jun 2024 17:26:21 +0200 Subject: [PATCH] Provide ability to track dependencies during batch compilation (#2529) Added callback to track FileSystem.findType() results during compilation. The callback (if set by clients) will see all types requested by the compiler during compilation, so it will be able to track used/unused compilation dependencies (important for example for bazel Java toolchain). Fixes https://github.com/eclipse-jdt/eclipse.jdt.core/issues/2529 Also by: Andrey Loskutov --- .../internal/compiler/batch/FileSystem.java | 31 +++- .../jdt/internal/compiler/batch/Main.java | 10 +- .../NameEnvironmentAnswerListenerTest.java | 175 ++++++++++++++++++ .../tests/compiler/regression/TestAll.java | 1 + 4 files changed, 211 insertions(+), 6 deletions(-) create mode 100644 org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NameEnvironmentAnswerListenerTest.java diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/batch/FileSystem.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/batch/FileSystem.java index 7f2aa26de87..e32d77ef6c2 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/batch/FileSystem.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/batch/FileSystem.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2000, 2021 IBM Corporation and others. + * Copyright (c) 2000, 2024 IBM Corporation and others. * * This program and the accompanying materials * are made available under the terms of the Eclipse Public License 2.0 @@ -12,6 +12,8 @@ * IBM Corporation - initial API and implementation * Stephan Herrmann - Contribution for * Bug 440687 - [compiler][batch][null] improve command line option for external annotations + * Salesforce - Contribution for + * https://github.com/eclipse-jdt/eclipse.jdt.core/issues/2529 *******************************************************************************/ package org.eclipse.jdt.internal.compiler.batch; @@ -166,6 +168,7 @@ public static ArrayList normalize(ArrayList classpaths) { protected boolean annotationsFromClasspath; // should annotation files be read from the classpath (vs. explicit separate path)? private static HashMap JRT_CLASSPATH_CACHE = null; protected Map moduleLocations = new HashMap<>(); + private Consumer nameEnvironmentAnswerListener; // a listener for findType* answers /** Tasks resulting from --add-reads or --add-exports command line options. */ Map moduleUpdates = new HashMap<>(); @@ -447,7 +450,7 @@ private NameEnvironmentAnswer findClass(String qualifiedTypeName, char[] typeNam } answer.setBinaryType(ExternalAnnotationDecorator.create(answer.getBinaryType(), classpathEntry.getPath(), qualifiedTypeName, zip)); - return answer; + return notify(answer); } catch (IOException e) { // ignore broken entry, keep searching } finally { @@ -461,8 +464,20 @@ private NameEnvironmentAnswer findClass(String qualifiedTypeName, char[] typeNam // globally configured (annotationsFromClasspath), but no .eea found, decorate in order to answer NO_EEA_FILE: answer.setBinaryType(new ExternalAnnotationDecorator(answer.getBinaryType(), null)); } + return notify(answer); +} + +private NameEnvironmentAnswer notify(NameEnvironmentAnswer answer) { + if(answer == null) { + return answer; + } + Consumer listener = this.nameEnvironmentAnswerListener; + if(listener != null) { + listener.accept(answer); + } return answer; } + private NameEnvironmentAnswer internalFindClass(String qualifiedTypeName, char[] typeName, boolean asBinaryOnly, /*NonNull*/char[] moduleName) { if (this.knownFileNames.contains(qualifiedTypeName)) return null; // looking for a file which we know was provided at the beginning of the compilation @@ -750,4 +765,16 @@ public void applyModuleUpdates(IUpdatableModule compilerModule, IUpdatableModule } } } + +/** + * @param nameEnvironmentAnswerListener + * a listener for {@link NameEnvironmentAnswer} returned by findType* methods; useful for + * tracking used/answered dependencies during compilation (may be null to unset) + * @return a previously set listener (may be null) + */ +public Consumer setNameEnvironmentAnswerListener(Consumer nameEnvironmentAnswerListener) { + Consumer existing = this.nameEnvironmentAnswerListener; + this.nameEnvironmentAnswerListener = nameEnvironmentAnswerListener; + return existing; +} } 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 cd1e843ec6b..99e49286c0e 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 @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2000, 2023 IBM Corporation and others. + * Copyright (c) 2000, 2024 IBM Corporation and others. * * This program and the accompanying materials * are made available under the terms of the Eclipse Public License 2.0 @@ -27,8 +27,10 @@ * Bug 408815 - [batch][null] Add CLI option for COMPILER_PB_SYNTACTIC_NULL_ANALYSIS_FOR_FIELDS * Jesper S Moller - Contributions for * bug 407297 - [1.8][compiler] Control generation of parameter names by option - * Mat Booth - Contribution for bug 405176 - * Frits Jalvingh - fix for bug 533830. + * Mat Booth - Contribution for bug 405176 + * Frits Jalvingh - fix for bug 533830. + * Salesforce - Contribution for + * https://github.com/eclipse-jdt/eclipse.jdt.core/issues/2529 *******************************************************************************/ package org.eclipse.jdt.internal.compiler.batch; @@ -3842,7 +3844,7 @@ protected void handleWarningToken(String token, boolean isEnabling) { protected void handleErrorToken(String token, boolean isEnabling) { handleErrorOrWarningToken(token, isEnabling, ProblemSeverities.Error); } -private void setSeverity(String compilerOptions, int severity, boolean isEnabling) { +protected void setSeverity(String compilerOptions, int severity, boolean isEnabling) { if (isEnabling) { switch(severity) { case ProblemSeverities.Error : diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NameEnvironmentAnswerListenerTest.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NameEnvironmentAnswerListenerTest.java new file mode 100644 index 00000000000..d08d4acc6fd --- /dev/null +++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NameEnvironmentAnswerListenerTest.java @@ -0,0 +1,175 @@ +/******************************************************************************* + * Copyright (c) 2024 Salesforce and others. + * + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Salesforce - initial API and implementation + *******************************************************************************/ +package org.eclipse.jdt.core.tests.compiler.regression; + +import static java.util.stream.Collectors.joining; + +import java.io.File; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.PrintWriter; +import java.net.URI; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; + +import org.eclipse.jdt.core.JavaCore; +import org.eclipse.jdt.core.tests.util.Util; +import org.eclipse.jdt.internal.compiler.batch.FileSystem; +import org.eclipse.jdt.internal.compiler.batch.Main; +import org.eclipse.jdt.internal.compiler.env.NameEnvironmentAnswer; +import org.eclipse.jdt.internal.compiler.impl.CompilerOptions; +import org.eclipse.jdt.internal.compiler.problem.ProblemSeverities; +import org.junit.Assert; + +public class NameEnvironmentAnswerListenerTest extends AbstractComparableTest { + + public NameEnvironmentAnswerListenerTest(String name) { + super(name); + } + + /** + * Extension of ECJ Batch compiler to allow pre-configuration as well as registration of listener. + *

+ * This is modeled after real-world use in Bazel ECJ Toolchain. + *

+ * @see BlazeEcjMain.java + */ + static class EclipseBatchCompiler extends Main { + + Set answeredFileNames = new LinkedHashSet<>(); + + public EclipseBatchCompiler(PrintWriter errAndOutWriter) { + super(errAndOutWriter, errAndOutWriter, false /* systemExitWhenFinished */, null /* customDefaultOptions */, + null /* compilationProgress */); + + setSeverity(CompilerOptions.OPTION_ReportForbiddenReference, ProblemSeverities.Error, true); + setSeverity(CompilerOptions.OPTION_ReportDiscouragedReference, ProblemSeverities.Error, true); + } + + @Override + public FileSystem getLibraryAccess() { + // we use this to collect information about all used dependencies during + // compilation + FileSystem nameEnvironment = super.getLibraryAccess(); + nameEnvironment.setNameEnvironmentAnswerListener(this::recordNameEnvironmentAnswer); + return nameEnvironment; + } + + protected void recordNameEnvironmentAnswer(NameEnvironmentAnswer answer) { + Assert.assertNotNull("don't call without answer", answer); + + char[] fileName = null; + if(answer.getBinaryType() != null) { + URI uri = answer.getBinaryType().getURI(); + this.answeredFileNames.add(uri.toString()); + return; + } else if(answer.getCompilationUnit() != null) { + fileName = answer.getCompilationUnit().getFileName(); + } else if(answer.getSourceTypes() != null && answer.getSourceTypes().length > 0) { + fileName = answer.getSourceTypes()[0].getFileName(); // the first type is guaranteed to be the requested type + } else if(answer.getResolvedBinding() != null) { + fileName = answer.getResolvedBinding().getFileName(); + } + if (fileName != null) this.answeredFileNames.add(new String(fileName)); + } + } + + public void testNameEnvironmentAnswerListener() throws IOException { + String path = LIB_DIR; + if(!path.endsWith(File.separator)) { + path += File.separator; + } + String libPath = path + "lib.jar"; + Util.createJar( + new String[] { + "p/Color.java", + "package p;\n" + + "public enum Color {\n" + + " R, Y;\n" + + " public static Color getColor() {\n" + + " return R;\n" + + " }\n" + + "}", + }, + libPath, JavaCore.VERSION_17); + + String unusedLibPath = path + "lib_unused.jar"; + Util.createJar( + new String[] { + "p2/Color.java", + "package p2;\n" + + "public enum Color {\n" + + " R, Y;\n" + + " public static Color getColor() {\n" + + " return R;\n" + + " }\n" + + "}", + }, + unusedLibPath, JavaCore.VERSION_17); + + String srcDir = path + "src"; + String[] pathsAndContents = + new String[] { + "s/X.java", + "package s;\n" + + "import p.Color;\n" + + "public class X {\n" + + " public static final Color MY = Color.R;\n" + + "}" + }; + Util.createSourceDir(pathsAndContents, srcDir); + + List classpath = new ArrayList<>(Arrays.asList(getDefaultClassPaths())); + classpath.add(libPath); + classpath.add(unusedLibPath); + + File outputDirectory = new File(Util.getOutputDirectory()); + if (!outputDirectory.isDirectory()) { + outputDirectory.mkdirs(); + } + + List ecjArguments = new ArrayList<>(); + + ecjArguments.add("-classpath"); + ecjArguments.add(classpath.stream() + .map(jar -> jar.equals(unusedLibPath) ? String.format("%s[-**/*]", jar) : jar) + .collect(joining(File.pathSeparator))); + + + ecjArguments.add("-d"); + ecjArguments.add(outputDirectory.getAbsolutePath()); + + ecjArguments.add("--release"); + ecjArguments.add("17"); + + ecjArguments.add(srcDir+ File.separator + "s"+ File.separator + "X.java"); + + EclipseBatchCompiler compiler; + File logFile = new File(outputDirectory, "compile.log"); + try(PrintWriter log = new PrintWriter(new FileOutputStream(logFile))) { + compiler = new EclipseBatchCompiler(log); + boolean compileOK; + compileOK = compiler.compile(ecjArguments.toArray(new String[ecjArguments.size()])); + if(!compileOK) { + String logOutputString = Util.fileContent(logFile.getAbsolutePath()); + Assert.fail("Compile failed, output: '" + logOutputString + "'"); + } + } + Assert.assertTrue("must reference p.Color", compiler.answeredFileNames.stream().anyMatch(s -> s.contains(libPath))); + Assert.assertFalse("must not reference p2.Color", compiler.answeredFileNames.stream().anyMatch(s -> s.contains(unusedLibPath))); + } +} diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/TestAll.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/TestAll.java index 99ff634539f..4b373a7d0a4 100644 --- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/TestAll.java +++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/TestAll.java @@ -94,6 +94,7 @@ public static Test suite() { standardTests.add(InitializationTests.class); standardTests.add(ResourceLeakTests.class); standardTests.add(PackageBindingTest.class); + standardTests.add(NameEnvironmentAnswerListenerTest.class); // add all javadoc tests for (int i=0, l=JavadocTest.ALL_CLASSES.size(); i