stylelint icon indicating copy to clipboard operation
stylelint copied to clipboard

`selector-type-no-unknown` doesn't properly read the 'universal namespace selector' unless it is added to the list of ignored namespaces.

Open bedney opened this issue 4 years ago • 8 comments

Given this rule in .stylelintrc:

        "selector-type-no-unknown": [ true, {
            "ignoreNamespaces": [ "svg" ],
            "ignoreTypes": [ "/*/" ]
        }],

A selector like this will still produce a 'Cannot parse selector parseError' failure:

*|span {
...
}

Note that this rule means 'a span in any namespace, given properly namespaced XHTML. Also note that this 'universal namespace selector' is not the same as a 'default namespace' selector. It is a construct, like the '*' universal selector, that is built directly into the CSS specification.

Which rule, if any, is the bug related to?

selector-type-no-unknown

What code is needed to reproduce the bug?

See code above.

What stylelint configuration is needed to reproduce the bug?

See code above.

Which version of stylelint are you using?

13.3.1

How are you running stylelint: CLI, PostCSS plugin, Node.js API?

Calling it from NodeJS API.

Does the bug relate to non-standard syntax (e.g. SCSS, Less etc.)?

No, this is standard CSS3 namespace syntax.

What did you expect to happen?

No warnings to be flagged.

What actually happened (e.g. what warnings or errors did you get)?

Warnings were flagged as detailed above.

Note that this bug is fixed by adding '*' as a 'namespace' in the list of ignored namespaces:

        "selector-type-no-unknown": [ true, {
            "ignore": ["default-namespace"],
            "ignoreNamespaces": [ "*", "svg" ],
            "ignoreTypes": [ "/*/" ]
        }],

But this should be unnecessary given that, just like how * is considered 'the universal selector' by CSS, the * in a selector like *|sometag is considered 'the universal namespace selector'. My argument here would be that it should automatically added to the list of ignored namespaces since it's truly a 'built in' of the CSS specification, independent of user-agent or rendering environment.

Also, note that this rule passes (universal selector using universal namespace) when it should fail until this bug is fixed, which probably means the code is taking another path when it sees the universal tag selector.

*|* {
...
}

bedney avatar May 23 '21 16:05 bedney

Thanks for the detailed report and for using the template.

I can reproduce the throw using the demo.

I believe the parse error is due to the * in the ignoreTypes regex not being escaped. The error isn't very clear, though, as it's being classified as a parse error.

The ignoreTypes option is for specifying type selectors you'd like to ignore. The universal selector will automatically be ignored by the rule as the parser has a separate node type for it.

The ignoreNamespace: [] option ignores unknown type selectors namespaced with one of the specified namespaces. It's not for ignoring namespaces themselves.

Can you provide some examples of the CSS you'd like to warn against, please, so that we can better understand your use case and advise what options will work best?

Note that this bug is fixed by adding '*' as a 'namespace' in the list of ignored namespaces:

I think adding the ignoreNamespaces removes the parse error because that code returns before the ignoreTypes option runs into the unescaped regex:

https://github.com/stylelint/stylelint/blob/b01ed25dfa3e8231a976eef76bd5e81fb535b1e9/lib/rules/selector-type-no-unknown/index.js#L79-L85

jeddy3 avatar May 23 '21 17:05 jeddy3

Thank you for the prompt reply!

So, given the following configuration:

"selector-type-no-unknown": [ true, {
            "ignoreNamespaces": [ "svg" ],
            "ignoreTypes": [ "/*/" ]
        }],

I would expect the following to not produce a parse error:

/* The common universal selector (any element type name in the 'default' namespace) */
* {
}
/* The common universal selector along with the universal namespace selector (any element type name in *any* namespace) */
*|* {
}
/* The universal namespace selector along with a specific element type name */
*|foo {
}

My argument for the 3rd case passing (even though * is not in the list of ignoreNamespaces) is that I consider the * in this case (universal namespace) to be 'built in', just like the * is for the 1st case here.

Additionally, given the same configuration, I would expect these to fail:

bar|* {
}
bar|foo {
}

In this case, the second one does fail (properly - I have not said in my configuration to ignore the 'bar' namespace), but the first one passes and that one should fail as well.

bedney avatar May 23 '21 19:05 bedney

To allow the following:

* {}
*|* {}
*|foo {}

You can use:

{
  "rules": {
    "selector-type-no-unknown": [
      true,
      {
        "ignoreTypes": [ "foo" ]
      }
    ]
  }
}

