diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyCodeActionParticipant.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyCodeActionParticipant.java index a2092722..07befb11 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyCodeActionParticipant.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyCodeActionParticipant.java @@ -25,8 +25,10 @@ import io.openliberty.tools.langserver.lemminx.codeactions.AddAttribute; import io.openliberty.tools.langserver.lemminx.codeactions.AddFeature; +import io.openliberty.tools.langserver.lemminx.codeactions.AddTrailingSlash; import io.openliberty.tools.langserver.lemminx.codeactions.CreateFile; import io.openliberty.tools.langserver.lemminx.codeactions.EditAttribute; +import io.openliberty.tools.langserver.lemminx.codeactions.RemoveTrailingSlash; import io.openliberty.tools.langserver.lemminx.codeactions.ReplaceFeature; public class LibertyCodeActionParticipant implements ICodeActionParticipant { @@ -57,6 +59,8 @@ private void registerCodeActions() { codeActionParticipants.put(LibertyDiagnosticParticipant.IMPLICIT_NOT_OPTIONAL_CODE, new AddAttribute()); codeActionParticipants.put(LibertyDiagnosticParticipant.INCORRECT_FEATURE_CODE, new ReplaceFeature()); codeActionParticipants.put(LibertyDiagnosticParticipant.MISSING_CONFIGURED_FEATURE_CODE, new AddFeature()); + codeActionParticipants.put(LibertyDiagnosticParticipant.IS_FILE_NOT_DIR_CODE, new RemoveTrailingSlash()); + codeActionParticipants.put(LibertyDiagnosticParticipant.Is_DIR_NOT_FILE_CODE, new AddTrailingSlash()); } } } diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDiagnosticParticipant.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDiagnosticParticipant.java index a772a8b3..b003588d 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDiagnosticParticipant.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDiagnosticParticipant.java @@ -44,6 +44,8 @@ public class LibertyDiagnosticParticipant implements IDiagnosticsParticipant { public static final String LIBERTY_LEMMINX_SOURCE = "liberty-lemminx"; + public static final String NOT_XML_OR_DIR = "The specified resource is not an XML file. If it is a directory, it must end with a trailing slash."; + public static final String MISSING_FILE_MESSAGE = "The resource at the specified location could not be found."; public static final String MISSING_FILE_CODE = "missing_file"; @@ -55,6 +57,11 @@ public class LibertyDiagnosticParticipant implements IDiagnosticsParticipant { public static final String IMPLICIT_NOT_OPTIONAL_MESSAGE = "The specified resource cannot be skipped. Check location value or add optional attribute."; public static final String IMPLICIT_NOT_OPTIONAL_CODE = "implicit_not_optional"; + public static final String SPECIFIED_DIR_IS_FILE = "Path specified a directory, but resource exists as a file. Please remove the trailing slash."; + public static final String SPECIFIED_FILE_IS_DIR = "Path specified a file, but resource exists as a directory. Please add a trailing slash."; + public static final String IS_FILE_NOT_DIR_CODE = "is_file_not_dir"; + public static final String Is_DIR_NOT_FILE_CODE = "is_dir_not_file"; + public static final String INCORRECT_FEATURE_CODE = "incorrect_feature"; private Set includedFeatures; @@ -150,12 +157,14 @@ private void validateIncludeLocation(DOMDocument domDocument, List d if (locAttribute.startsWith("http") || locAttribute.contains("$")) { return; } + // Liberty uses this to determine if directory. + boolean isLibertyDirectory = locAttribute.endsWith("/") || locAttribute.endsWith(File.separator); DOMNode locNode = node.getAttributeNode("location"); Range range = XMLPositionUtility.createRange(locNode.getStart(), locNode.getEnd(), domDocument); - if (!locAttribute.endsWith(".xml")) { - String message = "The specified resource is not an XML file."; - diagnosticsList.add(new Diagnostic(range, message, DiagnosticSeverity.Warning, LIBERTY_LEMMINX_SOURCE)); + if (!locAttribute.endsWith(".xml") + && !isLibertyDirectory) { + diagnosticsList.add(new Diagnostic(range, NOT_XML_OR_DIR, DiagnosticSeverity.Warning, LIBERTY_LEMMINX_SOURCE)); return; } @@ -175,11 +184,28 @@ private void validateIncludeLocation(DOMDocument domDocument, List d } diagnosticsList.add(new Diagnostic(range, MISSING_FILE_MESSAGE, DiagnosticSeverity.Warning, LIBERTY_LEMMINX_SOURCE, MISSING_FILE_CODE)); } + validateFileOrDirIncludeLocation(configFile, isLibertyDirectory, range, diagnosticsList); } catch (IllegalArgumentException e) { diagnosticsList.add(new Diagnostic(range, MISSING_FILE_MESSAGE, DiagnosticSeverity.Warning, "liberty-lemminx-exception", MISSING_FILE_CODE)); } } + /** + * Checks if specified file or dir is the correct filetype. + * Adds diagnostics if it is mismatched. + * @param f - location file + * @param isLibertyDirectory - whether Liberty considers this specified as a dir (path ends in slash) + * @param range - Range to apply diagnostic message + * @param diagnosticsList + */ + private void validateFileOrDirIncludeLocation(File f, boolean isLibertyDirectory, Range range, List diagnosticsList) { + if (f.isFile() && isLibertyDirectory) { + diagnosticsList.add(new Diagnostic(range, SPECIFIED_DIR_IS_FILE, DiagnosticSeverity.Error, LIBERTY_LEMMINX_SOURCE, IS_FILE_NOT_DIR_CODE)); + } else if (f.isDirectory() && !isLibertyDirectory) { + diagnosticsList.add(new Diagnostic(range, SPECIFIED_FILE_IS_DIR, DiagnosticSeverity.Error, LIBERTY_LEMMINX_SOURCE, Is_DIR_NOT_FILE_CODE)); + } + } + /** * Create temporary diagnostics for validation for single pass-through. * @param domDocument diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDocumentLinkParticipant.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDocumentLinkParticipant.java index 1af9d5c7..7083a275 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDocumentLinkParticipant.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDocumentLinkParticipant.java @@ -27,9 +27,9 @@ import org.eclipse.lsp4j.DocumentLink; import org.eclipse.lsp4j.Range; +import io.openliberty.tools.langserver.lemminx.util.LibertyConstants; import io.openliberty.tools.langserver.lemminx.util.LibertyUtils; - public class LibertyDocumentLinkParticipant implements IDocumentLinkParticipant { private static final Logger LOGGER = Logger.getLogger(LibertyDocumentLinkParticipant.class.getName()); @@ -43,7 +43,7 @@ public void findDocumentLinks(DOMDocument document, List links) { List nodes = document.getDocumentElement().getChildren(); // collect all nodes that are children of the document element - List includeDomNodes = nodes.stream().filter(n -> ((n.getNodeName() != null) && n.getNodeName().equals("include"))) + List includeDomNodes = nodes.stream().filter(n -> ((n.getNodeName() != null) && n.getNodeName().equals(LibertyConstants.INCLUDE_ELEMENT))) .collect(Collectors.toList()); for (DOMNode includeNode : includeDomNodes) { diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddTrailingSlash.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddTrailingSlash.java new file mode 100644 index 00000000..97b1ffa8 --- /dev/null +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddTrailingSlash.java @@ -0,0 +1,66 @@ +/******************************************************************************* +* Copyright (c) 2023 IBM Corporation and others. +* +* This program and the accompanying materials are made available under the +* terms of the Eclipse Public License v. 2.0 which is available at +* http://www.eclipse.org/legal/epl-2.0. +* +* SPDX-License-Identifier: EPL-2.0 +* +* Contributors: +* IBM Corporation +*******************************************************************************/ +package io.openliberty.tools.langserver.lemminx.codeactions; + +import java.io.File; +import java.util.List; +import java.util.logging.Logger; + +import org.eclipse.lemminx.commons.CodeActionFactory; +import org.eclipse.lemminx.dom.DOMDocument; +import org.eclipse.lemminx.services.extensions.codeaction.ICodeActionParticipant; +import org.eclipse.lemminx.services.extensions.codeaction.ICodeActionRequest; +import org.eclipse.lsp4j.CodeAction; +import org.eclipse.lsp4j.Diagnostic; +import org.eclipse.lsp4j.jsonrpc.CancelChecker; + +public class AddTrailingSlash implements ICodeActionParticipant { + private static final Logger LOGGER = Logger.getLogger(AddTrailingSlash.class.getName()); + + public static final String CODEACTION_TITLE = "Add trailing slash to specify directory."; + + public static final String FORWARD_SLASH = "/"; + public static final String BACK_SLASH = "\\"; + + @Override + public void doCodeAction(ICodeActionRequest request, List codeActions, CancelChecker cancelChecker) { + Diagnostic diagnostic = request.getDiagnostic(); + DOMDocument document = request.getDocument(); + try { + String fileSeparator = FORWARD_SLASH; + String locationText = document.findNodeAt(document.offsetAt(diagnostic.getRange().getEnd())).getAttribute("location"); + String replaceText = getReplaceText(fileSeparator, locationText); + codeActions.add(CodeActionFactory.replace(CODEACTION_TITLE, diagnostic.getRange(), replaceText, document.getTextDocument(), diagnostic)); + } catch (Exception e) { + LOGGER.warning("Could not generate code action for adding trailing slash." + e); + } + } + + /** + * Gets replace text based on OS and slash usage + * @param fileSeparator + * @param locationText + * @return + */ + public static String getReplaceText(String fileSeparator, String locationText) { + if (locationText.contains(BACK_SLASH) && locationText.contains(FORWARD_SLASH)) { + // if using mismatched slashes, replace all with / + locationText = locationText.replace(BACK_SLASH, FORWARD_SLASH); + } else if (File.separator.equals(BACK_SLASH) && locationText.contains(BACK_SLASH)) { + // if Windows and path using \, continue using it + fileSeparator = BACK_SLASH; + } + String replaceText = "location=\"" + locationText + fileSeparator + "\""; + return replaceText; + } +} diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/RemoveTrailingSlash.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/RemoveTrailingSlash.java new file mode 100644 index 00000000..ac3d2180 --- /dev/null +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/RemoveTrailingSlash.java @@ -0,0 +1,42 @@ +/******************************************************************************* +* Copyright (c) 2023 IBM Corporation and others. +* +* This program and the accompanying materials are made available under the +* terms of the Eclipse Public License v. 2.0 which is available at +* http://www.eclipse.org/legal/epl-2.0. +* +* SPDX-License-Identifier: EPL-2.0 +* +* Contributors: +* IBM Corporation +*******************************************************************************/ +package io.openliberty.tools.langserver.lemminx.codeactions; + +import java.util.List; +import java.util.logging.Logger; + +import org.eclipse.lemminx.commons.CodeActionFactory; +import org.eclipse.lemminx.dom.DOMDocument; +import org.eclipse.lemminx.services.extensions.codeaction.ICodeActionParticipant; +import org.eclipse.lemminx.services.extensions.codeaction.ICodeActionRequest; +import org.eclipse.lsp4j.CodeAction; +import org.eclipse.lsp4j.Diagnostic; +import org.eclipse.lsp4j.jsonrpc.CancelChecker; + +public class RemoveTrailingSlash implements ICodeActionParticipant { + private static final Logger LOGGER = Logger.getLogger(RemoveTrailingSlash.class.getName()); + + public static final String CODEACTION_TITLE = "Remove trailing slash to specify file."; + @Override + public void doCodeAction(ICodeActionRequest request, List codeActions, CancelChecker cancelChecker) { + Diagnostic diagnostic = request.getDiagnostic(); + DOMDocument document = request.getDocument(); + try { + String locationText = document.findNodeAt(document.offsetAt(diagnostic.getRange().getEnd())).getAttribute("location"); + String replaceText = "location=\"" + locationText.substring(0, locationText.length()-1) + "\""; + codeActions.add(CodeActionFactory.replace(CODEACTION_TITLE, diagnostic.getRange(), replaceText, document.getTextDocument(), diagnostic)); + } catch (Exception e) { + LOGGER.warning("Could not generate code action for removing trailing slash." + e); + } + } +} diff --git a/lemminx-liberty/src/test/java/io/openliberty/CodeActionUtilitiesTest.java b/lemminx-liberty/src/test/java/io/openliberty/CodeActionUtilitiesTest.java index 88513389..ce4cc106 100644 --- a/lemminx-liberty/src/test/java/io/openliberty/CodeActionUtilitiesTest.java +++ b/lemminx-liberty/src/test/java/io/openliberty/CodeActionUtilitiesTest.java @@ -1,9 +1,14 @@ package io.openliberty; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.File; import org.junit.jupiter.api.Test; +import io.openliberty.tools.langserver.lemminx.codeactions.AddTrailingSlash; import io.openliberty.tools.langserver.lemminx.codeactions.IndentUtil; public class CodeActionUtilitiesTest { @@ -30,4 +35,32 @@ public void indentTabsTest() { String expectedText = System.lineSeparator() + indent + indent + "test"; assertEquals(expectedText, IndentUtil.formatText(sampleText, indent, column), "Incorrect whitespace buffer calculation."); } + + /** + * Test that a location string with mismatched slashes will be converted to all to forward slashes + */ + @Test + public void slashConversionTest() { + String fileSeparator = "/"; + String locationText = "..\\../test.xml"; + String replaceText = AddTrailingSlash.getReplaceText(fileSeparator, locationText); + assertFalse(replaceText.contains("\\")); + assertTrue(replaceText.endsWith("/\"")); // ends with /" + } + + /** + * Windows test that backslashes will be retained if it was the only type of slash used + */ + @Test + public void retainWindowsBackSlash() { + if (!File.separator.equals("\\")) { + return; + } + String fileSeparator = "/"; + String locationText = "..\\..\\test.xml"; + String replaceText = AddTrailingSlash.getReplaceText(fileSeparator, locationText); + assertTrue(replaceText.contains("\\")); + assertFalse(replaceText.contains("/")); + assertTrue(replaceText.endsWith("\\\"")); // ends with \" + } } diff --git a/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java b/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java index 2f4c9a38..cdcd5547 100644 --- a/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java +++ b/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java @@ -143,7 +143,7 @@ public void testTrimmedFeatureDiagnostic() { } @Test - public void testDiagnosticsForInclude() throws IOException { + public void testDiagnosticsForInclude() throws IOException, BadLocationException { // LibertyWorkspace must be initialized List initList = new ArrayList(); initList.add(new WorkspaceFolder(new File("src/test/resources").toURI().toString())); @@ -158,6 +158,8 @@ public void testDiagnosticsForInclude() throws IOException { " optional=\"true\" location=\"MULTI LINER\"/>", // " ", // " ", // + " ", // + " ", // "" ); @@ -169,11 +171,11 @@ public void testDiagnosticsForInclude() throws IOException { Diagnostic not_xml = new Diagnostic(); not_xml.setRange(r(3, 29, 3, 52)); - not_xml.setMessage("The specified resource is not an XML file."); + not_xml.setMessage("The specified resource is not an XML file. If it is a directory, it must end with a trailing slash."); Diagnostic multi_liner = new Diagnostic(); multi_liner.setRange(r(5, 28, 5, 50)); - multi_liner.setMessage("The specified resource is not an XML file."); + multi_liner.setMessage("The specified resource is not an XML file. If it is a directory, it must end with a trailing slash."); Diagnostic not_optional = new Diagnostic(); not_optional.setRange(r(6, 13, 6, 29)); @@ -195,9 +197,86 @@ public void testDiagnosticsForInclude() throws IOException { missing_xml2.setCode("missing_file"); missing_xml2.setMessage("The resource at the specified location could not be found."); + Diagnostic dirIsFile = new Diagnostic(); + dirIsFile.setRange(r(8, 13, 8, 42)); + dirIsFile.setCode("is_file_not_dir"); + dirIsFile.setMessage("Path specified a directory, but resource exists as a file. Please remove the trailing slash."); + + Diagnostic fileIsDir = new Diagnostic(); + fileIsDir.setRange(r(9, 13, 9, 36)); + fileIsDir.setCode("is_dir_not_file"); + fileIsDir.setMessage("Path specified a file, but resource exists as a directory. Please add a trailing slash."); XMLAssert.testDiagnosticsFor(serverXML, null, null, serverXMLFile.toURI().toString(), - not_xml, multi_liner, not_optional, missing_xml, optional_not_defined, missing_xml2); + not_xml, multi_liner, not_optional, missing_xml, optional_not_defined, missing_xml2, + dirIsFile, fileIsDir); + + // Check code actions for add/remove trailing slashes + String fixedFilePath = "location=\"/empty_server.xml\""; + TextEdit dirIsFileTextEdit = te(dirIsFile.getRange().getStart().getLine(), dirIsFile.getRange().getStart().getCharacter(), + dirIsFile.getRange().getEnd().getLine(), dirIsFile.getRange().getEnd().getCharacter(), fixedFilePath); + CodeAction dirIsFileCodeAction = ca(dirIsFile, dirIsFileTextEdit); + + String fixedDirPath = "location=\"/testDir.xml/\""; + TextEdit fileIsDirTextEdit = te(fileIsDir.getRange().getStart().getLine(), fileIsDir.getRange().getStart().getCharacter(), + fileIsDir.getRange().getEnd().getLine(), fileIsDir.getRange().getEnd().getCharacter(), fixedDirPath); + CodeAction fileIsDirCodeAction = ca(fileIsDir, fileIsDirTextEdit); + + + XMLAssert.testCodeActionsFor(serverXML, dirIsFile, dirIsFileCodeAction); + + XMLAssert.testCodeActionsFor(serverXML, fileIsDir, fileIsDirCodeAction); + } + + @Test + public void testDiagnosticsForIncludeWindows() throws BadLocationException { + if (!File.separator.equals("\\")) { // skip test if not Windows + return; + } + // LibertyWorkspace must be initialized + List initList = new ArrayList(); + initList.add(new WorkspaceFolder(new File("src/test/resources").toURI().toString())); + LibertyProjectsManager.getInstance().setWorkspaceFolders(initList); + + String serverXML = String.join(newLine, // + "", // + " ", // + " ", // + "" + ); + + // Diagnostic location1 = new Diagnostic(); + File serverXMLFile = new File("src/test/resources/server.xml"); + assertFalse(serverXMLFile.exists()); + // Diagnostic will not be made if found + assertTrue(new File("src/test/resources/empty_server.xml").exists()); + + Diagnostic dirIsFile = new Diagnostic(); + dirIsFile.setRange(r(1, 13, 1, 42)); + dirIsFile.setCode("is_file_not_dir"); + dirIsFile.setMessage("Path specified a directory, but resource exists as a file. Please remove the trailing slash."); + + Diagnostic fileIsDir = new Diagnostic(); + fileIsDir.setRange(r(2, 13, 2, 36)); + fileIsDir.setCode("is_dir_not_file"); + fileIsDir.setMessage("Path specified a file, but resource exists as a directory. Please add a trailing slash."); + + XMLAssert.testDiagnosticsFor(serverXML, null, null, serverXMLFile.toURI().toString(), + dirIsFile, fileIsDir); + + String fixedFilePath = "location=\"\\empty_server.xml\""; + TextEdit dirIsFileTextEdit = te(dirIsFile.getRange().getStart().getLine(), dirIsFile.getRange().getStart().getCharacter(), + dirIsFile.getRange().getEnd().getLine(), dirIsFile.getRange().getEnd().getCharacter(), fixedFilePath); + CodeAction dirIsFileCodeAction = ca(dirIsFile, dirIsFileTextEdit); + + String fixedDirPath = "location=\"\\testDir.xml\\\""; + TextEdit fileIsDirTextEdit = te(fileIsDir.getRange().getStart().getLine(), fileIsDir.getRange().getStart().getCharacter(), + fileIsDir.getRange().getEnd().getLine(), fileIsDir.getRange().getEnd().getCharacter(), fixedDirPath); + CodeAction fileIsDirCodeAction = ca(fileIsDir, fileIsDirTextEdit); + + XMLAssert.testCodeActionsFor(serverXML, dirIsFile, dirIsFileCodeAction); + + XMLAssert.testCodeActionsFor(serverXML, fileIsDir, fileIsDirCodeAction); } @Test diff --git a/lemminx-liberty/src/test/resources/testDir.xml/empty_file.xml b/lemminx-liberty/src/test/resources/testDir.xml/empty_file.xml new file mode 100644 index 00000000..e69de29b