Stop flagging valid custom web component tag names with S4670

Versions: SQ 8.3, scanner 4.3.0, css plugin 1.2.0

The subject rule flags CSS selectors that specify a “non-existent” tag name. My problem is that the definition of “existent” is very difficult to nail down once you start writing your own web components.

I like the idea of having a rule that catches when I misspell one of the core HTML tag names, but it all falls apart when your project uses web components, or Angular, or Vue, or any other framework that works in a similar way. I don’t currently limit my components to a single prefixed namespace, but even if I did, the rule would be of limited value since it would check for typos in predefined tag names, but not in the names of components defined in the project.

The current implementation defaults to exclude some popular prefixes (md-, mat-, fa-) but it can’t possibly be a good idea to ship a rule whose functionality depends on what libraries are popular at the time. It requires that the rule be re-configured to suit any project that uses custom components, and forces those components to be named with a prefix scheme that can be whitelisted via regex. (This is arguably a good practice, but the web component naming guidelines I found only say that at least one dash is required, and explicitly state that prefixing is not necessary for private-use components.)

I originally titled this post “The impact of web components on S4670” but while I was researching and writing it I had a realization: this rule will never have a true negative – that is, it will never¹ examine a selector and find that it passes the rule – if there’s a dash in the tag name, because no “existing” tag names include a dash (except per footnote). Valid web components must have a dash in the name per the custom element spec. So, if a user typed a dash in their CSS selector tag name, they must be referring to a custom element. As far as I can tell, it’s impossible for the CSS plugin to verify the existence of custom elements by name. Therefore, flagging tags with a dash in the name is useless.

The rule should ignore tags with dashes in the name and assume that they refer to a web component / custom element. This would also allow the rule to get rid of the ignoredTypes parameter entirely, since all the current default cases would be covered².


¹ There’s a short list of exceptions in this page from webcomponents.org from SVG and MathML, but no dashes in HTML. I’d argue that the value of catching “missng-glyph” as a CSS selector is minimal compared to the reduction in false positives that we’d see with this proposal.

² You could argue that users have already customized this property for their own projects, but if they configured it to any value that doesn’t end with a dash, their components are violating the custom components spec, which is an actual code-quality issue that should be addressed.

1 Like

Hi @Thw0rted ,

thank you for your super detailed explanation of why S4670 is too noisy in the described situation.
We will discuss it in the team and inform you about possible next steps.

Best,
Nils

2 Likes

Hello @Thw0rted ,

we have decided not to change the behavior of the rule in general. If we ignore all tags that must contain only a dash, we might get too many false negatives. So the rule would not have much value.

As you described in your message, you can adjust the expression that selects the tags that are ignored. Of course, you can also just filter by a dash. To make this possibility sustainable we have added this capability to the description of the rule.
It will be available on https://rules.sonarsource.com/ at the latest by the next LTS.

Best,
Nils

The documentation update is appreciated, and will probably help users get the most value out of the rule as currently implemented.

However, I strongly disagree with the sentiment that the rule “would not have much value” if it ignored dashed tags by default. Do you have any kind of instrumentation or metrics to show what tags are being caught in real-world scenarios? (ETA: do you have specific, real-world examples of where false negatives would occur?) As far as I can tell, the only “official elements” that have a dash in the names are the ones listed on the page I linked in the original post:

  • annotation-xml
  • color-profile
  • font-face
  • font-face-src
  • font-face-uri
  • font-face-format
  • font-face-name
  • missing-glyph

Per that page, all of them come from SVG and MathML. Remember, this rule is to check that selectors in CSS are applied to a known tag. The spirit of the rule is to catch typographic errors when spelling the name of an official element. How many people are using Sonar to check CSS that is applied to SVG or MathML¹? How many are using it for CSS that is applied to HTML? I would strongly argue that the defaults for the rule should cover the most common case, and by far the most common case is CSS applied to HTML – which cannot have dashes in the tag names for official elements.

At the very least, if the ignoreTypes property is not removed completely, it should default to simply /-/, which will best cover 99% of users without any tweaking.


¹ Further, even if people are using Sonar to check CSS that is applied to SVG or MathML, how likely are they to be applying style to a font-face-uri tag? I just checked MDN and all of the SVG tags in the list are deprecated, which means the lone holdout is the annotation-xml tag from MathML. You’re really going to configure the default behavior of the rule to favor one tag from an little-used specialist-domain language (MathML) over the most commonly used language pair (HTML/CSS) in the world?

1 Like

Hi @Thw0rted,

When @Nils_Werner and I discussed this, we thought that additional tags containing dashes might be added to later versions of HTML/SVG/MathML standards. If this happened, it would be complex to revert the default behavior of this rule, i.e. raising on elements containing a dash, without suddenly blocking the quality gate of already analyzed projects.
After looking at the standard drafts for HTML, SVG and MathML I couldn’t find any new tag with a dash.

This is a good point.

As no new tags are coming and existing ones are deprecated it makes indeed sense to change the default behavior for this rule.

It turns out that the styling rule on which we base S4670 has an option custom-elements. We will simply enable it. You can follow this ticket.

1 Like

Thanks, Nicolas, that’s great to hear. I have followed the GH issue and will watch for progress.