Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor language tweaks to x-parser error language #248

Open
klebba opened this issue Feb 13, 2025 · 4 comments
Open

Minor language tweaks to x-parser error language #248

klebba opened this issue Feb 13, 2025 · 4 comments

Comments

@klebba
Copy link
Collaborator

klebba commented Feb 13, 2025

It's awesome that these are all wrapped up in this single lookup — thanks for that! I'll turn this into a PR soon... in the meantime I'm fiddling here in the ticket for some unknown reason (still in progress...)

const messages = [
  ['#100', 'Could not parse template markup (at template start)'],
  //['#100', 'Markup at the start of your template could not be parsed.'],
  
  ['#101', 'Could not parse template markup (after text content)'],
  //['#101', 'Markup after content text found in your template could not be parsed.'],
  
  ['#102', 'Could not parse template markup (after a comment)'],
  //['#102', 'Markup after a comment found in your template could not be parsed.'],
  
  ['#103', 'Could not parse template markup (after interpolated content)'],
  //['#103', 'Markup after a content interpolation in your template could not be parsed.'],
  
  ['#104', 'Could not parse template markup (after an opening tag name)'],
  //['#104', 'Markup after the tag name in an opening tag in your template could not be parsed.'],
  
  ['#105', 'Could not parse template markup (after a spacing within opening tag)'],
  //['#105', 'Markup after a spacing in an opening tag in your template could not be parsed.'],
  
  ['#106', 'Could not parse template markup (after an opening tag)'],
  //['#106', 'Markup after an opening tag in your template could not be parsed.'],
  
  ['#107', 'Could not parse template markup (after a boolean attribute in an opening tag)'],
  //['#107', 'Markup after a boolean attribute in an opening tag in your template could not be parsed.'],
  
  ['#108', 'Could not parse template markup (after an attribute in an opening tag)'],
  //['#108', 'Markup after an attribute in an opening tag in your template could not be parsed.'],
  
  ['#109', 'Could not parse template markup (after a boolean attribute interpolation in an opening tag)'],
  //['#109', 'Markup after a boolean attribute interpolation in an opening tag in your template could not be parsed.'],
  
  ['#110', 'Could not parse template markup (after a defined attribute interpolation in an opening tag)'],
  //['#110', 'Markup after a defined attribute interpolation in an opening tag in your template could not be parsed.'],
  
  ['#111', 'Could not parse template markup (after an attribute interpolation in an opening tag)'],
  //['#111', 'Markup after an attribute interpolation in an opening tag in your template could not be parsed.'],
  
  ['#112', 'Could not parse template markup (after a property interpolation in an opening tag)'],
  //['#112', 'Markup after a property interpolation in an opening tag in your template could not be parsed.'],
  
  ['#113', 'Could not parse template markup (after the closing quote of an interpolated attribute or property in an opening tag)'],
  //['#113', 'Markup after the closing quote of an interpolated attribute or property in an opening tag in your template could not be parsed.'],
  
  ['#114', 'Could not parse template markup (after a closing tag)'],
  //['#114', 'Markup after a closing tag in your template could not be parsed.'],
  
  ['#120', 'Invalid tag name - refer to https://developer.mozilla.org/en-US/docs/Web/API/CustomElementRegistry/define#valid_custom_element_names)'],
  //['#120', 'Malformed open start tag — tag names must be alphanumeric, lowercase, cannot start or end with hyphens, and cannot start with a number.'],
  
  ['#121', 'Invalid tag whitespace (extraneous whitespace at opening tag)'],
  //['#121', 'Malformed open tag space — spaces in open tags must be literal whitespace characters or newlines. Inter-declaration spaces must be singular. Spaces after newlines may be used for indentation. Only one newline is allowed.'],
  
  ['#122', 'Invalid opening tag (extraneous whitespace at end of opening tag)'],
  //['#122', 'Malformed end to an opening tag — opening tags must close without any extraneous spaces or newlines.'],
  
  ['#123', 'Invalid closing tag'],
  //['#123', 'Malformed close tag — close tags must not contain any extraneous spaces or newlines and tag names must be alphanumeric, lowercase, cannot start or end with hyphens, and cannot start with a number.'],
  
  ['#124', 'Invalid tag attribute (must use kebab-case names and double quoted values)],
  //['#124', 'Malformed boolean attribute text — attribute names must be alphanumeric, must be lowercase, must not start or end with hyphens, and cannot start with a number — and, attribute values must be enclosed in double-quotes.'],
  
  ['#125', 'Invalid tag attribute (must use kebab-case names and double quoted values)'],
  //['#125', 'Malformed attribute text — attribute names must be alphanumeric, must be lowercase, must not start or end with hyphens, and cannot start with a number — and, attribute values must be enclosed in double-quotes.'],
  
  ['#126', 'Invalid tag attribute interpolation (must use kebab-case names and double quoted values)'],
  //['#126', 'Malformed boolean attribute interpolation — attribute names must be alphanumeric, must be lowercase, must not start or end with hyphens, and cannot start with a number — and, attribute values must be enclosed in double-quotes.'],
  
  ['#127', 'Invalid tag attribute interpolation (must use kebab-case names and double quoted values)'],
  //['#127', 'Malformed defined attribute interpolation — attribute names must be alphanumeric, must be lowercase, must not start or end with hyphens, and cannot start with a number — and, attribute values must be enclosed in double-quotes.'],
  
  ['#128', 'Invalid tag attribute interpolation (must use kebab-case names and double quoted values)'],
  //['#128', 'Malformed attribute interpolation — attribute names must be alphanumeric, must be lowercase, must not start or end with hyphens, and cannot start with a number — and, attribute values must be enclosed in double-quotes.'],
  
  ['#129', 'Invalid tag property interpolation (must use kebab-case names and double quoted values)'],
  //['#129', 'Malformed property interpolation — property names must be alphanumeric, must be lowercase, must not start or end with underscores, and cannot start with a number — and, property values must be enclosed in double-quotes.'],
  
  ['#130', 'Invalid closing quote on tag attribute or property'],
  //['#130', 'Malformed closing quote to a bound attribute or property. Enclosing quotes must be simple, double-quotes.'],
  
  ['#140', 'CDATA sections are not supported. Use HTML character references instead: https://developer.mozilla.org/en-US/docs/Glossary/Character_reference'],
  //['#140', 'CDATA sections are forbidden. Use html entities (character encodings) instead.'],
  
  ['#150', 'Bad escape in tagged template string. Only escapes to create a literal slash (“\\”) or back tick (“`”) are allowed. HTML character references are also supported: https://developer.mozilla.org/en-US/docs/Glossary/Character_reference'],
  //['#150', 'Improper javascript escape (\\x, \\u, \\t, \\n, etc.) in raw template string. Only escapes to create a literal slash (“\\”) or back tick (“`”) is allowed. Only valid HTML entities (character references) are supported in html as code points. Use literal characters (e.g., newlines) to enter newlines in your templates.'],
  
  ['#151', 'Ambiguous ampersand character or invalid hexadecimal character reference.'],
  //['#151', 'Malformed hexadecimal character reference (html entity) or ambiguous ampersand.'],
  
  //['#152', 'Malformed HTML comment. Comments cannot start with “>” or “->” characters, they cannot include a set of “--” characters, and they cannot end with a “-” character.'],
  ['#152', 'Malformed html comment. Comments cannot start with “>” or “->” characters, they cannot include a set of “--” characters, and they cannot end with a “-” character.'],
  
  // this doc would need updating
  ['#153', 'Unsupported HTML tag - supported native tags are listed here: https://github.com/Netflix/x-element/blob/main/doc/TEMPLATE_ENGINE.md'],
  //['#153', 'Forbidden html element used — this parser is opinionated about which elements are allowed in order to reduce complexity and improve performance.'],
  
  //['#154', 'Unmatched closing tag at the end of your template. To avoid unintended markup, non-void tags must explicitly be closed.'],
  ['#154', 'Unmatched closing tag at the end of your template. To avoid unintended markup, non-void tags must explicitly be closed.'],
  
  //['#155', 'Mismatched closing tag used. To avoid unintended markup, non-void tags must explicitly be closed and all closing tag names must be a case-sensitive match.'],
  ['#155', 'Mismatched closing tag used. To avoid unintended markup, non-void tags must explicitly be closed and all closing tag names must be a case-sensitive match.'],
  
  //['#156', 'Forbidden, nontrivial interpolation of <textarea> tag used. Only basic interpolation is allowed — e.g., <textarea>${…}</textarea>.'],
  ['#156', 'Forbidden, nontrivial interpolation of <textarea> tag used. Only basic interpolation is allowed — e.g., <textarea>${…}</textarea>.'],
  
  //['#157', 'Forbidden declarative shadow root used (e.g., `<template shadowrootmode="open">`).'],
  ['#157', 'Forbidden declarative shadow root used (e.g., `<template shadowrootmode="open">`).'],
];
const ignored = [
  // TODO: #236: Remove support for <svg> completely.
  ['#190', 'Forbidden svg element used — this parser is opinionated about which elements are allowed in order to reduce complexity and improve performance.'],
  
  // TODO: #237: Remove support for <style> tags completely.
  ['#191', 'Interpolation of <style> tags is not allowed.'],
  
  // TODO: #236: Obviated once foreign elements like <svg> are gone.
  ['#192', 'Forbidden uppercase letters in html attribute name. Because html attributes are case-insensitive, prefer to use the lowercased equivalent.'],
  
  // TODO: #236: Obviated once patterns are updated after <svg> is gone.
  ['#193', 'Forbidden uppercase letters in html tag name. Because html tag names are case-insensitive, prefer to use the lowercased equivalent.'],
  
  // TODO: #236: Obviated once <svg> is gone and namespacing is deleted.
  ['#194', 'Forbidden language provided — only svg and html are allowed. In the future this configuration will be removed completely.'],
];

On malformed vs invalid — I read this and I still don't know which term to use here!

@klebba
Copy link
Collaborator Author

klebba commented Feb 18, 2025

To discuss: if there are semantic implications for groups of numbers (e.g. 100-120 is a group) perhaps we should consider a more apparent group numbering scheme

@theengineear
Copy link
Collaborator

I was thinking that generally — we would have x-parser throw 1xx errors, x-template throw 2xx errors, and x-element throw 3xx errors. I would be surprised if any individual file needed more than one hundred errors… but who knows…

Within each group, I have no real opinion on the number allocations, but yes there are semantic differences. I tried to comment it in the code — I’ll repeat it here for reference:

// Block #100-#119 — Invalid transition errors.
// Block #120-#139 — Common mistakes.
// Block #140-#149 — Forbidden transitions.
// Block #150+ — Special, named issues.

(and the 190+ block is gone, that was for “deprecations”, but we’ve fixed all those up already).

Invalid transition errors. This is a generic error when we cannot move the state machine forward and we don’t really have a great guess as to why we got stuck. We just know it failed and where.

Common mistakes. This is a subroutine we run after getting stuck to try and pinpoint exactly why we think we got stuck. E.g., we first found we got stuck after a piece of “content” and then ran this subroutine to figure out that there is a malformed opening tag.

Forbidden transitions. In this case we know exactly what the next transition is — but we specifically restrict it. Right now, only CDATA sections fall in this category.

Special, named issues. We were able to parse the state transition, but then some other check happened. E.g., oh yah, we read your “attribute” and that’s cool, but unfortunately, you’re trying to set shadowrootmode on a template tag and we don’t currently support that for [reasons].

So “yes” is the short answer, there are definitely semantics there, but of course — I’m totally open to improving upon this stuff.

@theengineear
Copy link
Collaborator

FYI, I’m going to start using “start tag” and “end tag” as I’ve seen that in the specifications and it disambiguates usage of “open” and “close”. I.e., you can say some of the following things:

  • The opening of a start tag.
  • The closing of a start tag.
  • The opening of a comment.
  • The opening of an end tag.

I know that’s minor — but then again… that’s what this ticket is all about 😅

@theengineear
Copy link
Collaborator

Another thing you’ll see… I’m going to convert all these Map instances into switch statements — this way, our code coverage enforcement will force us to prove that these cases / messages are actually possible. For example, it’s not actually possible to get to some of these messages currently!

theengineear added a commit that referenced this issue Mar 6, 2025
Taking on some of the language tweaks from #248. Also `switch`-ing on
the values (versus using a `Map`) so that these cases can be enforced by
code coverage.

We likely have a couple messages which are impossible to get to. It’d be
good to be forced to either remove them or find a way to prove that they
are reachable via our test suite.

For now, I reduced the coverage target to keep this change set smaller,
but I will address that in follow-up efforts.
theengineear added a commit that referenced this issue Mar 6, 2025
Taking on some of the language tweaks from #248. Also `switch`-ing on
the values (versus using a `Map`) so that these cases can be enforced by
code coverage.

We likely have a couple messages which are impossible to get to. It’d be
good to be forced to either remove them or find a way to prove that they
are reachable via our test suite.

For now, I reduced the coverage target to keep this change set smaller,
but I will address that in follow-up efforts.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants