Skip to content

Commit

Permalink
validate element in plugin <configuration>
Browse files Browse the repository at this point in the history
Fix #9

Signed-off-by: Andrew Obuchowicz <aobuchow@redhat.com>
  • Loading branch information
AObuchow committed Apr 25, 2020
1 parent 8755a74 commit f5d144c
Show file tree
Hide file tree
Showing 10 changed files with 139 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,18 @@ public static DOMNode findClosestParentNode(final IPositionRequest request, fina
return null;
}
DOMNode pluginNode = request.getNode();
return internalFindClosestParentNode(localName, pluginNode);
}

public static DOMNode findClosestParentNode(final DiagnosticRequest request, final String localName) {
if (localName == null || request == null) {
return null;
}
DOMNode pluginNode = request.getNode();
return internalFindClosestParentNode(localName, pluginNode);
}

private static DOMNode internalFindClosestParentNode(final String localName, DOMNode pluginNode) {
try {
while (!localName.equals(pluginNode.getLocalName())) {
pluginNode = pluginNode.getParentNode();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,15 @@ public class DiagnosticRequest {

public DiagnosticRequest(DOMNode node, DOMDocument xmlDocument, List<Diagnostic> diagnostics) {
this.setNode(node);
this.setDOMDocument(xmlDocument);
this.setXMLDocument(xmlDocument);
this.setDiagnostics(diagnostics);
}

public DOMDocument getDOMDocument() {
public DOMDocument getXMLDocument() {
return xmlDocument;
}

public void setDOMDocument(DOMDocument xmlDocument) {
public void setXMLDocument(DOMDocument xmlDocument) {
this.xmlDocument = xmlDocument;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import org.apache.maven.model.building.ModelProblem;
import org.apache.maven.model.building.ModelProblem.Severity;
import org.apache.maven.plugin.MavenPluginManager;
import org.eclipse.lemminx.dom.DOMDocument;
import org.eclipse.lemminx.dom.DOMElement;
import org.eclipse.lemminx.dom.DOMNode;
Expand All @@ -31,9 +32,11 @@
public class MavenDiagnosticParticipant implements IDiagnosticsParticipant {

private MavenProjectCache projectCache;
MavenPluginManager pluginManager;

public MavenDiagnosticParticipant(MavenProjectCache projectCache) {
public MavenDiagnosticParticipant(MavenProjectCache projectCache, MavenPluginManager pluginManager) {
this.projectCache = projectCache;
this.pluginManager = pluginManager;
}

@Override
Expand Down Expand Up @@ -79,24 +82,10 @@ public void doDiagnostics(DOMDocument xmlDocument, List<Diagnostic> diagnostics,

private HashMap<String, Function<DiagnosticRequest, Diagnostic>> configureDiagnosticFunctions(
DOMDocument xmlDocument) {
// SubModuleValidator subModuleValidator= new SubModuleValidator();
// try {
// subModuleValidator.setPomFile(new File(xmlDocument.getDocumentURI().substring(5)));
// } catch (IOException | XmlPullParserException e) {
// // TODO: Use plug-in error logger
// e.printStackTrace();
// }
//Function<DiagnosticRequest, Diagnostic> versionFunc = VersionValidator::validateVersion;
//Function<DiagnosticRequest, Diagnostic> submoduleExistenceFunc = subModuleValidator::validateSubModuleExistence;
// Below is a mock Diagnostic function which creates a warning between inside
// <configuration> tags
//Function<DiagnosticRequest, Diagnostic> configFunc = diagnosticReq -> new Diagnostic(diagnosticReq.getRange(),
// "Configuration Error", DiagnosticSeverity.Warning, xmlDocument.getDocumentURI(), "XML");
Function<DiagnosticRequest, Diagnostic> validatePluginConfiguration = new PluginValidator(projectCache, pluginManager)::validateConfiguration;

HashMap<String, Function<DiagnosticRequest, Diagnostic>> tagDiagnostics = new HashMap<>();
//tagDiagnostics.put("version", versionFunc);
//tagDiagnostics.put("configuration", configFunc);
//tagDiagnostics.put("module", submoduleExistenceFunc);
tagDiagnostics.put("configuration", validatePluginConfiguration);
return tagDiagnostics;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,11 @@ public void start(InitializeParams params, XMLExtensionsRegistry registry) {
e.printStackTrace();
}
registry.registerCompletionParticipant(completionParticipant);
diagnosticParticipant = new MavenDiagnosticParticipant(cache);
try {
diagnosticParticipant = new MavenDiagnosticParticipant(cache, container.lookup(MavenPluginManager.class));
} catch (ComponentLookupException e) {
e.printStackTrace();
}
registry.registerDiagnosticsParticipant(diagnosticParticipant);
try {
hoverParticipant = new MavenHoverParticipant(cache, indexSearcher, (MavenPluginManager) container.lookup(MavenPluginManager.class));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,29 @@ public static List<Parameter> collectPluginConfigurationParameters(IPositionRequ
.collect(Collectors.toList());
return parameters;
}

public static List<Parameter> collectPluginConfigurationParameters(DiagnosticRequest request,
MavenProjectCache cache, MavenPluginManager pluginManager) {
PluginDescriptor pluginDescriptor = MavenPluginUtils.getContainingPluginDescriptor(request, cache,
pluginManager);
if (pluginDescriptor == null) {
return Collections.emptyList();
}
List<MojoDescriptor> mojosToConsiderList = pluginDescriptor.getMojos();
DOMNode executionElementDomNode = DOMUtils.findClosestParentNode(request, "execution");
// TODO: Verify this works and factor with other collectPluginConfigurationParameters method
if (executionElementDomNode != null) {
Set<String> interestingMojos = executionElementDomNode.getChildren().stream()
.filter(node -> "goals".equals(node.getLocalName())).flatMap(node -> node.getChildren().stream())
.filter(node -> "goal".equals(node.getLocalName())).flatMap(node -> node.getChildren().stream())
.filter(DOMNode::isText).map(DOMNode::getTextContent).collect(Collectors.toSet());
mojosToConsiderList = mojosToConsiderList.stream().filter(mojo -> interestingMojos.contains(mojo.getGoal()))
.collect(Collectors.toList());
}
List<Parameter> parameters = mojosToConsiderList.stream().flatMap(mojo -> mojo.getParameters().stream())
.collect(Collectors.toList());
return parameters;
}

public static RemoteRepository toRemoteRepo(Repository modelRepo) {
Builder builder = new RemoteRepository.Builder(modelRepo.getId(), modelRepo.getLayout(), modelRepo.getLayout());
Expand All @@ -82,6 +105,24 @@ public static PluginDescriptor getContainingPluginDescriptor(IPositionRequest re
if (pluginNode == null) {
return null;
}
return getNodePluginDescriptor(cache, pluginManager, project, pluginNode);
}

public static PluginDescriptor getContainingPluginDescriptor(DiagnosticRequest request, MavenProjectCache cache,
MavenPluginManager pluginManager) {
MavenProject project = cache.getLastSuccessfulMavenProject(request.getXMLDocument());
if (project == null) {
return null;
}
DOMNode pluginNode = request.getNode().getParentNode();
if (pluginNode == null) {
return null;
}
return getNodePluginDescriptor(cache, pluginManager, project, pluginNode);
}

private static PluginDescriptor getNodePluginDescriptor(MavenProjectCache cache, MavenPluginManager pluginManager,
MavenProject project, DOMNode pluginNode) {
Optional<String> groupId = DOMUtils.findChildElementText(pluginNode, "groupId");
Optional<String> artifactId = DOMUtils.findChildElementText(pluginNode, "artifactId");
String pluginKey = "";
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package org.eclipse.lemminx.maven;

import java.util.List;

import org.apache.maven.plugin.MavenPluginManager;
import org.apache.maven.plugin.descriptor.Parameter;
import org.eclipse.lemminx.dom.DOMNode;
import org.eclipse.lsp4j.Diagnostic;
import org.eclipse.lsp4j.DiagnosticSeverity;

public class PluginValidator {
private final MavenProjectCache cache;
private final MavenPluginManager pluginManager;

public PluginValidator(MavenProjectCache cache, MavenPluginManager pluginManager ) {
this.cache = cache;
this.pluginManager = pluginManager;
}

public Diagnostic validateConfiguration(DiagnosticRequest diagnosticRequest) {
DOMNode node = diagnosticRequest.getNode();
if (node == null) {
return null;
}
if (node.isElement() && node.hasChildNodes()) {
List<Parameter> parameters = MavenPluginUtils.collectPluginConfigurationParameters(diagnosticRequest, cache, pluginManager);
for (DOMNode childNode : node.getChildren()) {
DiagnosticRequest childDiagnosticReq = new DiagnosticRequest(childNode, diagnosticRequest.getXMLDocument(), diagnosticRequest.getDiagnostics());
Diagnostic diag = internalValidateConfiguration(childDiagnosticReq, parameters);
if (diag != null) {
diagnosticRequest.getDiagnostics().add(diag);
}
}
}
return null;
}

private static Diagnostic internalValidateConfiguration(DiagnosticRequest diagnosticReq, List<Parameter> parameters) {
DOMNode node = diagnosticReq.getNode();
if (node.getLocalName() == null) {
return null;
}
for (Parameter parameter : parameters) {
if (node.getLocalName().equals(parameter.getName())) {
return null;
}
}
return new Diagnostic(diagnosticReq.getRange(),
"Invalid plugin configuration", DiagnosticSeverity.Warning, diagnosticReq.getXMLDocument().getDocumentURI(), "XML");

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public void setPomFile(File pomFile) throws FileNotFoundException, IOException,

public Diagnostic validateSubModuleExistence(DiagnosticRequest diagnosticRequest) {
DOMNode node = diagnosticRequest.getNode();
DOMDocument xmlDocument = diagnosticRequest.getDOMDocument();
DOMDocument xmlDocument = diagnosticRequest.getXMLDocument();
Diagnostic diagnostic = null;
Range range = diagnosticRequest.getRange();
String tagContent = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public class VersionValidator {

public static Diagnostic validateVersion(DiagnosticRequest diagnosticRequest) {
DOMNode node = diagnosticRequest.getNode();
DOMDocument xmlDocument = diagnosticRequest.getDOMDocument();
DOMDocument xmlDocument = diagnosticRequest.getXMLDocument();
Dependency model = MavenParseUtils.parseArtifact(node);
Artifact artifact = null;
Range range = diagnosticRequest.getRange();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.eclipse.lsp4j.CompletionItem;
import org.eclipse.lsp4j.CompletionList;
import org.eclipse.lsp4j.CompletionParams;
import org.eclipse.lsp4j.Diagnostic;
import org.eclipse.lsp4j.DidOpenTextDocumentParams;
import org.eclipse.lsp4j.Hover;
import org.eclipse.lsp4j.MarkupContent;
Expand All @@ -32,7 +33,6 @@
import org.eclipse.lsp4j.jsonrpc.messages.Either;
import org.junit.After;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;

Expand Down Expand Up @@ -135,4 +135,18 @@ public void testPluginArtifactHover() throws IOException, InterruptedException,
assertTrue((((MarkupContent) hover.getContents().getRight()).getValue()
.contains("Maven Surefire MOJO in maven-surefire-plugin")));
}


// Diagnostic related tests

@Test(timeout=30000)
public void testPluginConfigurationDiagnostics() throws IOException, InterruptedException, ExecutionException, URISyntaxException {
TextDocumentItem textDocumentItem = MavenLemminxTestsUtils.createTextDocumentItem("/pom-plugin-configuration-hover.xml");
DidOpenTextDocumentParams params = new DidOpenTextDocumentParams(textDocumentItem);
connection.languageServer.getTextDocumentService().didOpen(params);
assertTrue(connection.waitForDiagnostics(diagnostics -> diagnostics.stream().map(Diagnostic::getMessage)
.anyMatch(message -> message.contains("Invalid plugin configuration")), 15000));
assertTrue(connection.waitForDiagnostics(diagnostics -> diagnostics.size() == 2, 15000));

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
<version>3.8.0</version>
<configuration>
<source>true</source>
<notValidConfig><!-- This tag should cause a warning --></notValidConfig>
<anotherNotValidConfig><!-- This tag should cause a warning --></anotherNotValidConfig>
</configuration>
</plugin>
</plugins>
Expand Down

0 comments on commit f5d144c

Please sign in to comment.