Fix false positives for objects in declaration-block-trailing-semicolon

[srawlins]

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

Fixes #4650

Is there anything in the PR that needs further explanation?

Yes; @jeddy3 suggests a solution in #4650 (comment) that did not work for me: isStandardSyntaxRule only accepts Rule nodes, but if used in declaration-block-trailing-semicolon, it would also be given AtRule nodes (like in the SCSS test case: a { @foo { color: pink } }). This complicates things further because isStandardSyntaxRule examines the selector of the Rule which it receives, and AtRule has no selector.

This simple solution seems to work for now.

[jeddy3]

Thanks!

I've requested two changes. One to the tests and another to the fix itself.

Yes; @jeddy3 suggests a solution in #4650 (comment) that did not work for me: isStandardSyntaxRule only accepts Rule nodes, but if used in declaration-block-trailing-semicolon, it would also be given AtRule nodes

I've been meaning to write up an issue about this. I realised shortly after writing that suggestion that the isStandardSyntax* utils should only be used when a syntax parses a non-standard construct as a standard one, e.g. postcss-scss will parse a SCSS map as a decl node.

I think we need a new set of utilities for when a syntax parses a standard construct correctly, but the punctuation of the construct is non-standard, e.g. postcss-css-in-js will correctly parse the object pair {{ color: "red" }} as a decl, but it has non-standard punctuation.

The first of theseisStandardPunctuation* utils can be isStandardPunctuationDeclarationList and would use the type === "object" conditional.

I'll create a new issue about this so I can flesh out the rationale behind it. I think we can merge this pull request with a type === "object" conditional directly in the rule.

[jeddy3]

Let's replace these two tests with a single one at the bottom of the file that tests the problematic syntax, e.g.:

testRule({
	ruleName,
	config: ['always'],
	syntax: 'css-in-js',
	fix: true,

	accept: [
		{
			code: 'const C = () => { return <a style={{ color: "red" }}></a> }',
			description: 'css-in-js object',
		},
	],

	reject: [
		{
			code: 'const C = styled.a`color: red`;',
			fixed: 'const C = styled.a`color: red;`;',
			description: 'css-in-js template literal',
			message: messages.expected,
			line: 1,
			column: 29,
		},
	],
});

So that we're consistent with the conventions:

You should:

• use standard CSS syntax by default, and only swap parsers when testing a specific piece of non-standard syntax

I think this will also highlight that template literals are incorrectly excluded by the proposed fix.

[jeddy3]

I've updated the snippet above to use the new testRule signature because we now dogfood our Jest preset for testing rules (#4758).

You'll need to rebase your branch off master when updating this pull request.

As an aside, as a member of the stylelint organisation you can, if you like, clone this repository directly rather than work on a fork. It can make collaboration easier.

[srawlins]

Ah, very nice. Thanks much!

[jeddy3]

I think we should use:

if (decl.parent.type === 'object') return;

As not to exclude template literals, e.g.:

const C = styled.a`color: red;`;

[srawlins]

Done

[jeddy3]

Thanks for making the changes, LGTM!

[m-allanson]

LGTM, thank you @srawlins.

[jeddy3]

Changelog entry:

• Fixed: declaration-block-trailing-semicolon false positives for CSS-in-JS object notation (#4749).