And to allow anything in the * "namespace":

{
  "rules": {
    "selector-type-no-unknown": [
      true,
      {
        "ignoreNamespaces": [ "*" ]
      }
    ]
  }
}

As the:

  • first * in * {}
  • second star in *|* {}

are parsed as universal nodes and therefore not checked by this rule.

My argument for the 3rd case passing (even though * is not in the list of ignoreNamespaces) is that I consider the * in this case (universal namespace) to be 'built in', just like the * is for the 1st case here.

The rules in stylelint are strict by default, but can be made more permissive using secondary options. So, even though it may not be technically correct to treat * as a namespace, it will give users the choice of allowing (or not):

*|foo {}

Additionally, given the same configuration, I would expect these to fail:

As * is parsed as the universal node, it won't be checked by this rule and the following will never give an error:

bar|* {}

The following will produce an error as foo is an unknown type selector:

bar|foo {}

It can be ignored either by using ignoreTypes: ["foo"] or "ignoreNamespaces: ["bar"].

If you want to disallow certain namespaces then we can add another rule to stylelint to do this, e.g. selector-type-namespace-disallowed-list: [], which would disallow both:

bar|* {}
bar|foo {}

Or anything else namespaced with bar. Is that something you're trying to do?

I think some confusion lies in that stylelint treats the thing after the | as the type selector rather than the whole selector. Hence, ignoreTypes: [/\\*/] will never match anything in *|* {} as the first * is considered the namespace and the second * is parsed as a universal node.

I think we can improve the rule documentation around this to make it clearer.

jeddy3 avatar May 24 '21 13:05 jeddy3

Thanks for the detailed answer in what is, I'm sure, a pretty fringe issue (not a lot of folks out there doing namespaced markup ;-) ).

even though it may not be technically correct to treat * as a namespace, it will give users the choice of allowing (or not)

Fair enough. It's not a huge problem for me to put * in my list of 'ignored namespaces'

As * is parsed as the universal node, it won't be checked by this rule and the following will never give an error

