Skip to content

Commit

Permalink
Fix false positives for trailing combinator in selector-combinator-sp…
Browse files Browse the repository at this point in the history
…ace-after (#4878)
  • Loading branch information
mattxwang committed Aug 28, 2020
1 parent e2da124 commit 2c4d77f
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 53 deletions.
56 changes: 4 additions & 52 deletions lib/rules/selector-combinator-space-after/__tests__/index.js
Expand Up @@ -554,33 +554,9 @@ testRule({
code: 'a { > /*comment*/a, > /*comment*/.b{} }',
description: 'scss nesting and comment',
},
],

reject: [
{
code: 'a { >/*comment*/a {} }',
fixed: 'a { > /*comment*/a {} }',
description: 'scss nesting and comment',
message: messages.expectedAfter('>'),
line: 1,
column: 5,
},
{
code: 'a { >/*comment*/a, >/*comment*/.b{} }',
fixed: 'a { > /*comment*/a, > /*comment*/.b{} }',
description: 'scss nesting, comment and comma',
warnings: [
{
message: messages.expectedAfter('>'),
line: 1,
column: 5,
},
{
message: messages.expectedAfter('>'),
line: 1,
column: 20,
},
],
code: 'a ~, b {}',
description: 'scss trailing combinator',
},
],
});
Expand All @@ -596,33 +572,9 @@ testRule({
code: 'a { >/*comment*/a, >/*comment*/.b {} }',
description: 'scss nesting and comment',
},
],

reject: [
{
code: 'a { > /*comment*/a {} }',
fixed: 'a { >/*comment*/a {} }',
description: 'scss nesting and comment',
message: messages.rejectedAfter('>'),
line: 1,
column: 5,
},
{
code: 'a { > /*comment*/a, > /*comment*/.b {} }',
fixed: 'a { >/*comment*/a, >/*comment*/.b {} }',
description: 'scss nesting, comment and comma',
warnings: [
{
message: messages.rejectedAfter('>'),
line: 1,
column: 5,
},
{
message: messages.rejectedAfter('>'),
line: 1,
column: 21,
},
],
code: 'a ~, b {}',
description: 'scss trailing combinator',
},
],
});
6 changes: 6 additions & 0 deletions lib/rules/selectorCombinatorSpaceChecker.js
Expand Up @@ -2,6 +2,7 @@

'use strict';

const isStandardSyntaxCombinator = require('../utils/isStandardSyntaxCombinator');
const isStandardSyntaxRule = require('../utils/isStandardSyntaxRule');
const parseSelector = require('../utils/parseSelector');
const report = require('../utils/report');
Expand All @@ -19,6 +20,11 @@ module.exports = function (opts) {

const fixedSelector = parseSelector(selector, opts.result, rule, (selectorTree) => {
selectorTree.walkCombinators((node) => {
// Ignore non-standard combinators
if (!isStandardSyntaxCombinator(node)) {
return;
}

// Ignore spaced descendant combinator
if (/\s/.test(node.value)) {
return;
Expand Down
20 changes: 20 additions & 0 deletions lib/utils/__tests__/isStandardSyntaxCombinator.test.js
Expand Up @@ -60,6 +60,26 @@ describe('isStandardSyntaxCombinator', () => {
expect(isStandardSyntaxCombinator(func)).toBeFalsy();
});
});
it('last node is combinator', () => {
rules('a ~, {}', (func) => {
expect(isStandardSyntaxCombinator(func)).toBeFalsy();
});
});
it('first node is combinator', () => {
rules('~ b {}', (func) => {
expect(isStandardSyntaxCombinator(func)).toBeFalsy();
});
});
it('last node (in first container) is combinator', () => {
rules('a ~, b {}', (func) => {
expect(isStandardSyntaxCombinator(func)).toBeFalsy();
});
});
it('first node (in second container) is combinator', () => {
rules('a, ~ b {}', (func) => {
expect(isStandardSyntaxCombinator(func)).toBeFalsy();
});
});
});

function rules(css, cb) {
Expand Down
24 changes: 23 additions & 1 deletion lib/utils/isStandardSyntaxCombinator.js
Expand Up @@ -7,6 +7,28 @@
* @return {boolean} If `true`, the combinator is standard
*/
module.exports = function (node) {
// if it's not a combinator, then it's not a standard combinator
if (node.type !== 'combinator') {
return false;
}

// Ignore reference combinators like `/deep/`
return node.type === 'combinator' && !node.value.startsWith('/') && !node.value.endsWith('/');
if (node.value.startsWith('/') || node.value.endsWith('/')) {
return false;
}

// ignore the combinators that are the first or last node in their container
if (node.parent !== undefined && node.parent !== null) {
let parent = node.parent;

if (node === parent.first) {
return false;
}

if (node === parent.last) {
return false;
}
}

return true;
};

0 comments on commit 2c4d77f

Please sign in to comment.