Add annotation-no-unknown

[mattxwang]

Which issue, if any, is this issue related to?

Closes #4508.

Is there anything in the PR that needs further explanation?

A late turnaround on this one! I think there's probably a few missing things before this is mergeable.

Possible questions with my implementation:

• how should I be checking for nonstandard syntax? ex just at the declaration level?
• should !important be case-insensitive? on one hand, syntax highlighters and the literal reading of the spec only allow for lowercase, but it seems like Chrome respects any capitalization.
• I've opted to include the ! in the secondary options; is that too clunky?
• should I add any testcases for nonstandard syntax?

[mattxwang]

Actually - going to mark this as draft; I think I've made an implementation mistake that the code coverage sneakily catches. I'll revisit and mark as ready when relevant!

[ybiquitous]

@mattxwang Thanks for creating the pull request! 👍🏼

Here are my answers to your questions in the description:

• how should I be checking for nonstandard syntax? ex just at the declaration level?
• should I add any testcases for nonstandard syntax?

If you would have clear test cases for non-standard syntaxes, it might be nice to check them to reduce potential false positives. But if you wouldn't have them or the rule implementation would be much more complex by adding such a check, it would be acceptable not to check them.

• should !important be case-insensitive? on one hand, syntax highlighters and the literal reading of the spec only allow for lowercase, but it seems like Chrome respects any capitalization.

It seems good that !important be case-sensitive. We have similar code already:

stylelint/lib/utils/getImportantPosition.js

Line 11 in 592b149

const pattern = /!\s*important\b/gi;

• I've opted to include the ! in the secondary options; is that too clunky?

I think it's unnecessary for consistency to include ! because the ignoreAtRules option of property-no-unknown doesn't @.

stylelint/lib/rules/property-no-unknown/README.md

Lines 130 to 138 in 592b149

	### `ignoreAtRules: ["/regex/", /regex/, "string"]`
	Ignores properties nested within specified at-rules.
	Given:
	```json
	["supports"]
	```

[jeddy3]

@mattxwang Thanks for making a start on this. Looking good so far.

I've requested some changes to make the docs, tests and rule consistent with existing rules.

Let's not add additional tests for non-standard syntax. Only if users report an issue, should we add that code.

I agree with @ybiquitous that it's unnecessary to include !.

[mattxwang]

Thank you @ybiquitous and @jeddy3 for the in-depth feedback, I appreciate both of you and your patience as I still getting familiar with the codebase. I definitely need to do a better job of following the conventions doc!

I think I've resolved all of the outstanding changes, and I've removed the need to use ! (not sure if the way that I've done it is hacky).

One note:

It seems good that !important be case-sensitive.

With my understanding of the PostCSS parser, the top-level declaration doesn't differentiate between different capitalizations of !important. There is a different value in the raws field that I could use to check if the annotation is a lowercase !important. With that in mind, should I add a check for that in the rule (making the rule case-sensitive)? Or, should I leave it as is (case-insensitive)?

$ node -e 'require("postcss").parse("a { color: red !IMPORTANT }").walkDecls(decl => console.log(decl))'
<ref *1> Declaration {
  raws: { before: ' ', between: ': ', important: ' !IMPORTANT' },
  type: 'decl',
  parent: Rule {
    raws: { before: '', between: ' ', semicolon: false, after: ' ' },
    type: 'rule',
    nodes: [ [Circular *1] ],
    parent: Root {
      raws: [Object],
      type: 'root',
      nodes: [Array],
      source: [Object],
      lastEach: 1,
      indexes: [Object],
      [Symbol(isClean)]: false,
      [Symbol(my)]: true
    },
    source: { start: [Object], input: [Input], end: [Object] },
    selector: 'a',
    lastEach: 1,
    indexes: { '1': 0 },
    [Symbol(isClean)]: false,
    [Symbol(my)]: true
  },
  source: {
    start: { offset: 4, line: 1, column: 5 },
    input: Input {
      css: 'a { color: red !IMPORTANT }',
      hasBOM: false,
      id: '<input css HyYU1z>',
      [Symbol(fromOffsetCache)]: [Array]
    },
    end: { offset: 24, line: 1, column: 25 }
  },
  prop: 'color',
  important: true,
  value: 'red',
  [Symbol(isClean)]: false,
  [Symbol(my)]: true
}

[ybiquitous]

With my understanding of the PostCSS parser, the top-level declaration doesn't differentiate between different capitalizations of !important.

Sorry, let's follow the current PostCSS parser behavior!

[ybiquitous]

@mattxwang Thanks for the quick fix! I've left a suggestion comment, but this looks mostly good to me. 👍🏼

[ybiquitous]

Thank you. LGTM 👍🏼

[jeddy3]

Thanks for making the changes. LGTM, thank you!

[jeddy3]

• Added: annotation-no-unknown rule (#6155).