Skip to content

Commit

Permalink
Consider profiles in the maven.config as well
Browse files Browse the repository at this point in the history 
One can define profiles in the .mvn/maven.config file but currently
these are ignored by lemminx leading to unexpected or even fault project
models.

This now parses the profile option form the maven config and pass them
to the building request.

Also adding two test cases that check properties are taken into account.
  • Loading branch information
@laeubi @mickaelistria
laeubi authored and mickaelistria committed Feb 18, 2024
1 parent 18b343b commit e1e70d0
Show file tree
Hide file tree
Showing 6 changed files with 164 additions and 35 deletions.
24 changes: 18 additions & 6 deletions ...n/java/org/eclipse/lemminx/extensions/maven/participants/hover/MavenHoverParticipant.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
import org.apache.maven.plugin.descriptor.MojoDescriptor;
import org.apache.maven.plugin.descriptor.PluginDescriptor;
import org.apache.maven.project.MavenProject;
import org.apache.maven.project.ProjectBuildingRequest;
import org.eclipse.aether.artifact.Artifact;
import org.eclipse.lemminx.dom.DOMDocument;
import org.eclipse.lemminx.dom.DOMElement;
Expand Down Expand Up @@ -566,7 +567,8 @@ private Hover collectProperty(IHoverRequest request, Map.Entry<Range, String> pr

// Find location
MavenProject parentProject = project, childProj = project;
while (parentProject != null && parentProject.getProperties().containsKey(property.getValue())) {
String propertyName = property.getValue();
while (parentProject != null && parentProject.getProperties().containsKey(propertyName)) {
cancelChecker.checkCanceled();
childProj = parentProject;
parentProject = parentProject.getParent();
Expand All @@ -580,24 +582,34 @@ private Hover collectProperty(IHoverRequest request, Map.Entry<Range, String> pr
cancelChecker.checkCanceled();
if (childProjectUri.equals(thisProjectUri)) {
// Property is defined in the same file as the request
propertyDeclaration = DOMUtils.findNodesByLocalName(doc, property.getValue()).stream()
propertyDeclaration = DOMUtils.findNodesByLocalName(doc, propertyName).stream()
.filter(isMavenProperty).findFirst().orElse(null);
} else {
DOMDocument propertyDeclaringDocument = org.eclipse.lemminx.utils.DOMUtils.loadDocument(
childProj.getFile().toURI().toString(),
request.getNode().getOwnerDocument().getResolverExtensionManager());
cancelChecker.checkCanceled();
propertyDeclaration = DOMUtils.findNodesByLocalName(propertyDeclaringDocument, property.getValue())
propertyDeclaration = DOMUtils.findNodesByLocalName(propertyDeclaringDocument, propertyName)
.stream().filter(isMavenProperty).findFirst().orElse(null);
}

if (propertyDeclaration != null) {
cancelChecker.checkCanceled();
String uri = childProj.getFile().getAbsolutePath();
Range targetRange = XMLPositionUtility.createRange(propertyDeclaration);
String sourceModelId = childProj.getGroupId() + ':' + childProj.getArtifactId() + ':' + childProj.getVersion();
message.append(toBold.apply(MessageFormat.format(PomTextHover_property_location,
supportsMarkdown ? MarkdownUtils.toLink(uri, targetRange, sourceModelId, null) : sourceModelId)));
String sourceModelId = childProj.getGroupId() + ':' + childProj.getArtifactId() + ':'
+ childProj.getVersion();
message.append(toBold.apply(MessageFormat.format(PomTextHover_property_location,
supportsMarkdown ? MarkdownUtils.toLink(uri, targetRange, sourceModelId, null)
: sourceModelId)));
} else {
ProjectBuildingRequest projectRequest = project.getProjectBuildingRequest();
if (projectRequest != null) {
if (projectRequest.getUserProperties().getProperty(propertyName) != null) {
message.append(toBold.apply(MessageFormat.format(PomTextHover_property_location,
"the user properties")));
}
}
}

Hover hover = new Hover(
Expand Down
52 changes: 42 additions & 10 deletions ...x-maven/src/main/java/org/eclipse/lemminx/extensions/maven/project/MavenProjectCache.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import java.util.Objects;
import java.util.Optional;
import java.util.Properties;
import java.util.StringTokenizer;
import java.util.concurrent.CancellationException;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;
Expand Down Expand Up @@ -172,8 +173,8 @@ public LoadedMavenProject build(FileModelSource source, CancelChecker cancelChec
DependencyResolutionResult dependencyResolutionResult = null;
MavenProject project = null;
File file = source.getFile();
try {
ProjectBuildingRequest request = newProjectBuildingRequest(file);
try {
ProjectBuildingRequest request = newProjectBuildingRequest(file, problems);

ProjectBuildingResult buildResult = projectBuilder.build(source, request);
cancelChecker.checkCanceled();
Expand Down Expand Up @@ -349,7 +350,7 @@ public Optional<MavenProject> getSnapshotProject(File file) {
}

try {
return Optional.of(projectBuilder.build(file, newProjectBuildingRequest(file)).getProject());
return Optional.of(projectBuilder.build(file, newProjectBuildingRequest(file, null)).getProject());
} catch (ProjectBuildingException e) {
List<ProjectBuildingResult> result = e.getResults();
if (result != null && result.size() == 1 && result.get(0).getProject() != null) {
Expand All @@ -376,7 +377,7 @@ public Optional<MavenProject> getSnapshotProject(File file) {
*/
public MavenProject getSnapshotProject(DOMDocument document, String profileId, boolean resolve) {
// it would be nice to directly rebuild from Model instead of re-parsing text
ProjectBuildingRequest request = newProjectBuildingRequest(resolve, getFileForDocument(document));
ProjectBuildingRequest request = newProjectBuildingRequest(resolve, getFileForDocument(document), null);
if (profileId != null) {
request.setActiveProfileIds(List.of(profileId));
}
Expand All @@ -401,12 +402,17 @@ public MavenProject getSnapshotProject(DOMDocument document, String profileId, b
* Creates a new default Maven Project Building request (with dependency resolve
* enabled)
*
* @param projectFile the project file or base directory of the building request
* @param projectFile the project file or base directory of the building
* request
* @param problemsCollector if not <code>null</code> this method will add
* problems to the collection if there is a problem
* while creating the request
*
* @return A ProjectBuildingRequest object
*/
public ProjectBuildingRequest newProjectBuildingRequest(File projectFile) {
return newProjectBuildingRequest(true, projectFile);
public ProjectBuildingRequest newProjectBuildingRequest(File projectFile,
Collection<ModelProblem> problemsCollector) {
return newProjectBuildingRequest(true, projectFile, problemsCollector);
}

/**
Expand All @@ -416,10 +422,14 @@ public ProjectBuildingRequest newProjectBuildingRequest(File projectFile) {
* to be enabled on the request.
* @param projectFile the project file or base directory of the building
* request
* @param problemsCollector if not <code>null</code> this method will add
* problems to the collection if there is a problem
* while creating the request
*
* @return A ProjectBuildingRequest object
*/
public ProjectBuildingRequest newProjectBuildingRequest(boolean resolveDependencies, File projectFile) {
public ProjectBuildingRequest newProjectBuildingRequest(boolean resolveDependencies, File projectFile,
Collection<ModelProblem> problemsCollector) {
ProjectBuildingRequest request = new DefaultProjectBuildingRequest();
MavenExecutionRequest mavenRequest = mavenSession.getRequest();
request.setSystemProperties(mavenRequest.getSystemProperties());
Expand All @@ -435,7 +445,7 @@ public ProjectBuildingRequest newProjectBuildingRequest(boolean resolveDependenc
userProperties.setProperty("aether.syncContext.named.factory", "noop");
File multiModuleProjectDirectory = computeMultiModuleProjectDirectory(projectFile);
if (multiModuleProjectDirectory != null) {
File mavenConfig = new File(multiModuleProjectDirectory, MAVEN_CONFIG);
File mavenConfig = new File(multiModuleProjectDirectory, MVN_FOLDER + "/" + MAVEN_CONFIG);
if (mavenConfig.isFile()) {
try {
CLIManager manager = new CLIManager();
Expand All @@ -458,8 +468,30 @@ public ProjectBuildingRequest newProjectBuildingRequest(boolean resolveDependenc
}
}
}
if (commandline.hasOption(CLIManager.ACTIVATE_PROFILES)) {
String[] profileOptionValues = commandline.getOptionValues(CLIManager.ACTIVATE_PROFILES);
if (profileOptionValues != null) {
for (String profileOptionValue : profileOptionValues) {
StringTokenizer tokenizer = new StringTokenizer(profileOptionValue, ",");
while (tokenizer.hasMoreTokens()) {
String profileToken = tokenizer.nextToken().trim();
if (profileToken.startsWith("-") || profileToken.startsWith("!")) {
request.getInactiveProfileIds().add(profileToken.substring(1));
} else if (profileToken.startsWith("+")) {
request.getActiveProfileIds().add(profileToken.substring(1));
} else {
request.getActiveProfileIds().add(profileToken);
}
}
}
}
}
} catch (IOException | ParseException e) {
// TODO how to best propagate this?!?
if (problemsCollector != null) {
problemsCollector.add(new DefaultModelProblem("Problem parsing " + mavenConfig,
ModelProblem.Severity.ERROR, null, null,
-1, -1, e));
}
}
}
}
Expand Down
55 changes: 36 additions & 19 deletions lemminx-maven/src/main/java/org/eclipse/lemminx/extensions/maven/utils/ParticipantUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,22 +30,23 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Properties;
import java.util.concurrent.CancellationException;
import java.util.Map.Entry;
import java.util.Optional;
import java.util.logging.Level;
import java.util.logging.Logger;

import org.apache.commons.lang3.Validate;
import org.apache.maven.artifact.versioning.DefaultArtifactVersion;
import org.apache.maven.model.Build;
import org.apache.maven.model.Dependency;
import org.apache.maven.model.DependencyManagement;
import org.apache.maven.plugin.InvalidPluginDescriptorException;
import org.apache.maven.plugin.PluginDescriptorParsingException;
import org.apache.maven.plugin.PluginResolutionException;
import org.apache.maven.plugin.descriptor.PluginDescriptor;
import org.apache.maven.project.MavenProject;
import org.apache.maven.project.ProjectBuildingRequest;
import org.apache.maven.properties.internal.EnvironmentUtils;
import org.codehaus.plexus.component.repository.exception.ComponentLookupException;
import org.eclipse.aether.artifact.Artifact;
Expand Down Expand Up @@ -293,28 +294,44 @@ public static String resolveValueWithProperties(MavenProject project, String val
}

public static Map<String, String> getMavenProjectProperties(MavenProject project) {
if (project == null) {
return Map.of();
}
// See org.apache.maven.plugin.PluginParameterExpressionEvaluator
Map<String, String> allProps = new HashMap<>();
Properties projectProperties = project.getProperties();
projectProperties.putAll(getEnvironmentProperties());
if (project.getProperties() != null) {
for (Entry<Object, Object> prop : projectProperties.entrySet()) {
allProps.put((String) prop.getKey(), (String) prop.getValue());
}
putProperties(getEnvironmentProperties(), allProps);
putProperties(project.getProperties(), allProps);
ProjectBuildingRequest buildingRequest = project.getProjectBuildingRequest();
if (buildingRequest != null) {
// user properties overwrite project properties
putProperties(buildingRequest.getUserProperties(), allProps);
// but system properties overwrite even user properties
putProperties(buildingRequest.getSystemProperties(), allProps);
}
allProps.put("basedir", project.getBasedir().toString());
allProps.put("project.basedir", project.getBasedir().toString());
allProps.put("project.version", project.getVersion());
allProps.put("project.groupId", project.getGroupId());
allProps.put("project.artifactId", project.getArtifactId());
allProps.put("project.name", project.getName());
Build build = project.getBuild();
if (build != null) {
allProps.put("project.build.directory", build.getDirectory());
allProps.put("project.build.outputDirectory", build.getOutputDirectory());
allProps.put("project.build.testOutputDirectory", build.getTestOutputDirectory());
}
allProps.put("basedir", project == null ? "unknown" : project.getBasedir().toString());
allProps.put("project.basedir", project == null ? "unknown" : project.getBasedir().toString());
allProps.put("project.version", project == null ? "unknown" : project.getVersion());
allProps.put("project.groupId", project == null ? "unknown" : project.getGroupId());
allProps.put("project.artifactId", project == null ? "unknown" : project.getArtifactId());
allProps.put("project.name", project == null ? "unknown" : project.getName());
allProps.put("project.build.directory",
project.getBuild() == null ? "unknown" : project.getBuild().getDirectory());
allProps.put("project.build.outputDirectory",
project.getBuild() == null ? "unknown" : project.getBuild().getOutputDirectory());

return allProps;
}

private static void putProperties(Properties properties, Map<String, String> map) {
if (properties == null || map == null) {
return;
}
properties.stringPropertyNames().forEach(envProperty -> {
map.put(envProperty, properties.getProperty(envProperty));
});
}

public static Map.Entry<Range, String> getMavenPropertyInRequest(IPositionRequest request) {
return getMavenProperty(request.getNode(), request.getOffset());
}
Expand Down
22 changes: 22 additions & 0 deletions ...minx/extensions/maven/participants/hover/MavenHoverParticipantVerionWithPropertyTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
package org.eclipse.lemminx.extensions.maven.participants.hover;

import static org.eclipse.lemminx.extensions.maven.utils.MavenLemminxTestsUtils.createDOMDocument;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.io.IOException;
Expand Down Expand Up @@ -60,6 +61,27 @@ void testPluginManagementPluginWithVerionProperty() throws IOException, Interrup
assertTrue((hover.getContents().getRight().getValue().contains("1.8")));
}

@Test
void testPluginWithVersionPropertyFromMavenConfig()
throws IOException, InterruptedException, ExecutionException, URISyntaxException {
DOMDocument document = createDOMDocument("/maven.config/properties/pom.xml", languageService);
Hover hover = languageService.doHover(document, new Position(14, 32), new SharedSettings());
assertNotNull(hover, "Hover not found, property from .mvn/maven.config missing?");
String value = hover.getContents().getRight().getValue();
assertTrue(value.contains("antrun.plugin.version"));
assertTrue(value.contains("1.8"));
assertTrue(value.contains("The property is defined in the user properties"));
}

@Test
void testPluginWithVersionPropertyFromMavenConfigProfile()
throws IOException, InterruptedException, ExecutionException, URISyntaxException {
DOMDocument document = createDOMDocument("/maven.config/properties/pom.xml", languageService);
Hover hover = languageService.doHover(document, new Position(22, 28), new SharedSettings());
assertNotNull(hover, "Hover not found, property from profile activated in .mvn/maven.config missing?");
assertTrue((hover.getContents().getRight().getValue().contains("3.5.1")));
}

@Test
void testPluginWithVerionProperty() throws IOException, InterruptedException, ExecutionException, URISyntaxException {
DOMDocument document = createDOMDocument("/pom-with-property-in-version.xml", languageService);
Expand Down
2 changes: 2 additions & 0 deletions lemminx-maven/src/test/resources/maven.config/properties/.mvn/maven.config
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
-Dantrun.plugin.version=1.8
-Ptestme
44 changes: 44 additions & 0 deletions lemminx-maven/src/test/resources/maven.config/properties/pom.xml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<groupId>foo.bar</groupId>
<artifactId>test-hover-with-properties-from-maven-config</artifactId>
<version>0.0.1-SNAPSHOT</version>
<properties>
<maven.version>3.0</maven.version>
</properties>
<build>
<pluginManagement>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-antrun-plugin</artifactId>
<version>${antrun.plugin.version}</version>
</plugin>
</plugins>
</pluginManagement>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<version>${compiler.plugin.version}</version>
</plugin>
</plugins>
</build>
<dependencies>
<dependency>
<groupId>org.apache.maven</groupId>
<artifactId>maven-embedder</artifactId>
<version>${maven.version}</version>
<scope>compile</scope>
</dependency>
</dependencies>

<profiles>
<profile>
<id>testme</id>
<properties>
<compiler.plugin.version>3.5.1</compiler.plugin.version>
</properties>
</profile>
</profiles>
</project>

0 comments on commit e1e70d0

Please sign in to comment.