Ignore ERB template tags in selectors

[hudochenkov]

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

Fixes #4489.

Is there anything in the PR that needs further explanation?

Problem lies in many places. We are passing selector <% COLORS.each do |color| %>\na to postcss-selector-parser, and the later falls into endless loop. Which is understandable. This case might be fixed in postcss-selector-parser, but it won't happen any time soon. Currently, we're using postcss-selector-parser@3.1.0, while the latest version is 6.0.2. We can't upgrade, because there are still some issues in new version of a parser which needs to be resolved in a parser.

So, the only choice we have is to fix problem in stylelint. My solution is not universal, but if will fix the problem. It might create false negatives for isStandardSyntaxSelector, when selector like [data-something="<%"] is used. I think it's very unlikely to have a selector with <% or %> in it.

---

There is also a question when to release the fix. The next version of stylelint should be major, because we have some breaking changes in master. On the other hand we want to drop Node.js 8 from January 1. Which will require to release new major version. I don't want to release two major versions, as it's bad UX for our users have to update two major versions.

If this fix has to be release as soon as possible, we can create a new branch from latest release tag and cherry pick this PR there, and release patch version from that branch.

[alexander-akait]

Good job!

[hudochenkov]

@stylelint/core What we decided about release? (second part of my top message)

[alexander-akait]

If this fix has to be release as soon as possible, we can create a new branch from latest release tag and cherry pick this PR there, and release patch version from that branch.

Sounds good for me

[hudochenkov]

• Fixed: string-no-newline memory leak for ERB templates (#4491)

[vilchik-elena]

@hudochenkov thanks for the quick fix!
but looks like there is [at least] one more place to fix:

stylelint/lib/rules/no-descending-specificity/index.js

Lines 79 to 81 in 42f9bf2

	if (!isStandardSyntaxSelector(resolvedSelector)) {
	return;
	}

As I can reproduce the same problem with no-descending-specificity rule

As far as I understand this condition should be checked before trying to parse selector.

[hudochenkov]

@vilchik-elena make sense! Can you send a PR, please?

[vilchik-elena]

@hudochenkov I tried to do that, but I fail to reproduce with unit test :( (while I do with CLI with the same .erb file and { "rules": { "no-descending-specificity": true } } as config)

Also I checked that that ~40 usages of parseSelector. I'm not sure if all of them should be preceeded with isStandardSyntaxSelector check.

[hudochenkov]

@vilchik-elena I believe this test will be ok:

testRule(rule, {
	ruleName,
	config: [true],
	syntax: 'html',
	skipBasicChecks: true,

	accept: [
		{
			code: `<style>
				.highlight {
					padding: 2px;
				}
				<%- SEVERITY_COLORS.each do |severity, color| %>
				.severity.<%= severity %> {
					color: <%= color %>;
				}
				.highlight.<%= severity %> {
					background-color: <%= color.fade_out(0.4) %>;
				}
				</style>`,
			description: 'ERB templates are ignored',
		},
	],
});

Also I checked that that ~40 usages of parseSelector. I'm not sure if all of them should be preceeded with isStandardSyntaxSelector check.

Let's fix rules which confirmed has issues.

Maybe we can change parseSelector.js and check for standard syntax there.

[vilchik-elena]

Ok, I finally found out what's the problem. I was trying to create patch from master branch. And the problem of no-descending-specificity does not exist on master, yet it does exist on fresh release 12.0.1 as for some reason (you know better) commit 0f04539 is not part of 12.0.1.

So what should be done next according to you @hudochenkov ?

[hudochenkov]

Good catch :) In commit you found we added isStandardSyntaxSelector check to isStandardSyntaxRule, so now isStandardSyntaxSelector is not needed in no-descending-specificity.

stylelint/lib/rules/no-descending-specificity/index.js

Lines 53 to 56 in 42f9bf2

	// Ignore nested property `foo: {};`
	if (!isStandardSyntaxRule(rule)) {
	return;
	}

[vilchik-elena]

@hudochenkov so when this will be part of release?

[hudochenkov]

We plan to release in the beginning of January.