Skip to content

Commit

Permalink
Fix NPE in MavenLemminxExtension.sortProjects and the similar ones
Browse files Browse the repository at this point in the history
If a pom.xml file is empty or has inconsstent content, its DOMDocument's Document Element can be null:

Cannot invoke "org.eclipse.lemminx.dom.DOMElement.getNodeName()" because the return value of "org.eclipse.lemminx.dom.DOMDocument.getDocumentElement()" is null
  at org.eclipse.lemminx.extensions.maven.MavenLemminxExtension.lambda$sortProjects$2(MavenLemminxExtension.java:460)
  • Loading branch information
vrubezhny committed Apr 25, 2023
1 parent f2d2ebe commit c17d73c
Show file tree
Hide file tree
Showing 7 changed files with 122 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,8 @@ private Collection<URI> sortProjects(Collection<URI> projectsUris) {
Optional.ofNullable(projectsUris).ifPresent(uris -> {
uris.stream().filter(Objects::nonNull).forEach(uri ->{
Optional.ofNullable(createDOMDocument(uri)).ifPresent(doc -> {
if (PROJECT_ELT.equals(doc.getDocumentElement().getNodeName())) {
if (doc.getDocumentElement() != null
&& PROJECT_ELT.equals(doc.getDocumentElement().getNodeName())) {
Optional.ofNullable(MavenParseUtils.parseArtifact(doc.getDocumentElement())).ifPresent(a -> {
Parent p = MavenParseUtils.parseParent(DOMUtils.findChildElement(doc.getDocumentElement(), PARENT_ELT).orElse(null));
// If artifact groupId is null - set it from parent
Expand All @@ -480,12 +481,17 @@ private Collection<URI> sortProjects(Collection<URI> projectsUris) {
});
});

LinkedHashSet<URI> skippedUris = new LinkedHashSet<>();
uris.stream().filter(Objects::nonNull).forEach(uri -> {
String a = depByUri.get(uri);
if (a != null) {
adUrisdParentFirst(a, parentByDep, uriByDep, resultUris);
} else {
skippedUris.add(uri);
}
});
// Add skipped IRIs to the end of the collection
resultUris.addAll(skippedUris);
});
return resultUris;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,8 @@ public InputStream getInputStream() throws IOException {
projectCache.put(uri, buildResult.getProject());
projectParsedListeners.forEach(listener -> listener.accept(buildResult.getProject()));
}
lastCheckedVersion.put(uri, document.getTextDocument().getVersion());
problemCache.put(uri, problems);
} catch (ProjectBuildingException e) {
if (e.getResults() == null) {
if (e.getCause() instanceof ModelBuildingException modelBuildingException) {
Expand Down Expand Up @@ -193,12 +195,14 @@ public InputStream getInputStream() throws IOException {
}
}
}
lastCheckedVersion.put(uri, document.getTextDocument().getVersion());
problemCache.put(uri, problems);
} catch (Exception e) {
// Do not add any info, like lastCheckedVersion or problems, to the cache
// In case of project/problems etc. is not available due to an exception happened.
//
LOGGER.log(Level.SEVERE, e.getMessage(), e);
}

lastCheckedVersion.put(uri, document.getTextDocument().getVersion());
problemCache.put(uri, problems);
}

private ProjectBuildingRequest newProjectBuildingRequest() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -588,19 +588,21 @@ private void addPluginPackagingTypes(Set<String> packagingTypes, Artifact artifa
JarEntry componentsxml = jarFile.getJarEntry(COMPONENTS_PATH);
if (componentsxml != null) {
Document doc = db.parse(jarFile.getInputStream(componentsxml));
doc.getDocumentElement().normalize();
NodeList components = doc.getElementsByTagName(COMPONENTS_COMPONENT_ELT);
for (int i = 0; i < components.getLength(); i++) {
Node component = components.item(i);
if (component.getNodeType() == Node.ELEMENT_NODE) {
Element element = (Element) component;
String role = element.getElementsByTagName(COMPONENTS_ROLE_ELT).item(0).getTextContent();
if (ArtifactHandler.ROLE.equals(role)) {
Node config = element.getElementsByTagName(COMPONENTS_CONFIGURATION_ELT).item(0);
if (config.getNodeType() == Node.ELEMENT_NODE) {
Element configEl = (Element) config;
String name = configEl.getElementsByTagName(COMPONENTS_TYPE_ELT).item(0).getTextContent();
packagingTypes.add(name);
if (doc.getDocumentElement() != null) {
doc.getDocumentElement().normalize();
NodeList components = doc.getElementsByTagName(COMPONENTS_COMPONENT_ELT);
for (int i = 0; i < components.getLength(); i++) {
Node component = components.item(i);
if (component.getNodeType() == Node.ELEMENT_NODE) {
Element element = (Element) component;
String role = element.getElementsByTagName(COMPONENTS_ROLE_ELT).item(0).getTextContent();
if (ArtifactHandler.ROLE.equals(role)) {
Node config = element.getElementsByTagName(COMPONENTS_CONFIGURATION_ELT).item(0);
if (config.getNodeType() == Node.ELEMENT_NODE) {
Element configEl = (Element) config;
String name = configEl.getElementsByTagName(COMPONENTS_TYPE_ELT).item(0).getTextContent();
packagingTypes.add(name);
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ public void doDiagnostics(DOMDocument xmlDocument, List<Diagnostic> diagnostics,
}

DOMElement documentElement = xmlDocument.getDocumentElement();
if (documentElement == null) {
return;
}
Map<String, Function<DiagnosticRequest, Optional<List<Diagnostic>>>> tagDiagnostics = configureDiagnosticFunctions();

// Validate project element
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,9 @@ public class WorkspaceProjectsHyperlinkDetectorTest {
public static String TOOLS_SPRINGFRAMEWORK_PARENT = "tools.internal";
public static String TOOLS_BOM_PATH = "/tools/modules/BOM/pom.xml";
public static String TOOLS_BOM_PARENT = "tools";

public static String PROJECT_PATH = "/project/pom.xml";
public static String PROJECT_PARENT = "root";

public static String TOOLS_RETRY_SPRINGFRAMEWORK_PATH = "/tools/modules/retry-springframework/pom.xml";
public static String TOOLS_RETRY_SPRINGFRAMEWORK_SPRING_BOOT_DEPENDENCIES_TARGET_ARTIFACT = "spring-boot-dependencies";
public static String TOOLS_RETRY_SPRINGFRAMEWORK_SPRING_BOOT_DEPENDENCIES_TARGET_GROUP = "org.springframework.boot";
Expand Down Expand Up @@ -242,6 +244,47 @@ public void testHyperlinkFromToolsBomToParentDocument()
assertTrue(definitions.stream().map(LocationLink::getTargetUri).anyMatch(uri -> uriiRawPathEndsWith(uri, "/issue-345/tools/pom.xml")));
}

/**
* Test Ctrl-clicking on parent for artifact 'project'.
* The resulting URI should lead to parent artifact 'root'
*
* @throws IOException
* @throws InterruptedException
* @throws ExecutionException
* @throws URISyntaxException
* @throws BadLocationException
*/
@Test
public void testHyperlinkFromProjectToParentDocument()
throws IOException, InterruptedException, ExecutionException, URISyntaxException, BadLocationException {
// We need the WORKSPACE projects to be placed to MavenProjectCache
IWorkspaceServiceParticipant workspaceService = languageService.getWorkspaceServiceParticipants().stream().filter(MavenWorkspaceService.class::isInstance).findAny().get();
assertNotNull(workspaceService);

URI folderUri = getClass().getResource(WORKSPACE_PATH).toURI();
WorkspaceFolder wsFolder = new WorkspaceFolder(folderUri.toString());

// Add folders to MavenProjectCache
workspaceService.didChangeWorkspaceFolders(
new DidChangeWorkspaceFoldersParams(
new WorkspaceFoldersChangeEvent (
Arrays.asList(new WorkspaceFolder[] {wsFolder}),
Arrays.asList(new WorkspaceFolder[0]))));

DOMDocument document = createDOMDocument(WORKSPACE_PATH + PROJECT_PATH, languageService);
DOMElement parent = DOMUtils.findChildElement(document.getDocumentElement(), DOMConstants.PARENT_ELT).orElse(null);
assertNotNull(parent, "Parent element not found!");
DOMElement parentArtifactId = DOMUtils.findChildElement(parent, DOMConstants.ARTIFACT_ID_ELT).orElse(null);
assertNotNull(parentArtifactId, "Parent ArtifactId element not found!");
int offset = (parentArtifactId.getStart() + parentArtifactId.getEnd()) / 2;

TextDocument textDocument = document.getTextDocument();
Position offsetPosition = textDocument.positionAt(offset);
List<? extends LocationLink> definitions = languageService.findDefinition(document, offsetPosition, ()->{});
definitions.stream().map(LocationLink::getTargetUri).forEach(uri -> System.out.println("testHyperlinkFromProjectToParentDocument(): " + uri));
assertTrue(definitions.stream().map(LocationLink::getTargetUri).anyMatch(uri -> uriiRawPathEndsWith(uri, "/issue-345/root/pom.xml")));
}

//
// Test Ctrl-clicking on a dependency element
//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ public class WorkspaceProjectsHoverTest {
public static String TOOLS_SPRINGFRAMEWORK_PARENT = "tools.internal";
public static String TOOLS_BOM_PATH = "/tools/modules/BOM/pom.xml";
public static String TOOLS_BOM_PARENT = "tools";
public static String PROJECT_PATH = "/project/pom.xml";
public static String PROJECT_PARENT = "root";

public static String TOOLS_RETRY_SPRINGFRAMEWORK_PATH = "/tools/modules/retry-springframework/pom.xml";
public static String TOOLS_RETRY_SPRINGFRAMEWORK_SPRING_BOOT_DEPENDENCIES_TARGET_ARTIFACT = "spring-boot-dependencies";
Expand Down Expand Up @@ -252,6 +254,49 @@ public void testHoverFromToolsBomToParentDocument()
r(8, 14, 8, 19), settings);
}

/**
* Test hover on parent for artifact 'project'.
* The resulting hover should show parent artifact 'root'
*
* @throws IOException
* @throws InterruptedException
* @throws ExecutionException
* @throws URISyntaxException
* @throws BadLocationException
*/
@Test
public void testHoverFromProjectToParentDocument()
throws IOException, InterruptedException, ExecutionException, URISyntaxException, BadLocationException {
// We need the WORKSPACE projects to be placed to MavenProjectCache
IWorkspaceServiceParticipant workspaceService = languageService.getWorkspaceServiceParticipants().stream().filter(MavenWorkspaceService.class::isInstance).findAny().get();
assertNotNull(workspaceService);

URI folderUri = getClass().getResource(WORKSPACE_PATH).toURI();
WorkspaceFolder wsFolder = new WorkspaceFolder(folderUri.toString());

// Add folders to MavenProjectCache
workspaceService.didChangeWorkspaceFolders(
new DidChangeWorkspaceFoldersParams(
new WorkspaceFoldersChangeEvent (
Arrays.asList(new WorkspaceFolder[] {wsFolder}),
Arrays.asList(new WorkspaceFolder[0]))));

DOMDocument document = createDOMDocument(WORKSPACE_PATH + PROJECT_PATH, languageService);
DOMElement parent = DOMUtils.findChildElement(document.getDocumentElement(), DOMConstants.PARENT_ELT).orElse(null);
assertNotNull(parent, "Parent element not found!");
DOMElement parentArtifactId = DOMUtils.findChildElement(parent, DOMConstants.ARTIFACT_ID_ELT).orElse(null);
assertNotNull(parentArtifactId, "Parent ArtifactId element not found!");
int offset = (parentArtifactId.getStart() + parentArtifactId.getEnd()) / 2;

String text = document.getText();
text = text.substring(0, offset) + '|' + text.substring(offset);
ContentModelSettings settings = new ContentModelSettings();
settings.setUseCache(false);
assertHover(languageService, text, null, document.getDocumentURI(),
"**" + PROJECT_PARENT + "**",
r(8, 14, 8, 18), settings);
}

//
// Test hovering over external dependency element
//
Expand Down
1 change: 1 addition & 0 deletions lemminx-maven/src/test/resources/issue-345/broken/pom.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<?

0 comments on commit c17d73c

Please sign in to comment.