From cc21c5021426763942edd879380009bc7905f320 Mon Sep 17 00:00:00 2001 From: Alexander Chen Date: Wed, 2 Mar 2022 17:12:05 -0500 Subject: [PATCH] Added diagnostic and CodeAction to use let to replace with Signed-off-by: Alexander Chen --- .../qute/ls/commons/CodeActionFactory.java | 10 ++ .../redhat/qute/services/QuteCodeActions.java | 144 +++++++++++++++++- .../redhat/qute/services/QuteDiagnostics.java | 33 +++- .../diagnostics/DiagnosticDataFactory.java | 3 +- .../services/diagnostics/QuteErrorCode.java | 5 +- ...osticsInExpressionWithWithSectionTest.java | 73 +++++++-- 6 files changed, 243 insertions(+), 25 deletions(-) diff --git a/qute.ls/com.redhat.qute.ls/src/main/java/com/redhat/qute/ls/commons/CodeActionFactory.java b/qute.ls/com.redhat.qute.ls/src/main/java/com/redhat/qute/ls/commons/CodeActionFactory.java index 90271990f..c53a0e104 100644 --- a/qute.ls/com.redhat.qute.ls/src/main/java/com/redhat/qute/ls/commons/CodeActionFactory.java +++ b/qute.ls/com.redhat.qute.ls/src/main/java/com/redhat/qute/ls/commons/CodeActionFactory.java @@ -106,6 +106,16 @@ public static CodeAction replace(String title, Range range, String replaceText, return replace(title, Collections.singletonList(replace), document, diagnostic); } + @SuppressWarnings("null") + public static CodeAction replace(String title, List ranges, String replaceText, TextDocumentItem document, + Diagnostic diagnostic) { + List edits = null; + for (Range range : ranges) { + edits.add(new TextEdit(range, replaceText)); + } + return replace(title, edits, document, diagnostic); + } + public static CodeAction replace(String title, List replace, TextDocumentItem document, Diagnostic diagnostic) { diff --git a/qute.ls/com.redhat.qute.ls/src/main/java/com/redhat/qute/services/QuteCodeActions.java b/qute.ls/com.redhat.qute.ls/src/main/java/com/redhat/qute/services/QuteCodeActions.java index 52faba10e..c47726d29 100644 --- a/qute.ls/com.redhat.qute.ls/src/main/java/com/redhat/qute/services/QuteCodeActions.java +++ b/qute.ls/com.redhat.qute.ls/src/main/java/com/redhat/qute/services/QuteCodeActions.java @@ -19,7 +19,9 @@ import java.text.MessageFormat; import java.util.ArrayList; import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.logging.Level; import java.util.logging.Logger; @@ -29,6 +31,7 @@ import org.eclipse.lsp4j.Diagnostic; import org.eclipse.lsp4j.Position; import org.eclipse.lsp4j.Range; +import org.eclipse.lsp4j.TextEdit; import com.google.gson.JsonObject; import com.redhat.qute.ls.commons.BadLocationException; @@ -40,8 +43,11 @@ import com.redhat.qute.parser.template.Expression; import com.redhat.qute.parser.template.Node; import com.redhat.qute.parser.template.NodeKind; +import com.redhat.qute.parser.template.Parameter; import com.redhat.qute.parser.template.Section; +import com.redhat.qute.parser.template.SectionKind; import com.redhat.qute.parser.template.Template; +import com.redhat.qute.parser.template.sections.WithSection; import com.redhat.qute.project.QuteProject; import com.redhat.qute.services.commands.QuteClientCommandConstants; import com.redhat.qute.services.diagnostics.QuteErrorCode; @@ -50,7 +56,7 @@ /** * Qute code actions support. - * + * * @author Angelo ZERR * */ @@ -72,6 +78,8 @@ class QuteCodeActions { private static final String EXCLUDED_VALIDATION_TITLE = "Exclude this file from validation."; + private static final String QUTE_DEPRICATED_WITH_SECTION = "Replace `#with` with `#let`."; + public CompletableFuture> doCodeActions(Template template, CodeActionContext context, Range range, SharedSettings sharedSettings) { List codeActions = new ArrayList<>(); @@ -109,6 +117,15 @@ public CompletableFuture> doCodeActions(Template template, Code // Create `undefinedTag`" doCodeActionsForUndefinedSectionTag(template, diagnostic, codeActions); break; + case NotRecommendedWithSection: + // The following Qute template: + // {#with } + // + // will provide a quickfix like: + // + // Replace `with` with `let`. + doCodeActionsForNotRecommendedWithSection(template, diagnostic, codeActions); + break; default: break; } @@ -186,15 +203,136 @@ private static void doCodeActionToDisableValidation(Template template, List + * {#with item} + * {name} + * {/with} + * becomes + * {#let name=item.name} + * {name} + * {/let} + * + * + * @param template the Qute template. + * @param diagnostic the diagnostic list that this CodeAction will fix. + * @param codeActions the list of CodeActions to perform. + * + */ + private static void doCodeActionsForNotRecommendedWithSection(Template template, Diagnostic diagnostic, + List codeActions) { + Range withSectionRange = diagnostic.getRange(); + try { + // 1. Retrieve the #with section node using diagnostic range + int withSectionStart = template.offsetAt(withSectionRange.getStart()); + WithSection withSection = (WithSection) template.findNodeAt(withSectionStart + 1); + + // 2. Initialize TextEdit array + List edits = new ArrayList(); + + // 2.1 Create text edit to update section start from #with to #let + // and collect all object parts from expression for text edit + // (e.g. {name} -> {#let name=item.name}) + edits.add(createWithSectionOpenEdit(template, withSection)); + + // 2.2 Create text edit to update section end from /with to /let + edits.add(createWithSectionCloseEdit(template, withSection)); + + // 3. Create CodeAction + CodeAction replaceWithSection = CodeActionFactory.replace(QUTE_DEPRICATED_WITH_SECTION, edits, + template.getTextDocument(), diagnostic); + codeActions.add(replaceWithSection); + } catch (BadLocationException e) { + LOGGER.log(Level.SEVERE, "Creation of not recommended with section code action failed", e); + } + } + + /** + * Create text edit to replace unrecommended with section opening tag + * + * @param template the Qute template. + * @param withSection the Qute with section + * @return + * @throws BadLocationException + */ + private static TextEdit createWithSectionOpenEdit(Template template, WithSection withSection) + throws BadLocationException { + String objectPartName = withSection.getObjectParameter() != null ? withSection.getObjectParameter().getName() + : ""; + // Use set to avoid duplicate parameters + Set withObjectParts = new HashSet(); + + // Retrieve all expressions in #with section + findObjectParts(withSection, withObjectParts); + List letObjectPartParameterList = new ArrayList(); + for (String objectPart : withObjectParts) { + letObjectPartParameterList.add(MessageFormat.format("{1}={0}.{1}", objectPartName, objectPart)); + } + + // Build text edit string + String letObjectPartParameters = String.join(" ", letObjectPartParameterList); + String withSectionOpenReplacementText = MessageFormat.format("#let {0}", letObjectPartParameters); + Range withSectionOpen = new Range(template.positionAt(withSection.getStartTagNameOpenOffset()), + template.positionAt(withSection.getStartTagCloseOffset())); + + return new TextEdit(withSectionOpen, withSectionOpenReplacementText); + } + + /** + * Find all object parts by traversing AST Nodes, while retrieveing Expressions + * nested in Sections with recursion + * + * @param node current node in AST traversal + * @param objectParts set of found object parts + */ + private static void findObjectParts(Node node, Set objectParts) { + List children = node.getChildren(); + // Base case: Node is an expression + if (children.isEmpty()) { + if (node.getKind() == NodeKind.Expression) { + objectParts.add(((Expression) node).getContent()); + } + } + for (Node child : children) { + if (child.getKind() == NodeKind.Expression) { + objectParts.add(((Expression) child).getContent()); + } else if (child.getKind() == NodeKind.Section && ((Section) child).getSectionKind() != SectionKind.WITH) { + for (Parameter param : ((Section) child).getParameters()) { + objectParts.add(param.getValue()); + } + // Recursive call to traverse nested non-WithSection Sections + findObjectParts(child, objectParts); + } + } + } + + /** + * Create text edit to replace unrecommended with section closing tag + * + * @param template the Qute template. + * @param withSection the Qute with section + * @return + * @throws BadLocationException + */ + private static TextEdit createWithSectionCloseEdit(Template template, WithSection withSection) + throws BadLocationException { + String withSectionCloseReplacementText = "/let"; + Range withSectionClose = new Range(template.positionAt(withSection.getEndTagNameOpenOffset()), + template.positionAt(withSection.getEndTagCloseOffset())); + return new TextEdit(withSectionClose, withSectionCloseReplacementText); + } + /** * Create the configuration update (done on client side) quick fix. - * + * * @param title the displayed name of the QuickFix. * @param sectionName the section name of the settings to update. * @param item the section value of the settings to update. * @param editType the configuration edit type. * @param diagnostic the diagnostic list that this CodeAction will fix. - * + * * @return the configuration update (done on client side) quick fix. */ private static CodeAction createConfigurationUpdateCodeAction(String title, String scopeUri, String sectionName, diff --git a/qute.ls/com.redhat.qute.ls/src/main/java/com/redhat/qute/services/QuteDiagnostics.java b/qute.ls/com.redhat.qute.ls/src/main/java/com/redhat/qute/services/QuteDiagnostics.java index 59a1b0b51..a09adcf4c 100644 --- a/qute.ls/com.redhat.qute.ls/src/main/java/com/redhat/qute/services/QuteDiagnostics.java +++ b/qute.ls/com.redhat.qute.ls/src/main/java/com/redhat/qute/services/QuteDiagnostics.java @@ -54,6 +54,7 @@ import com.redhat.qute.parser.template.Template; import com.redhat.qute.parser.template.sections.IncludeSection; import com.redhat.qute.parser.template.sections.LoopSection; +import com.redhat.qute.parser.template.sections.WithSection; import com.redhat.qute.project.JavaMemberResult; import com.redhat.qute.project.QuteProject; import com.redhat.qute.project.datamodel.JavaDataModelCache; @@ -246,6 +247,9 @@ private void validateDataModel(Node parent, Template template, ResolvingJavaType case INCLUDE: validateIncludeSection((IncludeSection) section, diagnostics); break; + case WITH: + validateWithSection((WithSection) section, diagnostics); + break; default: validateSectionTag(section, template, resolvingJavaTypeContext, diagnostics); } @@ -345,6 +349,21 @@ private static void validateIncludeSection(IncludeSection includeSection, List diagnostics) { + if (withSection.getObjectParameter() != null) { + Range range = QutePositionUtility.createRange(withSection); + Diagnostic diagnostic = createDiagnostic(range, DiagnosticSeverity.Warning, + QuteErrorCode.NotRecommendedWithSection); + diagnostics.add(diagnostic); + } + } + private ResolvedJavaTypeInfo validateExpression(Expression expression, Section ownerSection, Template template, ResolutionContext resolutionContext, ResolvingJavaTypeContext resolvingJavaTypeContext, List diagnostics) { @@ -391,7 +410,7 @@ private ResolvedJavaTypeInfo validateExpressionParts(Parts parts, Section ownerS ResolvedJavaTypeInfo resolvedJavaType = null; String namespace = null; for (int i = 0; i < parts.getChildCount(); i++) { - Part current = ((Part) parts.getChild(i)); + Part current = parts.getChild(i); if (current.isLast()) { // It's the last part, check if it is not ended with '.' @@ -586,7 +605,7 @@ private ResolvedJavaTypeInfo validateObjectPart(ObjectPart objectPart, Section o /** * Validate the given property, method part. - * + * * @param part the property, method part to validate. * @param ownerSection the owner section and null otherwise. * @param template the template. @@ -596,7 +615,7 @@ private ResolvedJavaTypeInfo validateObjectPart(ObjectPart objectPart, Section o * @param iterableOfType the iterable of type. * @param diagnostics the diagnostic list to fill. * @param resolvingJavaTypeContext the resolving Java type context. - * + * * @return the Java type returned by the member part and null otherwise. */ private ResolvedJavaTypeInfo validateMemberPart(Part part, Section ownerSection, Template template, @@ -617,7 +636,7 @@ private ResolvedJavaTypeInfo validateMemberPart(Part part, Section ownerSection, /** * Validate the given property part. - * + * * @param part the property part to validate. * @param ownerSection the owner section and null otherwise. * @param template the template. @@ -627,7 +646,7 @@ private ResolvedJavaTypeInfo validateMemberPart(Part part, Section ownerSection, * @param iterableOfType the iterable of type. * @param diagnostics the diagnostic list to fill. * @param resolvingJavaTypeContext the resolving Java type context. - * + * * @return the Java type returned by the member part and null otherwise. */ private ResolvedJavaTypeInfo validatePropertyPart(PropertyPart part, Section ownerSection, Template template, @@ -650,7 +669,7 @@ private ResolvedJavaTypeInfo validatePropertyPart(PropertyPart part, Section own /** * Validate the given method part. - * + * * @param part the method part to validate. * @param ownerSection the owner section and null otherwise. * @param template the template. @@ -660,7 +679,7 @@ private ResolvedJavaTypeInfo validatePropertyPart(PropertyPart part, Section own * @param iterableOfType the iterable of type. * @param diagnostics the diagnostic list to fill. * @param resolvingJavaTypeContext the resolving Java type context. - * + * * @return the Java type returned by the member part and null otherwise. */ private ResolvedJavaTypeInfo validateMethodPart(MethodPart methodPart, Section ownerSection, Template template, diff --git a/qute.ls/com.redhat.qute.ls/src/main/java/com/redhat/qute/services/diagnostics/DiagnosticDataFactory.java b/qute.ls/com.redhat.qute.ls/src/main/java/com/redhat/qute/services/diagnostics/DiagnosticDataFactory.java index b0d16a59c..74ff4471a 100644 --- a/qute.ls/com.redhat.qute.ls/src/main/java/com/redhat/qute/services/diagnostics/DiagnosticDataFactory.java +++ b/qute.ls/com.redhat.qute.ls/src/main/java/com/redhat/qute/services/diagnostics/DiagnosticDataFactory.java @@ -24,7 +24,7 @@ /** * Diagnostic factory. - * + * * @author Angelo ZERR * */ @@ -55,4 +55,5 @@ public static Diagnostic createDiagnostic(Range range, String message, Diagnosti errorCode != null ? errorCode.getCode() : null); return diagnostic; } + } diff --git a/qute.ls/com.redhat.qute.ls/src/main/java/com/redhat/qute/services/diagnostics/QuteErrorCode.java b/qute.ls/com.redhat.qute.ls/src/main/java/com/redhat/qute/services/diagnostics/QuteErrorCode.java index 2838d4d03..dd07affa2 100644 --- a/qute.ls/com.redhat.qute.ls/src/main/java/com/redhat/qute/services/diagnostics/QuteErrorCode.java +++ b/qute.ls/com.redhat.qute.ls/src/main/java/com/redhat/qute/services/diagnostics/QuteErrorCode.java @@ -45,7 +45,10 @@ public enum QuteErrorCode implements IQuteErrorCode { UndefinedSectionTag("No section helper found for `{0}`."), // - SyntaxError("Syntax error: `{0}`."); + SyntaxError("Syntax error: `{0}`."), + + // Error code for deprecated #with section + NotRecommendedWithSection("`with` is not recommended. Use `let` instead."); private final String rawMessage; diff --git a/qute.ls/com.redhat.qute.ls/src/test/java/com/redhat/qute/services/diagnostics/QuteDiagnosticsInExpressionWithWithSectionTest.java b/qute.ls/com.redhat.qute.ls/src/test/java/com/redhat/qute/services/diagnostics/QuteDiagnosticsInExpressionWithWithSectionTest.java index e40f3b4d1..de87498df 100644 --- a/qute.ls/com.redhat.qute.ls/src/test/java/com/redhat/qute/services/diagnostics/QuteDiagnosticsInExpressionWithWithSectionTest.java +++ b/qute.ls/com.redhat.qute.ls/src/test/java/com/redhat/qute/services/diagnostics/QuteDiagnosticsInExpressionWithWithSectionTest.java @@ -23,34 +23,69 @@ /** * Test with #with section - * + * * @author Angelo ZERR * */ public class QuteDiagnosticsInExpressionWithWithSectionTest { + @Test + public void missingObject() throws Exception { + String template = "{#with}\r\n" + // + "{/with}"; + Diagnostic d = d(0, 6, 0, 6, QuteErrorCode.SyntaxError, + "Parser error on line 1: mandatory section parameters not declared for {#with}: [object]", + DiagnosticSeverity.Error); + + testDiagnosticsFor(template, d); + testCodeActionsFor(template, d); + } + @Test public void undefinedObject() throws Exception { String template = "{#with item}\r\n" + // "{/with}"; - Diagnostic d = d(0, 7, 0, 11, QuteErrorCode.UndefinedVariable, "`item` cannot be resolved to a variable.", + Diagnostic d1 = d(0, 7, 0, 11, QuteErrorCode.UndefinedVariable, "`item` cannot be resolved to a variable.", DiagnosticSeverity.Warning); - d.setData(DiagnosticDataFactory.createUndefinedVariableData("item", false)); + d1.setData(DiagnosticDataFactory.createUndefinedVariableData("item", false)); - testDiagnosticsFor(template, d); - testCodeActionsFor(template, d, // - ca(d, te(0, 0, 0, 0, "{@java.lang.String item}\r\n"))); + Diagnostic d2 = d(0, 0, 1, 7, QuteErrorCode.NotRecommendedWithSection, + "`with` is not recommended. Use `let` instead.", DiagnosticSeverity.Warning); + + testDiagnosticsFor(template, d1, d2); + testCodeActionsFor(template, d1, // + ca(d1, te(0, 0, 0, 0, "{@java.lang.String item}\r\n"))); + testCodeActionsFor(template, d2, // + ca(d2, te(0, 1, 0, 11, "#let "), te(1, 1, 1, 6, "/let"))); } @Test - public void noError() throws Exception { + public void singleSection() throws Exception { String template = "{@org.acme.Item item}\r\n" + // "{#with item}\r\n" + // "

{name}

\r\n" + // "

{price}

\r\n" + // "{/with}"; - testDiagnosticsFor(template); + Diagnostic d = d(1, 0, 4, 7, QuteErrorCode.NotRecommendedWithSection, + "`with` is not recommended. Use `let` instead.", DiagnosticSeverity.Warning); + testDiagnosticsFor(template, d); + testCodeActionsFor(template, d, // + ca(d, te(1, 1, 1, 11, "#let price=item.price name=item.name"), te(4, 1, 4, 6, "/let"))); + } + + @Test + public void nestedParameter() throws Exception { + String template = "{@org.acme.Item item}\r\n" + // + "{#with item}\r\n" + // + " {#let foo=name} \r\n" + // + " {/let} \r\n" + // + "{/with}"; + Diagnostic d = d(1, 0, 4, 7, QuteErrorCode.NotRecommendedWithSection, + "`with` is not recommended. Use `let` instead.", DiagnosticSeverity.Warning); + testDiagnosticsFor(template, d); + testCodeActionsFor(template, d, // + ca(d, te(1, 1, 1, 11, "#let name=item.name"), te(4, 1, 4, 6, "/let"))); } @Test @@ -68,13 +103,25 @@ public void nested() throws Exception { " {/with}\r\n" + // "{/with}"; - Diagnostic d = d(6, 5, 6, 12, QuteErrorCode.UndefinedVariable, "`average` cannot be resolved to a variable.", + Diagnostic d1 = d(1, 0, 11, 7, QuteErrorCode.NotRecommendedWithSection, + "`with` is not recommended. Use `let` instead.", DiagnosticSeverity.Warning); + + Diagnostic d2 = d(4, 2, 10, 9, QuteErrorCode.NotRecommendedWithSection, + "`with` is not recommended. Use `let` instead.", DiagnosticSeverity.Warning); + + Diagnostic d3 = d(6, 5, 6, 12, QuteErrorCode.UndefinedVariable, "`average` cannot be resolved to a variable.", DiagnosticSeverity.Warning); - d.setData(DiagnosticDataFactory.createUndefinedVariableData("average", false)); + d3.setData(DiagnosticDataFactory.createUndefinedVariableData("average", false)); - testDiagnosticsFor(template, d); - testCodeActionsFor(template, d, // - ca(d, te(0, 0, 0, 0, "{@java.lang.String average}\r\n"))); + testDiagnosticsFor(template, d1, d2, d3); + testCodeActionsFor(template, d1, // + ca(d1, te(1, 1, 1, 11, "#let reviews=item.reviews name=item.name"), te(11, 1, 11, 6, "/let"))); + testCodeActionsFor(template, d2, // + ca(d2, te(4, 3, 4, 16, + "#let average=reviews.average size=reviews.size name=reviews.name data:item.name=reviews.data:item.name item.name=reviews.item.name"), + te(10, 3, 10, 8, "/let"))); + testCodeActionsFor(template, d3, // + ca(d3, te(0, 0, 0, 0, "{@java.lang.String average}\r\n"))); } }