Skip to content

Commit

Permalink
Fix spaceBeforeBrace reporting on multi-line selectors
Browse files Browse the repository at this point in the history 
Closes #231
  • Loading branch information
@jwilsson
jwilsson committed Jul 20, 2017
1 parent e254527 commit 431dac4
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 55 deletions.
21 changes: 7 additions & 14 deletions lib/linters/space_before_brace.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,8 @@ module.exports = {
throw new Error(`Invalid setting value for spaceBeforeBrace: ${ config.style }`);
}

/**
* If the node is a bodiless rule, or it's an import statement, or we
* are dealing with an atrule without a body, bail.
*/
if (node.empty || node.name === 'import' ||
// If the node is a bodiless rule or atrule, bail
if (node.empty ||
/**
* Hopefully node.empty will be implemented on AtRule nodes
* in the future: https://github.com/webschik/postcss-less/issues/55
Expand All @@ -34,17 +31,13 @@ module.exports = {
}

if (!styles[config.style].test(node.raws.between)) {
let column = node.source.start.column;

if (node.type === 'rule') {
column += node.selector.length;
} else if (node.type === 'atrule') {
column += node.name.length + node.raws.afterName.length + node.params.length + 1;
}
const position = node.positionBy({
word: '{'
});

return [{
column: column,
line: node.source.start.line,
column: position.column,
line: position.line,
message: this.message[config.style]
}];
}
Expand Down
70 changes: 29 additions & 41 deletions test/specs/linters/space_before_brace.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe('lesshint', function () {
it('should not allow one space when "style" is "no_space"', function () {
const source = '.foo {}';
const expected = [{
column: 5,
column: 6,
line: 1,
message: 'Opening curly brace should not be preceded by a space or new line.'
}];
Expand All @@ -48,7 +48,7 @@ describe('lesshint', function () {
it('should not allow multiple spaces when "style" is "no_space"', function () {
const source = '.foo {}';
const expected = [{
column: 5,
column: 7,
line: 1,
message: 'Opening curly brace should not be preceded by a space or new line.'
}];
Expand All @@ -67,8 +67,8 @@ describe('lesshint', function () {
it('should not allow one new line when "style" is "no_space"', function () {
const source = '.foo\n{}';
const expected = [{
column: 5,
line: 1,
column: 1,
line: 2,
message: 'Opening curly brace should not be preceded by a space or new line.'
}];

Expand All @@ -86,8 +86,8 @@ describe('lesshint', function () {
it('should not allow multiple new lines when "style" is "no_space"', function () {
const source = '.foo\n\n{}';
const expected = [{
column: 5,
line: 1,
column: 1,
line: 3,
message: 'Opening curly brace should not be preceded by a space or new line.'
}];

Expand Down Expand Up @@ -169,26 +169,7 @@ describe('lesshint', function () {
it('should not allow multiple spaces when "style" is "one_space"', function () {
const source = '.foo {}';
const expected = [{
column: 5,
line: 1,
message: 'Opening curly brace should be preceded by one space.'
}];

const options = {
style: 'one_space'
};

return spec.parse(source, function (ast) {
const result = spec.linter.lint(options, ast.root.first);

expect(result).to.deep.equal(expected);
});
});

it('should not allow multiple spaces when "style" is "one_space"', function () {
const source = '.foo {}';
const expected = [{
column: 5,
column: 7,
line: 1,
message: 'Opening curly brace should be preceded by one space.'
}];
Expand All @@ -207,8 +188,8 @@ describe('lesshint', function () {
it('should not allow new line when "style" is "one_space"', function () {
const source = '.foo, .bar\n{}';
const expected = [{
column: 11,
line: 1,
column: 1,
line: 2,
message: 'Opening curly brace should be preceded by one space.'
}];

Expand All @@ -226,7 +207,7 @@ describe('lesshint', function () {
it('should not allow tab when "style" is "one_space"', function () {
const source = '.foo, .bar\t{}';
const expected = [{
column: 11,
column: 12,
line: 1,
message: 'Opening curly brace should be preceded by one space.'
}];
Expand All @@ -245,7 +226,7 @@ describe('lesshint', function () {
it('should not allow multiple spaces when multiple simple selectors are used and "style" is "one_space"', function () {
const source = '.foo, .bar {}';
const expected = [{
column: 11,
column: 13,
line: 1,
message: 'Opening curly brace should be preceded by one space.'
}];
Expand Down Expand Up @@ -316,8 +297,8 @@ describe('lesshint', function () {
it('should not allow multiple new lines when "style" is "new_line"', function () {
const source = '.foo\n\n{}';
const expected = [{
column: 5,
line: 1,
column: 1,
line: 3,
message: 'Opening curly brace should be on its own line.'
}];

Expand All @@ -335,8 +316,8 @@ describe('lesshint', function () {
it('should not allow multiple new lines when a selector group is used and "style" is "new_line"', function () {
const source = '.foo, .bar\n\n{}';
const expected = [{
column: 11,
line: 1,
column: 1,
line: 3,
message: 'Opening curly brace should be on its own line.'
}];

Expand Down Expand Up @@ -421,8 +402,8 @@ describe('lesshint', function () {
});
});

it('should not check imports', function () {
const source = '@import \'lib/colors\';';
it('should ignore nodes without a following block', function () {
const source = '.foo();';
const options = {
style: 'one_space'
};
Expand All @@ -434,8 +415,8 @@ describe('lesshint', function () {
});
});

it('should ignore nodes without a following block', function () {
const source = '.foo();';
it('should ignore atrule nodes without a following block', function () {
const source = '@foo();';
const options = {
style: 'one_space'
};
Expand All @@ -447,16 +428,23 @@ describe('lesshint', function () {
});
});

it('should ignore atrule nodes without a following block', function () {
const source = '@foo();';
it('should report the correct line on multi-line selectors (#231)', function () {
const source = '.foo,\n.bar { color: red; }';

const expected = [{
column: 7,
line: 2,
message: 'Opening curly brace should be preceded by one space.'
}];

const options = {
style: 'one_space'
};

return spec.parse(source, function (ast) {
const result = spec.linter.lint(options, ast.root.first);

expect(result).to.be.undefined;
expect(result).to.deep.equal(expected);
});
});

Expand Down

0 comments on commit 431dac4

Please sign in to comment.