From 3033c7a045166853283c23e83a671f9934a809f5 Mon Sep 17 00:00:00 2001 From: Jinbo Wang Date: Tue, 11 Jun 2024 14:40:06 +0800 Subject: [PATCH] Address review comments & fix test cases --- .../internal/AptCompilationParticipant.java | 5 +- .../core/tests/builder/BasicBuildTests.java | 41 ++++++---- .../core/compiler/CompilationParticipant.java | 3 +- .../compiler/CompilerConfiguration.java | 18 ++--- .../eclipse/jdt/internal/compiler/Either.java | 80 +++++++++++++++++++ .../core/builder/AbstractImageBuilder.java | 75 +++++++---------- 6 files changed, 149 insertions(+), 73 deletions(-) create mode 100644 org.eclipse.jdt.core/model/org/eclipse/jdt/internal/compiler/Either.java diff --git a/org.eclipse.jdt.apt.core/src/org/eclipse/jdt/apt/core/internal/AptCompilationParticipant.java b/org.eclipse.jdt.apt.core/src/org/eclipse/jdt/apt/core/internal/AptCompilationParticipant.java index 488f9598d08..0d000e9522f 100644 --- a/org.eclipse.jdt.apt.core/src/org/eclipse/jdt/apt/core/internal/AptCompilationParticipant.java +++ b/org.eclipse.jdt.apt.core/src/org/eclipse/jdt/apt/core/internal/AptCompilationParticipant.java @@ -24,6 +24,7 @@ import java.util.Map; import java.util.Set; +import org.eclipse.core.resources.IContainer; import org.eclipse.core.resources.IFile; import org.eclipse.core.resources.IFolder; import org.eclipse.core.resources.IMarker; @@ -237,7 +238,7 @@ public String[] getAnnotationProcessorPaths(IJavaProject project, boolean isTest } @Override - public String[] getGeneratedSourcePaths(IJavaProject project, boolean isTest) { + public IContainer[] getGeneratedSourcePaths(IJavaProject project, boolean isTest) { AptProject aptProject = AptPlugin.getAptProject(project); if (aptProject == null) { return null; @@ -249,7 +250,7 @@ public String[] getGeneratedSourcePaths(IJavaProject project, boolean isTest) { } IFolder folder = generatedSourceFolderManager.getFolder(); - return folder == null || folder.getLocation() == null ? null : new String[] { folder.getLocation().toOSString() }; + return folder == null? null : new IContainer[] { folder }; } @Override diff --git a/org.eclipse.jdt.core.tests.builder/src/org/eclipse/jdt/core/tests/builder/BasicBuildTests.java b/org.eclipse.jdt.core.tests.builder/src/org/eclipse/jdt/core/tests/builder/BasicBuildTests.java index 3afe0c31dd4..f29239be54b 100644 --- a/org.eclipse.jdt.core.tests.builder/src/org/eclipse/jdt/core/tests/builder/BasicBuildTests.java +++ b/org.eclipse.jdt.core.tests.builder/src/org/eclipse/jdt/core/tests/builder/BasicBuildTests.java @@ -33,6 +33,7 @@ import junit.framework.*; +import org.eclipse.core.resources.IContainer; import org.eclipse.core.resources.IFile; import org.eclipse.core.resources.IMarker; import org.eclipse.core.resources.IProject; @@ -53,6 +54,7 @@ import org.eclipse.jdt.internal.compiler.CompilationResult; import org.eclipse.jdt.internal.compiler.Compiler; import org.eclipse.jdt.internal.compiler.CompilerConfiguration; +import org.eclipse.jdt.internal.compiler.Either; import org.eclipse.jdt.internal.compiler.ICompilerFactory; import org.eclipse.jdt.internal.compiler.ICompilerRequestor; import org.eclipse.jdt.internal.compiler.IErrorHandlingPolicy; @@ -766,15 +768,17 @@ public void testCustomerCompilerFactoryWithAP() throws Exception { configs.add(mockCompiler.compilerConfig); } }; + File projectRoot = null; + IProject project = null; try { MockCompilerFactory.addListener(listener); System.setProperty(AbstractImageBuilder.COMPILER_FACTORY_KEY, CUSTOM_COMPILER_VALUE); - File projectRoot = copyFiles("autoValueSnippet", true); + projectRoot = copyFiles("autoValueSnippet", true); IPath dotProjectPath = new org.eclipse.core.runtime.Path(new File(projectRoot, IProjectDescription.DESCRIPTION_FILE_NAME).getAbsolutePath()); IWorkspace workspace = ResourcesPlugin.getWorkspace(); IProjectDescription descriptor = workspace.loadProjectDescription(dotProjectPath); String projectName = descriptor.getName(); - IProject project = workspace.getRoot().getProject(projectName); + project = workspace.getRoot().getProject(projectName); project.create(descriptor, new NullProgressMonitor()); project.open(IResource.NONE, new NullProgressMonitor()); @@ -782,34 +786,37 @@ public void testCustomerCompilerFactoryWithAP() throws Exception { project.build(IncrementalProjectBuilder.FULL_BUILD, null); // It creates compiler 4 times (MAIN full build, TEST full build, MAIN incremental build, TEST incremental build) - assertEquals(4, configs.size()); + assertFalse(configs.isEmpty()); CompilerConfiguration config = configs.get(0); List processorPaths = config.annotationProcessorPaths(); assertEquals(2, processorPaths.size()); assertTrue(processorPaths.get(0).endsWith("auto-value-1.6.5.jar")); assertTrue(processorPaths.get(1).endsWith("auto-value-annotations-1.6.5.jar")); - List generatedSourcePaths = config.generatedSourcePaths(); + List generatedSourcePaths = config.generatedSourcePaths(); assertEquals(1, generatedSourcePaths.size()); - assertTrue(generatedSourcePaths.get(0).endsWith(".apt_generated")); + assertEquals(".apt_generated", generatedSourcePaths.get(0).getRawLocation().lastSegment()); - List sourcePaths = config.sourcepaths(); + List sourcePaths = config.sourcepaths(); assertEquals(2, sourcePaths.size()); - assertTrue(sourcePaths.get(0).endsWith("src")); - assertTrue(sourcePaths.get(1).endsWith(".apt_generated")); - List classPaths = config.classpaths(); + assertEquals("src", sourcePaths.get(0).getRawLocation().lastSegment()); + assertEquals(".apt_generated", sourcePaths.get(1).getRawLocation().lastSegment()); + + List> classPaths = config.classpaths(); assertEquals(2, classPaths.size()); - assertTrue(classPaths.get(0).endsWith("bin")); - assertTrue(classPaths.get(1).endsWith("auto-value-annotations-1.6.5.jar")); + assertNotNull(classPaths.get(0).getLeft()); + assertEquals("bin", classPaths.get(0).getLeft().getRawLocation().lastSegment()); + assertNotNull(classPaths.get(1).getRight()); + assertTrue(classPaths.get(1).getRight().endsWith("auto-value-annotations-1.6.5.jar")); - List moduleSourcePaths = config.moduleSourcepaths(); + List moduleSourcePaths = config.moduleSourcepaths(); assertEquals(0, moduleSourcePaths.size()); - List modulePaths = config.modulepaths(); + List> modulePaths = config.modulepaths(); assertEquals(0, modulePaths.size()); - Map sourceOutputMapping = config.sourceOutputMapping(); + Map sourceOutputMapping = config.sourceOutputMapping(); assertEquals(2, sourceOutputMapping.size()); IFile aptGeneratedFile = project.getFile(".apt_generated/AutoValue_Outer.java"); @@ -817,6 +824,12 @@ public void testCustomerCompilerFactoryWithAP() throws Exception { } finally { MockCompilerFactory.removeListener(listener); System.clearProperty(AbstractImageBuilder.COMPILER_FACTORY_KEY); + if (project != null && project.exists()) { + project.close(new NullProgressMonitor()); + } + if (projectRoot != null) { + deleteDirectory(projectRoot.toPath()); + } } } diff --git a/org.eclipse.jdt.core/model/org/eclipse/jdt/core/compiler/CompilationParticipant.java b/org.eclipse.jdt.core/model/org/eclipse/jdt/core/compiler/CompilationParticipant.java index 029a9d0e711..2634f1b08e8 100644 --- a/org.eclipse.jdt.core/model/org/eclipse/jdt/core/compiler/CompilationParticipant.java +++ b/org.eclipse.jdt.core/model/org/eclipse/jdt/core/compiler/CompilationParticipant.java @@ -20,6 +20,7 @@ import java.io.ByteArrayInputStream; import java.util.Optional; +import org.eclipse.core.resources.IContainer; import org.eclipse.jdt.core.IJavaProject; /** @@ -177,7 +178,7 @@ public String[] getAnnotationProcessorPaths(IJavaProject project, boolean isTest * @since 3.38 * @noreference Provisional API for experimental support for an alternative compiler. This method is not intended to be referenced by clients. */ -public String[] getGeneratedSourcePaths(IJavaProject project, boolean isTest) { +public IContainer[] getGeneratedSourcePaths(IJavaProject project, boolean isTest) { return null; } diff --git a/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/compiler/CompilerConfiguration.java b/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/compiler/CompilerConfiguration.java index ec2b4f3c5fa..1c2e6d7d0a0 100644 --- a/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/compiler/CompilerConfiguration.java +++ b/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/compiler/CompilerConfiguration.java @@ -13,10 +13,11 @@ package org.eclipse.jdt.internal.compiler; -import java.io.File; import java.util.List; import java.util.Map; +import org.eclipse.core.resources.IContainer; + /** * This class encapsulates the standard compiler options that can be * used to compile Java files. It provides methods to set and retrieve @@ -28,25 +29,24 @@ * like Javac to compile Java files. * * @since 3.38 - * @noextend This class is not intended to be subclassed by clients. */ -public record CompilerConfiguration( +public final record CompilerConfiguration( /** * List of file paths where the compiler can find source files. */ - List sourcepaths, + List sourcepaths, /** * List of file paths where the compiler can find source files for modules. */ - List moduleSourcepaths, + List moduleSourcepaths, /** * List of file paths where the compiler can find user class files and annotation processors. */ - List classpaths, + List> classpaths, /** * List of file paths where the compiler can find modules. */ - List modulepaths, + List> modulepaths, /** * Location to search for annotation processors. */ @@ -54,11 +54,11 @@ public record CompilerConfiguration( /** * Locations to place generated source files. */ - List generatedSourcePaths, + List generatedSourcePaths, /** * The mapping of source files to output directories. */ - Map sourceOutputMapping, + Map sourceOutputMapping, /** * The compiler options used to control the compilation behavior. * See {@link org.eclipse.jdt.internal.compiler.impl.CompilerOptions} for a list of available options. diff --git a/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/compiler/Either.java b/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/compiler/Either.java new file mode 100644 index 00000000000..2976efc6bff --- /dev/null +++ b/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/compiler/Either.java @@ -0,0 +1,80 @@ +/******************************************************************************* +* Copyright (c) 2024 Microsoft Corporation and others. +* All rights reserved. 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: +* Microsoft Corporation - initial API and implementation +*******************************************************************************/ + +package org.eclipse.jdt.internal.compiler; + +import java.util.Objects; + +public class Either { + private final L left; + private final R right; + + public static Either forLeft(L left) { + return new Either<>(left, null); + } + + public static Either forRight(R right) { + return new Either<>(null, right); + } + + protected Either(L left, R right) { + this.left = left; + this.right = right; + } + + public L getLeft() { + return this.left; + } + + public R getRight() { + return this.right; + } + + public Object get() { + if (this.left != null) + return this.left; + if (this.right != null) + return this.right; + return null; + } + + public boolean isLeft() { + return this.left != null; + } + + public boolean isRight() { + return this.right != null; + } + + @Override + public int hashCode() { + return Objects.hash(this.left, this.right); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) + return true; + if (obj == null) + return false; + if (getClass() != obj.getClass()) + return false; + Either other = (Either) obj; + return Objects.equals(this.left, other.left) && Objects.equals(this.right, other.right); + } + + @Override + public String toString() { + return "Either [left=" + this.left + ", right=" + this.right + "]"; //$NON-NLS-1$ //$NON-NLS-2$//$NON-NLS-3$ + } +} diff --git a/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/builder/AbstractImageBuilder.java b/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/builder/AbstractImageBuilder.java index bc6703ca78b..f3b6dee0d8b 100644 --- a/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/builder/AbstractImageBuilder.java +++ b/org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/builder/AbstractImageBuilder.java @@ -583,12 +583,9 @@ protected Compiler newCompiler() { Class compilerFactoryClass = (Class) Class.forName(compilerFactoryClassName); Constructor constructor = compilerFactoryClass.getDeclaredConstructor(); compilerFactory = constructor.newInstance(); - } catch (ClassNotFoundException e) { - ILog.get().error("Could not load class " + compilerFactoryClassName, e); //$NON-NLS-1$ - } catch (NoSuchMethodException e) { - ILog.get().error("Couldn't find compatible constructor " + compilerFactoryClassName); //$NON-NLS-1$ - } catch (IllegalAccessException | IllegalArgumentException | InstantiationException | InvocationTargetException e) { - ILog.get().error("Failed invoking constructor " + compilerFactoryClassName); //$NON-NLS-1$ + } catch (ClassNotFoundException | NoSuchMethodException | IllegalAccessException + | IllegalArgumentException | InstantiationException | InvocationTargetException e) { + ILog.get().error("Failed to initialize the custom compiler factory - " + compilerFactoryClassName, e); //$NON-NLS-1$ } } @@ -623,7 +620,7 @@ protected Compiler newCompiler() { private CompilerConfiguration prepareCompilerConfiguration(CompilerOptions options) { try { List annotationProcessorPaths = new ArrayList<>(); - List generatedSourcePaths = new ArrayList<>(); + List generatedSourcePaths = new ArrayList<>(); boolean isTest = this.compilationGroup == CompilationGroup.TEST; if (this.javaBuilder.participants != null) { for (CompilationParticipant participant : this.javaBuilder.participants) { @@ -632,7 +629,7 @@ private CompilerConfiguration prepareCompilerConfiguration(CompilerOptions optio if (paths != null) { annotationProcessorPaths.addAll(Arrays.asList(paths)); } - String[] generatedSrc = participant.getGeneratedSourcePaths(this.javaBuilder.javaProject, isTest); + IContainer[] generatedSrc = participant.getGeneratedSourcePaths(this.javaBuilder.javaProject, isTest); if (generatedSrc != null) { generatedSourcePaths.addAll(Arrays.asList(generatedSrc)); } @@ -641,67 +638,49 @@ private CompilerConfiguration prepareCompilerConfiguration(CompilerOptions optio } ClasspathLocation[] classpathLocations = this.nameEnvironment.binaryLocations; - Set classpaths = new LinkedHashSet<>(); - Set modulepaths = new LinkedHashSet<>(); + Set> classpaths = new LinkedHashSet<>(); + Set> modulepaths = new LinkedHashSet<>(); for (ClasspathLocation location : classpathLocations) { if (location instanceof ClasspathDirectory cpDirectory) { - IPath binaryLocation = cpDirectory.binaryFolder.getLocation(); - if (binaryLocation == null) { - continue; - } - - String filepath = binaryLocation.toFile().getAbsolutePath(); if (cpDirectory.isOnModulePath) { - modulepaths.add(filepath); + modulepaths.add(Either.forLeft(cpDirectory.binaryFolder)); } else { - classpaths.add(filepath); + classpaths.add(Either.forLeft(cpDirectory.binaryFolder)); } } else if (location instanceof ClasspathJar cpJar) { String filepath = cpJar.zipFilename; if (cpJar.isOnModulePath) { - modulepaths.add(filepath); + modulepaths.add(Either.forRight(filepath)); } else { - classpaths.add(filepath); + classpaths.add(Either.forRight(filepath)); } } } - Map sourceOutputMapping = new HashMap<>(); - Set sourcepaths = new LinkedHashSet<>(); - Set moduleSourcepaths = new LinkedHashSet<>(); + Map sourceOutputMapping = new HashMap<>(); + Set sourcepaths = new LinkedHashSet<>(); + Set moduleSourcepaths = new LinkedHashSet<>(); ClasspathMultiDirectory[] srcLocations = this.nameEnvironment.sourceLocations; for (ClasspathMultiDirectory sourceLocation : srcLocations) { - IPath sourcepath = sourceLocation.sourceFolder.getLocation(); - if (sourcepath == null) { - continue; - } - - File sourceFolder = sourcepath.toFile(); - IPath binarypath = sourceLocation.binaryFolder.getLocation(); - if (binarypath != null) { - File outputFolder = binarypath.toFile(); - sourceOutputMapping.put(sourceFolder, outputFolder); - } - - String absoluteSourcepath = sourceFolder.getAbsolutePath(); + sourceOutputMapping.put(sourceLocation.sourceFolder, sourceLocation.binaryFolder); if (sourceLocation.isOnModulePath) { - moduleSourcepaths.add(absoluteSourcepath); + moduleSourcepaths.add(sourceLocation.sourceFolder); } else { - sourcepaths.add(absoluteSourcepath); + sourcepaths.add(sourceLocation.sourceFolder); } } return new CompilerConfiguration( - new ArrayList<>(sourcepaths), - new ArrayList<>(moduleSourcepaths), - new ArrayList<>(classpaths), - new ArrayList<>(modulepaths), + List.copyOf(sourcepaths), + List.copyOf(moduleSourcepaths), + List.copyOf(classpaths), + List.copyOf(modulepaths), annotationProcessorPaths, generatedSourcePaths, sourceOutputMapping, options.getMap()); } catch (Exception e) { - ILog.get().error("Failed computing compiler configuration " + e); //$NON-NLS-1$ + ILog.get().error("Failed computing compiler configuration", e); //$NON-NLS-1$ return new CompilerConfiguration( null, null, null, null, null, null, null, options.getMap()); } @@ -778,10 +757,12 @@ protected void processAnnotations(CompilationParticipantResult[] results) { } boolean isEcjUsed = Compiler.class.equals(this.compiler.getClass()); - // even if no files have annotations, must still tell every annotation processor in case the file used to have them - for (CompilationParticipant participant : this.javaBuilder.participants) - if (isEcjUsed && participant.isAnnotationProcessor()) - participant.processAnnotations(results); + if (isEcjUsed) { + // even if no files have annotations, must still tell every annotation processor in case the file used to have them + for (CompilationParticipant participant : this.javaBuilder.participants) + if (participant.isAnnotationProcessor()) + participant.processAnnotations(results); + } processAnnotationResults(results); }