My only nit here is that * is only considered the universal node when no default namespace has been defined (see https://drafts.csswg.org/selectors-3/#universal-selector, section 6.2.1, last example).

If a default namespace has been defined:

@namespace url("http://www.w3.org/1999/xhtml");

Then it will not match "universally" - only "universally within the default namespace". In order to get "all elements" in this case, you have to use *|*.

Assume:

<span>Hi</span>
<foo:span>Hi</foo:span>

This is insufficient to style them both red:

@namespace url("http://www.w3.org/1999/xhtml");

* {color: red}

(the <foo:span> will not be red)

You must use:

@namespace url("http://www.w3.org/1999/xhtml");

*|* {color: red}

So, this case, whether the namespace is a * or something else, the namespace needs to be considered.

bedney avatar May 24 '21 14:05 bedney

Thanks for the detailed answer

No worries. I'd like to wrap my head around it, even if it is a fringe issue.

As * is parsed as the universal node, it won't be checked by this rule and the following will never give an error

My only nit here is that * is only considered the universal node when no default namespace has been defined (see https://drafts.csswg.org/selectors-3/#universal-selector, section 6.2.1, last example).

I'm referring to how it is parsed by the library we use in stylelint, i.e.

*|* {}

Is parsed as:

{
  "nodes": [{ 
     "type": "universal",
     "value": "*",
     "_namespace": "*"
  }]
}

Regardless of context.

Fair enough. It's not a huge problem for me to put * in my list of 'ignored namespaces'

It's best to think of the ignoreNamespaces option as ignoring the bit after | for the matching namespaces, rather than ignoring the namespace itself.

So, using ignoreNamespaces: ["*"] will allow anything to come after *|. For example:

*|unknown {}
*|spsn {}

The selector-type-no-unknown rule was added to help users catch spelling mistakes in their type selectors. For example, to catch when they accidentally type spsn {} instead of span {}. It looks like it's coming up short for your use case, though. Can you, as a next step, write down the patterns you'd like to allow and those you'd like to disallow? Forget about the existing options and rules for now. Just list the CSS patterns (with a comment to explain each).

We can then work out how best to meet that need (if it's possible with static analysis), e.g. by changing the behaviour of the existing options, adding new options to this rule or adding new rules entirely.

jeddy3 avatar May 24 '21 16:05 jeddy3

So since you asked ;-), I'm going to lay out how I think this rule should work in general and back up all of the way to the beginning. So some of this will not be specific to this bug, but just general comments.

For purposes of this discussion, I'll call * the 'universal selector' and the first * part of a selector like *|* the 'any namespace'. This is in keeping with the CSS3 specification.

  1. If a namespace is defined, as it is in this file: https://gist.github.com/bedney/c84d63b12284caffa3539a56c4b59bda, then I shouldn't have to put it into the ignoreNamespaces entry in my .stylelintrc. For instance, in that file, foo is defined as a namespace in the scope of that file by using the @namespace at-rule. This behavior has always bothered me about stylelint. Why am I having to say 'ignore this namespace'? The namespace was defined properly using the CSS3 @namespace at-rule. Therefore, I shouldn't have to inform stylelint that it needs to ignore this namespace, but I currently have to. Otherwise I get a parseError. What is suspect is happening here is that @namespace at-rules are not processed by stylelint and therefore it can't know that "Ok, the foo namespace [in this case] is a valid namespace for the scope of this file".

  2. If a universal selector is defined without any namespace prefix, then it applies to all elements, namespaced or not. See https://gist.github.com/bedney/1c28b7b4ff89a36b1bf319c827fa0fa6

  3. If a universal selector is defined with a 'any namespace' prefix (i.e. *|*) but with no default namespace defined, then it is equivalent to the non-namespaced universal selector *. In the following example, you'll see that the text is either green or blue depending on the order of the rules. If their order is flipped, the color will change: https://gist.github.com/bedney/031046044cc17f180f5d9591bd39bad3

  4. If a universal selector is defined with a 'any namespace' prefix (i.e. *|*) and a default namespace is defined, then it will always take precedence over a simple universal selector. See: https://gist.github.com/bedney/843b031bf8399f0f0eb05c7ddfc9a999

  5. Note that the namespace (whether it is a specific namespace or the 'any namespace' specifier) has no impact on specificity. This example shows that the *|* rule will override foo|*, even though it would look to the casual observer that the second one has less 'precedence': https://gist.github.com/bedney/60881e572b09c946ff55637d5dc87669

So, with that long-winded introduction out of the way, here's what my expectations for this rule would be:

  1. It should always allow the 'any namespace' namespace (i.e. the first * of *|*) without me having to add * to 'ignoreNamespaces'. It's a 'built-in', like the universal selector. I don't have to add the universal selector to the list of ignoreTypes and I feel the same way about the * namespace.

  2. If a namespace prefix is known in the file because of a @namespace at-rule (i.e. @namespace foo url("http://www.foo.com");) then it is 'known' (or should be) to stylelint. I shouldn't have to add it to the list of ignoreNamespaces. Note that this really should operate on a per-file basis to do checking according to the CSS3 specification.

  3. If a namespace prefix is not known in the file because there is a missing @namespace at-rule, then it should be considered a violation. I should either have to define it in that file using a @namespace at-rule OR add it to the list of ignoreNamespaces.

In looking back at my use-cases and my comments above, those 3 statements really summarize what I'm trying to achieve here.

Thanks very much for looking at this. This project has amazing maintainers :-).

Cheers,

  • Bill

bedney avatar May 26 '21 21:05 bedney

Thanks for the detailed write-up! I feel I have a better grasp of namespaces now.

The good news is that the rule already does most of what you described. An unescaped regex (i.e. the regex in "ignoreTypes": [ "/*/" ] should be /\\*/) in your configuration caused the parse error that got in the way of you experiencing this. Once you remove the option with the unescaped regex, the rule will work as you imagine it should.

Given this config:

{
  "rules": {
    "selector-type-no-unknown": true
  }
}

And this CSS:

* {}
span {}
spsn {}

*|* {}
*|span {}
*|spsn {}

foo|* {}
foo|span {}
foo|spsn {}

We get:

3:1 error Unexpected unknown type selector "spsn" (selector-type-no-unknown)
7:3 error Unexpected unknown type selector "spsn" (selector-type-no-unknown)
11:5 error Unexpected unknown type selector "spsn" (selector-type-no-unknown)

i.e.

  • the * universal selector is allowed (and doesn't result in a parse error).
  • the foo and * namespaces are allowed (and don't result in a parse error).
  • the misspelt spsn is caught three times

This is because the rule is only interested in the bit after | in type selectors.

See this demo.

The ignoreNamespaces: [] option is for ignoring bit after | for the matching namespaces, rather than ignoring the namespace itself.

So, given CSS above and the following config:

{
  "rules": {
    "selector-type-no-unknown": [true, { "ignoreNamespaces": ["foo"] } ]
  }
}

We get one less violation:

3:1 error Unexpected unknown type selector "spsn" (selector-type-no-unknown)
7:3 error Unexpected unknown type selector "spsn" (selector-type-no-unknown)

Because the last "spsn" is now ignored as we configured the rule to ignore all type selectors with the "foo" namespace. See this demo.

The ignoreTypes: [] option is for ignoring the bit after | regardless of the namespace

So, given CSS above and the following config:

{
  "rules": {
    "selector-type-no-unknown": [true, { "ignoreTypes": ["spsn"] } ]
  }
}

We get no violations as we configured the rule to ignore the "spsn" type selector. See this demo.

I think we can improve our error messaging around malformed regex. Presenting it as a parse error related to selectors is understandably confusing.

If a namespace prefix is not known in the file because there is a missing @namespace at-rule, then it should be considered a violation.

This is a great candidate for a new rule. It would be very similar to the existing no-unknown-animations rule, which warns if an undefined animation is used.

Rule blueprint:

  • Name: no-unknown-namespaces
  • Primary option: true
  • Secondary options: None, although possibly ignoreNamespaces: [] down the line
  • Autofixable: No
  • Messages: Unexpected unknown namespace "${namespace}"`
  • Description: "Disallow unknown namespaces."
  • Section: "Limit language features" -> "Namespaces"

Allowed:

@namespace foo url("http://www.foo.com");
foo|* {}

Disallowed:

foo|* {}

Does that look good to you? If so, I'll label this issue as a new rule and ready to implement.

jeddy3 avatar May 28 '21 11:05 jeddy3

First of all, I apologize for the late reply here. Thank you so much for your attention to this issue!

  1. Thanks for pointing out that I had malformed RegExp. Yes, better messaging around this rather than an opaque parseError would really helpful! :-)

  2. Regarding ignoreNamespaces vs ignoreType, thanks for the deeper explanation. One suggestion I have for the documentation, however, is that you should clearly point out that "*" matches the literal * namespace/type and that to match any namespace/type you need to use /.*/ (quoted as a String or not). This confusion arises because * is used in both CSS and RegExp syntax. Also note in the sample config below the ignoreTypes portion appears to not be considered since ignoreNamespaces matches all (as expected).

"selector-type-no-unknown": [ true, {
            "ignoreNamespaces": [ /.*/ ],
            "ignoreTypes": [ /.*/ ]
        }],
  1. There does still seem to be some subtlety around default namespaces.

First issue: ignoreNamespaces doesn't ignore the default when it's not HTML5.

Given this rule configuration:

"selector-type-no-unknown": [ true, {
            "ignoreNamespaces": [ /.*/ ]
        }],

In the following file, the default namespace is HTML5, but I've configured an @namespace at-rule to redefine the default namespace to http://www.foo.com.

@namespace url("http://www.foo.com");

* {}
span {}
spsn {}

Because I've configured the rule to ignore non-HTML5 namespaces and, by definition, all of these rules are non-HTML5 (because of changing the default namespace), there should be no error messages. But there is one for the spsn rule.

Second issue: ignoreTypes cannot be used properly.

Given this rule configuration:

"selector-type-no-unknown": [ true, {
            "ignoreTypes": [ "spsn" ]
        }],

and the same CSS file content above, I would expect an error for span because span is not in my ignoreTypes list, but I get no error.

I realize that I can ignore the entire default namespace by using "ignore": ["default-namespace"], but that is a blunt instrument that doesn't allow me to leverage the ignoreTypes setting.

Both of these issues point to improperly considering HTML5 tags even when HTML5 is not the default namespace.

  1. The new no-unknown-namespaces rule looks really great! I might suggest a corollary rule of no-unused-namespaces to go along with it that would lint for @namespace foo definitions where there are no foo| prefixes on any rules in the file. Please be aware that namespace prefixes can also be used inside of attribute selectors like so: *[foo|bar="fluffy"] for any of these rules.

Thanks again! This is really great stuff!

Cheers,

  • Bill

bedney avatar Jun 12 '21 17:06 bedney

I see 3 requests in one in this issue:

  • a proposed documentation update
  • that we support custom namespaces (hence load foreign code)
  • the creation of the no-unknown-namespaces and no-unused-namespaces rules (which are both also affected by #4088)

Ill close this issue instead of trying to refurbish it. Since we don't have any no-unused-* rules yet, you could start by creating a new issue for no-unknown-namespaces.

Mouvedia avatar Oct 09 '23 23:10 Mouvedia