Skip to content

Commit

Permalink
Fix false positives for parametric mixins in selector-class-pattern (#…
Browse files Browse the repository at this point in the history
…5378)

* Fix processing of less parametric mixins

* Remove redundant conditionals

* Fix not-rule test cases

* Replace checks for selector

* Add tests for selector
  • Loading branch information
immitsu committed Jul 26, 2021
1 parent 04be2a2 commit 56e2453
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 57 deletions.
23 changes: 7 additions & 16 deletions lib/utils/__tests__/isStandardSyntaxRule.test.js
Expand Up @@ -35,11 +35,11 @@ describe('isStandardSyntaxRule', () => {
it('custom-property-set', () => {
expect(isStandardSyntaxRule(node('--custom-property-set: {}'))).toBeFalsy();
});
it('Scss nested properties', () => {
it('scss nested properties', () => {
expect(isStandardSyntaxRule(node('foo: {};'))).toBeFalsy();
});
it('called Less class parametric mixin', () => {
expect(isStandardSyntaxRule(lessNode('.mixin-name(@var);'))).toBeFalsy();
it('less class parametric mixin', () => {
expect(isStandardSyntaxRule(lessNode('.mixin-name(@var) {}'))).toBeFalsy();
});
it('non-outputting parametric Less class mixin definition', () => {
expect(isStandardSyntaxRule(lessNode('.mixin-name() {}'))).toBeFalsy();
Expand All @@ -53,32 +53,23 @@ describe('isStandardSyntaxRule', () => {
it('non-outputting Less ID mixin definition', () => {
expect(isStandardSyntaxRule(lessNode('#mixin-name() {}'))).toBeFalsy();
});
it('called Less ID mixin', () => {
expect(isStandardSyntaxRule(lessNode('#mixin-name;'))).toBeFalsy();
});
it('called namespaced Less mixin (child)', () => {
expect(isStandardSyntaxRule(lessNode('#namespace > .mixin-name;'))).toBeFalsy();
});
it('called namespaced Less mixin (descendant)', () => {
expect(isStandardSyntaxRule(lessNode('#namespace .mixin-name;'))).toBeFalsy();
});
it('called namespaced Less mixin (compound)', () => {
expect(isStandardSyntaxRule(lessNode('#namespace.mixin-name;'))).toBeFalsy();
});
it('less mixin', () => {
expect(
isStandardSyntaxRule(lessNode('.box-shadow(@style, @c) when (iscolor(@c)) {}')),
).toBeFalsy();
});
it('less extend', () => {
expect(isStandardSyntaxRule(lessNode('&:extend(.inline);'))).toBeFalsy();
expect(isStandardSyntaxRule(lessNode('&:extend(.inline) {}'))).toBeFalsy();
});
it('less detached rulesets', () => {
expect(isStandardSyntaxRule(lessNode('@foo: {};'))).toBeFalsy();
});
it('less guarded namespaces', () => {
expect(isStandardSyntaxRule(lessNode('#namespace when (@mode=huge) {}'))).toBeFalsy();
});
it('less parametric mixins', () => {
expect(isStandardSyntaxRule(lessNode('.mixin (@variable: 5) {}'))).toBeFalsy();
});
it('mixin guards', () => {
expect(
isStandardSyntaxRule(lessNode('.mixin (@variable) when (@variable = 10px) {}')),
Expand Down
15 changes: 13 additions & 2 deletions lib/utils/__tests__/isStandardSyntaxSelector.test.js
Expand Up @@ -42,6 +42,13 @@ describe('isStandardSyntaxSelector', () => {
it('SCSS interpolation (pseudo)', () => {
expect(isStandardSyntaxSelector(':n-#{$n}')).toBeFalsy();
});
it('SCSS placeholder', () => {
expect(isStandardSyntaxSelector('%foo')).toBeFalsy();
});
it('SCSS nested properties', () => {
expect(isStandardSyntaxSelector('.a { .b }')).toBeFalsy();
expect(isStandardSyntaxSelector('.a { &:hover }')).toBeFalsy();
});
it('Less interpolation', () => {
expect(isStandardSyntaxSelector('.n-@{n}')).toBeFalsy();
});
Expand All @@ -54,8 +61,8 @@ describe('isStandardSyntaxSelector', () => {
it('Less extend inside ruleset', () => {
expect(isStandardSyntaxSelector('a { &:extend(.a all) }')).toBeFalsy();
});
it('SCSS placeholder', () => {
expect(isStandardSyntaxSelector('%foo')).toBeFalsy();
it('Less mixin', () => {
expect(isStandardSyntaxSelector('.foo()')).toBeFalsy();
});
it('Less mixin with resolved nested selectors', () => {
expect(isStandardSyntaxSelector('.foo().bar')).toBeFalsy();
Expand All @@ -69,6 +76,10 @@ describe('isStandardSyntaxSelector', () => {
expect(isStandardSyntaxSelector('.foo()[bar]')).toBeFalsy();
expect(isStandardSyntaxSelector(".foo()[bar='baz']")).toBeFalsy();
});
it('Less parametric mixin', () => {
expect(isStandardSyntaxSelector('.foo(@a)')).toBeFalsy();
expect(isStandardSyntaxSelector('.foo(@a: 5px)')).toBeFalsy();
});

it('ERB templates', () => {
// E. g. like in https://github.com/stylelint/stylelint/issues/4489
Expand Down
40 changes: 1 addition & 39 deletions lib/utils/isStandardSyntaxRule.js
Expand Up @@ -13,50 +13,12 @@ module.exports = function (rule) {
return false;
}

const raws = rule.raws;

// Get full selector
const selector = (raws.selector && raws.selector.raw) || rule.selector;

if (!isStandardSyntaxSelector(rule.selector)) {
return false;
}

// Called Less mixin (e.g. a { .mixin() })
// @ts-expect-error -- TODO TYPES support LESS and SASS types somehow
if (rule.mixin) {
return false;
}

// Less detached rulesets
if (selector.startsWith('@') && selector.endsWith(':')) {
return false;
}

// Ignore Less &:extend rule
if ('extend' in rule && rule.extend) {
return false;
}

// Ignore mixin or &:extend rule
// https://github.com/shellscape/postcss-less/blob/master/lib/less-parser.js#L52
// @ts-expect-error -- TODO TYPES support LESS and SASS types somehow
if (rule.params && rule.params[0]) {
return false;
}

// Non-outputting Less mixin definition (e.g. .mixin() {})
if (selector.endsWith(')') && !selector.includes(':')) {
return false;
}

// Less guards
if (/when\s+(not\s+)*\(/.test(selector)) {
return false;
}

// Ignore Scss nested properties
if (selector.endsWith(':')) {
if (!isStandardSyntaxSelector(rule.selector)) {
return false;
}

Expand Down
15 changes: 15 additions & 0 deletions lib/utils/isStandardSyntaxSelector.js
Expand Up @@ -19,6 +19,11 @@ module.exports = function (selector) {
return false;
}

// SCSS nested properties
if (selector.endsWith(':')) {
return false;
}

// Less :extend()
if (/:extend(\(.*?\))?/.test(selector)) {
return false;
Expand All @@ -29,6 +34,16 @@ module.exports = function (selector) {
return false;
}

// Less non-outputting mixin definition (e.g. .mixin() {})
if (selector.endsWith(')') && !selector.includes(':')) {
return false;
}

// Less Parametric mixins (e.g. .mixin(@variable: x) {})
if (/\(@.*\)$/.test(selector)) {
return false;
}

// ERB template tags
if (selector.includes('<%') || selector.includes('%>')) {
return false;
Expand Down

0 comments on commit 56e2453

Please sign in to comment.