Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/wise-grapes-applaud.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"lit-analyzer-fork": patch
---

Rework no-unclosed-tag rule to improve to cleanly check for begining and ending tags
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ export function parseHtmlNode(p5Node: IP5TagNode, parent: HtmlNode | undefined,

const htmlNodeBase: IHtmlNodeBase = {
tagName: p5Node.tagName.toLowerCase(),
selfClosed: isSelfClosed(p5Node, context),
attributes: [],
location: makeHtmlNodeLocation(p5Node, context),
children: [],
Expand All @@ -71,17 +70,6 @@ export function parseHtmlNode(p5Node: IP5TagNode, parent: HtmlNode | undefined,
return htmlNode;
}

/**
* Returns if this node is self-closed.
* @param p5Node
* @param context
*/
function isSelfClosed(p5Node: IP5TagNode, context: ParseHtmlContext) {
const isEmpty = p5Node.childNodes == null || p5Node.childNodes.length === 0;
const isSelfClosed = getSourceLocation(p5Node)!.startTag.endOffset === getSourceLocation(p5Node)!.endOffset;
return isEmpty && isSelfClosed;
}

/**
* Creates source code location from a p5Node.
* @param p5Node
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ export interface IHtmlNodeBase {
attributes: HtmlNodeAttr[];
parent?: HtmlNode;
children: HtmlNode[];
selfClosed: boolean;
document: HtmlDocument;
}

Expand Down
43 changes: 33 additions & 10 deletions packages/lit-analyzer/src/lib/rules/no-unclosed-tag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,29 @@ import type { RuleModule } from "../analyze/types/rule/rule-module.js";
import { isCustomElementTagName } from "../analyze/util/is-valid-name.js";
import { rangeFromHtmlNode } from "../analyze/util/range-util.js";

// List taken from https://html.spec.whatwg.org/multipage/syntax.html#void-elements
// and parse5 list of void elements: https://github.com/inikulin/parse5/blob/86f09edd5a6840ab2269680b0eef2945e78c38fd/packages/parse5/lib/serializer/index.ts#L7-L26
const VOID_ELEMENTS = new Set([
"area",
"base",
"basefont",
"bgsound",
"br",
"col",
"embed",
"frame",
"hr",
"img",
"input",
"keygen",
"link",
"meta",
"param",
"source",
"track",
"wbr"
]);

/**
* This rule validates that all tags are closed properly.
*/
Expand All @@ -11,18 +34,18 @@ const rule: RuleModule = {
priority: "low"
},
visitHtmlNode(htmlNode, context) {
if (!htmlNode.selfClosed && htmlNode.location.endTag == null) {
// Report specifically that a custom element cannot be self closing
// if the user is trying to close a custom element.
const isCustomElement = isCustomElementTagName(htmlNode.tagName);

context.report({
location: rangeFromHtmlNode(htmlNode),
message: `This tag isn't closed.${isCustomElement ? " Custom elements cannot be self closing." : ""}`
});
if (VOID_ELEMENTS.has(htmlNode.tagName.toLowerCase()) || htmlNode.location.endTag != null) {
return;
}

return;
// Report specifically that a custom element cannot be self closing
// if the user is trying to close a custom element.
const isCustomElement = isCustomElementTagName(htmlNode.tagName);

context.report({
location: rangeFromHtmlNode(htmlNode),
message: `This tag isn't closed.${isCustomElement ? " Custom elements cannot be self closing." : ""}`
});
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ tsTest("Cannot assign 'undefined' in attribute binding", t => {
tsTest("Can assign 'undefined' in property binding", t => {
const { diagnostics } = getDiagnostics([
makeElement({ slots: ["foo: number | undefined"] }),
'html`<my-element .foo="${{} as number | undefined}" />`'
'html`<my-element .foo="${{} as number | undefined}"></my-element>`'
]);
hasNoDiagnostics(t, diagnostics);
});
Expand Down
44 changes: 43 additions & 1 deletion packages/lit-analyzer/src/test/rules/no-unclosed-tag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,49 @@ tsTest("Report unclosed tags", t => {
hasDiagnostic(t, diagnostics, "no-unclosed-tag");
});

tsTest("Don't report self closed tags", t => {
tsTest("Don't report void elements", t => {
const { diagnostics } = getDiagnostics("html`<img>`", { rules: { "no-unclosed-tag": true } });
hasNoDiagnostics(t, diagnostics);
});

tsTest("Don't report void elements with self closing syntax", t => {
const { diagnostics } = getDiagnostics("html`<img />`", { rules: { "no-unclosed-tag": true } });
hasNoDiagnostics(t, diagnostics);
});

// The `<p>` tag will be closed automatically if immediately followed by a lot of other elements,
// including `<div>`.
// Ref: https://html.spec.whatwg.org/multipage/grouping-content.html#the-p-element
tsTest("Report unclosed 'p' tag that was implicitly closed via tag omission", t => {
const { diagnostics } = getDiagnostics("html`<p><div></div></p>`", { rules: { "no-unclosed-tag": true } });
hasDiagnostic(t, diagnostics, "no-unclosed-tag");
});

tsTest("Report unclosed 'p' tag that is implicitly closed via tag omission containing text content", t => {
const { diagnostics } = getDiagnostics("html`<p>Unclosed Content<div></div></p>`", { rules: { "no-unclosed-tag": true } });
hasDiagnostic(t, diagnostics, "no-unclosed-tag");
});

// Regeression test for https://github.com/runem/lit-analyzer/issues/283
tsTest("Report 'p' tag that is implicitly closed via tag omission containing a space", t => {
// Note, the browser will parse this case into: `<p> </p><div></div><p></p>` which can be
// unexpected, but technically means the first `<p>` tag is not explicitly closed.
const { diagnostics } = getDiagnostics("html`<p> <div></div></p>`", { rules: { "no-unclosed-tag": true } });
hasDiagnostic(t, diagnostics, "no-unclosed-tag");
});

// Self-closing tags do not exist in HTML. They are only valid in SVG and MathML.
tsTest("Report non-void element using self closing syntax", t => {
const { diagnostics } = getDiagnostics("html`<p /><div></div>`", { rules: { "no-unclosed-tag": true } });
hasDiagnostic(t, diagnostics, "no-unclosed-tag");
});

tsTest("Report self closing 'p' tag containing text content", t => {
const { diagnostics } = getDiagnostics("html`<p />Unclosed Content<div></div>`", { rules: { "no-unclosed-tag": true } });
hasDiagnostic(t, diagnostics, "no-unclosed-tag");
});

tsTest("Don't report explicit closing 'p' tag containing text content", t => {
const { diagnostics } = getDiagnostics("html`<p>Unclosed Content</p><div></div>`", { rules: { "no-unclosed-tag": true } });
hasNoDiagnostics(t, diagnostics);
});