Skip to content

Commit

Permalink
Make isSelfClosed check far more specific, checking void tags and sel…
Browse files Browse the repository at this point in the history
…f closing syntax.
  • Loading branch information
AndrewJakubowicz committed Jan 6, 2024
1 parent 9f44f6f commit dfbbfae
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ import { getSourceLocation, IP5TagNode, P5Node } from "../parse-html-p5/parse-ht
import { parseHtmlNodeAttrs } from "./parse-html-attribute.js";
import { ParseHtmlContext } from "./parse-html-context.js";

// List taken from https://html.spec.whatwg.org/multipage/syntax.html#void-elements
const VOID_ELEMENTS = new Set(["area", "base", "br", "col", "embed", "hr", "img", "input", "link", "meta", "source", "track", "wbr"]);

/**
* Parses multiple p5Nodes into multiple html nodes.
* @param p5Nodes
Expand Down Expand Up @@ -70,14 +73,26 @@ export function parseHtmlNode(p5Node: IP5TagNode, parent: HtmlNode | undefined,
}

/**
* Returns if this node is self-closed.
* Returns if this node was explicitly self-closed or void.
*
* Note: Self-closing tags do not actually exist in HTML
* https://developer.mozilla.org/en-US/docs/Glossary/Void_element#self-closing_tags
*
* Therefore this function returns `true` if `p5Node` is
* a void element, or was explicitly self-closed using XML/JSX
* syntax. If the node is implicitly closed, for example a
* `p` element followed by a `div`, then `false` is returned.
*
* @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;
if (VOID_ELEMENTS.has(p5Node.tagName.toLowerCase())) {
return true;
}
const loc = getSourceLocation(p5Node)!;
const isSelfClosed = context.html[loc.startTag.endOffset - 2] === "/";
return isSelfClosed;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ export interface IHtmlNodeBase {
attributes: HtmlNodeAttr[];
parent?: HtmlNode;
children: HtmlNode[];
/**
* Is true when an HTML node is either a void element, or was
* explicitly closed with self-closing XML syntax.
*/
selfClosed: boolean;
document: HtmlDocument;
}
Expand Down
6 changes: 5 additions & 1 deletion packages/lit-analyzer/src/lib/rules/no-unclosed-tag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@ const rule: RuleModule = {
priority: "low"
},
visitHtmlNode(htmlNode, context) {
if (!htmlNode.selfClosed && htmlNode.location.endTag == null) {
const {
selfClosed,
location: { endTag }
} = htmlNode;
if (!selfClosed && 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);
Expand Down
30 changes: 30 additions & 0 deletions packages/lit-analyzer/src/test/rules/no-unclosed-tag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,33 @@ tsTest("Don't report self closed tags", t => {
const { diagnostics } = getDiagnostics("html`<img />`", { rules: { "no-unclosed-tag": true } });
hasNoDiagnostics(t, diagnostics);
});

tsTest("Don't report void tags", 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 is 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");
});

// Self-closing tags do not exist in HTML, but we can use them to check
// that the user intended that the tag be closed.
tsTest("Don't report self closing 'p' tag", t => {
const { diagnostics } = getDiagnostics("html`<p /><div></div>`", { rules: { "no-unclosed-tag": true } });
hasNoDiagnostics(t, diagnostics);
});

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

0 comments on commit dfbbfae

Please sign in to comment.