Skip to content

Commit

Permalink
Provide ability to track dependencies during batch compilation (eclip…
Browse files Browse the repository at this point in the history
…se-jdt#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 eclipse-jdt#2529

Also by: Andrey Loskutov <[email protected]>
  • Loading branch information
guw committed Jun 13, 2024
1 parent 18d9a53 commit c3fd232
Show file tree
Hide file tree
Showing 4 changed files with 211 additions and 6 deletions.
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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;

Expand Down Expand Up @@ -166,6 +168,7 @@ public static ArrayList<Classpath> normalize(ArrayList<Classpath> classpaths) {
protected boolean annotationsFromClasspath; // should annotation files be read from the classpath (vs. explicit separate path)?
private static HashMap<File, Classpath> JRT_CLASSPATH_CACHE = null;
protected Map<String,Classpath> moduleLocations = new HashMap<>();
private Consumer<NameEnvironmentAnswer> nameEnvironmentAnswerListener; // a listener for findType* answers

/** Tasks resulting from --add-reads or --add-exports command line options. */
Map<String,UpdatesByKind> moduleUpdates = new HashMap<>();
Expand Down Expand Up @@ -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 {
Expand All @@ -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<NameEnvironmentAnswer> 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

Expand Down Expand Up @@ -750,4 +765,16 @@ public void applyModuleUpdates(IUpdatableModule compilerModule, IUpdatableModule
}
}
}

/**
* @param nameEnvironmentAnswerListener
* a listener for {@link NameEnvironmentAnswer} returned by <code>findType*</code> methods; useful for
* tracking used/answered dependencies during compilation (may be <code>null</code> to unset)
* @return a previously set listener (may be <code>null</code>)
*/
public Consumer<NameEnvironmentAnswer> setNameEnvironmentAnswerListener(Consumer<NameEnvironmentAnswer> nameEnvironmentAnswerListener) {
Consumer<NameEnvironmentAnswer> existing = this.nameEnvironmentAnswerListener;
this.nameEnvironmentAnswerListener = nameEnvironmentAnswerListener;
return existing;
}
}
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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 :
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
* <p>
* This is modeled after real-world use in Bazel ECJ Toolchain.
* </p>
* @see <a href="https://github.com/salesforce/bazel-jdt-java-toolchain/blob/04c27501b9623300ff38f500cd53af8ce0a11835/compiler/src/main/buildjar/com/google/devtools/build/buildjar/javac/BlazeEcjMain.java#L98">BlazeEcjMain.java</a>
*/
static class EclipseBatchCompiler extends Main {

Set<String> 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<String> classpath = new ArrayList<>(Arrays.asList(getDefaultClassPaths()));
classpath.add(libPath);
classpath.add(unusedLibPath);

File outputDirectory = new File(Util.getOutputDirectory());
if (!outputDirectory.isDirectory()) {
outputDirectory.mkdirs();
}

List<String> 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)));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<l; i++) {
Expand Down

0 comments on commit c3fd232

Please sign in to comment.