From ecdcc983d8eacd69370b3bef51f010a4a94d2d05 Mon Sep 17 00:00:00 2001 From: Dan Purdy Date: Thu, 25 Aug 2016 02:11:56 +0100 Subject: [PATCH 01/67] :mag: Add max-line-length rule #833 --- docs/rules/max-line-length.md | 20 +++++++++++ lib/config/sass-lint.yml | 1 + lib/rules/max-line-length.js | 32 +++++++++++++++++ tests/rules/max-line-length.js | 63 +++++++++++++++++++++++++++++++++ tests/sass/max-line-length.sass | 20 +++++++++++ tests/sass/max-line-length.scss | 22 ++++++++++++ 6 files changed, 158 insertions(+) create mode 100644 docs/rules/max-line-length.md create mode 100644 lib/rules/max-line-length.js create mode 100644 tests/rules/max-line-length.js create mode 100644 tests/sass/max-line-length.sass create mode 100644 tests/sass/max-line-length.scss diff --git a/docs/rules/max-line-length.md b/docs/rules/max-line-length.md new file mode 100644 index 00000000..89c58bdf --- /dev/null +++ b/docs/rules/max-line-length.md @@ -0,0 +1,20 @@ +# Max Line Length + +Rule `max-line-length` will enforce that lines do not exceed a max length / limit. + +## Examples + +When enabled, the following are disallowed: + +```scss +.really--long--class-name--that-unfortunately--isnt--very--succint--and-looks-stupid { + color: red; +} + +// ============================================================================== +// +// This comment is too long clearly, we should probably make sure we have a rule to +// determine when we breach this length +// +// ============================================================================== +``` diff --git a/lib/config/sass-lint.yml b/lib/config/sass-lint.yml index 1b3ed511..6fc22342 100644 --- a/lib/config/sass-lint.yml +++ b/lib/config/sass-lint.yml @@ -67,6 +67,7 @@ rules: hex-notation: 1 indentation: 1 leading-zero: 1 + max-line-length: 0 nesting-depth: 1 property-sort-order: 1 pseudo-element: 1 diff --git a/lib/rules/max-line-length.js b/lib/rules/max-line-length.js new file mode 100644 index 00000000..2c447294 --- /dev/null +++ b/lib/rules/max-line-length.js @@ -0,0 +1,32 @@ +'use strict'; + +var helpers = require('../helpers'); + +module.exports = { + 'name': 'max-line-length', + 'defaults': { + length: 80 + }, + 'detect': function (ast, parser) { + var result = []; + + ast.traverseByType('space', function (space) { + var lineLength = 0; + if (helpers.hasEOL(space.content)) { + lineLength = space.start.column - 1; + } + + if (lineLength > parser.options.length) { + result = helpers.addUnique(result, { + 'ruleId': parser.rule.name, + 'severity': parser.severity, + 'line': space.start.line, + 'column': 0, + 'message': 'line ' + space.start.line + ' exceeds the maximum line length of ' + parser.options.length + }); + } + }); + + return result; + } +}; diff --git a/tests/rules/max-line-length.js b/tests/rules/max-line-length.js new file mode 100644 index 00000000..0109174e --- /dev/null +++ b/tests/rules/max-line-length.js @@ -0,0 +1,63 @@ +'use strict'; + +var lint = require('./_lint'); + +////////////////////////////// +// SCSS syntax tests +////////////////////////////// +describe('max-line-length - scss', function () { + var file = lint.file('max-line-length.scss'); + + it('enforce [default]', function (done) { + lint.test(file, { + 'max-line-length': 1 + }, function (data) { + lint.assert.equal(5, data.warningCount); + done(); + }); + }); + + it('enforce [length: 79]', function (done) { + lint.test(file, { + 'max-line-length': [ + 1, + { + length: 79 + } + ] + }, function (data) { + lint.assert.equal(8, data.warningCount); + done(); + }); + }); +}); + +////////////////////////////// +// Sass syntax tests +////////////////////////////// +describe('max-line-length - sass', function () { + var file = lint.file('max-line-length.sass'); + + it('enforce', function (done) { + lint.test(file, { + 'max-line-length': 1 + }, function (data) { + lint.assert.equal(5, data.warningCount); + done(); + }); + }); + + it('enforce [length: 79]', function (done) { + lint.test(file, { + 'max-line-length': [ + 1, + { + length: 79 + } + ] + }, function (data) { + lint.assert.equal(8, data.warningCount); + done(); + }); + }); +}); diff --git a/tests/sass/max-line-length.sass b/tests/sass/max-line-length.sass new file mode 100644 index 00000000..a753bb7f --- /dev/null +++ b/tests/sass/max-line-length.sass @@ -0,0 +1,20 @@ +.really--long--class-name--that-unfortunately--isnt--very--succint--and-looks-stupid + color: red + +@function($aReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyLongVariableName) + @return 'test' + +// ============================================================================== +// +// This comment is too long clearly, we should probably make sure we have a rule to +// determine when we breach this length +// +// ============================================================================== + + +// ============================================================================= +// +// This comment comment on the other hand should be the perfect length, unless a +// user decides to make their max line length === 79! +// +// ============================================================================= diff --git a/tests/sass/max-line-length.scss b/tests/sass/max-line-length.scss new file mode 100644 index 00000000..07021d26 --- /dev/null +++ b/tests/sass/max-line-length.scss @@ -0,0 +1,22 @@ +.really--long--class-name--that-unfortunately--isnt--very--succint--and-looks-stupid { + color: red; +} + +@function($aReallyReallyReallyReallyReallyReallyReallyReallyReallyReallyLongVariableName) { + @return 'test'; +} + +// ============================================================================== +// +// This comment is too long clearly, we should probably make sure we have a rule to +// determine when we breach this length +// +// ============================================================================== + + +// ============================================================================= +// +// This comment comment on the other hand should be the perfect length, unless a +// user decides to make their max line length === 79! +// +// ============================================================================= From 983696033bdf2fbf15657458691ae96a73bbe936 Mon Sep 17 00:00:00 2001 From: Dan Purdy Date: Thu, 25 Aug 2016 02:18:08 +0100 Subject: [PATCH 02/67] :memo: Add missing options to max-line-length docs --- docs/rules/max-line-length.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/rules/max-line-length.md b/docs/rules/max-line-length.md index 89c58bdf..9e0b3aad 100644 --- a/docs/rules/max-line-length.md +++ b/docs/rules/max-line-length.md @@ -2,6 +2,10 @@ Rule `max-line-length` will enforce that lines do not exceed a max length / limit. +## Options + +* `length`: `number`, (defaults to 80) + ## Examples When enabled, the following are disallowed: From a1200022900dfde1e6266ad2a14431e24a6230d2 Mon Sep 17 00:00:00 2001 From: Dan Wasilewski Date: Wed, 24 Aug 2016 18:28:56 -0400 Subject: [PATCH 03/67] issue-813 -- Adds enforce-domains flag to no-url-protocols rule --- lib/rules/no-url-protocols.js | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/lib/rules/no-url-protocols.js b/lib/rules/no-url-protocols.js index 389b4e7a..d3b43bb4 100644 --- a/lib/rules/no-url-protocols.js +++ b/lib/rules/no-url-protocols.js @@ -2,7 +2,8 @@ var helpers = require('../helpers'); -var isUrlRegex = /^(https?:)?\/\//; +var isUrlRegex = /^(https?:)?\/\//, + noProtocolRegex = /^(https?:)\/\//; var stripQuotes = function (str) { return str.substring(1, str.length - 1); @@ -10,16 +11,20 @@ var stripQuotes = function (str) { module.exports = { 'name': 'no-url-protocols', - 'defaults': {}, + 'defaults': { + 'enforce-domains' : true + }, 'detect': function (ast, parser) { var result = []; ast.traverseByType('uri', function (uri) { uri.traverse(function (item) { if (item.is('string')) { - var stripped = stripQuotes(item.content); + var stripped = stripQuotes(item.content), + regexSelector = parser.options['enforce-domains'] ? + isUrlRegex : noProtocolRegex; - if (stripped.match(isUrlRegex)) { + if (stripped.match(regexSelector)) { result = helpers.addUnique(result, { 'ruleId': parser.rule.name, 'severity': parser.severity, From cf9472ae1cefa32efbb9ab62306eff985359d4f1 Mon Sep 17 00:00:00 2001 From: Dan Wasilewski Date: Thu, 25 Aug 2016 11:09:52 -0400 Subject: [PATCH 04/67] issue-813 -- Adds no-domains rule --- lib/config/sass-lint.yml | 1 + lib/rules/no-domains.js | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 lib/rules/no-domains.js diff --git a/lib/config/sass-lint.yml b/lib/config/sass-lint.yml index 1b3ed511..da93ea4a 100644 --- a/lib/config/sass-lint.yml +++ b/lib/config/sass-lint.yml @@ -25,6 +25,7 @@ rules: no-css-comments: 1 no-debug: 1 no-disallowed-properties: 0 + no-domains: 1 no-duplicate-properties: 1 no-empty-rulesets: 1 no-extends: 0 diff --git a/lib/rules/no-domains.js b/lib/rules/no-domains.js new file mode 100644 index 00000000..d769ab72 --- /dev/null +++ b/lib/rules/no-domains.js @@ -0,0 +1,37 @@ +'use strict'; + +var helpers = require('../helpers'); +var url = require('url'); + +var stripQuotes = function (str) { + return str.substring(1, str.length - 1); +}; + +module.exports = { + 'name': 'no-domains', + 'defaults': {}, + 'detect': function (ast, parser) { + var result = []; + + ast.traverseByType('uri', function (uri) { + uri.traverse(function (item) { + if (item.is('string')) { + var stripped = stripQuotes(item.content), + parsedUrl = url.parse(stripped, false, true); + + if (parsedUrl.host) { + result = helpers.addUnique(result, { + 'ruleId': parser.rule.name, + 'severity': parser.severity, + 'line': item.end.line, + 'column': item.end.column, + 'message': 'Domains in URLs are disallowed' + }); + } + } + }); + }); + + return result; + } +}; From d506b26cf2c78a492ff9dc53c2bc7194bc927321 Mon Sep 17 00:00:00 2001 From: Dan Wasilewski Date: Thu, 25 Aug 2016 11:26:15 -0400 Subject: [PATCH 05/67] issue-813 -- DRYs up helper functions. Adds conditional messaging to no-protocols. --- lib/rules/no-domains.js | 10 +++------- lib/rules/no-url-protocols.js | 13 ++++++------- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/lib/rules/no-domains.js b/lib/rules/no-domains.js index d769ab72..e392c053 100644 --- a/lib/rules/no-domains.js +++ b/lib/rules/no-domains.js @@ -1,11 +1,7 @@ 'use strict'; -var helpers = require('../helpers'); -var url = require('url'); - -var stripQuotes = function (str) { - return str.substring(1, str.length - 1); -}; +var helpers = require('../helpers'), + url = require('url'); module.exports = { 'name': 'no-domains', @@ -16,7 +12,7 @@ module.exports = { ast.traverseByType('uri', function (uri) { uri.traverse(function (item) { if (item.is('string')) { - var stripped = stripQuotes(item.content), + var stripped = helpers.stripQuotes(item.content), parsedUrl = url.parse(stripped, false, true); if (parsedUrl.host) { diff --git a/lib/rules/no-url-protocols.js b/lib/rules/no-url-protocols.js index d3b43bb4..cea5558d 100644 --- a/lib/rules/no-url-protocols.js +++ b/lib/rules/no-url-protocols.js @@ -5,10 +5,6 @@ var helpers = require('../helpers'); var isUrlRegex = /^(https?:)?\/\//, noProtocolRegex = /^(https?:)\/\//; -var stripQuotes = function (str) { - return str.substring(1, str.length - 1); -}; - module.exports = { 'name': 'no-url-protocols', 'defaults': { @@ -20,9 +16,12 @@ module.exports = { ast.traverseByType('uri', function (uri) { uri.traverse(function (item) { if (item.is('string')) { - var stripped = stripQuotes(item.content), + var stripped = helpers.stripQuotes(item.content), regexSelector = parser.options['enforce-domains'] ? - isUrlRegex : noProtocolRegex; + isUrlRegex : noProtocolRegex, + message = parser.options['enforce-domains'] ? + "Protocols and domains in URLs are disallowed" : + "Protocols in URLS are disallowed"; if (stripped.match(regexSelector)) { result = helpers.addUnique(result, { @@ -30,7 +29,7 @@ module.exports = { 'severity': parser.severity, 'line': item.end.line, 'column': item.end.column, - 'message': 'Protocols and domains in URLs are disallowed' + 'message': message }); } } From 75ac1f076950c5884ba8dc48aed9d54a0dc73a1b Mon Sep 17 00:00:00 2001 From: Dan Wasilewski Date: Thu, 25 Aug 2016 12:32:33 -0400 Subject: [PATCH 06/67] issue-813 -- Adds unit tests --- lib/rules/no-domains.js | 2 +- lib/rules/no-url-protocols.js | 6 +++--- tests/rules/no-domains.js | 35 +++++++++++++++++++++++++++++++++ tests/rules/no-url-protocols.js | 29 +++++++++++++++++++++++++++ tests/sass/no-domains.sass | 25 +++++++++++++++++++++++ tests/sass/no-domains.scss | 27 +++++++++++++++++++++++++ 6 files changed, 120 insertions(+), 4 deletions(-) create mode 100644 tests/rules/no-domains.js create mode 100644 tests/sass/no-domains.sass create mode 100644 tests/sass/no-domains.scss diff --git a/lib/rules/no-domains.js b/lib/rules/no-domains.js index e392c053..213e7b24 100644 --- a/lib/rules/no-domains.js +++ b/lib/rules/no-domains.js @@ -15,7 +15,7 @@ module.exports = { var stripped = helpers.stripQuotes(item.content), parsedUrl = url.parse(stripped, false, true); - if (parsedUrl.host) { + if (parsedUrl.host && parsedUrl.protocol !== 'data:') { result = helpers.addUnique(result, { 'ruleId': parser.rule.name, 'severity': parser.severity, diff --git a/lib/rules/no-url-protocols.js b/lib/rules/no-url-protocols.js index cea5558d..2193b1b2 100644 --- a/lib/rules/no-url-protocols.js +++ b/lib/rules/no-url-protocols.js @@ -8,7 +8,7 @@ var isUrlRegex = /^(https?:)?\/\//, module.exports = { 'name': 'no-url-protocols', 'defaults': { - 'enforce-domains' : true + 'enforce-domains': true }, 'detect': function (ast, parser) { var result = []; @@ -20,8 +20,8 @@ module.exports = { regexSelector = parser.options['enforce-domains'] ? isUrlRegex : noProtocolRegex, message = parser.options['enforce-domains'] ? - "Protocols and domains in URLs are disallowed" : - "Protocols in URLS are disallowed"; + 'Protocols and domains in URLs are disallowed' : + 'Protocols in URLS are disallowed'; if (stripped.match(regexSelector)) { result = helpers.addUnique(result, { diff --git a/tests/rules/no-domains.js b/tests/rules/no-domains.js new file mode 100644 index 00000000..39973fbf --- /dev/null +++ b/tests/rules/no-domains.js @@ -0,0 +1,35 @@ +'use strict'; + +var lint = require('./_lint'); + +////////////////////////////// +// SCSS syntax tests +////////////////////////////// +describe('no domains - scss', function () { + var file = lint.file('no-domains.scss'); + + it('enforce', function (done) { + lint.test(file, { + 'no-domains': 1 + }, function (data) { + lint.assert.equal(3, data.warningCount); + done(); + }); + }); +}); + +////////////////////////////// +// Sass syntax tests +////////////////////////////// +describe('no domains - sass', function () { + var file = lint.file('no-domains.sass'); + + it('enforce', function (done) { + lint.test(file, { + 'no-domains': 1 + }, function (data) { + lint.assert.equal(3, data.warningCount); + done(); + }); + }); +}); diff --git a/tests/rules/no-url-protocols.js b/tests/rules/no-url-protocols.js index 87b5422a..17ca8253 100644 --- a/tests/rules/no-url-protocols.js +++ b/tests/rules/no-url-protocols.js @@ -16,8 +16,23 @@ describe('no url protocols - scss', function () { done(); }); }); + + it('allow protocol-relative urls [enforce-domains: false]', function (done) { + lint.test(file, { + 'no-url-protocols': [ + 1, + { + 'enforce-domains': false + } + ] + }, function (data) { + lint.assert.equal(2, data.warningCount); + done(); + }); + }); }); + ////////////////////////////// // Sass syntax tests ////////////////////////////// @@ -32,4 +47,18 @@ describe('no url protocols - sass', function () { done(); }); }); + + it('allow protocol-relative urls [enforce-domains: false]', function (done) { + lint.test(file, { + 'no-url-protocols': [ + 1, + { + 'enforce-domains': false + } + ] + }, function (data) { + lint.assert.equal(2, data.warningCount); + done(); + }); + }); }); diff --git a/tests/sass/no-domains.sass b/tests/sass/no-domains.sass new file mode 100644 index 00000000..21a17035 --- /dev/null +++ b/tests/sass/no-domains.sass @@ -0,0 +1,25 @@ +.foo + background-image: url('https://foo.com/img/bar.png') + + +.foo + background-image: url('http://foo.com/img/bar.png') + + +.foo + background-image: url('//foo.com/img/bar.png') + + +.foo + background-image: url('/img/bar.png') + + +.foo + background-image: url('img/bar.png') + + +.foo + background-image: url('bar.png') + +.foo + background-image: url('data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7') diff --git a/tests/sass/no-domains.scss b/tests/sass/no-domains.scss new file mode 100644 index 00000000..98ba24c0 --- /dev/null +++ b/tests/sass/no-domains.scss @@ -0,0 +1,27 @@ +.foo { + background-image: url('https://foo.com/img/bar.png'); +} + +.foo { + background-image: url('http://foo.com/img/bar.png'); +} + +.foo { + background-image: url('//foo.com/img/bar.png'); +} + +.foo { + background-image: url('/img/bar.png'); +} + +.foo { + background-image: url('img/bar.png'); +} + +.foo { + background-image: url('bar.png'); +} + +.foo { + background-image: url('data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7'); +} From 45074f71ac4ba459f2b4faadeafa4594a1584550 Mon Sep 17 00:00:00 2001 From: Dan Wasilewski Date: Thu, 25 Aug 2016 13:15:35 -0400 Subject: [PATCH 07/67] issue-813 -- Updates documentation for new and updated rules. --- docs/rules/no-domains.md | 37 +++++++++++++++++++++++++++++ docs/rules/no-url-protocols.md | 42 +++++++++++++++++++++++++++++++-- lib/rules/no-url-protocols.js | 10 ++++---- tests/rules/no-url-protocols.js | 8 +++---- 4 files changed, 86 insertions(+), 11 deletions(-) create mode 100644 docs/rules/no-domains.md diff --git a/docs/rules/no-domains.md b/docs/rules/no-domains.md new file mode 100644 index 00000000..762d7174 --- /dev/null +++ b/docs/rules/no-domains.md @@ -0,0 +1,37 @@ +# No Domains + +Rule `no-domains` will enforce that domains are not used within urls. + +## Examples + +When enabled, the following are allowed: + +```scss +.foo { + background-image: url('/img/bar.png'); +} + +.foo { + background-image: url('img/bar.png'); +} + +.foo { + background-image: url('bar.png'); +} +``` + +When enabled, the following are disallowed: + +```scss +.foo { + background-image: url('https://foo.com/img/bar.png'); +} + +.foo { + background-image: url('http://foo.com/img/bar.png'); +} + +.foo { + background-image: url('//foo.com/img/bar.png'); +} +``` diff --git a/docs/rules/no-url-protocols.md b/docs/rules/no-url-protocols.md index ebf620b3..a585445f 100644 --- a/docs/rules/no-url-protocols.md +++ b/docs/rules/no-url-protocols.md @@ -2,9 +2,15 @@ Rule `no-url-protocols` will enforce that protocols and domains are not used within urls. +## Options + +* `protocol-relative-urls`: `true`/`false` (defaults to `false`) + ## Examples -When enabled, the following are allowed: +### `protocol-relative-urls` + +When `protocol-relative-urls: false`, the following are allowed: ```scss .foo { @@ -20,7 +26,7 @@ When enabled, the following are allowed: } ``` -When enabled, the following are disallowed: +When `protocol-relative-urls: false`, the following are disallowed: ```scss .foo { @@ -35,3 +41,35 @@ When enabled, the following are disallowed: background-image: url('//foo.com/img/bar.png'); } ``` + +When `protocol-relative-urls: true`, the following are allowed: + +```scss +.foo { + background-image: url('//foo.com/img/bar.png'); +} + +.foo { + background-image: url('/img/bar.png'); +} + +.foo { + background-image: url('img/bar.png'); +} + +.foo { + background-image: url('bar.png'); +} +``` + +When `protocol-relative-urls: true`, the following are disallowed: + +```scss +.foo { + background-image: url('https://foo.com/img/bar.png'); +} + +.foo { + background-image: url('http://foo.com/img/bar.png'); +} +``` diff --git a/lib/rules/no-url-protocols.js b/lib/rules/no-url-protocols.js index 2193b1b2..6cdd14d6 100644 --- a/lib/rules/no-url-protocols.js +++ b/lib/rules/no-url-protocols.js @@ -3,12 +3,12 @@ var helpers = require('../helpers'); var isUrlRegex = /^(https?:)?\/\//, - noProtocolRegex = /^(https?:)\/\//; + protocolRelativeRegex = /^(https?:)\/\//; module.exports = { 'name': 'no-url-protocols', 'defaults': { - 'enforce-domains': true + 'protocol-relative-urls': false }, 'detect': function (ast, parser) { var result = []; @@ -17,9 +17,9 @@ module.exports = { uri.traverse(function (item) { if (item.is('string')) { var stripped = helpers.stripQuotes(item.content), - regexSelector = parser.options['enforce-domains'] ? - isUrlRegex : noProtocolRegex, - message = parser.options['enforce-domains'] ? + regexSelector = !parser.options['protocol-relative-urls'] ? + isUrlRegex : protocolRelativeRegex, + message = !parser.options['protocol-relative-urls'] ? 'Protocols and domains in URLs are disallowed' : 'Protocols in URLS are disallowed'; diff --git a/tests/rules/no-url-protocols.js b/tests/rules/no-url-protocols.js index 17ca8253..680881f5 100644 --- a/tests/rules/no-url-protocols.js +++ b/tests/rules/no-url-protocols.js @@ -17,12 +17,12 @@ describe('no url protocols - scss', function () { }); }); - it('allow protocol-relative urls [enforce-domains: false]', function (done) { + it('allow protocol-relative-urls urls [protocol-relative-urls: true]', function (done) { lint.test(file, { 'no-url-protocols': [ 1, { - 'enforce-domains': false + 'protocol-relative-urls': true } ] }, function (data) { @@ -48,12 +48,12 @@ describe('no url protocols - sass', function () { }); }); - it('allow protocol-relative urls [enforce-domains: false]', function (done) { + it('allow protocol-relative-urls urls [protocol-relative-urls: true]', function (done) { lint.test(file, { 'no-url-protocols': [ 1, { - 'enforce-domains': false + 'protocol-relative-urls': true } ] }, function (data) { From 1a8d9842e91257aa62e2923b7dd19662793e6468 Mon Sep 17 00:00:00 2001 From: Dan Wasilewski Date: Thu, 25 Aug 2016 13:20:46 -0400 Subject: [PATCH 08/67] issue-813 -- Disables no-domains rule by default --- lib/config/sass-lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/config/sass-lint.yml b/lib/config/sass-lint.yml index da93ea4a..fc87c186 100644 --- a/lib/config/sass-lint.yml +++ b/lib/config/sass-lint.yml @@ -25,7 +25,7 @@ rules: no-css-comments: 1 no-debug: 1 no-disallowed-properties: 0 - no-domains: 1 + no-domains: 0 no-duplicate-properties: 1 no-empty-rulesets: 1 no-extends: 0 From 6fff0a00bec68ae251461523759f9e609dcc4d46 Mon Sep 17 00:00:00 2001 From: Dan Purdy Date: Fri, 26 Aug 2016 00:15:15 +0100 Subject: [PATCH 09/67] :bug: Fix missing string type from value checker --- lib/rules/shorthand-values.js | 8 +++++++- tests/rules/shorthand-values.js | 28 ++++++++++++++-------------- tests/sass/shorthand-values.sass | 7 +++++++ tests/sass/shorthand-values.scss | 9 +++++++++ 4 files changed, 37 insertions(+), 15 deletions(-) diff --git a/lib/rules/shorthand-values.js b/lib/rules/shorthand-values.js index 232ae45b..3df76a4a 100644 --- a/lib/rules/shorthand-values.js +++ b/lib/rules/shorthand-values.js @@ -93,7 +93,13 @@ var scanValue = function (node) { fullVal += '#' + val.content + ''; } - else if (val.is('operator') || val.is('ident') || val.is('number') || val.is('unaryOperator')) { + else if ( + val.is('operator') || + val.is('ident') || + val.is('number') || + val.is('unaryOperator') || + val.is('string') + ) { fullVal += val.content; } diff --git a/tests/rules/shorthand-values.js b/tests/rules/shorthand-values.js index e80cc21c..5bf8dbe7 100644 --- a/tests/rules/shorthand-values.js +++ b/tests/rules/shorthand-values.js @@ -12,7 +12,7 @@ describe('shorthand values - scss', function () { lint.test(file, { 'shorthand-values': 1 }, function (data) { - lint.assert.equal(77, data.warningCount); + lint.assert.equal(78, data.warningCount); done(); }); }); @@ -44,7 +44,7 @@ describe('shorthand values - scss', function () { } ] }, function (data) { - lint.assert.equal(39, data.warningCount); + lint.assert.equal(40, data.warningCount); done(); }); }); @@ -60,7 +60,7 @@ describe('shorthand values - scss', function () { } ] }, function (data) { - lint.assert.equal(46, data.warningCount); + lint.assert.equal(47, data.warningCount); done(); }); }); @@ -92,7 +92,7 @@ describe('shorthand values - scss', function () { } ] }, function (data) { - lint.assert.equal(58, data.warningCount); + lint.assert.equal(59, data.warningCount); done(); }); }); @@ -109,7 +109,7 @@ describe('shorthand values - scss', function () { } ] }, function (data) { - lint.assert.equal(65, data.warningCount); + lint.assert.equal(66, data.warningCount); done(); }); }); @@ -126,7 +126,7 @@ describe('shorthand values - scss', function () { } ] }, function (data) { - lint.assert.equal(58, data.warningCount); + lint.assert.equal(59, data.warningCount); done(); }); }); @@ -144,7 +144,7 @@ describe('shorthand values - scss', function () { } ] }, function (data) { - lint.assert.equal(77, data.warningCount); + lint.assert.equal(78, data.warningCount); done(); }); }); @@ -161,7 +161,7 @@ describe('shorthand values - sass', function () { lint.test(file, { 'shorthand-values': 1 }, function (data) { - lint.assert.equal(77, data.warningCount); + lint.assert.equal(78, data.warningCount); done(); }); }); @@ -193,7 +193,7 @@ describe('shorthand values - sass', function () { } ] }, function (data) { - lint.assert.equal(39, data.warningCount); + lint.assert.equal(40, data.warningCount); done(); }); }); @@ -209,7 +209,7 @@ describe('shorthand values - sass', function () { } ] }, function (data) { - lint.assert.equal(46, data.warningCount); + lint.assert.equal(47, data.warningCount); done(); }); }); @@ -241,7 +241,7 @@ describe('shorthand values - sass', function () { } ] }, function (data) { - lint.assert.equal(58, data.warningCount); + lint.assert.equal(59, data.warningCount); done(); }); }); @@ -258,7 +258,7 @@ describe('shorthand values - sass', function () { } ] }, function (data) { - lint.assert.equal(65, data.warningCount); + lint.assert.equal(66, data.warningCount); done(); }); }); @@ -275,7 +275,7 @@ describe('shorthand values - sass', function () { } ] }, function (data) { - lint.assert.equal(58, data.warningCount); + lint.assert.equal(59, data.warningCount); done(); }); }); @@ -293,7 +293,7 @@ describe('shorthand values - sass', function () { } ] }, function (data) { - lint.assert.equal(77, data.warningCount); + lint.assert.equal(78, data.warningCount); done(); }); }); diff --git a/tests/sass/shorthand-values.sass b/tests/sass/shorthand-values.sass index b766c6ce..6e9dbffd 100644 --- a/tests/sass/shorthand-values.sass +++ b/tests/sass/shorthand-values.sass @@ -305,3 +305,10 @@ .test border-color: transparent #095b97 transparent #095b97 + +// Issue #847 - Ignoring function arguments +.foo + padding: 0 size('half-shim') 0 size('spacer') + +.foo + padding: 0 size('half-shim') 0 size('half-shim') diff --git a/tests/sass/shorthand-values.scss b/tests/sass/shorthand-values.scss index 7ffbccfb..54a422a4 100644 --- a/tests/sass/shorthand-values.scss +++ b/tests/sass/shorthand-values.scss @@ -369,3 +369,12 @@ .test { border-color: transparent #095b97 transparent #095b97; } + +// Issue #847 - Ignoring function arguments +.foo { + padding: 0 size('half-shim') 0 size('spacer'); +} + +.foo { + padding: 0 size('half-shim') 0 size('half-shim'); +} From 56d3e4f53d51b1151c9578c4521f6740f64ad959 Mon Sep 17 00:00:00 2001 From: Dan Wasilewski Date: Thu, 25 Aug 2016 19:31:52 -0400 Subject: [PATCH 10/67] issue-813 -- Addresses PR feedback --- docs/rules/{no-domains.md => no-url-domains.md} | 4 ++-- docs/rules/no-url-protocols.md | 14 ++++++++------ lib/config/sass-lint.yml | 2 +- lib/rules/{no-domains.js => no-url-domains.js} | 2 +- lib/rules/no-url-protocols.js | 6 +++--- tests/rules/{no-domains.js => no-url-domains.js} | 12 ++++++------ tests/rules/no-url-protocols.js | 8 ++++---- .../sass/{no-domains.sass => no-url-domains.sass} | 0 .../sass/{no-domains.scss => no-url-domains.scss} | 0 9 files changed, 25 insertions(+), 23 deletions(-) rename docs/rules/{no-domains.md => no-url-domains.md} (83%) rename lib/rules/{no-domains.js => no-url-domains.js} (96%) rename tests/rules/{no-domains.js => no-url-domains.js} (68%) rename tests/sass/{no-domains.sass => no-url-domains.sass} (100%) rename tests/sass/{no-domains.scss => no-url-domains.scss} (100%) diff --git a/docs/rules/no-domains.md b/docs/rules/no-url-domains.md similarity index 83% rename from docs/rules/no-domains.md rename to docs/rules/no-url-domains.md index 762d7174..8c536b1a 100644 --- a/docs/rules/no-domains.md +++ b/docs/rules/no-url-domains.md @@ -1,6 +1,6 @@ -# No Domains +# No Url Domains -Rule `no-domains` will enforce that domains are not used within urls. +Rule `no-url-domains` will enforce that domains are not used within urls. ## Examples diff --git a/docs/rules/no-url-protocols.md b/docs/rules/no-url-protocols.md index a585445f..50baa5ff 100644 --- a/docs/rules/no-url-protocols.md +++ b/docs/rules/no-url-protocols.md @@ -4,13 +4,15 @@ Rule `no-url-protocols` will enforce that protocols and domains are not used wit ## Options -* `protocol-relative-urls`: `true`/`false` (defaults to `false`) +* `allow-protocol-relative-urls`: `true`/`false` (defaults to `false`) +> This option is scheduled to be deprecated in favour of the [no-url-domains](https://github.com/sasstools/sass-lint/blob/develop/docs/rules/no-url-domains.md) rule in sass-lint 2.0. ## Examples -### `protocol-relative-urls` +### `allow-protocol-relative-urls` -When `protocol-relative-urls: false`, the following are allowed: + +When `allow-protocol-relative-urls: false`, the following are allowed: ```scss .foo { @@ -26,7 +28,7 @@ When `protocol-relative-urls: false`, the following are allowed: } ``` -When `protocol-relative-urls: false`, the following are disallowed: +When `allow-protocol-relative-urls: false`, the following are disallowed: ```scss .foo { @@ -42,7 +44,7 @@ When `protocol-relative-urls: false`, the following are disallowed: } ``` -When `protocol-relative-urls: true`, the following are allowed: +When `allow-protocol-relative-urls: true`, the following are allowed: ```scss .foo { @@ -62,7 +64,7 @@ When `protocol-relative-urls: true`, the following are allowed: } ``` -When `protocol-relative-urls: true`, the following are disallowed: +When `allow-protocol-relative-urls: true`, the following are disallowed: ```scss .foo { diff --git a/lib/config/sass-lint.yml b/lib/config/sass-lint.yml index fc87c186..a2a19473 100644 --- a/lib/config/sass-lint.yml +++ b/lib/config/sass-lint.yml @@ -25,7 +25,7 @@ rules: no-css-comments: 1 no-debug: 1 no-disallowed-properties: 0 - no-domains: 0 + no-url-domains: 1 no-duplicate-properties: 1 no-empty-rulesets: 1 no-extends: 0 diff --git a/lib/rules/no-domains.js b/lib/rules/no-url-domains.js similarity index 96% rename from lib/rules/no-domains.js rename to lib/rules/no-url-domains.js index 213e7b24..73b491fe 100644 --- a/lib/rules/no-domains.js +++ b/lib/rules/no-url-domains.js @@ -4,7 +4,7 @@ var helpers = require('../helpers'), url = require('url'); module.exports = { - 'name': 'no-domains', + 'name': 'no-url-domains', 'defaults': {}, 'detect': function (ast, parser) { var result = []; diff --git a/lib/rules/no-url-protocols.js b/lib/rules/no-url-protocols.js index 6cdd14d6..90080479 100644 --- a/lib/rules/no-url-protocols.js +++ b/lib/rules/no-url-protocols.js @@ -8,7 +8,7 @@ var isUrlRegex = /^(https?:)?\/\//, module.exports = { 'name': 'no-url-protocols', 'defaults': { - 'protocol-relative-urls': false + 'allow-protocol-relative-urls': false }, 'detect': function (ast, parser) { var result = []; @@ -17,9 +17,9 @@ module.exports = { uri.traverse(function (item) { if (item.is('string')) { var stripped = helpers.stripQuotes(item.content), - regexSelector = !parser.options['protocol-relative-urls'] ? + regexSelector = !parser.options['allow-protocol-relative-urls'] ? isUrlRegex : protocolRelativeRegex, - message = !parser.options['protocol-relative-urls'] ? + message = !parser.options['allow-protocol-relative-urls'] ? 'Protocols and domains in URLs are disallowed' : 'Protocols in URLS are disallowed'; diff --git a/tests/rules/no-domains.js b/tests/rules/no-url-domains.js similarity index 68% rename from tests/rules/no-domains.js rename to tests/rules/no-url-domains.js index 39973fbf..2caabdb1 100644 --- a/tests/rules/no-domains.js +++ b/tests/rules/no-url-domains.js @@ -5,12 +5,12 @@ var lint = require('./_lint'); ////////////////////////////// // SCSS syntax tests ////////////////////////////// -describe('no domains - scss', function () { - var file = lint.file('no-domains.scss'); +describe('no url domains - scss', function () { + var file = lint.file('no-url-domains.scss'); it('enforce', function (done) { lint.test(file, { - 'no-domains': 1 + 'no-url-domains': 1 }, function (data) { lint.assert.equal(3, data.warningCount); done(); @@ -21,12 +21,12 @@ describe('no domains - scss', function () { ////////////////////////////// // Sass syntax tests ////////////////////////////// -describe('no domains - sass', function () { - var file = lint.file('no-domains.sass'); +describe('no url domains - sass', function () { + var file = lint.file('no-url-domains.sass'); it('enforce', function (done) { lint.test(file, { - 'no-domains': 1 + 'no-url-domains': 1 }, function (data) { lint.assert.equal(3, data.warningCount); done(); diff --git a/tests/rules/no-url-protocols.js b/tests/rules/no-url-protocols.js index 680881f5..65b5b5f1 100644 --- a/tests/rules/no-url-protocols.js +++ b/tests/rules/no-url-protocols.js @@ -17,12 +17,12 @@ describe('no url protocols - scss', function () { }); }); - it('allow protocol-relative-urls urls [protocol-relative-urls: true]', function (done) { + it('[allow-protocol-relative-urls: true]', function (done) { lint.test(file, { 'no-url-protocols': [ 1, { - 'protocol-relative-urls': true + 'allow-protocol-relative-urls': true } ] }, function (data) { @@ -48,12 +48,12 @@ describe('no url protocols - sass', function () { }); }); - it('allow protocol-relative-urls urls [protocol-relative-urls: true]', function (done) { + it('[allow-protocol-relative-urls: true]', function (done) { lint.test(file, { 'no-url-protocols': [ 1, { - 'protocol-relative-urls': true + 'allow-protocol-relative-urls': true } ] }, function (data) { diff --git a/tests/sass/no-domains.sass b/tests/sass/no-url-domains.sass similarity index 100% rename from tests/sass/no-domains.sass rename to tests/sass/no-url-domains.sass diff --git a/tests/sass/no-domains.scss b/tests/sass/no-url-domains.scss similarity index 100% rename from tests/sass/no-domains.scss rename to tests/sass/no-url-domains.scss From 21f836ea41627b3b72e8d3f780000907327d2aac Mon Sep 17 00:00:00 2001 From: danwaz Date: Fri, 26 Aug 2016 08:11:09 -0400 Subject: [PATCH 11/67] issue-813 -- Alphabetizes rules config --- lib/config/sass-lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/config/sass-lint.yml b/lib/config/sass-lint.yml index a2a19473..33dcf7dd 100644 --- a/lib/config/sass-lint.yml +++ b/lib/config/sass-lint.yml @@ -25,7 +25,6 @@ rules: no-css-comments: 1 no-debug: 1 no-disallowed-properties: 0 - no-url-domains: 1 no-duplicate-properties: 1 no-empty-rulesets: 1 no-extends: 0 @@ -39,6 +38,7 @@ rules: no-trailing-zero: 1 no-transition-all: 1 no-universal-selectors: 0 + no-url-domains: 1 no-url-protocols: 1 no-vendor-prefixes: 1 no-warn: 1 From fbaf73825dfe57bc6827d1d4bd79bba513fd5934 Mon Sep 17 00:00:00 2001 From: Robin Winslow Date: Wed, 31 Aug 2016 18:50:54 +0100 Subject: [PATCH 12/67] Reimplement max-warnings as a proper option - Remove custom, slightly hacky code for `max-warnings` from `bin/sass-lint.js` - Tidy up `bin/sass-lint.js` more generally - Add `max-warnings` as an option to be passed to `failOnError`, either through a config file or through passing options directly - Add documentation for the max-warnings option - Create new descriptive exceptions for `failOnError` - MaxWarningsExceededError and SassLintFailureError - Test the functionality of `failOnError` --- bin/sass-lint.js | 57 +++++--------------- docs/options/max-warnings.md | 29 ++++++++++ docs/sass-lint.yml | 2 + index.js | 25 +++++++-- lib/exceptions.js | 19 +++++++ tests/cli.js | 12 ++--- tests/failures.js | 90 ++++++++++++++++++++++++++++++++ tests/sass/success.scss | 3 ++ tests/yml/.indentation-error.yml | 2 + tests/yml/.indentation-warn.yml | 2 + tests/yml/.max-0-warnings.yml | 2 + tests/yml/.max-10-warnings.yml | 2 + tests/yml/.max-100-warnings.yml | 2 + 13 files changed, 192 insertions(+), 55 deletions(-) create mode 100644 docs/options/max-warnings.md create mode 100644 lib/exceptions.js create mode 100644 tests/failures.js create mode 100644 tests/sass/success.scss create mode 100644 tests/yml/.indentation-error.yml create mode 100644 tests/yml/.indentation-warn.yml create mode 100644 tests/yml/.max-0-warnings.yml create mode 100644 tests/yml/.max-10-warnings.yml create mode 100644 tests/yml/.max-100-warnings.yml diff --git a/bin/sass-lint.js b/bin/sass-lint.js index 787e9330..a6218b8f 100755 --- a/bin/sass-lint.js +++ b/bin/sass-lint.js @@ -6,15 +6,7 @@ var program = require('commander'), lint = require('../index'); var configPath, - ignores, - configOptions = {}, - exitCode = 0; - -var tooManyWarnings = function (detects) { - var warningCount = lint.warningCount(detects).count; - - return warningCount > 0 && warningCount > program.maxWarnings; -}; + configOptions = {}; var detectPattern = function (pattern) { var detects; @@ -25,13 +17,7 @@ var detectPattern = function (pattern) { lint.outputResults(detects, configOptions, configPath); } - if (lint.errorCount(detects).count || tooManyWarnings(detects)) { - exitCode = 1; - } - - if (program.exit) { - lint.failOnError(detects); - } + lint.failOnError(detects, configOptions, configPath); }; program @@ -47,21 +33,16 @@ program .option('--max-warnings [integer]', 'Number of warnings to trigger nonzero exit code') .parse(process.argv); +// Create "options" and "files" dictionaries if they don't exist +configOptions.files = configOptions.files || {}; +configOptions.options = configOptions.options || {}; if (program.config && program.config !== true) { configPath = program.config; } if (program.ignore && program.ignore !== true) { - ignores = program.ignore.split(', '); - if (configOptions.hasOwnProperty('files')) { - configOptions.files.ignore = ignores; - } - else { - configOptions.files = { - 'ignore': ignores - }; - } + configOptions.files.ignore = program.ignore.split(', '); } if (program.syntax && ['sass', 'scss'].indexOf(program.syntax) > -1) { @@ -69,25 +50,15 @@ if (program.syntax && ['sass', 'scss'].indexOf(program.syntax) > -1) { } if (program.format && program.format !== true) { - if (configOptions.hasOwnProperty('options')) { - configOptions.options.formatter = program.format; - } - else { - configOptions.options = { - 'formatter': program.format - }; - } + configOptions.options.formatter = program.format; } if (program.output && program.output !== true) { - if (configOptions.hasOwnProperty('options')) { - configOptions.options['output-file'] = program.output; - } - else { - configOptions.options = { - 'output-file': program.output - }; - } + configOptions.options['output-file'] = program.output; +} + +if (program.maxWarnings && program.maxWarnings !== true) { + configOptions.options['max-warnings'] = program.maxWarnings; } if (program.args.length === 0) { @@ -98,7 +69,3 @@ else { detectPattern(path); }); } - -process.on('exit', function () { - process.exit(exitCode); // eslint-disable-line no-process-exit -}); diff --git a/docs/options/max-warnings.md b/docs/options/max-warnings.md new file mode 100644 index 00000000..32d6e8ed --- /dev/null +++ b/docs/options/max-warnings.md @@ -0,0 +1,29 @@ +# Max warnings + +An error will be thrown if the total number of warnings exceeds the `max-warnings` setting. + +## Examples + +This can be set as a command-line option: + +``` bash +$ sass-lint --max-warnings 50 +``` + +In `.sass-lint.yml`: + +``` yaml +options: + max-warnings: 50 +``` + +Or inside a script: + +``` javascript +var sassLint = require('sass-lint'), + config = {options: {'max-warnings': 50}}; + +results = sassLint.lintFiles('sass/**/*.scss', config) +sassLint.failOnError(results, config); +``` + diff --git a/docs/sass-lint.yml b/docs/sass-lint.yml index 3de8a760..682ec6c5 100644 --- a/docs/sass-lint.yml +++ b/docs/sass-lint.yml @@ -9,6 +9,8 @@ options: formatter: html # Output file instead of logging results output-file: 'linters/sass-lint.html' + # Raise an error if more than 50 warnings are generated + max-warnings: 50 # File Options files: include: 'sass/**/*.s+(a|c)ss' diff --git a/index.js b/index.js index decbc923..e222c616 100644 --- a/index.js +++ b/index.js @@ -2,6 +2,7 @@ var slConfig = require('./lib/config'), groot = require('./lib/groot'), + exceptions = require('./lib/exceptions'), helpers = require('./lib/helpers'), slRules = require('./lib/rules'), glob = require('glob'), @@ -284,14 +285,30 @@ sassLint.outputResults = function (results, options, configPath) { * Throws an error if there are any errors detected. The error includes a count of all errors * and a list of all files that include errors. * - * @param {object} results our results object + * @param {object} results - our results object + * @param {object} [options] - extra options to use when running failOnError, e.g. max-warnings + * @param {string} [configPath] - path to the config file * @returns {void} */ -sassLint.failOnError = function (results) { - var errorCount = this.errorCount(results); +sassLint.failOnError = function (results, options, configPath) { + // Default parameters + options = typeof options !== 'undefined' ? options : {}; + configPath = typeof configPath !== 'undefined' ? configPath : null; + + var errorCount = this.errorCount(results), + warningCount = this.warningCount(results), + configOptions = this.getConfig(options, configPath).options; if (errorCount.count > 0) { - throw new Error(errorCount.count + ' errors were detected in \n- ' + errorCount.files.join('\n- ')); + throw new exceptions.SassLintFailureError(errorCount.count + ' errors were detected in \n- ' + errorCount.files.join('\n- ')); + } + + if (!isNaN(configOptions['max-warnings']) && warningCount.count > configOptions['max-warnings']) { + throw new exceptions.MaxWarningsExceededError( + 'Number of warnings (' + warningCount.count + + ') exceeds the allowed maximum of ' + configOptions['max-warnings'] + + '.\n' + ); } }; diff --git a/lib/exceptions.js b/lib/exceptions.js new file mode 100644 index 00000000..77e15683 --- /dev/null +++ b/lib/exceptions.js @@ -0,0 +1,19 @@ +'use strict'; + +var util = require('util'); + +module.exports = { + SassLintFailureError: function (message) { + Error.captureStackTrace(this, this.constructor); + this.name = 'SassLintFailureError'; + this.message = message; + }, + MaxWarningsExceededError: function (message) { + Error.captureStackTrace(this, this.constructor); + this.name = 'MaxWarningsExceededError'; + this.message = message; + } +}; + +util.inherits(module.exports.SassLintFailureError, Error); +util.inherits(module.exports.MaxWarningsExceededError, Error); diff --git a/tests/cli.js b/tests/cli.js index 4cc618c3..a3d6698d 100644 --- a/tests/cli.js +++ b/tests/cli.js @@ -383,27 +383,27 @@ describe('cli', function () { }); - it('should exit with exit code 1 when quiet', function (done) { + it('should exit with error when quiet', function (done) { var command = 'sass-lint -c tests/yml/.error-output.yml tests/cli/cli-error.scss --verbose --no-exit'; exec(command, function (err) { - if (err.code === 1) { + if (err) { return done(); } - return done(new Error('Error code not 1')); + return done(new Error('No error on exit')); }); }); - it('should exit with exit code 1 when more warnings than --max-warnings', function (done) { + it('should exit with error when more warnings than --max-warnings', function (done) { var command = 'sass-lint -c tests/yml/.color-keyword-errors.yml tests/cli/cli.scss --max-warnings 0'; exec(command, function (err) { - if (err && err.code === 1) { + if (err) { return done(); } - return done(new Error('Error code not 1')); + return done(new Error('No error on exit')); }); }); diff --git a/tests/failures.js b/tests/failures.js new file mode 100644 index 00000000..3127d451 --- /dev/null +++ b/tests/failures.js @@ -0,0 +1,90 @@ +'use strict'; + +var lint = require('../index'), + assert = require('assert'), + exceptions = require('../lib/exceptions'); + +describe('failures', function () { + it('should raise SassLintFailureError if indentation is set to error', function (done) { + assert.throws( + function () { + var results = lint.lintFiles('tests/sass/indentation/indentation-spaces.scss', {rules: {indentation: 2}}); // 14 errors + lint.failOnError(results); // Set indentation to error + }, + exceptions.SassLintFailureError + ); + assert.throws( + function () { + var results = lint.lintFiles('tests/sass/indentation/indentation-spaces.scss', {}, 'tests/yml/.indentation-error.yml'); // 14 errors + lint.failOnError(results); // Set indentation to error + }, + exceptions.SassLintFailureError + ); + + done(); + }); + + it('should not raise error if indentation is only set to warn', function (done) { + // These should produce 55 warnings and 0 errors + var directResults = lint.lintFiles('sass/indentation/indentation-spaces.scss', {rules: {indentation: 1}}); + var configResults = lint.lintFiles('sass/indentation/indentation-spaces.scss', {}, 'yml/.indentation-warn.yml'); + lint.failOnError(directResults); + lint.failOnError(configResults); + + done(); + }); + + it('should raise MaxWarningsExceededError if warnings exceed `max-warnings` setting', function (done) { + assert.throws( + function () { + var results = lint.lintFiles('tests/sass/indentation/indentation-spaces.scss', {}); // 55 warnings + lint.failOnError(results, {options: {'max-warnings': 10}}); + }, + exceptions.MaxWarningsExceededError + ); + assert.throws( + function () { + var results = lint.lintFiles('tests/sass/indentation/indentation-spaces.scss', {}); // 55 warnings + lint.failOnError(results, {}, 'tests/yml/.max-10-warnings.yml'); + }, + exceptions.MaxWarningsExceededError + ); + + done(); + }); + + it('should raise MaxWarningsExceededError if warnings exceed `max-warnings` of zero', function (done) { + assert.throws( + function () { + var results = lint.lintFiles('tests/sass/indentation/indentation-spaces.scss', {}); // 55 warnings + lint.failOnError(results, {options: {'max-warnings': 0}}); + }, + exceptions.MaxWarningsExceededError + ); + assert.throws( + function () { + var results = lint.lintFiles('tests/sass/indentation/indentation-spaces.scss', {}); // 55 warnings + lint.failOnError(results, {}, 'tests/yml/.max-0-warnings.yml'); + }, + exceptions.MaxWarningsExceededError + ); + + done(); + }); + + it('should not raise error if warnings do not exceed `max-warnings` setting', function (done) { + var results = lint.lintFiles('sass/indentation/indentation-spaces.scss', {}); // 55 warnings + lint.failOnError(results, {'max-warnings': 100}); // should succceed + lint.failOnError(results, {}, 'yml/.max-100-warnings.yml'); // should succeed + + done(); + }); + + it('should not raise error if no warnings even if `max-warnings` is zero', function (done) { + var results = lint.lintFiles('sass/success.scss', {}); // no warnings + lint.failOnError(results, {'max-warnings': 0}); // should still succceed + lint.failOnError(results, {}, 'yml/.max-0-warnings.yml'); // should still succeed + + done(); + }); +}); diff --git a/tests/sass/success.scss b/tests/sass/success.scss new file mode 100644 index 00000000..ae771e48 --- /dev/null +++ b/tests/sass/success.scss @@ -0,0 +1,3 @@ +.one { + margin-top: 0; +} diff --git a/tests/yml/.indentation-error.yml b/tests/yml/.indentation-error.yml new file mode 100644 index 00000000..57dbbfe6 --- /dev/null +++ b/tests/yml/.indentation-error.yml @@ -0,0 +1,2 @@ +rules: + indentation: 2 diff --git a/tests/yml/.indentation-warn.yml b/tests/yml/.indentation-warn.yml new file mode 100644 index 00000000..71a1f08f --- /dev/null +++ b/tests/yml/.indentation-warn.yml @@ -0,0 +1,2 @@ +rules: + indentation: 1 diff --git a/tests/yml/.max-0-warnings.yml b/tests/yml/.max-0-warnings.yml new file mode 100644 index 00000000..0a3dc120 --- /dev/null +++ b/tests/yml/.max-0-warnings.yml @@ -0,0 +1,2 @@ +options: + max-warnings: 0 diff --git a/tests/yml/.max-10-warnings.yml b/tests/yml/.max-10-warnings.yml new file mode 100644 index 00000000..85b0f1c9 --- /dev/null +++ b/tests/yml/.max-10-warnings.yml @@ -0,0 +1,2 @@ +options: + max-warnings: 10 diff --git a/tests/yml/.max-100-warnings.yml b/tests/yml/.max-100-warnings.yml new file mode 100644 index 00000000..759639a6 --- /dev/null +++ b/tests/yml/.max-100-warnings.yml @@ -0,0 +1,2 @@ +options: + max-warnings: 100 From 6aa9544e1ccfa9f82982b4e4f120c9b4173f3271 Mon Sep 17 00:00:00 2001 From: Adam Onishi Date: Tue, 6 Sep 2016 15:41:50 +0100 Subject: [PATCH 13/67] :mag: Add rule: declarations before nesting --- docs/rules/declarations-before-nesting.md | 29 +++++++++++++ lib/rules/declarations-before-nesting.js | 47 +++++++++++++++++++++ tests/rules/declarations-before-nesting.js | 36 ++++++++++++++++ tests/sass/declarations-before-nesting.sass | 27 ++++++++++++ tests/sass/declarations-before-nesting.scss | 35 +++++++++++++++ 5 files changed, 174 insertions(+) create mode 100644 docs/rules/declarations-before-nesting.md create mode 100644 lib/rules/declarations-before-nesting.js create mode 100644 tests/rules/declarations-before-nesting.js create mode 100644 tests/sass/declarations-before-nesting.sass create mode 100644 tests/sass/declarations-before-nesting.scss diff --git a/docs/rules/declarations-before-nesting.md b/docs/rules/declarations-before-nesting.md new file mode 100644 index 00000000..d68aebcd --- /dev/null +++ b/docs/rules/declarations-before-nesting.md @@ -0,0 +1,29 @@ +# Declarations Before Nesting + +Rule `declarations-before-nesting` will enforce that declarations should be written before nesting in a ruleset. + +## Examples + +When enabled, the following are allowed: + +```scss +.foo { + content: 'baz'; + + .bar { + content: 'qux'; + } +} +``` + +When enabled, the following are disallowed: + +```scss +.foo { + .bar { + content: 'qux'; + } + + content: 'baz'; +} +``` diff --git a/lib/rules/declarations-before-nesting.js b/lib/rules/declarations-before-nesting.js new file mode 100644 index 00000000..cfc97f10 --- /dev/null +++ b/lib/rules/declarations-before-nesting.js @@ -0,0 +1,47 @@ +'use strict'; + +var helpers = require('../helpers'); + +module.exports = { + 'name': 'declarations-before-nesting', + 'defaults': {}, + 'detect': function (ast, parser) { + var result = [], + error; + + ast.traverseByType('block', function (block) { + if (block.contains('ruleset') && block.contains('declaration')) { + var rulesetIndex; + + block.forEach(function (item, j) { + var declarationIndex; + var declaration; + + if (item.is('ruleset') && rulesetIndex === void 0) { + rulesetIndex = j; + } + + if (item.is('declaration')) { + declarationIndex = j; + declaration = item; + } + + if (rulesetIndex < declarationIndex && declaration) { + error = { + 'ruleId': parser.rule.name, + 'line': declaration.start.line, + 'column': declaration.start.column, + 'message': 'declarations should come before nestings', + 'severity': parser.severity + }; + result = helpers.addUnique(result, error); + } + }); + + rulesetIndex = null; + } + }); + + return result; + } +}; diff --git a/tests/rules/declarations-before-nesting.js b/tests/rules/declarations-before-nesting.js new file mode 100644 index 00000000..d7009a7e --- /dev/null +++ b/tests/rules/declarations-before-nesting.js @@ -0,0 +1,36 @@ +'use strict'; + +var lint = require('./_lint'); + +////////////////////////////// +// SCSS syntax tests +////////////////////////////// +describe('declarations before nesting - scss', function () { + var file = lint.file('declarations-before-nesting.scss'); + + it('enforce', function (done) { + lint.test(file, { + 'declarations-before-nesting': 1 + }, function (data) { + console.log(data); + lint.assert.equal(4, data.warningCount); + done(); + }); + }); +}); + +////////////////////////////// +// Sass syntax tests +////////////////////////////// +describe('declarations before nesting - sass', function () { + var file = lint.file('declarations-before-nesting.sass'); + + it('enforce', function (done) { + lint.test(file, { + 'declarations-before-nesting': 1 + }, function (data) { + lint.assert.equal(4, data.warningCount); + done(); + }); + }); +}); diff --git a/tests/sass/declarations-before-nesting.sass b/tests/sass/declarations-before-nesting.sass new file mode 100644 index 00000000..00bbb6a9 --- /dev/null +++ b/tests/sass/declarations-before-nesting.sass @@ -0,0 +1,27 @@ +.bar + content: 'baz' + + .qux + content: 'baz' + +.foo + .bar + content: 'where' + + content: 'baz' + + .baz + content: 'where' + + content: 'baz' + +.foo + .bar + content: 'where' + + .baz + content: 'quz' + + content: 'baz' + + content: 'baz' diff --git a/tests/sass/declarations-before-nesting.scss b/tests/sass/declarations-before-nesting.scss new file mode 100644 index 00000000..efb31d67 --- /dev/null +++ b/tests/sass/declarations-before-nesting.scss @@ -0,0 +1,35 @@ +.bar { + content: 'baz'; + + .qux { + content: 'baz'; + } +} + +.foo { + .bar { + content: 'where'; + } + + content: 'baz'; + + .baz { + content: 'where'; + } + + content: 'baz'; +} + +.foo { + .bar { + content: 'where'; + + .baz { + content: 'quz'; + } + + content: 'baz'; + } + + content: 'baz'; +} From 715332013ee77098f92b7e64a4f545ba15af7ed0 Mon Sep 17 00:00:00 2001 From: Adam Onishi Date: Tue, 6 Sep 2016 15:48:42 +0100 Subject: [PATCH 14/67] :mag: Add declarations-before-nesting rule to default config --- lib/config/sass-lint.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/config/sass-lint.yml b/lib/config/sass-lint.yml index dd3e5113..e98c3db9 100644 --- a/lib/config/sass-lint.yml +++ b/lib/config/sass-lint.yml @@ -43,6 +43,7 @@ rules: property-units: 0 # Nesting + declarations-before-nesting: 1 force-attribute-nesting: 1 force-element-nesting: 1 force-pseudo-nesting: 1 From 9a7b528b9ca23e45fd0606522d52a4070480eba4 Mon Sep 17 00:00:00 2001 From: Adam Onishi Date: Tue, 6 Sep 2016 15:49:53 +0100 Subject: [PATCH 15/67] Capitalise warning message --- lib/rules/declarations-before-nesting.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/declarations-before-nesting.js b/lib/rules/declarations-before-nesting.js index cfc97f10..992cad24 100644 --- a/lib/rules/declarations-before-nesting.js +++ b/lib/rules/declarations-before-nesting.js @@ -31,7 +31,7 @@ module.exports = { 'ruleId': parser.rule.name, 'line': declaration.start.line, 'column': declaration.start.column, - 'message': 'declarations should come before nestings', + 'message': 'Declarations should come before nestings', 'severity': parser.severity }; result = helpers.addUnique(result, error); From c08b4a85101b03fa2d44a09e9db8e00dad22d51c Mon Sep 17 00:00:00 2001 From: Dan Purdy Date: Tue, 18 Oct 2016 00:25:08 +0100 Subject: [PATCH 16/67] :bug: Fix boolean strictbem modifiers --- docs/rules/class-name-format.md | 4 ++++ lib/helpers.js | 2 +- tests/rules/class-name-format.js | 36 +++++++++++++++---------------- tests/sass/class-name-format.sass | 13 +++++++++++ tests/sass/class-name-format.scss | 17 +++++++++++++++ 5 files changed, 53 insertions(+), 19 deletions(-) diff --git a/docs/rules/class-name-format.md b/docs/rules/class-name-format.md index 45cf6b59..0a3fb285 100644 --- a/docs/rules/class-name-format.md +++ b/docs/rules/class-name-format.md @@ -191,6 +191,10 @@ When enabled, the following are allowed: .owner-name_mod-name_mod-val { content: ''; } + +.block-name__elem-name_mod-bool { + content: ''; +} ``` When enabled, the following are disallowed: diff --git a/lib/helpers.js b/lib/helpers.js index 236126a0..d2cb3306 100644 --- a/lib/helpers.js +++ b/lib/helpers.js @@ -169,7 +169,7 @@ helpers.isSnakeCase = function (str) { * @returns {boolean} Whether str adheres to strict-BEM format */ helpers.isStrictBEM = function (str) { - return /^[a-z](\-?[a-z0-9]+)*(__[a-z0-9](\-?[a-z0-9]+)*)?((_[a-z0-9](\-?[a-z0-9]+)*){2})?$/.test(str); + return /^[a-z](\-?[a-z0-9]+)*(__[a-z0-9](\-?[a-z0-9]+)*)?((_[a-z0-9](\-?[a-z0-9]+)*){0,2})?$/.test(str); }; /** diff --git a/tests/rules/class-name-format.js b/tests/rules/class-name-format.js index 220f382c..c5f5365c 100644 --- a/tests/rules/class-name-format.js +++ b/tests/rules/class-name-format.js @@ -12,7 +12,7 @@ describe('class name format - scss', function () { lint.test(file, { 'class-name-format': 1 }, function (data) { - lint.assert.equal(27, data.warningCount); + lint.assert.equal(31, data.warningCount); done(); }); }); @@ -26,7 +26,7 @@ describe('class name format - scss', function () { } ] }, function (data) { - lint.assert.equal(26, data.warningCount); + lint.assert.equal(30, data.warningCount); done(); }); }); @@ -40,7 +40,7 @@ describe('class name format - scss', function () { } ] }, function (data) { - lint.assert.equal(36, data.warningCount); + lint.assert.equal(40, data.warningCount); done(); }); }); @@ -54,7 +54,7 @@ describe('class name format - scss', function () { } ] }, function (data) { - lint.assert.equal(36, data.warningCount); + lint.assert.equal(40, data.warningCount); done(); }); }); @@ -68,7 +68,7 @@ describe('class name format - scss', function () { } ] }, function (data) { - lint.assert.equal(32, data.warningCount); + lint.assert.equal(36, data.warningCount); done(); }); }); @@ -82,7 +82,7 @@ describe('class name format - scss', function () { } ] }, function (data) { - lint.assert.equal(22, data.warningCount); + lint.assert.equal(17, data.warningCount); done(); }); }); @@ -96,7 +96,7 @@ describe('class name format - scss', function () { } ] }, function (data) { - lint.assert.equal(20, data.warningCount); + lint.assert.equal(24, data.warningCount); done(); }); }); @@ -110,7 +110,7 @@ describe('class name format - scss', function () { } ] }, function (data) { - lint.assert.equal(38, data.warningCount); + lint.assert.equal(42, data.warningCount); done(); }); }); @@ -126,7 +126,7 @@ describe('class name format - scss', function () { } ] }, function (data) { - lint.assert.equal(34, data.warningCount); + lint.assert.equal(38, data.warningCount); lint.assert.equal(data.messages[0].message, message); done(); }); @@ -143,7 +143,7 @@ describe('class name format - sass', function () { lint.test(file, { 'class-name-format': 1 }, function (data) { - lint.assert.equal(27, data.warningCount); + lint.assert.equal(31, data.warningCount); done(); }); }); @@ -157,7 +157,7 @@ describe('class name format - sass', function () { } ] }, function (data) { - lint.assert.equal(26, data.warningCount); + lint.assert.equal(30, data.warningCount); done(); }); }); @@ -171,7 +171,7 @@ describe('class name format - sass', function () { } ] }, function (data) { - lint.assert.equal(36, data.warningCount); + lint.assert.equal(40, data.warningCount); done(); }); }); @@ -185,7 +185,7 @@ describe('class name format - sass', function () { } ] }, function (data) { - lint.assert.equal(36, data.warningCount); + lint.assert.equal(40, data.warningCount); done(); }); }); @@ -199,7 +199,7 @@ describe('class name format - sass', function () { } ] }, function (data) { - lint.assert.equal(32, data.warningCount); + lint.assert.equal(36, data.warningCount); done(); }); }); @@ -213,7 +213,7 @@ describe('class name format - sass', function () { } ] }, function (data) { - lint.assert.equal(22, data.warningCount); + lint.assert.equal(17, data.warningCount); done(); }); }); @@ -227,7 +227,7 @@ describe('class name format - sass', function () { } ] }, function (data) { - lint.assert.equal(20, data.warningCount); + lint.assert.equal(24, data.warningCount); done(); }); }); @@ -241,7 +241,7 @@ describe('class name format - sass', function () { } ] }, function (data) { - lint.assert.equal(38, data.warningCount); + lint.assert.equal(42, data.warningCount); done(); }); }); @@ -257,7 +257,7 @@ describe('class name format - sass', function () { } ] }, function (data) { - lint.assert.equal(34, data.warningCount); + lint.assert.equal(38, data.warningCount); lint.assert.equal(data.messages[0].message, message); done(); }); diff --git a/tests/sass/class-name-format.sass b/tests/sass/class-name-format.sass index 07a34e9e..f63b1fe9 100644 --- a/tests/sass/class-name-format.sass +++ b/tests/sass/class-name-format.sass @@ -81,3 +81,16 @@ .APascalCase .camelCase color: red + +// Issue #872 - incorrect strict bem regex +.strict-bem__elem_bool + color: red + +.strict-bem__elem_bool-modifier + color: red + +.strict-bem__elem_key-val + color: red + +.strict-bem__elem_key--fail + color: red diff --git a/tests/sass/class-name-format.scss b/tests/sass/class-name-format.scss index 3a3af0c2..bbb2452e 100644 --- a/tests/sass/class-name-format.scss +++ b/tests/sass/class-name-format.scss @@ -116,3 +116,20 @@ } } } + +// Issue #872 - incorrect strict bem regex +.strict-bem__elem_bool { + color: red; +} + +.strict-bem__elem_bool-modifier { + color: red; +} + +.strict-bem__elem_key-val { + color: red; +} + +.strict-bem__elem_key--fail { + color: red; +} From 074e47b51d7b6b42878c37a49fd635f0fa6d37b7 Mon Sep 17 00:00:00 2001 From: Dan Purdy Date: Tue, 18 Oct 2016 00:41:25 +0100 Subject: [PATCH 17/67] :bug: fix strictbem *-name-format --- tests/helpers/isStrictBEM.js | 4 ++-- tests/rules/function-name-format.js | 4 ++-- tests/rules/mixin-name-format.js | 4 ++-- tests/rules/placeholder-name-format.js | 4 ++-- tests/rules/variable-name-format.js | 4 ++-- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/helpers/isStrictBEM.js b/tests/helpers/isStrictBEM.js index 6c474519..0fa85d39 100644 --- a/tests/helpers/isStrictBEM.js +++ b/tests/helpers/isStrictBEM.js @@ -89,11 +89,11 @@ describe('helpers - isStrictBEM', function () { done(); }); - it('isStrictBEM - [\'abc_def\' - false]', function (done) { + it('isStrictBEM - [\'abc_def\' - true]', function (done) { var result = helpers.isStrictBEM('abc_def'); - assert.equal(false, result); + assert.equal(true, result); done(); }); diff --git a/tests/rules/function-name-format.js b/tests/rules/function-name-format.js index ba2bb02a..beda9f51 100644 --- a/tests/rules/function-name-format.js +++ b/tests/rules/function-name-format.js @@ -65,7 +65,7 @@ describe('function name format - scss', function () { } ] }, function (data) { - lint.assert.equal(13, data.warningCount); + lint.assert.equal(10, data.warningCount); done(); }); }); @@ -177,7 +177,7 @@ describe('function name format - sass', function () { } ] }, function (data) { - lint.assert.equal(13, data.warningCount); + lint.assert.equal(10, data.warningCount); done(); }); }); diff --git a/tests/rules/mixin-name-format.js b/tests/rules/mixin-name-format.js index 3b57f45e..4c985802 100644 --- a/tests/rules/mixin-name-format.js +++ b/tests/rules/mixin-name-format.js @@ -65,7 +65,7 @@ describe('mixin name format - scss', function () { } ] }, function (data) { - lint.assert.equal(13, data.warningCount); + lint.assert.equal(9, data.warningCount); done(); }); }); @@ -177,7 +177,7 @@ describe('mixin name format - sass', function () { } ] }, function (data) { - lint.assert.equal(13, data.warningCount); + lint.assert.equal(9, data.warningCount); done(); }); }); diff --git a/tests/rules/placeholder-name-format.js b/tests/rules/placeholder-name-format.js index a5f8e40d..0d30033e 100644 --- a/tests/rules/placeholder-name-format.js +++ b/tests/rules/placeholder-name-format.js @@ -69,7 +69,7 @@ describe('placeholder name format - scss', function () { } ] }, function (data) { - lint.assert.equal(13, data.warningCount); + lint.assert.equal(9, data.warningCount); done(); }); }); @@ -185,7 +185,7 @@ describe('placeholder name format - sass', function () { } ] }, function (data) { - lint.assert.equal(13, data.warningCount); + lint.assert.equal(9, data.warningCount); done(); }); }); diff --git a/tests/rules/variable-name-format.js b/tests/rules/variable-name-format.js index 9d3cf406..3572d28f 100644 --- a/tests/rules/variable-name-format.js +++ b/tests/rules/variable-name-format.js @@ -69,7 +69,7 @@ describe('variable name format - scss', function () { } ] }, function (data) { - lint.assert.equal(12, data.warningCount); + lint.assert.equal(9, data.warningCount); done(); }); }); @@ -185,7 +185,7 @@ describe('variable name format - sass', function () { } ] }, function (data) { - lint.assert.equal(12, data.warningCount); + lint.assert.equal(9, data.warningCount); done(); }); }); From c8f2e6ce353b67cef6a3f6adefdd8e8795b074fc Mon Sep 17 00:00:00 2001 From: Dan Purdy Date: Wed, 19 Oct 2016 01:19:45 +0100 Subject: [PATCH 18/67] :art: Protect against unhandled error --- lib/rules/space-before-colon.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/space-before-colon.js b/lib/rules/space-before-colon.js index 0d82b468..f61791fa 100644 --- a/lib/rules/space-before-colon.js +++ b/lib/rules/space-before-colon.js @@ -14,7 +14,7 @@ module.exports = { if (delimiter.content === ':') { var previous = parent.content[i - 1]; - if (previous.is('space')) { + if (previous && previous.is('space')) { if (!parser.options.include) { result = helpers.addUnique(result, { 'ruleId': parser.rule.name, From 87e3f0d2dd0c4464cc876f6f65927a70f62a267e Mon Sep 17 00:00:00 2001 From: greenkeeperio-bot Date: Thu, 20 Oct 2016 05:59:28 -0400 Subject: [PATCH 19/67] chore(package): update gonzales-pe to version 3.4.5 https://greenkeeper.io/ --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 61f91546..646c5944 100644 --- a/package.json +++ b/package.json @@ -33,7 +33,7 @@ "fs-extra": "^0.30.0", "glob": "^7.0.0", "globule": "^1.0.0", - "gonzales-pe": "3.4.4", + "gonzales-pe": "3.4.5", "js-yaml": "^3.5.4", "lodash.capitalize": "^4.1.0", "lodash.kebabcase": "^4.0.0", From 48a7b7793c361e939091cffc8032c19d17085769 Mon Sep 17 00:00:00 2001 From: Dan Purdy Date: Fri, 21 Oct 2016 01:27:29 +0100 Subject: [PATCH 20/67] :art: Skip all front matter --- README.md | 18 ++++++++++++++++++ lib/groot.js | 8 +++++++- package.json | 1 + tests/main.js | 12 ++++++++++++ tests/sass/front-matter/front-matter.scss | 7 +++++++ 5 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 tests/sass/front-matter/front-matter.scss diff --git a/README.md b/README.md index aab7434a..5610ce1c 100644 --- a/README.md +++ b/README.md @@ -169,6 +169,24 @@ For further information you can visit our CLI documentation linked below. --- +## Front matter + +Certain static site generators such as [Jekyll](http://jekyllrb.com/docs/frontmatter/) include the YAML front matter block at the top of their scss file. Sass-lint by default checks a file for this block and attempts to parse your Sass without this front matter. You can see an example of a front matter block below. + +```scss + +--- +# Only the main Sass file needs front matter (the dashes are enough) +--- + +.test { + color: red; +} + +``` + +--- + ## Contributions We welcome all contributions to this project but please do read our [contribution guidelines](https://github.com/sasstools/sass-lint/blob/master/CONTRIBUTING.md) first, especially before opening a pull request. It would also be good to read our [code of conduct](https://github.com/sasstools/sass-lint/blob/master/CODE_OF_CONDUCT.md). diff --git a/lib/groot.js b/lib/groot.js index f3bad09e..986ce8b7 100644 --- a/lib/groot.js +++ b/lib/groot.js @@ -3,7 +3,8 @@ ////////////////////////////// 'use strict'; -var gonzales = require('gonzales-pe'); +var gonzales = require('gonzales-pe'), + fm = require('front-matter'); module.exports = function (text, syntax, filename) { var tree; @@ -11,6 +12,11 @@ module.exports = function (text, syntax, filename) { // Run `.toString()` to allow Buffers to be passed in text = text.toString(); + // if we're skipping front matter do it here, fall back to just our text in case it fails + if (fm.test(text)) { + text = fm(text).body || text; + } + try { tree = gonzales.parse(text, { 'syntax': syntax diff --git a/package.json b/package.json index 646c5944..318d03b2 100644 --- a/package.json +++ b/package.json @@ -30,6 +30,7 @@ "dependencies": { "commander": "^2.8.1", "eslint": "^2.7.0", + "front-matter": "2.1.0", "fs-extra": "^0.30.0", "glob": "^7.0.0", "globule": "^1.0.0", diff --git a/tests/main.js b/tests/main.js index 9c54feb2..05c7d9d6 100644 --- a/tests/main.js +++ b/tests/main.js @@ -159,6 +159,18 @@ describe('sass lint', function () { }); }); + // ============================================================================== + // Parse files with YAML front matter + // ============================================================================== + + it('should parse a file with front matter correctly and without parse error', function (done) { + lintFile('front-matter/front-matter.scss', function (data) { + assert.equal(0, data.errorCount); + assert.equal(2, data.warningCount); + done(); + }); + }); + // ============================================================================== // Parse Errors should return as lint errors // ============================================================================== diff --git a/tests/sass/front-matter/front-matter.scss b/tests/sass/front-matter/front-matter.scss new file mode 100644 index 00000000..93f12e57 --- /dev/null +++ b/tests/sass/front-matter/front-matter.scss @@ -0,0 +1,7 @@ +--- +# Only the main Sass file needs front matter (the dashes are enough) +--- + +.test { + color: red; +} From 36b3b8faa01bbadb704850eaf72d122fd4596292 Mon Sep 17 00:00:00 2001 From: Dan Purdy Date: Sat, 22 Oct 2016 13:06:50 +0100 Subject: [PATCH 21/67] :bug: Fix operators in variable names --- lib/rules/variable-name-format.js | 4 +++- tests/rules/variable-name-format.js | 16 ++++++++-------- tests/sass/variable-name-format.sass | 4 ++++ tests/sass/variable-name-format.scss | 5 +++++ 4 files changed, 20 insertions(+), 9 deletions(-) diff --git a/lib/rules/variable-name-format.js b/lib/rules/variable-name-format.js index 71e17238..9801e60d 100644 --- a/lib/rules/variable-name-format.js +++ b/lib/rules/variable-name-format.js @@ -16,7 +16,9 @@ module.exports = { ast.traverseByType('variable', function (variable) { var strippedName, violationMessage = false, - name = variable.first().content; + // Gonzales would parse variable names with operators without this regex i.e. '$mobile-site-gutter*1' + name = variable.first().content.split(/(\*|\+|\/)/)[0]; + strippedName = name; diff --git a/tests/rules/variable-name-format.js b/tests/rules/variable-name-format.js index 9d3cf406..470d8d91 100644 --- a/tests/rules/variable-name-format.js +++ b/tests/rules/variable-name-format.js @@ -27,7 +27,7 @@ describe('variable name format - scss', function () { } ] }, function (data) { - lint.assert.equal(15, data.warningCount); + lint.assert.equal(16, data.warningCount); done(); }); }); @@ -41,7 +41,7 @@ describe('variable name format - scss', function () { } ] }, function (data) { - lint.assert.equal(16, data.warningCount); + lint.assert.equal(17, data.warningCount); done(); }); }); @@ -55,7 +55,7 @@ describe('variable name format - scss', function () { } ] }, function (data) { - lint.assert.equal(10, data.warningCount); + lint.assert.equal(11, data.warningCount); done(); }); }); @@ -98,7 +98,7 @@ describe('variable name format - scss', function () { } ] }, function (data) { - lint.assert.equal(16, data.warningCount); + lint.assert.equal(17, data.warningCount); done(); }); }); @@ -143,7 +143,7 @@ describe('variable name format - sass', function () { } ] }, function (data) { - lint.assert.equal(15, data.warningCount); + lint.assert.equal(16, data.warningCount); done(); }); }); @@ -157,7 +157,7 @@ describe('variable name format - sass', function () { } ] }, function (data) { - lint.assert.equal(10, data.warningCount); + lint.assert.equal(11, data.warningCount); done(); }); }); @@ -171,7 +171,7 @@ describe('variable name format - sass', function () { } ] }, function (data) { - lint.assert.equal(16, data.warningCount); + lint.assert.equal(17, data.warningCount); done(); }); }); @@ -214,7 +214,7 @@ describe('variable name format - sass', function () { } ] }, function (data) { - lint.assert.equal(16, data.warningCount); + lint.assert.equal(17, data.warningCount); done(); }); }); diff --git a/tests/sass/variable-name-format.sass b/tests/sass/variable-name-format.sass index 3c3ce96f..9738a0a7 100644 --- a/tests/sass/variable-name-format.sass +++ b/tests/sass/variable-name-format.sass @@ -19,3 +19,7 @@ $_does_NOT-fitSTANDARD: 1 .class width: $snake_case + +// Issue #901 - operators not recognized as separate tokens by gonzales-pe +.content-main + padding: $mobile-site-gutter*1.5 diff --git a/tests/sass/variable-name-format.scss b/tests/sass/variable-name-format.scss index fd8df367..e04be3ae 100644 --- a/tests/sass/variable-name-format.scss +++ b/tests/sass/variable-name-format.scss @@ -20,3 +20,8 @@ $_does_NOT-fitSTANDARD: 1; .class { width: $snake_case; } + +// Issue #901 - operators not recognized as separate tokens by gonzales-pe +.content-main { + padding: $mobile-site-gutter*1.5; +} From bbb5507d4d459acc270a7c287db1cd74a3dcb6ab Mon Sep 17 00:00:00 2001 From: greenkeeperio-bot Date: Sat, 22 Oct 2016 14:33:01 -0700 Subject: [PATCH 22/67] chore(package): update gonzales-pe to version 3.4.7 https://greenkeeper.io/ --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 646c5944..7c204593 100644 --- a/package.json +++ b/package.json @@ -33,7 +33,7 @@ "fs-extra": "^0.30.0", "glob": "^7.0.0", "globule": "^1.0.0", - "gonzales-pe": "3.4.5", + "gonzales-pe": "3.4.7", "js-yaml": "^3.5.4", "lodash.capitalize": "^4.1.0", "lodash.kebabcase": "^4.0.0", From 987e096f9781aabb53b5927315eb757100afef17 Mon Sep 17 00:00:00 2001 From: James Riley Date: Mon, 24 Oct 2016 11:26:16 +0100 Subject: [PATCH 23/67] clearer readme for configuring a rule --- README.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index aab7434a..8a03182d 100644 --- a/README.md +++ b/README.md @@ -79,13 +79,14 @@ For all [rules](https://github.com/sasstools/sass-lint/tree/master/docs/rules), If you want to configure options, set the rule to an array, where the first item in the array is the severity, and the second item in the array is an object including the options you would like to set. -An example configuration of a rule with options look like the following: +Here an is example configuration of a rule, where we are specifying that breaking the [indentation rule](https://github.com/sasstools/sass-lint/blob/master/docs/rules/indentation.md) should be treated as an error (its severity set to two), and setting the `size` option of the rule to 2 spaces: ```yml -indentation: - - 2 - - - size: 2 +rules: + indentation: + - 2 + - + size: 2 ``` ### [Rules Documentation](https://github.com/sasstools/sass-lint/tree/master/docs/rules) From 31421049978e48cd3a9e51a41a71cdd2ba8fc425 Mon Sep 17 00:00:00 2001 From: Dan Purdy Date: Tue, 25 Oct 2016 09:26:19 +0100 Subject: [PATCH 24/67] Remove gonzales patch --- lib/rules/variable-name-format.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/rules/variable-name-format.js b/lib/rules/variable-name-format.js index 9801e60d..2ea0577d 100644 --- a/lib/rules/variable-name-format.js +++ b/lib/rules/variable-name-format.js @@ -16,8 +16,7 @@ module.exports = { ast.traverseByType('variable', function (variable) { var strippedName, violationMessage = false, - // Gonzales would parse variable names with operators without this regex i.e. '$mobile-site-gutter*1' - name = variable.first().content.split(/(\*|\+|\/)/)[0]; + name = variable.first().content; strippedName = name; From 3e05a346f254c515c77b1b1e17b6bfdd173877e5 Mon Sep 17 00:00:00 2001 From: Ben Griffith Date: Mon, 24 Oct 2016 20:15:25 +0100 Subject: [PATCH 25/67] :bug: Correct cli tests to use bin --- tests/cli.js | 52 ++++++++++++++++++++++++++-------------------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/tests/cli.js b/tests/cli.js index a3d6698d..8df2996e 100644 --- a/tests/cli.js +++ b/tests/cli.js @@ -6,7 +6,7 @@ var assert = require('assert'), describe('cli', function () { it('should return help instructions', function (done) { - var command = 'sass-lint -h'; + var command = 'node bin/sass-lint -h'; exec(command, function (err, stdout) { if (err) { @@ -20,7 +20,7 @@ describe('cli', function () { }); it('should return a version', function (done) { - var command = 'sass-lint --version'; + var command = 'node bin/sass-lint --version'; exec(command, function (err, stdout) { if (err) { @@ -34,7 +34,7 @@ describe('cli', function () { }); it('should not try to read and lint a directory', function (done) { - var command = 'sass-lint "tests/dir-test/**/*.scss" --no-exit --verbose --format json'; + var command = 'node bin/sass-lint "tests/dir-test/**/*.scss" --no-exit --verbose --format json'; exec(command, function (err, stdout) { var result = JSON.parse(stdout); @@ -52,7 +52,7 @@ describe('cli', function () { }); it('Should accept multiple input paths', function (done) { - var command = 'sass-lint "tests/cli/cli-error.scss, tests/cli/cli-error.sass" --no-exit --verbose'; + var command = 'node bin/sass-lint "tests/cli/cli-error.scss, tests/cli/cli-error.sass" --no-exit --verbose'; exec(command, function (err, stdout) { @@ -68,7 +68,7 @@ describe('cli', function () { }); it('Should accept multiple input globs', function (done) { - var command = 'sass-lint "tests/cli/*.scss, tests/cli/*.sass" --no-exit --verbose'; + var command = 'node bin/sass-lint "tests/cli/*.scss, tests/cli/*.sass" --no-exit --verbose'; exec(command, function (err, stdout) { @@ -84,7 +84,7 @@ describe('cli', function () { }); it('Should accept multiple input paths from a config file', function (done) { - var command = 'sass-lint -c tests/yml/.multiple-inputs.yml --no-exit --verbose'; + var command = 'node bin/sass-lint -c tests/yml/.multiple-inputs.yml --no-exit --verbose'; exec(command, function (err, stdout) { @@ -100,7 +100,7 @@ describe('cli', function () { }); it('Should accept multiple input paths and multiple ignore paths', function (done) { - var command = 'sass-lint "tests/cli/cli-error.scss, tests/cli/cli-error.sass" -i "tests/cli/cli-error.scss, tests/cli/cli-error.sass" --no-exit --verbose'; + var command = 'node bin/sass-lint "tests/cli/cli-error.scss, tests/cli/cli-error.sass" -i "tests/cli/cli-error.scss, tests/cli/cli-error.sass" --no-exit --verbose'; exec(command, function (err, stdout) { @@ -116,7 +116,7 @@ describe('cli', function () { }); it('Should accept multiple input paths and multiple ignores from a config file', function (done) { - var command = 'sass-lint -c tests/yml/.multiple-ignores.yml --no-exit --verbose'; + var command = 'node bin/sass-lint -c tests/yml/.multiple-ignores.yml --no-exit --verbose'; exec(command, function (err, stdout) { @@ -131,7 +131,7 @@ describe('cli', function () { }); it('CLI format option should output JSON', function (done) { - var command = 'sass-lint -c tests/yml/.stylish-output.yml tests/cli/cli.scss --verbose --format json'; + var command = 'node bin/sass-lint -c tests/yml/.stylish-output.yml tests/cli/cli.scss --verbose --format json'; exec(command, function (err, stdout) { @@ -151,7 +151,7 @@ describe('cli', function () { }); it('CLI output option should write to test file', function (done) { - var command = 'sass-lint -c tests/yml/.stylish-output.yml tests/cli/cli.scss --verbose --format json --output tests/cli-output.json', + var command = 'node bin/sass-lint -c tests/yml/.stylish-output.yml tests/cli/cli.scss --verbose --format json --output tests/cli-output.json', outputFile = path.resolve(process.cwd(), 'tests/cli-output.json'); exec(command, function (err) { @@ -174,7 +174,7 @@ describe('cli', function () { }); it('CLI output option should write JSON to test file', function (done) { - var command = 'sass-lint -c tests/yml/.stylish-output.yml tests/cli/cli.scss --verbose --format json --output tests/cli-output.json', + var command = 'node bin/sass-lint -c tests/yml/.stylish-output.yml tests/cli/cli.scss --verbose --format json --output tests/cli-output.json', outputFile = path.resolve(process.cwd(), 'tests/cli-output.json'); exec(command, function (err) { @@ -207,7 +207,7 @@ describe('cli', function () { }); it('CLI output option should write JSON to test file when upper case format is used', function (done) { - var command = 'sass-lint -c tests/yml/.stylish-output.yml tests/cli/cli.scss --verbose --format JSON --output tests/cli-output.json', + var command = 'node bin/sass-lint -c tests/yml/.stylish-output.yml tests/cli/cli.scss --verbose --format JSON --output tests/cli-output.json', outputFile = path.resolve(process.cwd(), 'tests/cli-output.json'); exec(command, function (err) { @@ -242,7 +242,7 @@ describe('cli', function () { // Test custom config path it('should return JSON from a custom config', function (done) { - var command = 'sass-lint -c tests/yml/.color-keyword-errors.yml tests/cli/cli.scss --verbose'; + var command = 'node bin/sass-lint -c tests/yml/.color-keyword-errors.yml tests/cli/cli.scss --verbose'; exec(command, function (err, stdout) { @@ -264,7 +264,7 @@ describe('cli', function () { // Test 0 errors/warnings when rules set to 0 in config it('output should return no errors/warnings', function (done) { - var command = 'sass-lint -c tests/yml/.json-lint.yml tests/cli/cli.scss --verbose'; + var command = 'node bin/sass-lint -c tests/yml/.json-lint.yml tests/cli/cli.scss --verbose'; exec(command, function (err, stdout) { @@ -286,7 +286,7 @@ describe('cli', function () { // Test 1 warning when rules set to 0 in config it('should return a warning', function (done) { - var command = 'sass-lint -c tests/yml/.color-keyword-errors.yml tests/cli/cli.scss --verbose'; + var command = 'node bin/sass-lint -c tests/yml/.color-keyword-errors.yml tests/cli/cli.scss --verbose'; exec(command, function (err, stdout) { @@ -315,7 +315,7 @@ describe('cli', function () { }); it('should return a warning - stylish', function (done) { - var command = 'sass-lint -c tests/yml/.stylish-errors.yml tests/cli/cli.scss --verbose', + var command = 'node bin/sass-lint -c tests/yml/.stylish-errors.yml tests/cli/cli.scss --verbose', expectedOutputLength = 154; exec(command, function (err, stdout) { @@ -332,7 +332,7 @@ describe('cli', function () { }); it('should not include ignored paths', function (done) { - var command = 'sass-lint -i "**/*.scss" -v -q --format json "**/cli/*.scss"'; + var command = 'node bin/sass-lint -i "**/*.scss" -v -q --format json "**/cli/*.scss"'; exec(command, function (err, stdout) { @@ -347,7 +347,7 @@ describe('cli', function () { }); it('should not include multiple ignored paths', function (done) { - var command = 'sass-lint -i "**/*.scss, **/*.sass" -q -v --format json'; + var command = 'node bin/sass-lint -i "**/*.scss, **/*.sass" -q -v --format json'; exec(command, function (err, stdout) { @@ -363,7 +363,7 @@ describe('cli', function () { }); it('should override filename convention if a valid --syntax is provided', function (done) { - var command = 'sass-lint --syntax scss tests/cli/cli.txt --verbose --format json'; + var command = 'node bin/sass-lint --syntax scss tests/cli/cli.txt --verbose --format json'; exec(command, function (err, stdout) { @@ -384,7 +384,7 @@ describe('cli', function () { }); it('should exit with error when quiet', function (done) { - var command = 'sass-lint -c tests/yml/.error-output.yml tests/cli/cli-error.scss --verbose --no-exit'; + var command = 'node bin/sass-lint -c tests/yml/.error-output.yml tests/cli/cli-error.scss --verbose --no-exit'; exec(command, function (err) { if (err) { @@ -396,7 +396,7 @@ describe('cli', function () { }); it('should exit with error when more warnings than --max-warnings', function (done) { - var command = 'sass-lint -c tests/yml/.color-keyword-errors.yml tests/cli/cli.scss --max-warnings 0'; + var command = 'node bin/sass-lint -c tests/yml/.color-keyword-errors.yml tests/cli/cli.scss --max-warnings 0'; exec(command, function (err) { if (err) { @@ -408,7 +408,7 @@ describe('cli', function () { }); it('should not exit with an error if no config is specified', function (done) { - var command = 'sass-lint tests/cli/cli-clean.scss --verbose --no-exit'; + var command = 'node bin/sass-lint tests/cli/cli-clean.scss --verbose --no-exit'; exec(command, function (err) { if (!err) { @@ -423,7 +423,7 @@ describe('cli', function () { * We disabled eslints handle callback err rule here as we are deliberately throwing errors that we don't care about */ it('parse errors should report as a lint error', function (done) { - var command = 'sass-lint --config tests/yml/.stylish-output.yml tests/sass/parse.scss --verbose --no-exit --format json'; + var command = 'node bin/sass-lint --config tests/yml/.stylish-output.yml tests/sass/parse.scss --verbose --no-exit --format json'; exec(command, function (err, stdout) { // eslint-disable-line handle-callback-err var result = JSON.parse(stdout)[0]; @@ -434,7 +434,7 @@ describe('cli', function () { }); it('parse errors should report as severity 2', function (done) { - var command = 'sass-lint --config tests/yml/.stylish-output.yml tests/sass/parse.scss --verbose --no-exit --format json'; + var command = 'node bin/sass-lint --config tests/yml/.stylish-output.yml tests/sass/parse.scss --verbose --no-exit --format json'; exec(command, function (err, stdout) { // eslint-disable-line handle-callback-err var result = JSON.parse(stdout)[0], @@ -447,7 +447,7 @@ describe('cli', function () { }); it('parse errors should report the correct message', function (done) { - var command = 'sass-lint --config tests/yml/.stylish-output.yml tests/sass/parse.scss --verbose --no-exit --format json'; + var command = 'node bin/sass-lint --config tests/yml/.stylish-output.yml tests/sass/parse.scss --verbose --no-exit --format json'; exec(command, function (err, stdout) { // eslint-disable-line handle-callback-err var result = JSON.parse(stdout)[0], @@ -460,7 +460,7 @@ describe('cli', function () { }); it('parse errors rule Id should be \'Fatal\'', function (done) { - var command = 'sass-lint --config tests/yml/.stylish-output.yml tests/sass/parse.scss --verbose --no-exit --format json'; + var command = 'node bin/sass-lint --config tests/yml/.stylish-output.yml tests/sass/parse.scss --verbose --no-exit --format json'; exec(command, function (err, stdout) { // eslint-disable-line handle-callback-err var result = JSON.parse(stdout)[0], From 55852163b0f38cfb54c3d2cca15d8c933206307a Mon Sep 17 00:00:00 2001 From: Ben Griffith Date: Tue, 25 Oct 2016 09:58:23 +0100 Subject: [PATCH 26/67] :fire: Remove link command in travis and appveyor configs --- .travis.yml | 1 - appveyor.yml | 2 -- 2 files changed, 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 2268ba40..d1b77185 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,7 +6,6 @@ node_js: - '5' - node - iojs -before_script: npm link after_success: npm run coveralls notifications: slack: diff --git a/appveyor.yml b/appveyor.yml index 22ade15c..8bab6ec5 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -24,8 +24,6 @@ install: - ps: Install-Product node $env:nodejs_version # install modules - npm install - # link - - npm link # Post-install test scripts. test_script: From c448971ab59e7a73ab7146f581bed67d0ab44690 Mon Sep 17 00:00:00 2001 From: James Riley Date: Tue, 25 Oct 2016 10:44:56 +0100 Subject: [PATCH 27/67] typo in readme --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 2be3db62..5e43b27e 100644 --- a/README.md +++ b/README.md @@ -79,7 +79,7 @@ For all [rules](https://github.com/sasstools/sass-lint/tree/master/docs/rules), If you want to configure options, set the rule to an array, where the first item in the array is the severity, and the second item in the array is an object including the options you would like to set. -Here an is example configuration of a rule, where we are specifying that breaking the [indentation rule](https://github.com/sasstools/sass-lint/blob/master/docs/rules/indentation.md) should be treated as an error (its severity set to two), and setting the `size` option of the rule to 2 spaces: +Here is an example configuration of a rule, where we are specifying that breaking the [indentation rule](https://github.com/sasstools/sass-lint/blob/master/docs/rules/indentation.md) should be treated as an error (its severity set to two), and setting the `size` option of the rule to 2 spaces: ```yml rules: From 3aada68bae9a856de840e0cac0c49a33ebbe9473 Mon Sep 17 00:00:00 2001 From: Richard de Wit Date: Mon, 17 Oct 2016 15:48:37 +0200 Subject: [PATCH 28/67] :bug: Fix false positive in border-zero This fixes a false positive when `convention` is set to the integer `0` (as opposed to the string, which does work), by casting the `convention` to a string. Fixes #861 --- lib/rules/border-zero.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/border-zero.js b/lib/rules/border-zero.js index 9fdc9d74..52506349 100644 --- a/lib/rules/border-zero.js +++ b/lib/rules/border-zero.js @@ -29,7 +29,7 @@ module.exports = { var node = item.content[0]; if (node.type === 'number' || node.type === 'ident') { if (node.content === '0' || node.content === 'none') { - if (parser.options.convention !== node.content) { + if (('' + parser.options.convention) !== node.content) { result = helpers.addUnique(result, { 'ruleId': parser.rule.name, 'line': node.start.line, From b291be86d746fe551185535579a23e8ead3dabd0 Mon Sep 17 00:00:00 2001 From: Ben Griffith Date: Wed, 26 Oct 2016 17:40:04 +0100 Subject: [PATCH 29/67] :art: Improve clarity of solution --- lib/rules/border-zero.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/border-zero.js b/lib/rules/border-zero.js index 52506349..3f3d2b54 100644 --- a/lib/rules/border-zero.js +++ b/lib/rules/border-zero.js @@ -29,7 +29,7 @@ module.exports = { var node = item.content[0]; if (node.type === 'number' || node.type === 'ident') { if (node.content === '0' || node.content === 'none') { - if (('' + parser.options.convention) !== node.content) { + if (parser.options.convention.toString() !== node.content) { result = helpers.addUnique(result, { 'ruleId': parser.rule.name, 'line': node.start.line, From 569b38f450692128c0a68ea3b57f7da4841e74bd Mon Sep 17 00:00:00 2001 From: Ben Griffith Date: Wed, 26 Oct 2016 17:40:33 +0100 Subject: [PATCH 30/67] :white_check_mark: Add extra tests for non-string value --- tests/rules/border-zero.js | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/rules/border-zero.js b/tests/rules/border-zero.js index da2c280a..21080c0b 100644 --- a/tests/rules/border-zero.js +++ b/tests/rules/border-zero.js @@ -9,6 +9,20 @@ describe('border zero - scss', function () { var file = lint.file('border-zero.scss'); it('[convention: 0]', function (done) { + lint.test(file, { + 'border-zero': [ + 1, + { + 'convention': 0 + } + ] + }, function (data) { + lint.assert.equal(3, data.warningCount); + done(); + }); + }); + + it('[convention: \'0\']', function (done) { lint.test(file, { 'border-zero': 1 }, function (data) { @@ -39,6 +53,20 @@ describe('border zero - sass', function () { var file = lint.file('border-zero.sass'); it('[convention: 0]', function (done) { + lint.test(file, { + 'border-zero': [ + 1, + { + 'convention': 0 + } + ] + }, function (data) { + lint.assert.equal(3, data.warningCount); + done(); + }); + }); + + it('[convention: \'0\']', function (done) { lint.test(file, { 'border-zero': 1 }, function (data) { From a13c755738b3ec99a95c76e161383d021fc8fdab Mon Sep 17 00:00:00 2001 From: Dan Purdy Date: Wed, 19 Oct 2016 00:42:05 +0100 Subject: [PATCH 31/67] :bug: Fix BOM markers causing parse issues --- lib/groot.js | 7 ++-- package.json | 1 + .../bom-utf8/starts-with-mixin-utf8-bom.scss | 5 +++ tests/bom-utf8/var-utf8-bom.scss | 5 +++ tests/cli.js | 40 +++++++++++++++++++ 5 files changed, 55 insertions(+), 3 deletions(-) create mode 100644 tests/bom-utf8/starts-with-mixin-utf8-bom.scss create mode 100644 tests/bom-utf8/var-utf8-bom.scss diff --git a/lib/groot.js b/lib/groot.js index 986ce8b7..6ccdfe61 100644 --- a/lib/groot.js +++ b/lib/groot.js @@ -3,14 +3,15 @@ ////////////////////////////// 'use strict'; -var gonzales = require('gonzales-pe'), - fm = require('front-matter'); +var gonzales = require('gonzales-pe'); +var fm = require('front-matter'); +var stripBom = require('strip-bom'); module.exports = function (text, syntax, filename) { var tree; // Run `.toString()` to allow Buffers to be passed in - text = text.toString(); + text = stripBom(text.toString()); // if we're skipping front matter do it here, fall back to just our text in case it fails if (fm.test(text)) { diff --git a/package.json b/package.json index 4cf4c0d9..2676b94c 100644 --- a/package.json +++ b/package.json @@ -40,6 +40,7 @@ "lodash.kebabcase": "^4.0.0", "merge": "^1.2.0", "path-is-absolute": "^1.0.0", + "strip-bom": "^3.0.0", "util": "^0.10.3" }, "devDependencies": { diff --git a/tests/bom-utf8/starts-with-mixin-utf8-bom.scss b/tests/bom-utf8/starts-with-mixin-utf8-bom.scss new file mode 100644 index 00000000..4f12e183 --- /dev/null +++ b/tests/bom-utf8/starts-with-mixin-utf8-bom.scss @@ -0,0 +1,5 @@ +@mixin foo( + $param: 'def' +) { + color: red; +} diff --git a/tests/bom-utf8/var-utf8-bom.scss b/tests/bom-utf8/var-utf8-bom.scss new file mode 100644 index 00000000..bc5580da --- /dev/null +++ b/tests/bom-utf8/var-utf8-bom.scss @@ -0,0 +1,5 @@ +$my-color: red; + +body { + color: $my-color; +} \ No newline at end of file diff --git a/tests/cli.js b/tests/cli.js index 8df2996e..e5389f50 100644 --- a/tests/cli.js +++ b/tests/cli.js @@ -472,3 +472,43 @@ describe('cli', function () { }); }); }); + +// ============================================================================== +// UTF-8 BOM +// ============================================================================== + +describe('Reading files with UTF-8 BOM', function () { + before(function () { + var testText = fs.readFileSync('tests/bom-utf8/starts-with-mixin-utf8-bom.scss').toString(); + if (testText.charCodeAt(0) !== 0xFEFF) { + throw new Error('BOM test files have no BOM markers'); + } + }); + + it('should not throw a parse error from file containing a BOM', function (done) { + var command = 'sass-lint -v tests/bom-utf8/starts-with-mixin-utf8-bom.scss --format json'; + + exec(command, function (err, stdout) { // eslint-disable-line handle-callback-err + var result = JSON.parse(stdout)[0]; + + // Files with BOM markers that start with a mixing throw a fatal error + // https://github.com/sasstools/sass-lint/issues/880 + assert.equal(result.errorCount, 0); + done(); + }); + }); + + it('should return the correct amount of warnings from a file containing BOM markers', function (done) { + var command = 'sass-lint -v tests/bom-utf8/var-utf8-bom.scss --format json'; + + exec(command, function (err, stdout) { // eslint-disable-line handle-callback-err + var result = JSON.parse(stdout)[0]; + + // Files starting with variables threw extra errors + // see https://github.com/sasstools/sass-lint/issues/880 + assert.equal(result.errorCount, 0); + assert.equal(result.warningCount, 2); + done(); + }); + }); +}); From 14c526831a4beb9ebcb4d1289584e5a45b20386e Mon Sep 17 00:00:00 2001 From: Dan Purdy Date: Wed, 19 Oct 2016 01:07:42 +0100 Subject: [PATCH 32/67] :white_check_mark: add helper tests for strip BOM --- lib/groot.js | 4 ++-- lib/helpers.js | 20 ++++++++++++++++++++ tests/helpers/stripBom.js | 28 ++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 2 deletions(-) create mode 100644 tests/helpers/stripBom.js diff --git a/lib/groot.js b/lib/groot.js index 6ccdfe61..1d2ee529 100644 --- a/lib/groot.js +++ b/lib/groot.js @@ -5,13 +5,13 @@ var gonzales = require('gonzales-pe'); var fm = require('front-matter'); -var stripBom = require('strip-bom'); +var helpers = require('./helpers'); module.exports = function (text, syntax, filename) { var tree; // Run `.toString()` to allow Buffers to be passed in - text = stripBom(text.toString()); + text = helpers.stripBom(text.toString()); // if we're skipping front matter do it here, fall back to just our text in case it fails if (fm.test(text)) { diff --git a/lib/helpers.js b/lib/helpers.js index d2cb3306..0b9af3ce 100644 --- a/lib/helpers.js +++ b/lib/helpers.js @@ -394,4 +394,24 @@ helpers.isPartialStringMatch = function (needle, haystack) { return false; }; +/** + * A copy of the the stripBom module from https://github.com/sindresorhus/strip-bom/blob/master/index.js + * The module requires node > 4 whereas we support earlier versions. + * This function strips the BOM marker from the beginning of a file + * + * @param {string} str - The string we wish to strip the BOM marker from + * @returns {string} The string without a BOM marker + */ +helpers.stripBom = function (str) { + if (typeof str !== 'string') { + throw new TypeError('Expected a string, got ' + typeof str); + } + + if (str.charCodeAt(0) === 0xFEFF) { + return str.slice(1); + } + + return str; +}; + module.exports = helpers; diff --git a/tests/helpers/stripBom.js b/tests/helpers/stripBom.js new file mode 100644 index 00000000..523e0266 --- /dev/null +++ b/tests/helpers/stripBom.js @@ -0,0 +1,28 @@ +'use strict'; + +var assert = require('assert'), + helpers = require('../../lib/helpers'), + fs = require('fs'); + +describe('helpers - stripBom', function () { + + ////////////////////////////// + // Strip BOM + ////////////////////////////// + + it('should remove the BOM marker', function (done) { + var file = fs.readFileSync('tests/bom-utf8/starts-with-mixin-utf8-bom.scss').toString(); + assert.equal(file.charCodeAt(0), 0xFEFF); + assert.notEqual(helpers.stripBom(file).charCodeAt(0), 0xFEFF); + done(); + }); + + it('should throw an error if not passed a string', function (done) { + assert.throws( + function () { + helpers.stripBom(8); + }, Error + ); + done(); + }); +}); From 5bbbc26f158701ad6ca1d365511319668c93dac0 Mon Sep 17 00:00:00 2001 From: Dan Purdy Date: Thu, 20 Oct 2016 00:49:40 +0100 Subject: [PATCH 33/67] Remove unnecessary package --- package.json | 1 - 1 file changed, 1 deletion(-) diff --git a/package.json b/package.json index 2676b94c..4cf4c0d9 100644 --- a/package.json +++ b/package.json @@ -40,7 +40,6 @@ "lodash.kebabcase": "^4.0.0", "merge": "^1.2.0", "path-is-absolute": "^1.0.0", - "strip-bom": "^3.0.0", "util": "^0.10.3" }, "devDependencies": { From 10037082aeff5578c813b46fad49844e08058a51 Mon Sep 17 00:00:00 2001 From: Dan Purdy Date: Thu, 27 Oct 2016 13:26:10 +0100 Subject: [PATCH 34/67] :art: Update tests to latest pattern --- tests/cli.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/cli.js b/tests/cli.js index e5389f50..bd8130be 100644 --- a/tests/cli.js +++ b/tests/cli.js @@ -486,7 +486,7 @@ describe('Reading files with UTF-8 BOM', function () { }); it('should not throw a parse error from file containing a BOM', function (done) { - var command = 'sass-lint -v tests/bom-utf8/starts-with-mixin-utf8-bom.scss --format json'; + var command = 'node bin/sass-lint -v tests/bom-utf8/starts-with-mixin-utf8-bom.scss --format json'; exec(command, function (err, stdout) { // eslint-disable-line handle-callback-err var result = JSON.parse(stdout)[0]; @@ -499,7 +499,7 @@ describe('Reading files with UTF-8 BOM', function () { }); it('should return the correct amount of warnings from a file containing BOM markers', function (done) { - var command = 'sass-lint -v tests/bom-utf8/var-utf8-bom.scss --format json'; + var command = 'node bin/sass-lint -v tests/bom-utf8/var-utf8-bom.scss --format json'; exec(command, function (err, stdout) { // eslint-disable-line handle-callback-err var result = JSON.parse(stdout)[0]; From 8ec572774688ef69e3bd502757217206a7261b56 Mon Sep 17 00:00:00 2001 From: Dan Purdy Date: Thu, 25 Aug 2016 02:48:28 +0100 Subject: [PATCH 35/67] :mag: Add max-file-line-count rule --- docs/rules/max-file-line-count.md | 27 +++ lib/config/sass-lint.yml | 1 + lib/rules/max-file-line-count.js | 48 +++++ tests/rules/max-file-line-count.js | 63 ++++++ tests/sass/max-file-line-count.sass | 301 ++++++++++++++++++++++++++++ tests/sass/max-file-line-count.scss | 301 ++++++++++++++++++++++++++++ 6 files changed, 741 insertions(+) create mode 100644 docs/rules/max-file-line-count.md create mode 100644 lib/rules/max-file-line-count.js create mode 100644 tests/rules/max-file-line-count.js create mode 100644 tests/sass/max-file-line-count.sass create mode 100644 tests/sass/max-file-line-count.scss diff --git a/docs/rules/max-file-line-count.md b/docs/rules/max-file-line-count.md new file mode 100644 index 00000000..a121b4cf --- /dev/null +++ b/docs/rules/max-file-line-count.md @@ -0,0 +1,27 @@ +# Max File Line Count + +Rule `max-file-line-count` will enforce that a files length doesn't exceed a certain number of lines + +## Options + +* `length`: `number`, (defaults to 300) + +## Examples + +When enabled, the following are disallowed: + +```scss +/* +* line count is represented along the +* left hand side of the following example +*/ + 1| .test { + 2| color: red + 3| } +===== +~ snip ~ +===== +299| .bar { +300| color: blue; +301| } +``` diff --git a/lib/config/sass-lint.yml b/lib/config/sass-lint.yml index 36ce1470..3298df52 100644 --- a/lib/config/sass-lint.yml +++ b/lib/config/sass-lint.yml @@ -70,6 +70,7 @@ rules: indentation: 1 leading-zero: 1 max-line-length: 0 + max-file-line-count: 0 nesting-depth: 1 property-sort-order: 1 pseudo-element: 1 diff --git a/lib/rules/max-file-line-count.js b/lib/rules/max-file-line-count.js new file mode 100644 index 00000000..3bad530c --- /dev/null +++ b/lib/rules/max-file-line-count.js @@ -0,0 +1,48 @@ +'use strict'; + +var helpers = require('../helpers'); + +/** + * Get the 'last' node of the tree + * + * @param {Object} node - The node whose last child we want to return + * @returns {Object} The last node + */ +var getLastNode = function (node) { + var last = node.last(); + + return last ? getLastNode(last) : node; +}; + +module.exports = { + 'name': 'max-file-line-count', + 'defaults': { + length: 300 + }, + 'detect': function (ast, parser) { + var result = []; + var last; + + // If the syntax is Sass we must recursively loop to determine the last node. + // This is not required for SCSS which will always use the last node in the + // content of the parent stylesheet node + if (ast.syntax === 'sass') { + last = getLastNode(ast); + } + else { + last = ast.content[ast.content.length - 1]; + } + + if (last.end.line > parser.options.length) { + result = helpers.addUnique(result, { + 'ruleId': parser.rule.name, + 'line': last.end.line, + 'column': 0, + 'message': 'This file has ' + last.end.line + ' lines, which exceeds the maximum of ' + parser.options.length + ' lines allowed.', + 'severity': parser.severity + }); + } + + return result; + } +}; diff --git a/tests/rules/max-file-line-count.js b/tests/rules/max-file-line-count.js new file mode 100644 index 00000000..89a8690f --- /dev/null +++ b/tests/rules/max-file-line-count.js @@ -0,0 +1,63 @@ +'use strict'; + +var lint = require('./_lint'); + +////////////////////////////// +// SCSS syntax tests +////////////////////////////// +describe('max-file-line-count - scss', function () { + var file = lint.file('max-file-line-count.scss'); + + it('enforce [default]', function (done) { + lint.test(file, { + 'max-file-line-count': 1 + }, function (data) { + lint.assert.equal(1, data.warningCount); + done(); + }); + }); + + it('enforce [length: 3000]', function (done) { + lint.test(file, { + 'max-file-line-count': [ + 1, + { + length: 3000 + } + ] + }, function (data) { + lint.assert.equal(0, data.warningCount); + done(); + }); + }); +}); + +////////////////////////////// +// Sass syntax tests +////////////////////////////// +describe('max-file-line-count - sass', function () { + var file = lint.file('max-file-line-count.sass'); + + it('enforce', function (done) { + lint.test(file, { + 'max-file-line-count': 1 + }, function (data) { + lint.assert.equal(1, data.warningCount); + done(); + }); + }); + + it('enforce [length: 3000]', function (done) { + lint.test(file, { + 'max-file-line-count': [ + 1, + { + length: 3000 + } + ] + }, function (data) { + lint.assert.equal(0, data.warningCount); + done(); + }); + }); +}); diff --git a/tests/sass/max-file-line-count.sass b/tests/sass/max-file-line-count.sass new file mode 100644 index 00000000..26d1540f --- /dev/null +++ b/tests/sass/max-file-line-count.sass @@ -0,0 +1,301 @@ +// ============================================ +// +// Really +// long +// comment +// for +// padding +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +.test + .other + .foo + .bar + .baz + .qux + color: red diff --git a/tests/sass/max-file-line-count.scss b/tests/sass/max-file-line-count.scss new file mode 100644 index 00000000..6319aa21 --- /dev/null +++ b/tests/sass/max-file-line-count.scss @@ -0,0 +1,301 @@ +// ============================================ +// +// Really +// long +// comment +// for +// padding +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +// +.test { + .other { + .foo { + .bar { + .baz { + .qux { + color: red; + } + } + } + } + } +} From 33c57c3ee86aa583f662989cc81991c3e720a5ce Mon Sep 17 00:00:00 2001 From: Dan Purdy Date: Wed, 26 Oct 2016 17:33:48 +0100 Subject: [PATCH 36/67] Fix typo --- docs/rules/max-file-line-count.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rules/max-file-line-count.md b/docs/rules/max-file-line-count.md index a121b4cf..c588a461 100644 --- a/docs/rules/max-file-line-count.md +++ b/docs/rules/max-file-line-count.md @@ -1,6 +1,6 @@ # Max File Line Count -Rule `max-file-line-count` will enforce that a files length doesn't exceed a certain number of lines +Rule `max-file-line-count` will enforce that a file's length doesn't exceed a certain number of lines ## Options From 8167edec570644d1a28f34da8a0d1912fc85fa44 Mon Sep 17 00:00:00 2001 From: Dan Purdy Date: Wed, 26 Oct 2016 17:35:29 +0100 Subject: [PATCH 37/67] :art: Simplify rule functionality --- lib/rules/max-file-line-count.js | 29 +++-------------------------- 1 file changed, 3 insertions(+), 26 deletions(-) diff --git a/lib/rules/max-file-line-count.js b/lib/rules/max-file-line-count.js index 3bad530c..5cc08125 100644 --- a/lib/rules/max-file-line-count.js +++ b/lib/rules/max-file-line-count.js @@ -2,18 +2,6 @@ var helpers = require('../helpers'); -/** - * Get the 'last' node of the tree - * - * @param {Object} node - The node whose last child we want to return - * @returns {Object} The last node - */ -var getLastNode = function (node) { - var last = node.last(); - - return last ? getLastNode(last) : node; -}; - module.exports = { 'name': 'max-file-line-count', 'defaults': { @@ -21,24 +9,13 @@ module.exports = { }, 'detect': function (ast, parser) { var result = []; - var last; - - // If the syntax is Sass we must recursively loop to determine the last node. - // This is not required for SCSS which will always use the last node in the - // content of the parent stylesheet node - if (ast.syntax === 'sass') { - last = getLastNode(ast); - } - else { - last = ast.content[ast.content.length - 1]; - } - if (last.end.line > parser.options.length) { + if (ast.end.line > parser.options.length) { result = helpers.addUnique(result, { 'ruleId': parser.rule.name, - 'line': last.end.line, + 'line': ast.end.line, 'column': 0, - 'message': 'This file has ' + last.end.line + ' lines, which exceeds the maximum of ' + parser.options.length + ' lines allowed.', + 'message': 'This file has ' + ast.end.line + ' lines, which exceeds the maximum of ' + parser.options.length + ' lines allowed.', 'severity': parser.severity }); } From c2c5fba7eb9189abefb94c83a5251b8e32c4cfd6 Mon Sep 17 00:00:00 2001 From: Dan Purdy Date: Thu, 27 Oct 2016 13:37:35 +0100 Subject: [PATCH 38/67] :fire: Remove stray console.log --- tests/rules/declarations-before-nesting.js | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/rules/declarations-before-nesting.js b/tests/rules/declarations-before-nesting.js index d7009a7e..e8f4b59c 100644 --- a/tests/rules/declarations-before-nesting.js +++ b/tests/rules/declarations-before-nesting.js @@ -12,7 +12,6 @@ describe('declarations before nesting - scss', function () { lint.test(file, { 'declarations-before-nesting': 1 }, function (data) { - console.log(data); lint.assert.equal(4, data.warningCount); done(); }); From 08640c87f12cad77dfa478d33295ffe5001b960b Mon Sep 17 00:00:00 2001 From: Dan Purdy Date: Thu, 27 Oct 2016 13:41:10 +0100 Subject: [PATCH 39/67] :art: Correct spelling --- tests/cli.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/cli.js b/tests/cli.js index bd8130be..df7c7fef 100644 --- a/tests/cli.js +++ b/tests/cli.js @@ -491,7 +491,7 @@ describe('Reading files with UTF-8 BOM', function () { exec(command, function (err, stdout) { // eslint-disable-line handle-callback-err var result = JSON.parse(stdout)[0]; - // Files with BOM markers that start with a mixing throw a fatal error + // Files with BOM markers that start with a mixin throw a fatal error // https://github.com/sasstools/sass-lint/issues/880 assert.equal(result.errorCount, 0); done(); From 46cdecb26112a491712145c9c5f3e2b14d8786c3 Mon Sep 17 00:00:00 2001 From: Dan Purdy Date: Fri, 28 Oct 2016 00:54:19 +0100 Subject: [PATCH 40/67] :white_check_mark: Add extra issue tests --- tests/rules/no-duplicate-properties.js | 12 ++++++------ tests/sass/no-duplicate-properties.sass | 10 ++++++++++ tests/sass/no-duplicate-properties.scss | 11 +++++++++++ 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/tests/rules/no-duplicate-properties.js b/tests/rules/no-duplicate-properties.js index c703d25a..d51bf824 100644 --- a/tests/rules/no-duplicate-properties.js +++ b/tests/rules/no-duplicate-properties.js @@ -12,7 +12,7 @@ describe('no duplicate properties - scss', function () { lint.test(file, { 'no-duplicate-properties': 1 }, function (data) { - lint.assert.equal(6, data.warningCount); + lint.assert.equal(7, data.warningCount); done(); }); }); @@ -28,7 +28,7 @@ describe('no duplicate properties - scss', function () { } ] }, function (data) { - lint.assert.equal(5, data.warningCount); + lint.assert.equal(6, data.warningCount); done(); }); }); @@ -45,7 +45,7 @@ describe('no duplicate properties - scss', function () { } ] }, function (data) { - lint.assert.equal(4, data.warningCount); + lint.assert.equal(5, data.warningCount); done(); }); }); @@ -61,7 +61,7 @@ describe('no duplicate properties - sass', function () { lint.test(file, { 'no-duplicate-properties': 1 }, function (data) { - lint.assert.equal(6, data.warningCount); + lint.assert.equal(7, data.warningCount); done(); }); }); @@ -77,7 +77,7 @@ describe('no duplicate properties - sass', function () { } ] }, function (data) { - lint.assert.equal(5, data.warningCount); + lint.assert.equal(6, data.warningCount); done(); }); }); @@ -94,7 +94,7 @@ describe('no duplicate properties - sass', function () { } ] }, function (data) { - lint.assert.equal(4, data.warningCount); + lint.assert.equal(5, data.warningCount); done(); }); }); diff --git a/tests/sass/no-duplicate-properties.sass b/tests/sass/no-duplicate-properties.sass index 1f3b4c5b..fd4bb85c 100644 --- a/tests/sass/no-duplicate-properties.sass +++ b/tests/sass/no-duplicate-properties.sass @@ -30,3 +30,13 @@ content: 'display' display: flex display: inline-block + +// issue #907 - interpolation/variable in property names +$paint-border-1: top +$paint-border-2: left + +.test + border: $size solid transparent + border-#{$paint-border-1}: $size solid $color + border-#{$paint-border-2}: $size solid $color + border-#{$paint-border-2}: $size solid $color diff --git a/tests/sass/no-duplicate-properties.scss b/tests/sass/no-duplicate-properties.scss index af11b692..eb9869c2 100644 --- a/tests/sass/no-duplicate-properties.scss +++ b/tests/sass/no-duplicate-properties.scss @@ -37,3 +37,14 @@ display: inline-block; } + +// issue #907 - interpolation/variable in property names +$paint-border-1: top; +$paint-border-2: left; + +.test { + border: $size solid transparent; + border-#{$paint-border-1}: $size solid $color; + border-#{$paint-border-2}: $size solid $color; + border-#{$paint-border-2}: $size solid $color; +} From 4180e344fac5e2dc6e15499c83632d39f583fb10 Mon Sep 17 00:00:00 2001 From: Dan Purdy Date: Fri, 28 Oct 2016 00:55:36 +0100 Subject: [PATCH 41/67] :bug: Fix no duplicate property interp issue --- lib/rules/no-duplicate-properties.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/rules/no-duplicate-properties.js b/lib/rules/no-duplicate-properties.js index 1302a77b..ad65f43a 100644 --- a/lib/rules/no-duplicate-properties.js +++ b/lib/rules/no-duplicate-properties.js @@ -1,6 +1,7 @@ 'use strict'; var helpers = require('../helpers'); +var selectorHelpers = require('../selector-helpers'); module.exports = { 'name': 'no-duplicate-properties', @@ -25,8 +26,12 @@ module.exports = { warnMessage = false; declaration.eachFor('property', function (item) { - var property = item.content[0].content; - + var property = ''; + item.content.forEach(function (subItem) { + // Although not a selector the method here helps us construct the proper property name + // taking into account any interpolation etc + property += selectorHelpers.constructSelector(subItem); + }); if (properties.indexOf(property) !== -1 && properties.length >= 1) { if (parser.options.exclude.indexOf(property) !== -1 && properties[properties.length - 1] !== property) { warnMessage = 'Excluded duplicate properties must directly follow each other.'; From 5b7cea705bf2ad611e12f64d2a474e4164988b65 Mon Sep 17 00:00:00 2001 From: Lucas Jahn Date: Mon, 31 Oct 2016 11:56:02 +0100 Subject: [PATCH 42/67] updated concentric rules to match the newest property sort order list (including flex) --- .../property-sort-orders/concentric.yml | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/lib/config/property-sort-orders/concentric.yml b/lib/config/property-sort-orders/concentric.yml index 5fb6fae5..b66cfa3e 100644 --- a/lib/config/property-sort-orders/concentric.yml +++ b/lib/config/property-sort-orders/concentric.yml @@ -11,6 +11,19 @@ order: - 'bottom' - 'left' + - 'flex' + - 'flex-basis' + - 'flex-direction' + - 'flex-flow' + - 'flex-grow' + - 'flex-shrink' + - 'flex-wrap' + - 'align-content' + - 'align-items' + - 'align-self' + - 'justify-content' + - 'order' + - 'columns' - 'column-gap' - 'column-fill' @@ -46,6 +59,10 @@ order: - 'margin-left' - 'outline' + - 'outline-offset' + - 'outline-width' + - 'outline-style' + - 'outline-color' - 'border' - 'border-top' @@ -79,6 +96,8 @@ order: - 'box-shadow' - 'background' + - 'background-attachment' + - 'background-clip' - 'background-color' - 'background-image' - 'background-repeat' From d05c747278a99ad76cf10eff853cb2ecc5dcb78b Mon Sep 17 00:00:00 2001 From: greenkeeperio-bot Date: Tue, 1 Nov 2016 10:05:51 -0400 Subject: [PATCH 43/67] chore(package): update fs-extra to version 1.0.0 https://greenkeeper.io/ --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 4cf4c0d9..a9ce9644 100644 --- a/package.json +++ b/package.json @@ -31,7 +31,7 @@ "commander": "^2.8.1", "eslint": "^2.7.0", "front-matter": "2.1.0", - "fs-extra": "^0.30.0", + "fs-extra": "^1.0.0", "glob": "^7.0.0", "globule": "^1.0.0", "gonzales-pe": "3.4.7", From 1045e5f1da1890cd4ff8f3234e037a0b0bd962c5 Mon Sep 17 00:00:00 2001 From: "don.abrams" Date: Thu, 12 Nov 2015 10:57:30 -0700 Subject: [PATCH 44/67] add docs --- docs/options/toggle-rules-in-src.md | 67 +++++++++++++++++++++++++++++ docs/rules/doc-disabled-rules.md | 48 +++++++++++++++++++++ 2 files changed, 115 insertions(+) create mode 100644 docs/options/toggle-rules-in-src.md create mode 100644 docs/rules/doc-disabled-rules.md diff --git a/docs/options/toggle-rules-in-src.md b/docs/options/toggle-rules-in-src.md new file mode 100644 index 00000000..0b831b82 --- /dev/null +++ b/docs/options/toggle-rules-in-src.md @@ -0,0 +1,67 @@ +# Toggling Rules Inside Source Files + +For special cases where a particular lint doesn't make sense in a specific area of a file, special inline comments can be used to enable/disable linters. Some examples are provided below: + +## Disable a rule for the entire file + +```scss +// sass-lint:disable border-zero +p { + border: none; // No lint reported +} +``` + +## Disable more than 1 rule + +```scss +// sass-lint:disable border-zero, quotes +p { + border: none; // No lint reported + content: "hello"; // No lint reported +} +``` + +## Disable all lints within a block (and all contained blocks) + +```scss +p { + // sass-lint:disable-block border-zero + border: none; // No result reported +} + +a { + border: none; // Failing result reported +} +``` + +## Disable and enable again + +```scss +// sass-lint:disable border-zero +p { + border: none; // No result reported +} +// sass-lint:enable border-zero + +a { + border: none; // Failing result reported +} +``` + +## Disable/enable all linters + +```scss +// sass-lint:disable-all +p { + border: none; // No result reported +} +// sass-lint:enable-all + +a { + border: none; // Failing result reported +} +``` + +## doc-disabled-rules rule + +It is highly recommended that you use the doc-disabled-rules rule. diff --git a/docs/rules/doc-disabled-rules.md b/docs/rules/doc-disabled-rules.md new file mode 100644 index 00000000..4902bced --- /dev/null +++ b/docs/rules/doc-disabled-rules.md @@ -0,0 +1,48 @@ +# Document Disabled Rules + +Rule `doc-disabled-rules` will enforce that disabled rules have a comment immediately preceding the disabled rule. + +## Examples + +When enabled, the following are disallowed. + +```scss +// sass-lint:disable-all +p { + border: none; +} +// sass-lint:enable-all + +// sass-lint:disable border-zero +a { + border: none; +} + +li { + // sass-lint:disable-block border-zero + border: none; +} +``` + +When enabled, the following are allowed. + +```scss +// Paragraphs are special ponies +// sass-lint:disable-all +p { + border: none; +} +// sass-lint:enable-all + +// We really prefer `border: none` in this file, for reasons. +// sass-lint:disable border-zero +a { + border: none; +} + +li { + // We really prefer `border: none` in this file, for reasons. + // sass-lint:disable-block border-zero + border: none; +} +``` From 66ae1aa79f603557627e31f98c545f990e4607b8 Mon Sep 17 00:00:00 2001 From: "don.abrams" Date: Thu, 12 Nov 2015 11:00:15 -0700 Subject: [PATCH 45/67] add disable-line --- docs/options/toggle-rules-in-src.md | 8 ++++++++ docs/rules/doc-disabled-rules.md | 9 +++++++++ 2 files changed, 17 insertions(+) diff --git a/docs/options/toggle-rules-in-src.md b/docs/options/toggle-rules-in-src.md index 0b831b82..6fca0aef 100644 --- a/docs/options/toggle-rules-in-src.md +++ b/docs/options/toggle-rules-in-src.md @@ -21,6 +21,14 @@ p { } ``` +## Disable a rule for a single line + +```scss +p { + border: none; // sass-lint:disable-line border-zero +} +``` + ## Disable all lints within a block (and all contained blocks) ```scss diff --git a/docs/rules/doc-disabled-rules.md b/docs/rules/doc-disabled-rules.md index 4902bced..b318d0c6 100644 --- a/docs/rules/doc-disabled-rules.md +++ b/docs/rules/doc-disabled-rules.md @@ -22,6 +22,10 @@ li { // sass-lint:disable-block border-zero border: none; } + +dd { + border: none; // sass-lint:disable-line border-zero +} ``` When enabled, the following are allowed. @@ -45,4 +49,9 @@ li { // sass-lint:disable-block border-zero border: none; } + +dd { + // We really prefer `border: none` in this file, for reasons. + border: none; // sass-lint:disable-line border-zero +} ``` From bbc9d1dffb74e6a52ac86821c9d3bb42c3a1d007 Mon Sep 17 00:00:00 2001 From: "don.abrams" Date: Thu, 12 Nov 2015 16:00:25 -0700 Subject: [PATCH 46/67] working commit-- currently adding a ton of tests --- index.js | 10 +- lib/ruleToggler.js | 169 ++++++++++++++++++ tests/ruleToggler.js | 121 +++++++++++++ tests/sass/ruleToggler-disable-all.scss | 17 ++ .../sass/ruleToggler-disable-border-zero.scss | 17 ++ tests/sass/ruleToggler.scss | 17 ++ 6 files changed, 349 insertions(+), 2 deletions(-) create mode 100644 lib/ruleToggler.js create mode 100644 tests/ruleToggler.js create mode 100644 tests/sass/ruleToggler-disable-all.scss create mode 100644 tests/sass/ruleToggler-disable-border-zero.scss create mode 100644 tests/sass/ruleToggler.scss diff --git a/index.js b/index.js index e222c616..2920a6e8 100644 --- a/index.js +++ b/index.js @@ -5,11 +5,15 @@ var slConfig = require('./lib/config'), exceptions = require('./lib/exceptions'), helpers = require('./lib/helpers'), slRules = require('./lib/rules'), + ruleToggler = require('./lib/ruleToggler'), glob = require('glob'), path = require('path'), fs = require('fs-extra'), globule = require('globule'); +var getToggledRules = ruleToggler.getToggledRules, + isResultEnabled = ruleToggler.isResultEnabled; + var sassLint = function (config) { config = require('./lib/config')(config); return; @@ -102,7 +106,8 @@ sassLint.lintText = function (file, options, configPath) { detects, results = [], errors = 0, - warnings = 0; + warnings = 0, + ruleToggles = getToggledRules(ast); try { ast = groot(file.text, file.format, file.filename); @@ -122,7 +127,8 @@ sassLint.lintText = function (file, options, configPath) { if (ast.content && ast.content.length > 0) { rules.forEach(function (rule) { - detects = rule.rule.detect(ast, rule); + detects = rule.rule.detect(ast, rule) + .filter(isResultEnabled(ruleToggles)); results = results.concat(detects); if (detects.length) { if (rule.severity === 1) { diff --git a/lib/ruleToggler.js b/lib/ruleToggler.js new file mode 100644 index 00000000..8aa970fb --- /dev/null +++ b/lib/ruleToggler.js @@ -0,0 +1,169 @@ +'use strict'; + +var addDisable = function (toggledRules, rules, line, column) { + rules.map(function (rule) { + toggledRules.ruleEnable[rule] = toggledRules.ruleEnable[rule] || []; + toggledRules.ruleEnable[rule].push([false, line, column]); + }); +}; + +var addEnable = function (toggledRules, rules, line, column) { + rules.map(function (rule) { + toggledRules.ruleEnable[rule] = toggledRules.ruleEnable[rule] || []; + toggledRules.ruleEnable[rule].push([true, line, column]); + }); +}; + +var getBoundsOfContainingBlock = function (blockChild) { + if (blockChild.type === 'block') { + return { + start: { + line: blockChild.start.line, + column: blockChild.start.column + }, + end: { + line: blockChild.end.line, + column: blockChild.end.column + } + }; + } + // TODO: not sure what the appropriate behavior is if there is no parent block; currently NPEs + return getBoundsOfContainingBlock(blockChild.parent); +}; + +var addDisableBlock = function (toggledRules, rules, blockChild) { + var blockBounds = getBoundsOfContainingBlock(blockChild); + rules.map(function (rule) { + toggledRules.ruleEnable[rule] = toggledRules.ruleEnable[rule] || []; + toggledRules.ruleEnable[rule] + .push([false, blockBounds.start.line, blockBounds.start.column]) + .push([true, blockBounds.end.line, blockBounds.end.column]); + }); +}; + +var addDisableAll = function (toggledRules, line, column) { + toggledRules.globalEnable + .push([false, line, column]); +}; + +var addEnableAll = function (toggledRules, line, column) { + toggledRules.globalEnable + .push([true, line, column]); +}; + +var addDisableLine = function (toggledRules, rules, line) { + rules.map(function (rule) { + toggledRules.ruleEnable[rule] = toggledRules.ruleEnable[rule] || []; + // NOTE: corner case not handled here: a 2nd disable inside an ignored line, which is unrealistically pathological. + toggledRules.ruleEnable[rule] + .push([false, line, 0]) + .push([true, line + 2, 0]); + }); +}; + +var sortRange = function (toggleRangeA, toggleRangeB) { + var aLine = toggleRangeA[1], + aCol = toggleRangeA[2], + bLine = toggleRangeB[1], + bCol = toggleRangeA[2]; + if (aLine < bLine) { + return -1; + } + if (bLine < aLine) { + return 1; + } + if (aCol < bCol) { + return -1; + } + if (bCol < aCol) { + return 1; + } + return 0; +}; + +module.exports.getToggledRules = function (ast) { + var toggledRules = { + ruleEnable: { + // Format in here is [isDisabled, line, column] + }, + globalEnable: [] + }; + if (!ast.traverseByTypes) { + return toggledRules; + } + ast.traverseByTypes(['multilineComment', 'singlelineComment'], function (comment) { + var content = comment.content; + if (!content) { + return; + } + var tokens = content.split(/[\s,]/); + if (!tokens.length) { + return; + } + var first = tokens[0], + rules = tokens.splice(1); + switch (first) { + case 'sass-lint:disable': + addDisable(toggledRules, rules, comment.end.line, comment.end.column); + break; + case 'sass-lint:enable': + addEnable(toggledRules, rules, comment.end.line, comment.end.column); + break; + case 'sass-lint:disable-block': + addDisableBlock(toggledRules, rules, comment); + break; + case 'sass-lint:disable-all': + addDisableAll(toggledRules, comment.end.line, comment.end.column); + break; + case 'sass-lint:enable-all': + addEnableAll(toggledRules, comment.end.line, comment.end.column); + break; + case 'sass-lint:disable-line': + addDisableLine(toggledRules, rules, comment.line); + break; + default: + return; + } + }); + // Sort these toggle stacks so reading them is easier (algorithmically). + // Hopefully it's a bubble sort cause they will generally be ordered and tiny. + toggledRules.globalEnable.sort(sortRange); + Object.keys(toggledRules.ruleEnable).map(function (ruleId) { + toggledRules.ruleEnable[ruleId].sort(sortRange); + }); + return toggledRules; +}; + +module.exports.isResultEnabled = function (toggledRules) { + return function (ruleResult) { + var ruleId = ruleResult.ruleId; + // Convention: if no column or line, assume rule is targetting 0. + var line = ruleResult.line || 0; + var column = ruleResult.column || 0; + var isGloballyEnabled = toggledRules.globalEnable + .reduce(function (acc, toggleRange) { + if (line <= toggleRange[1] && column <= toggleRange[2]) { + return acc; + } + return toggleRange[0]; + }, true); + if (!isGloballyEnabled) { + return false; + } + if (!toggledRules.ruleEnable[ruleId]) { + return true; + } + var isRuleEnabled = toggledRules.ruleEnable[ruleId] + .reduce(function (acc, toggleRange) { + if (line <= toggleRange[1] && column <= toggleRange[2]) { + return acc; + } + return toggleRange[0]; + }, true); + if (!isRuleEnabled) { + return false; + } + return true; + }; +}; + diff --git a/tests/ruleToggler.js b/tests/ruleToggler.js new file mode 100644 index 00000000..92e8b593 --- /dev/null +++ b/tests/ruleToggler.js @@ -0,0 +1,121 @@ +var path = require('path'), + fs = require('fs'), + groot = require('../lib/groot'), + ruleToggler = require('../lib/ruleToggler'), + assert = require('assert'), + deepEqual = require('deep-equal'); + +var getToggledRules = ruleToggler.getToggledRules, + isResultEnabled = ruleToggler.isResultEnabled; + +var generateToggledRules = function (filename) { + var filePath = path.join(process.cwd(), 'tests', 'sass', filename); + var file = { + 'text': fs.readFileSync(filePath), + 'format': path.extname(filePath).replace('.', ''), + 'filename': path.basename(filePath) + }; + var ast = groot(file.text, file.format, file.filename); + return getToggledRules(ast); +}; + +describe.only('rule toggling', function () { + describe('getToggledRules', function () { + it('should allow all rules to be disabled', function () { + deepEqual(generateToggledRules('ruleToggler-disable-all.scss'), { + globalEnable: [[false, 0, 0]], + ruleEnable: {} + }); + }); + it('should allow all rules to be disabled then re-enabled', function () { + deepEqual(generateToggledRules('ruleToggler-disable-all-then-reenable.scss'), { + globalEnable: [[false, 0, 0], [false, 1, 0]], + ruleEnable: {} + }); + }); + }); + describe('isResultEnabled', function () { + it('should disable all rules if global is disabled', function () { + assert(isResultEnabled({ + globalEnable: [[false, 0, 0]], + ruleEnable: {} + })({ + ruleId: 'anything', + line: 1, + column: 0 + }) === false); + }); + it('should disable a rule', function () { + assert(isResultEnabled({ + globalEnable: [], + ruleEnable: { + a: [[false, 0, 0]] + } + })({ + ruleId: 'a', + line: 1, + column: 0 + }) === false); + }); + it('should not disable an unrelated rule', function () { + assert(isResultEnabled({ + globalEnable: [], + ruleEnable: { + b: [[false, 0, 0]] + } + })({ + ruleId: 'a', + line: 1, + column: 0 + }) === true); + }); + it('should support enabling a previously disabled rule', function () { + assert(isResultEnabled({ + globalEnable: [], + ruleEnable: { + a: [[false, 0, 0], [true, 1, 0]] + } + })({ + ruleId: 'a', + line: 2, + column: 0 + }) === true); + }); + it('should support disabling a previously re-enabled rule', function () { + assert(isResultEnabled({ + globalEnable: [], + ruleEnable: { + a: [[false, 0, 0], [true, 1, 0], [false, 2, 0]] + } + })({ + ruleId: 'a', + line: 3, + column: 0 + }) === false); + }); + it('should support enabling a previously re-enabled then disabled rule', function () { + assert(isResultEnabled({ + globalEnable: [], + ruleEnable: { + a: [[false, 0, 0], [true, 1, 0], [false, 2, 0], [true, 3, 0]] + } + })({ + ruleId: 'a', + line: 4, + column: 0 + }) === true); + }); + it('should support disabling a rule that is later re-enabled', function () { + assert(isResultEnabled({ + globalEnable: [], + ruleEnable: { + a: [[false, 0, 0], [true, 2, 0], [false, 3, 0]] + } + })({ + ruleId: 'a', + line: 1, + column: 0 + }) === false); + }); + }); +}); diff --git a/tests/sass/ruleToggler-disable-all.scss b/tests/sass/ruleToggler-disable-all.scss new file mode 100644 index 00000000..79e23d67 --- /dev/null +++ b/tests/sass/ruleToggler-disable-all.scss @@ -0,0 +1,17 @@ +// sass-lint:disable-all +p { + border: none; +} + +a { + border: none; + content: "hello"; +} + +li { + border: none; +} + +dl { + border: none; +} diff --git a/tests/sass/ruleToggler-disable-border-zero.scss b/tests/sass/ruleToggler-disable-border-zero.scss new file mode 100644 index 00000000..885d2b32 --- /dev/null +++ b/tests/sass/ruleToggler-disable-border-zero.scss @@ -0,0 +1,17 @@ +// sass-lint:disable border-zero +p { + border: none; +} + +a { + border: none; + content: "hello"; +} + +li { + border: none; +} + +dl { + border: none; +} diff --git a/tests/sass/ruleToggler.scss b/tests/sass/ruleToggler.scss new file mode 100644 index 00000000..79e23d67 --- /dev/null +++ b/tests/sass/ruleToggler.scss @@ -0,0 +1,17 @@ +// sass-lint:disable-all +p { + border: none; +} + +a { + border: none; + content: "hello"; +} + +li { + border: none; +} + +dl { + border: none; +} From 600885cbeb306a782584af9160af9f5b92204d91 Mon Sep 17 00:00:00 2001 From: "don.abrams" Date: Thu, 12 Nov 2015 19:12:41 -0700 Subject: [PATCH 47/67] getting tests to pass, YEAH! --- index.js | 5 +- lib/ruleToggler.js | 35 +++---- tests/ruleToggler.js | 95 +++++++++++++------ tests/sass/ruleToggler-disable-a-block.scss | 3 + tests/sass/ruleToggler-disable-a-line.scss | 3 + tests/sass/ruleToggler-disable-a-rule.scss | 1 + ...ruleToggler-disable-all-then-reenable.scss | 3 + tests/sass/ruleToggler-disable-all.scss | 16 ---- .../ruleToggler-disable-multiple-rules.scss | 1 + 9 files changed, 100 insertions(+), 62 deletions(-) create mode 100644 tests/sass/ruleToggler-disable-a-block.scss create mode 100644 tests/sass/ruleToggler-disable-a-line.scss create mode 100644 tests/sass/ruleToggler-disable-a-rule.scss create mode 100644 tests/sass/ruleToggler-disable-all-then-reenable.scss create mode 100644 tests/sass/ruleToggler-disable-multiple-rules.scss diff --git a/index.js b/index.js index 2920a6e8..aab32048 100644 --- a/index.js +++ b/index.js @@ -107,7 +107,8 @@ sassLint.lintText = function (file, options, configPath) { results = [], errors = 0, warnings = 0, - ruleToggles = getToggledRules(ast); + ruleToggles = getToggledRules(ast), + isEnabledFilter = isResultEnabled(ruleToggles); try { ast = groot(file.text, file.format, file.filename); @@ -128,7 +129,7 @@ sassLint.lintText = function (file, options, configPath) { if (ast.content && ast.content.length > 0) { rules.forEach(function (rule) { detects = rule.rule.detect(ast, rule) - .filter(isResultEnabled(ruleToggles)); + .filter(isEnabledFilter); results = results.concat(detects); if (detects.length) { if (rule.severity === 1) { diff --git a/lib/ruleToggler.js b/lib/ruleToggler.js index 8aa970fb..ee7497d8 100644 --- a/lib/ruleToggler.js +++ b/lib/ruleToggler.js @@ -15,6 +15,7 @@ var addEnable = function (toggledRules, rules, line, column) { }; var getBoundsOfContainingBlock = function (blockChild) { + console.log(blockChild); if (blockChild.type === 'block') { return { start: { @@ -35,9 +36,8 @@ var addDisableBlock = function (toggledRules, rules, blockChild) { var blockBounds = getBoundsOfContainingBlock(blockChild); rules.map(function (rule) { toggledRules.ruleEnable[rule] = toggledRules.ruleEnable[rule] || []; - toggledRules.ruleEnable[rule] - .push([false, blockBounds.start.line, blockBounds.start.column]) - .push([true, blockBounds.end.line, blockBounds.end.column]); + toggledRules.ruleEnable[rule].push([false, blockBounds.start.line, blockBounds.start.column]); + toggledRules.ruleEnable[rule].push([true, blockBounds.end.line, blockBounds.end.column]); }); }; @@ -55,9 +55,8 @@ var addDisableLine = function (toggledRules, rules, line) { rules.map(function (rule) { toggledRules.ruleEnable[rule] = toggledRules.ruleEnable[rule] || []; // NOTE: corner case not handled here: a 2nd disable inside an ignored line, which is unrealistically pathological. - toggledRules.ruleEnable[rule] - .push([false, line, 0]) - .push([true, line + 2, 0]); + toggledRules.ruleEnable[rule].push([false, line, 1]); + toggledRules.ruleEnable[rule].push([true, line + 1, 1]); }); }; @@ -96,37 +95,39 @@ module.exports.getToggledRules = function (ast) { if (!content) { return; } - var tokens = content.split(/[\s,]/); + var tokens = content.split(/[\s,]+/) + .filter(function (s) { + return s.trim().length > 0; + }); if (!tokens.length) { return; } var first = tokens[0], - rules = tokens.splice(1); + rules = tokens.slice(1); switch (first) { case 'sass-lint:disable': - addDisable(toggledRules, rules, comment.end.line, comment.end.column); + addDisable(toggledRules, rules, comment.start.line, comment.start.column); break; case 'sass-lint:enable': - addEnable(toggledRules, rules, comment.end.line, comment.end.column); + addEnable(toggledRules, rules, comment.start.line, comment.start.column); break; case 'sass-lint:disable-block': addDisableBlock(toggledRules, rules, comment); break; case 'sass-lint:disable-all': - addDisableAll(toggledRules, comment.end.line, comment.end.column); + addDisableAll(toggledRules, comment.start.line, comment.start.column); break; case 'sass-lint:enable-all': - addEnableAll(toggledRules, comment.end.line, comment.end.column); + addEnableAll(toggledRules, comment.start.line, comment.start.column); break; case 'sass-lint:disable-line': - addDisableLine(toggledRules, rules, comment.line); + addDisableLine(toggledRules, rules, comment.start.line); break; default: return; } }); // Sort these toggle stacks so reading them is easier (algorithmically). - // Hopefully it's a bubble sort cause they will generally be ordered and tiny. toggledRules.globalEnable.sort(sortRange); Object.keys(toggledRules.ruleEnable).map(function (ruleId) { toggledRules.ruleEnable[ruleId].sort(sortRange); @@ -137,9 +138,9 @@ module.exports.getToggledRules = function (ast) { module.exports.isResultEnabled = function (toggledRules) { return function (ruleResult) { var ruleId = ruleResult.ruleId; - // Convention: if no column or line, assume rule is targetting 0. - var line = ruleResult.line || 0; - var column = ruleResult.column || 0; + // Convention: if no column or line, assume rule is targetting 1. + var line = ruleResult.line || 1; + var column = ruleResult.column || 1; var isGloballyEnabled = toggledRules.globalEnable .reduce(function (acc, toggleRange) { if (line <= toggleRange[1] && column <= toggleRange[2]) { diff --git a/tests/ruleToggler.js b/tests/ruleToggler.js index 92e8b593..30c3d4a8 100644 --- a/tests/ruleToggler.js +++ b/tests/ruleToggler.js @@ -22,99 +22,140 @@ var generateToggledRules = function (filename) { describe.only('rule toggling', function () { describe('getToggledRules', function () { it('should allow all rules to be disabled', function () { - deepEqual(generateToggledRules('ruleToggler-disable-all.scss'), { - globalEnable: [[false, 0, 0]], + assert(deepEqual(generateToggledRules('ruleToggler-disable-all.scss'), { + globalEnable: [[false, 1, 1]], ruleEnable: {} - }); + }) === true); }); it('should allow all rules to be disabled then re-enabled', function () { - deepEqual(generateToggledRules('ruleToggler-disable-all-then-reenable.scss'), { - globalEnable: [[false, 0, 0], [false, 1, 0]], + var ruleToggles = generateToggledRules('ruleToggler-disable-all-then-reenable.scss'); + assert(deepEqual(ruleToggles, { + globalEnable: [ + [false, 1, 1], + [true, 3, 1] + ], ruleEnable: {} - }); + }) === true); + }); + it('should allow a single rule to be disabled', function () { + assert(deepEqual(generateToggledRules('ruleToggler-disable-a-rule.scss'), { + globalEnable: [], + ruleEnable: { + a: [[false, 1, 1]] + } + }) === true); + }); + it('should allow multiple rules to be disabled', function () { + assert(deepEqual(generateToggledRules('ruleToggler-disable-multiple-rules.scss'), { + globalEnable: [], + ruleEnable: { + a: [[false, 1, 1]], + b: [[false, 1, 1]], + c: [[false, 1, 1]], + d: [[false, 1, 1]] + } + }) === true); + }); + it('should be able to disable a single line', function () { + var ruleToggles = generateToggledRules('ruleToggler-disable-a-line.scss'); + assert(deepEqual(ruleToggles, { + globalEnable: [], + ruleEnable: { + a: [[false, 2, 1], [true, 3, 1]] + } + }) === true); + }); + it('should be able to disable a block of code', function () { + var ruleToggles = generateToggledRules('ruleToggler-disable-a-block.scss'); + assert(deepEqual(ruleToggles, { + globalEnable: [], + ruleEnable: { + a: [[false, 1, 4], [true, 3, 1]] + } + }) === true); }); }); describe('isResultEnabled', function () { it('should disable all rules if global is disabled', function () { assert(isResultEnabled({ - globalEnable: [[false, 0, 0]], + globalEnable: [[false, 1, 1]], ruleEnable: {} })({ ruleId: 'anything', - line: 1, - column: 0 + line: 2, + column: 1 }) === false); }); it('should disable a rule', function () { assert(isResultEnabled({ globalEnable: [], ruleEnable: { - a: [[false, 0, 0]] + a: [[false, 1, 1]] } })({ ruleId: 'a', - line: 1, - column: 0 + line: 2, + column: 1 }) === false); }); it('should not disable an unrelated rule', function () { assert(isResultEnabled({ globalEnable: [], ruleEnable: { - b: [[false, 0, 0]] + b: [[false, 1, 1]] } })({ ruleId: 'a', - line: 1, - column: 0 + line: 2, + column: 1 }) === true); }); it('should support enabling a previously disabled rule', function () { assert(isResultEnabled({ globalEnable: [], ruleEnable: { - a: [[false, 0, 0], [true, 1, 0]] + a: [[false, 1, 1], [true, 2, 1]] } })({ ruleId: 'a', - line: 2, - column: 0 + line: 3, + column: 1 }) === true); }); it('should support disabling a previously re-enabled rule', function () { assert(isResultEnabled({ globalEnable: [], ruleEnable: { - a: [[false, 0, 0], [true, 1, 0], [false, 2, 0]] + a: [[false, 1, 1], [true, 2, 1], [false, 3, 1]] } })({ ruleId: 'a', - line: 3, - column: 0 + line: 4, + column: 1 }) === false); }); it('should support enabling a previously re-enabled then disabled rule', function () { assert(isResultEnabled({ globalEnable: [], ruleEnable: { - a: [[false, 0, 0], [true, 1, 0], [false, 2, 0], [true, 3, 0]] + a: [[false, 1, 1], [true, 2, 1], [false, 3, 1], [true, 4, 1]] } })({ ruleId: 'a', - line: 4, - column: 0 + line: 5, + column: 1 }) === true); }); it('should support disabling a rule that is later re-enabled', function () { assert(isResultEnabled({ globalEnable: [], ruleEnable: { - a: [[false, 0, 0], [true, 2, 0], [false, 3, 0]] + a: [[false, 1, 1], [true, 3, 1], [false, 4, 1]] } })({ ruleId: 'a', - line: 1, - column: 0 + line: 2, + column: 1 }) === false); }); }); diff --git a/tests/sass/ruleToggler-disable-a-block.scss b/tests/sass/ruleToggler-disable-a-block.scss new file mode 100644 index 00000000..8eddb3ea --- /dev/null +++ b/tests/sass/ruleToggler-disable-a-block.scss @@ -0,0 +1,3 @@ +p { + // sass-lint:disable-block a +} diff --git a/tests/sass/ruleToggler-disable-a-line.scss b/tests/sass/ruleToggler-disable-a-line.scss new file mode 100644 index 00000000..7d6ca7cc --- /dev/null +++ b/tests/sass/ruleToggler-disable-a-line.scss @@ -0,0 +1,3 @@ +p { + border-color: red; // sass-lint:disable-line a +} diff --git a/tests/sass/ruleToggler-disable-a-rule.scss b/tests/sass/ruleToggler-disable-a-rule.scss new file mode 100644 index 00000000..5f04af37 --- /dev/null +++ b/tests/sass/ruleToggler-disable-a-rule.scss @@ -0,0 +1 @@ +// sass-lint:disable a diff --git a/tests/sass/ruleToggler-disable-all-then-reenable.scss b/tests/sass/ruleToggler-disable-all-then-reenable.scss new file mode 100644 index 00000000..63eeb627 --- /dev/null +++ b/tests/sass/ruleToggler-disable-all-then-reenable.scss @@ -0,0 +1,3 @@ +// sass-lint:disable-all + +// sass-lint:enable-all diff --git a/tests/sass/ruleToggler-disable-all.scss b/tests/sass/ruleToggler-disable-all.scss index 79e23d67..1776554a 100644 --- a/tests/sass/ruleToggler-disable-all.scss +++ b/tests/sass/ruleToggler-disable-all.scss @@ -1,17 +1 @@ // sass-lint:disable-all -p { - border: none; -} - -a { - border: none; - content: "hello"; -} - -li { - border: none; -} - -dl { - border: none; -} diff --git a/tests/sass/ruleToggler-disable-multiple-rules.scss b/tests/sass/ruleToggler-disable-multiple-rules.scss new file mode 100644 index 00000000..8e44d373 --- /dev/null +++ b/tests/sass/ruleToggler-disable-multiple-rules.scss @@ -0,0 +1 @@ +// sass-lint:disable a b, c d From da73122ba775eb87abfdf8c15917ff4c4c44ea2a Mon Sep 17 00:00:00 2001 From: "don.abrams" Date: Fri, 13 Nov 2015 10:22:17 -0700 Subject: [PATCH 48/67] fix disable block with assumption direct parent is block --- lib/ruleToggler.js | 31 ++++++------------------------- tests/ruleToggler.js | 2 +- 2 files changed, 7 insertions(+), 26 deletions(-) diff --git a/lib/ruleToggler.js b/lib/ruleToggler.js index ee7497d8..7bd87881 100644 --- a/lib/ruleToggler.js +++ b/lib/ruleToggler.js @@ -14,30 +14,11 @@ var addEnable = function (toggledRules, rules, line, column) { }); }; -var getBoundsOfContainingBlock = function (blockChild) { - console.log(blockChild); - if (blockChild.type === 'block') { - return { - start: { - line: blockChild.start.line, - column: blockChild.start.column - }, - end: { - line: blockChild.end.line, - column: blockChild.end.column - } - }; - } - // TODO: not sure what the appropriate behavior is if there is no parent block; currently NPEs - return getBoundsOfContainingBlock(blockChild.parent); -}; - -var addDisableBlock = function (toggledRules, rules, blockChild) { - var blockBounds = getBoundsOfContainingBlock(blockChild); +var addDisableBlock = function (toggledRules, rules, block) { rules.map(function (rule) { toggledRules.ruleEnable[rule] = toggledRules.ruleEnable[rule] || []; - toggledRules.ruleEnable[rule].push([false, blockBounds.start.line, blockBounds.start.column]); - toggledRules.ruleEnable[rule].push([true, blockBounds.end.line, blockBounds.end.column]); + toggledRules.ruleEnable[rule].push([false, block.start.line, block.start.column]); + toggledRules.ruleEnable[rule].push([true, block.end.line, block.end.column]); }); }; @@ -90,7 +71,7 @@ module.exports.getToggledRules = function (ast) { if (!ast.traverseByTypes) { return toggledRules; } - ast.traverseByTypes(['multilineComment', 'singlelineComment'], function (comment) { + ast.traverseByTypes(['multilineComment', 'singlelineComment'], function (comment, i, parent) { var content = comment.content; if (!content) { return; @@ -112,7 +93,8 @@ module.exports.getToggledRules = function (ast) { addEnable(toggledRules, rules, comment.start.line, comment.start.column); break; case 'sass-lint:disable-block': - addDisableBlock(toggledRules, rules, comment); + // TODO: not sure what the appropriate behavior is if there is no parent block; currently NPEs + addDisableBlock(toggledRules, rules, parent); break; case 'sass-lint:disable-all': addDisableAll(toggledRules, comment.start.line, comment.start.column); @@ -167,4 +149,3 @@ module.exports.isResultEnabled = function (toggledRules) { return true; }; }; - diff --git a/tests/ruleToggler.js b/tests/ruleToggler.js index 30c3d4a8..07e18a67 100644 --- a/tests/ruleToggler.js +++ b/tests/ruleToggler.js @@ -70,7 +70,7 @@ describe.only('rule toggling', function () { assert(deepEqual(ruleToggles, { globalEnable: [], ruleEnable: { - a: [[false, 1, 4], [true, 3, 1]] + a: [[false, 1, 3], [true, 3, 1]] } }) === true); }); From 5777aed8fbd831b7ae6389e020c34d218b3f7fa1 Mon Sep 17 00:00:00 2001 From: "don.abrams" Date: Fri, 13 Nov 2015 10:54:08 -0700 Subject: [PATCH 49/67] fix range detection problem in isResultEnabled --- lib/ruleToggler.js | 18 ++++---- tests/ruleToggler.js | 46 ++++++++++++++++++- .../sass/ruleToggler-disable-then-enable.scss | 6 +++ tests/sass/ruleToggler-empty-comment.scss | 3 ++ tests/sass/ruleToggler-ignore-unknown.scss | 3 ++ 5 files changed, 66 insertions(+), 10 deletions(-) create mode 100644 tests/sass/ruleToggler-disable-then-enable.scss create mode 100644 tests/sass/ruleToggler-empty-comment.scss create mode 100644 tests/sass/ruleToggler-ignore-unknown.scss diff --git a/lib/ruleToggler.js b/lib/ruleToggler.js index 7bd87881..581dc6a0 100644 --- a/lib/ruleToggler.js +++ b/lib/ruleToggler.js @@ -117,6 +117,10 @@ module.exports.getToggledRules = function (ast) { return toggledRules; }; +var isBeforeOrSame = function (x, y, x2, y2) { + return x < x2 || (x === x2 && y < y2); +}; + module.exports.isResultEnabled = function (toggledRules) { return function (ruleResult) { var ruleId = ruleResult.ruleId; @@ -125,10 +129,9 @@ module.exports.isResultEnabled = function (toggledRules) { var column = ruleResult.column || 1; var isGloballyEnabled = toggledRules.globalEnable .reduce(function (acc, toggleRange) { - if (line <= toggleRange[1] && column <= toggleRange[2]) { - return acc; - } - return toggleRange[0]; + return isBeforeOrSame(line, column, toggleRange[1], toggleRange[2]) + ? acc + : toggleRange[0]; }, true); if (!isGloballyEnabled) { return false; @@ -138,10 +141,9 @@ module.exports.isResultEnabled = function (toggledRules) { } var isRuleEnabled = toggledRules.ruleEnable[ruleId] .reduce(function (acc, toggleRange) { - if (line <= toggleRange[1] && column <= toggleRange[2]) { - return acc; - } - return toggleRange[0]; + return isBeforeOrSame(line, column, toggleRange[1], toggleRange[2]) + ? acc + : toggleRange[0]; }, true); if (!isRuleEnabled) { return false; diff --git a/tests/ruleToggler.js b/tests/ruleToggler.js index 07e18a67..b9086890 100644 --- a/tests/ruleToggler.js +++ b/tests/ruleToggler.js @@ -19,7 +19,7 @@ var generateToggledRules = function (filename) { return getToggledRules(ast); }; -describe.only('rule toggling', function () { +describe('rule toggling', function () { describe('getToggledRules', function () { it('should allow all rules to be disabled', function () { assert(deepEqual(generateToggledRules('ruleToggler-disable-all.scss'), { @@ -74,6 +74,36 @@ describe.only('rule toggling', function () { } }) === true); }); + it('should be able to enable a disabled rule', function () { + var ruleToggles = generateToggledRules('ruleToggler-disable-then-enable.scss'); + assert(deepEqual(ruleToggles, { + globalEnable: [], + ruleEnable: { + a: [[false, 2, 5], [true, 4, 5]] + } + }) === true); + }); + it('should ignore comments that don\'t fit known formats', function () { + var ruleToggles = generateToggledRules('ruleToggler-ignore-unknown.scss'); + assert(deepEqual(ruleToggles, { + globalEnable: [], + ruleEnable: {} + }) === true); + }); + it('should ignore empty files', function () { + var ruleToggles = generateToggledRules('empty-file.scss'); + assert(deepEqual(ruleToggles, { + globalEnable: [], + ruleEnable: {} + }) === true); + }); + it('should ignore empty comments', function () { + var ruleToggles = generateToggledRules('ruleToggler-empty-comment.scss'); + assert(deepEqual(ruleToggles, { + globalEnable: [], + ruleEnable: {} + }) === true); + }); }); describe('isResultEnabled', function () { it('should disable all rules if global is disabled', function () { @@ -134,7 +164,7 @@ describe.only('rule toggling', function () { column: 1 }) === false); }); - it('should support enabling a previously re-enabled then disabled rule', function () { + it('should support enabling a previously re-enabled then disabled rule (in enabled part)', function () { assert(isResultEnabled({ globalEnable: [], ruleEnable: { @@ -146,6 +176,18 @@ describe.only('rule toggling', function () { column: 1 }) === true); }); + it('should support enabling a previously re-enabled then disabled rule (in disabled part)', function () { + assert(isResultEnabled({ + globalEnable: [], + ruleEnable: { + a: [[false, 1, 1], [true, 2, 1], [false, 3, 1], [true, 4, 1]] + } + })({ + ruleId: 'a', + line: 3, + column: 10 + }) === false); + }); it('should support disabling a rule that is later re-enabled', function () { assert(isResultEnabled({ globalEnable: [], diff --git a/tests/sass/ruleToggler-disable-then-enable.scss b/tests/sass/ruleToggler-disable-then-enable.scss new file mode 100644 index 00000000..44f55f67 --- /dev/null +++ b/tests/sass/ruleToggler-disable-then-enable.scss @@ -0,0 +1,6 @@ +p { + // sass-lint:disable a + border: none; + // sass-lint:enable a + color: blue; +} diff --git a/tests/sass/ruleToggler-empty-comment.scss b/tests/sass/ruleToggler-empty-comment.scss new file mode 100644 index 00000000..1d7676a4 --- /dev/null +++ b/tests/sass/ruleToggler-empty-comment.scss @@ -0,0 +1,3 @@ +p { + /**/ +} \ No newline at end of file diff --git a/tests/sass/ruleToggler-ignore-unknown.scss b/tests/sass/ruleToggler-ignore-unknown.scss new file mode 100644 index 00000000..698ec2e5 --- /dev/null +++ b/tests/sass/ruleToggler-ignore-unknown.scss @@ -0,0 +1,3 @@ +p { + //sass-lint:random a +} \ No newline at end of file From 51c0e93f70f21766685a671f4dc4c9268b9b1adc Mon Sep 17 00:00:00 2001 From: "don.abrams" Date: Fri, 13 Nov 2015 10:58:48 -0700 Subject: [PATCH 50/67] update comments --- lib/ruleToggler.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/ruleToggler.js b/lib/ruleToggler.js index 581dc6a0..47115569 100644 --- a/lib/ruleToggler.js +++ b/lib/ruleToggler.js @@ -64,7 +64,7 @@ var sortRange = function (toggleRangeA, toggleRangeB) { module.exports.getToggledRules = function (ast) { var toggledRules = { ruleEnable: { - // Format in here is [isDisabled, line, column] + // Format in here is [isEnabled, line, column] }, globalEnable: [] }; @@ -110,6 +110,7 @@ module.exports.getToggledRules = function (ast) { } }); // Sort these toggle stacks so reading them is easier (algorithmically). + // Usually already sorted but since it's not guaranteed by the contract with gonzales-pe, ensuring it is. toggledRules.globalEnable.sort(sortRange); Object.keys(toggledRules.ruleEnable).map(function (ruleId) { toggledRules.ruleEnable[ruleId].sort(sortRange); From 92c4226ca1aa3526de9462cdd142c080fe6e167f Mon Sep 17 00:00:00 2001 From: "don.abrams" Date: Fri, 13 Nov 2015 11:03:14 -0700 Subject: [PATCH 51/67] add toggle rule to source of border-zero test fixture --- tests/sass/border-zero.scss | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/sass/border-zero.scss b/tests/sass/border-zero.scss index 398d1cb8..5336c600 100644 --- a/tests/sass/border-zero.scss +++ b/tests/sass/border-zero.scss @@ -21,3 +21,9 @@ .norf { border: 1px; } + +.norf { + // sass-lint:disable border-zero + border: none; + // sass-lint:enable border-zero +} From ce01fa2991c499c7ce2201f1353bcdf41c962e95 Mon Sep 17 00:00:00 2001 From: "don.abrams" Date: Fri, 13 Nov 2015 11:07:22 -0700 Subject: [PATCH 52/67] remove references to doc-disabled-rules --- docs/options/toggle-rules-in-src.md | 4 -- docs/rules/doc-disabled-rules.md | 57 ------------------- .../sass/ruleToggler-disable-border-zero.scss | 17 ------ tests/sass/ruleToggler.scss | 17 ------ 4 files changed, 95 deletions(-) delete mode 100644 docs/rules/doc-disabled-rules.md delete mode 100644 tests/sass/ruleToggler-disable-border-zero.scss delete mode 100644 tests/sass/ruleToggler.scss diff --git a/docs/options/toggle-rules-in-src.md b/docs/options/toggle-rules-in-src.md index 6fca0aef..05e5ba78 100644 --- a/docs/options/toggle-rules-in-src.md +++ b/docs/options/toggle-rules-in-src.md @@ -69,7 +69,3 @@ a { border: none; // Failing result reported } ``` - -## doc-disabled-rules rule - -It is highly recommended that you use the doc-disabled-rules rule. diff --git a/docs/rules/doc-disabled-rules.md b/docs/rules/doc-disabled-rules.md deleted file mode 100644 index b318d0c6..00000000 --- a/docs/rules/doc-disabled-rules.md +++ /dev/null @@ -1,57 +0,0 @@ -# Document Disabled Rules - -Rule `doc-disabled-rules` will enforce that disabled rules have a comment immediately preceding the disabled rule. - -## Examples - -When enabled, the following are disallowed. - -```scss -// sass-lint:disable-all -p { - border: none; -} -// sass-lint:enable-all - -// sass-lint:disable border-zero -a { - border: none; -} - -li { - // sass-lint:disable-block border-zero - border: none; -} - -dd { - border: none; // sass-lint:disable-line border-zero -} -``` - -When enabled, the following are allowed. - -```scss -// Paragraphs are special ponies -// sass-lint:disable-all -p { - border: none; -} -// sass-lint:enable-all - -// We really prefer `border: none` in this file, for reasons. -// sass-lint:disable border-zero -a { - border: none; -} - -li { - // We really prefer `border: none` in this file, for reasons. - // sass-lint:disable-block border-zero - border: none; -} - -dd { - // We really prefer `border: none` in this file, for reasons. - border: none; // sass-lint:disable-line border-zero -} -``` diff --git a/tests/sass/ruleToggler-disable-border-zero.scss b/tests/sass/ruleToggler-disable-border-zero.scss deleted file mode 100644 index 885d2b32..00000000 --- a/tests/sass/ruleToggler-disable-border-zero.scss +++ /dev/null @@ -1,17 +0,0 @@ -// sass-lint:disable border-zero -p { - border: none; -} - -a { - border: none; - content: "hello"; -} - -li { - border: none; -} - -dl { - border: none; -} diff --git a/tests/sass/ruleToggler.scss b/tests/sass/ruleToggler.scss deleted file mode 100644 index 79e23d67..00000000 --- a/tests/sass/ruleToggler.scss +++ /dev/null @@ -1,17 +0,0 @@ -// sass-lint:disable-all -p { - border: none; -} - -a { - border: none; - content: "hello"; -} - -li { - border: none; -} - -dl { - border: none; -} From 1fa7f575f0002e3305af1508d30b2fee03a2c44d Mon Sep 17 00:00:00 2001 From: "don.abrams" Date: Tue, 17 Nov 2015 11:44:28 -0700 Subject: [PATCH 53/67] write test for disable ordering, find bug, fix bug --- lib/ruleToggler.js | 6 +++--- tests/ruleToggler.js | 16 ++++++++++++++++ tests/sass/ruleToggler-guarantee-order.scss | 17 +++++++++++++++++ 3 files changed, 36 insertions(+), 3 deletions(-) create mode 100644 tests/sass/ruleToggler-guarantee-order.scss diff --git a/lib/ruleToggler.js b/lib/ruleToggler.js index 47115569..736e6531 100644 --- a/lib/ruleToggler.js +++ b/lib/ruleToggler.js @@ -45,17 +45,17 @@ var sortRange = function (toggleRangeA, toggleRangeB) { var aLine = toggleRangeA[1], aCol = toggleRangeA[2], bLine = toggleRangeB[1], - bCol = toggleRangeA[2]; + bCol = toggleRangeB[2]; if (aLine < bLine) { return -1; } - if (bLine < aLine) { + if (aLine > bLine) { return 1; } if (aCol < bCol) { return -1; } - if (bCol < aCol) { + if (aCol > bCol) { return 1; } return 0; diff --git a/tests/ruleToggler.js b/tests/ruleToggler.js index b9086890..16210985 100644 --- a/tests/ruleToggler.js +++ b/tests/ruleToggler.js @@ -104,6 +104,22 @@ describe('rule toggling', function () { ruleEnable: {} }) === true); }); + it('should be ordered', function () { + var ruleToggles = generateToggledRules('ruleToggler-guarantee-order.scss'); + assert(deepEqual(ruleToggles, { + globalEnable: [], + ruleEnable: { + a: [[false, 1, 3], + [false, 2, 5], + [true, 6, 1], + [false, 8, 3], + [false, 8, 5], + [true, 12, 1], + [false, 14, 6], + [false, 14, 32]] + } + }) === true); + }); }); describe('isResultEnabled', function () { it('should disable all rules if global is disabled', function () { diff --git a/tests/sass/ruleToggler-guarantee-order.scss b/tests/sass/ruleToggler-guarantee-order.scss new file mode 100644 index 00000000..7c6d6178 --- /dev/null +++ b/tests/sass/ruleToggler-guarantee-order.scss @@ -0,0 +1,17 @@ +p { + // sass-lint:disable a + border: 0; + // sass-lint:disable-block a + font-size: 100%; +} + +a { // sass-lint:disable a + border: 0; + // sass-lint:disable-block a + font-size: 100%; +} + +li { /* sass-lint:disable a */ /* sass-lint:disable a */ + border: 0; + font-size: 100%; +} From d9cc73e61f88620c53016663c6a88efbbd930dbe Mon Sep 17 00:00:00 2001 From: saptarshi Date: Wed, 18 May 2016 21:50:01 +0530 Subject: [PATCH 54/67] Add changes to run isEnabledFilter after checking if the ast has content. Resolves #70 DCO 1.1 Signed-off-by: Saptarshi Dutta --- index.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/index.js b/index.js index aab32048..ac2d8807 100644 --- a/index.js +++ b/index.js @@ -127,6 +127,9 @@ sassLint.lintText = function (file, options, configPath) { } if (ast.content && ast.content.length > 0) { + + ruleToggles = getToggledRules(ast); + isEnabledFilter = isResultEnabled(ruleToggles); rules.forEach(function (rule) { detects = rule.rule.detect(ast, rule) .filter(isEnabledFilter); From 236e4f6659f2b5d0e17323f8eb4b1f30817ca078 Mon Sep 17 00:00:00 2001 From: saptarshi Date: Mon, 23 May 2016 08:12:39 +0530 Subject: [PATCH 55/67] Add changes to initialise rules toggle and isEnabledFilter as null. Resolves # 70 DCO 1.1 Signed-off-by: Saptarshi Dutta saptarshidutta31@yahoo.co.in --- index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/index.js b/index.js index ac2d8807..8f1fae84 100644 --- a/index.js +++ b/index.js @@ -107,8 +107,8 @@ sassLint.lintText = function (file, options, configPath) { results = [], errors = 0, warnings = 0, - ruleToggles = getToggledRules(ast), - isEnabledFilter = isResultEnabled(ruleToggles); + ruleToggles = null, + isEnabledFilter = null; try { ast = groot(file.text, file.format, file.filename); @@ -127,9 +127,9 @@ sassLint.lintText = function (file, options, configPath) { } if (ast.content && ast.content.length > 0) { - ruleToggles = getToggledRules(ast); isEnabledFilter = isResultEnabled(ruleToggles); + rules.forEach(function (rule) { detects = rule.rule.detect(ast, rule) .filter(isEnabledFilter); From e2466297921c83984aeec8ae274f860ee88e589e Mon Sep 17 00:00:00 2001 From: saptarshi Date: Thu, 16 Jun 2016 22:55:34 +0530 Subject: [PATCH 56/67] add test cases for sass and include necessary sass test files DCO 1.1 Signed-off-by: Saptarshi Dutta --- tests/ruleToggler.js | 106 ++++++++++++++++++ tests/sass/empty-file.sass | 0 tests/sass/ruleToggler-disable-a-block.sass | 2 + tests/sass/ruleToggler-disable-a-line.sass | 2 + tests/sass/ruleToggler-disable-a-rule.sass | 1 + ...ruleToggler-disable-all-then-reenable.sass | 3 + tests/sass/ruleToggler-disable-all.sass | 1 + .../ruleToggler-disable-multiple-rules.sass | 1 + .../sass/ruleToggler-disable-then-enable.sass | 5 + tests/sass/ruleToggler-empty-comment.sass | 2 + tests/sass/ruleToggler-guarantee-order.sass | 17 +++ tests/sass/ruleToggler-ignore-unknown.sass | 2 + 12 files changed, 142 insertions(+) create mode 100644 tests/sass/empty-file.sass create mode 100644 tests/sass/ruleToggler-disable-a-block.sass create mode 100644 tests/sass/ruleToggler-disable-a-line.sass create mode 100644 tests/sass/ruleToggler-disable-a-rule.sass create mode 100644 tests/sass/ruleToggler-disable-all-then-reenable.sass create mode 100644 tests/sass/ruleToggler-disable-all.sass create mode 100644 tests/sass/ruleToggler-disable-multiple-rules.sass create mode 100644 tests/sass/ruleToggler-disable-then-enable.sass create mode 100644 tests/sass/ruleToggler-empty-comment.sass create mode 100644 tests/sass/ruleToggler-guarantee-order.sass create mode 100644 tests/sass/ruleToggler-ignore-unknown.sass diff --git a/tests/ruleToggler.js b/tests/ruleToggler.js index 16210985..22b3c600 100644 --- a/tests/ruleToggler.js +++ b/tests/ruleToggler.js @@ -21,6 +21,9 @@ var generateToggledRules = function (filename) { describe('rule toggling', function () { describe('getToggledRules', function () { + ////////////////////////////// + // SCSS syntax tests + ////////////////////////////// it('should allow all rules to be disabled', function () { assert(deepEqual(generateToggledRules('ruleToggler-disable-all.scss'), { globalEnable: [[false, 1, 1]], @@ -120,6 +123,109 @@ describe('rule toggling', function () { } }) === true); }); + + ////////////////////////////// + // SASS syntax tests + ////////////////////////////// + it('should allow all rules to be disabled', function () { + assert(deepEqual(generateToggledRules('ruleToggler-disable-all.sass'), { + globalEnable: [[false, 1, 1]], + ruleEnable: {} + }) === true); + }); + it('should allow all rules to be disabled then re-enabled', function () { + var ruleToggles = generateToggledRules('ruleToggler-disable-all-then-reenable.sass'); + assert(deepEqual(ruleToggles, { + globalEnable: [ + [false, 1, 1], + [true, 3, 1] + ], + ruleEnable: {} + }) === true); + }); + it('should allow a single rule to be disabled', function () { + assert(deepEqual(generateToggledRules('ruleToggler-disable-a-rule.sass'), { + globalEnable: [], + ruleEnable: { + a: [[false, 1, 1]] + } + }) === true); + }); + it('should allow multiple rules to be disabled', function () { + assert(deepEqual(generateToggledRules('ruleToggler-disable-multiple-rules.sass'), { + globalEnable: [], + ruleEnable: { + a: [[false, 1, 1]], + b: [[false, 1, 1]], + c: [[false, 1, 1]], + d: [[false, 1, 1]] + } + }) === true); + }); + it('should be able to disable a single line', function () { + var ruleToggles = generateToggledRules('ruleToggler-disable-a-line.sass'); + assert(deepEqual(ruleToggles, { + globalEnable: [], + ruleEnable: { + a: [[false, 2, 1], [true, 3, 1]] + } + }) === true); + }); + it('should be able to disable a block of code', function () { + var ruleToggles = generateToggledRules('ruleToggler-disable-a-block.sass'); + assert(deepEqual(ruleToggles, { + globalEnable: [], + ruleEnable: { + a: [[false, 2, 1], [true, 2, 32]] + } + }) === true); + }); + it('should be able to enable a disabled rule', function () { + var ruleToggles = generateToggledRules('ruleToggler-disable-then-enable.sass'); + assert(deepEqual(ruleToggles, { + globalEnable: [], + ruleEnable: { + a: [[false, 2, 5], [true, 4, 5]] + } + }) === true); + }); + it('should ignore comments that don\'t fit known formats', function () { + var ruleToggles = generateToggledRules('ruleToggler-ignore-unknown.sass'); + assert(deepEqual(ruleToggles, { + globalEnable: [], + ruleEnable: {} + }) === true); + }); + it('should ignore empty files', function () { + var ruleToggles = generateToggledRules('empty-file.sass'); + assert(deepEqual(ruleToggles, { + globalEnable: [], + ruleEnable: {} + }) === true); + }); + it('should ignore empty comments', function () { + var ruleToggles = generateToggledRules('ruleToggler-empty-comment.sass'); + assert(deepEqual(ruleToggles, { + globalEnable: [], + ruleEnable: {} + }) === true); + }); + it('should be ordered', function () { + var ruleToggles = generateToggledRules('ruleToggler-guarantee-order.sass'); + assert(deepEqual(ruleToggles, { + globalEnable: [], + ruleEnable: { + a: [[false, 2, 1], + [false, 2, 5], + [true, 5, 20], + [false, 8, 1], + [false, 8, 5], + [true, 11, 20], + [false, 14, 5], + [false, 15, 5]] + } + }) === true); + }); }); describe('isResultEnabled', function () { it('should disable all rules if global is disabled', function () { diff --git a/tests/sass/empty-file.sass b/tests/sass/empty-file.sass new file mode 100644 index 00000000..e69de29b diff --git a/tests/sass/ruleToggler-disable-a-block.sass b/tests/sass/ruleToggler-disable-a-block.sass new file mode 100644 index 00000000..79981c64 --- /dev/null +++ b/tests/sass/ruleToggler-disable-a-block.sass @@ -0,0 +1,2 @@ +p + // sass-lint:disable-block a diff --git a/tests/sass/ruleToggler-disable-a-line.sass b/tests/sass/ruleToggler-disable-a-line.sass new file mode 100644 index 00000000..9c449198 --- /dev/null +++ b/tests/sass/ruleToggler-disable-a-line.sass @@ -0,0 +1,2 @@ +p + border-color: red // sass-lint:disable-line a diff --git a/tests/sass/ruleToggler-disable-a-rule.sass b/tests/sass/ruleToggler-disable-a-rule.sass new file mode 100644 index 00000000..5f04af37 --- /dev/null +++ b/tests/sass/ruleToggler-disable-a-rule.sass @@ -0,0 +1 @@ +// sass-lint:disable a diff --git a/tests/sass/ruleToggler-disable-all-then-reenable.sass b/tests/sass/ruleToggler-disable-all-then-reenable.sass new file mode 100644 index 00000000..63eeb627 --- /dev/null +++ b/tests/sass/ruleToggler-disable-all-then-reenable.sass @@ -0,0 +1,3 @@ +// sass-lint:disable-all + +// sass-lint:enable-all diff --git a/tests/sass/ruleToggler-disable-all.sass b/tests/sass/ruleToggler-disable-all.sass new file mode 100644 index 00000000..1776554a --- /dev/null +++ b/tests/sass/ruleToggler-disable-all.sass @@ -0,0 +1 @@ +// sass-lint:disable-all diff --git a/tests/sass/ruleToggler-disable-multiple-rules.sass b/tests/sass/ruleToggler-disable-multiple-rules.sass new file mode 100644 index 00000000..8e44d373 --- /dev/null +++ b/tests/sass/ruleToggler-disable-multiple-rules.sass @@ -0,0 +1 @@ +// sass-lint:disable a b, c d diff --git a/tests/sass/ruleToggler-disable-then-enable.sass b/tests/sass/ruleToggler-disable-then-enable.sass new file mode 100644 index 00000000..5fdc7cf4 --- /dev/null +++ b/tests/sass/ruleToggler-disable-then-enable.sass @@ -0,0 +1,5 @@ +p + // sass-lint:disable a + border: none + // sass-lint:enable a + color: blue diff --git a/tests/sass/ruleToggler-empty-comment.sass b/tests/sass/ruleToggler-empty-comment.sass new file mode 100644 index 00000000..eea459d1 --- /dev/null +++ b/tests/sass/ruleToggler-empty-comment.sass @@ -0,0 +1,2 @@ +p + /**/ diff --git a/tests/sass/ruleToggler-guarantee-order.sass b/tests/sass/ruleToggler-guarantee-order.sass new file mode 100644 index 00000000..62efa91c --- /dev/null +++ b/tests/sass/ruleToggler-guarantee-order.sass @@ -0,0 +1,17 @@ +p + // sass-lint:disable a + border: 0 + // sass-lint:disable-block a + font-size: 100% + +a + // sass-lint:disable a + border: 0 + // sass-lint:disable-block a + font-size: 100% + +li + /* sass-lint:disable a + /* sass-lint:disable a + border: 0 + font-size: 100% diff --git a/tests/sass/ruleToggler-ignore-unknown.sass b/tests/sass/ruleToggler-ignore-unknown.sass new file mode 100644 index 00000000..7913ac83 --- /dev/null +++ b/tests/sass/ruleToggler-ignore-unknown.sass @@ -0,0 +1,2 @@ +p + //sass-lint:random a From b7a883224225dea1feeba4e0bc4509e2ffe2cf04 Mon Sep 17 00:00:00 2001 From: Dan Purdy Date: Wed, 24 Aug 2016 00:38:03 +0100 Subject: [PATCH 57/67] :art: Tidy up and comment functions --- .eslintrc | 8 +++- lib/ruleToggler.js | 103 +++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 105 insertions(+), 6 deletions(-) diff --git a/.eslintrc b/.eslintrc index 1353aff2..236fef8c 100644 --- a/.eslintrc +++ b/.eslintrc @@ -135,8 +135,12 @@ rules: - global valid-jsdoc: - 2 - - prefer: - return: returns + - + requireParamDescription: true + requireReturnDescription: true + requireReturn: false + prefer: + return: "returns" wrap-iife: 2 yoda: - 2 diff --git a/lib/ruleToggler.js b/lib/ruleToggler.js index 736e6531..80eb05a5 100644 --- a/lib/ruleToggler.js +++ b/lib/ruleToggler.js @@ -1,5 +1,16 @@ 'use strict'; +/** + * Adds each rule in our array of rules in a disable comment into the toggledRules object + * under the correct rule name along with the line an column number where the disable comment + * was encountered + * + * @param {Object} toggledRules - Contains the information about each rule disable/enable + encountered and and what line/column it occurred on + * @param {Array} rules - An array of rule names + * @param {number} line - The line number the disable appeared on + * @param {number} column - The column number the disable appeared on + */ var addDisable = function (toggledRules, rules, line, column) { rules.map(function (rule) { toggledRules.ruleEnable[rule] = toggledRules.ruleEnable[rule] || []; @@ -7,6 +18,17 @@ var addDisable = function (toggledRules, rules, line, column) { }); }; +/** + * Adds each rule in our array of rules in a enable comment into the toggledRules object + * under the correct rule name along with the line an column number where the enable comment + * was encountered + * + * @param {Object} toggledRules - Contains the information about each rule enable + encountered and and what line/column it occurred on + * @param {Array} rules - An array of rule names + * @param {number} line - The line number the enable appeared on + * @param {number} column - The column number the enable appeared on + */ var addEnable = function (toggledRules, rules, line, column) { rules.map(function (rule) { toggledRules.ruleEnable[rule] = toggledRules.ruleEnable[rule] || []; @@ -14,6 +36,16 @@ var addEnable = function (toggledRules, rules, line, column) { }); }; +/** + * Adds each rule in our array of rules in a disable block comment into the toggledRules object + * under the correct rule name along with the line an column number where the disable block comment + * was encountered + * + * @param {Object} toggledRules - Contains the information about each rule enable + encountered and and what line/column that block occurred on + * @param {Array} rules - An array of rule names + * @param {Object} block - The block that is to be disabled + */ var addDisableBlock = function (toggledRules, rules, block) { rules.map(function (rule) { toggledRules.ruleEnable[rule] = toggledRules.ruleEnable[rule] || []; @@ -22,16 +54,43 @@ var addDisableBlock = function (toggledRules, rules, block) { }); }; +/** + * Adds a globally disabled flag to the toggled rules globalEnable property including the line and column + * that this comment was encountered on. + * + * @param {Object} toggledRules - Contains the information about the globally disable comment + encountered and and what line/column it occurred on + * @param {number} line - The line number the disable appeared on + * @param {number} column - The column number the disable appeared on + */ var addDisableAll = function (toggledRules, line, column) { toggledRules.globalEnable .push([false, line, column]); }; +/** + * Adds a globally enabled flag to the toggled rules globalEnable property including the line and column + * that this comment was encountered on. + * + * @param {Object} toggledRules - Contains the information about the globally enable comment + encountered and and what line/column it occurred on + * @param {number} line - The line number the enable appeared on + * @param {number} column - The column number the enable appeared on + */ var addEnableAll = function (toggledRules, line, column) { toggledRules.globalEnable .push([true, line, column]); }; +/** + * Adds a line disabled flag to the ruleEnable property of the toggledRules object for each rule name + * encountered in the comment and which line this comment was discovered on / refers to + * + * @param {Object} toggledRules - Contains the information about the line disable comment encountered, the rules + * it relates to and which line it was encountered on + * @param {Array} rules - An array of rule names to apply + * @param {number} line - The line number that this disable should refer to + */ var addDisableLine = function (toggledRules, rules, line) { rules.map(function (rule) { toggledRules.ruleEnable[rule] = toggledRules.ruleEnable[rule] || []; @@ -41,6 +100,15 @@ var addDisableLine = function (toggledRules, rules, line) { }); }; +/** + * This is the sorting function we use to sort the toggle stacks in our getToggledRules method + * First sorts by line and then by column if the lines are identical + * + * @param {Array} toggleRangeA - The first rule to sort + * @param {Array} toggleRangeB - The second rule to sort + * + * @returns {number} A pointer to signify to the sort method how the currently in focus value should be sorted + */ var sortRange = function (toggleRangeA, toggleRangeB) { var aLine = toggleRangeA[1], aCol = toggleRangeA[2], @@ -61,6 +129,29 @@ var sortRange = function (toggleRangeA, toggleRangeB) { return 0; }; +/** + * Checks if line number A is before line number B, if it's the same then it checks if the column of A + * is before the column of B + * + * @param {number} x - The line number of A + * @param {number} y - The column number of A + * @param {number} x2 - The line number of B + * @param {number} y2 - The column number of B + * + * @returns {Boolean} Whether the current line/column A is before or the same as B + */ +var isBeforeOrSame = function (x, y, x2, y2) { + return x < x2 || (x === x2 && y < y2); +}; + +/** + * Traverses the AST looking for sass-lint disable/enable comments and then building a Object representation + * of any it encounters + * + * @param {Object} ast - Gonzales PE abstract syntax tree + * + * @returns {Object} The toggledRules object containing all of our rule enable/disable information + */ module.exports.getToggledRules = function (ast) { var toggledRules = { ruleEnable: { @@ -118,10 +209,14 @@ module.exports.getToggledRules = function (ast) { return toggledRules; }; -var isBeforeOrSame = function (x, y, x2, y2) { - return x < x2 || (x === x2 && y < y2); -}; - +/** + * Filters our rule results by checking the lint result and it's line/column against our + * toggledRules object to see whether we should still be reporting this lint. + * + * @param {Object} toggledRules - The toggledRules object containing all of our rule enable/disable information + * + * @returns {Boolean} Whethere the current rule is disabled for this lint report + */ module.exports.isResultEnabled = function (toggledRules) { return function (ruleResult) { var ruleId = ruleResult.ruleId; From 395d76b2e91d51b575b8375a260ff4c702c0f230 Mon Sep 17 00:00:00 2001 From: greenkeeperio-bot Date: Mon, 31 Oct 2016 19:36:04 -0400 Subject: [PATCH 58/67] chore: drop support for Node.js 0.10 BREAKING CHANGE: This module no longer supports Node.js 0.10 --- .travis.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index d1b77185..a2cd1c3a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,11 +1,8 @@ language: node_js node_js: - - '0.10' - - '0.12' - '4' - '5' - node - - iojs after_success: npm run coveralls notifications: slack: From 32bc08865881aec2a084fd118002592f5a85944a Mon Sep 17 00:00:00 2001 From: Dan Purdy Date: Wed, 2 Nov 2016 00:18:15 +0000 Subject: [PATCH 59/67] :fire: Remove node 0.10 - 0.12 --- appveyor.yml | 8 -------- 1 file changed, 8 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index 8bab6ec5..6b46be8f 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -5,18 +5,10 @@ init: # Test against this version of Node.js environment: matrix: - - nodejs_version: '0.10' - - nodejs_version: '0.12' - # io.js - - nodejs_version: '1' - nodejs_version: '4' - nodejs_version: '5' # Latest stable version of Node - nodejs_version: 'stable' -matrix: - # Help instructions and version number don't show in appveyor - allow_failures: - - nodejs_version: '0.10' # Install scripts. (runs after repo cloning) install: From 96bd89d3ded8a71fc9e1ee454e4fb50a43c7d9c6 Mon Sep 17 00:00:00 2001 From: Dan Purdy Date: Wed, 2 Nov 2016 01:14:25 +0000 Subject: [PATCH 60/67] :art: Correct spelling issues --- lib/ruleToggler.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/ruleToggler.js b/lib/ruleToggler.js index 80eb05a5..f4bbd520 100644 --- a/lib/ruleToggler.js +++ b/lib/ruleToggler.js @@ -2,7 +2,7 @@ /** * Adds each rule in our array of rules in a disable comment into the toggledRules object - * under the correct rule name along with the line an column number where the disable comment + * under the correct rule name along with the line and column number where the disable comment * was encountered * * @param {Object} toggledRules - Contains the information about each rule disable/enable @@ -20,7 +20,7 @@ var addDisable = function (toggledRules, rules, line, column) { /** * Adds each rule in our array of rules in a enable comment into the toggledRules object - * under the correct rule name along with the line an column number where the enable comment + * under the correct rule name along with the line and column number where the enable comment * was encountered * * @param {Object} toggledRules - Contains the information about each rule enable @@ -38,7 +38,7 @@ var addEnable = function (toggledRules, rules, line, column) { /** * Adds each rule in our array of rules in a disable block comment into the toggledRules object - * under the correct rule name along with the line an column number where the disable block comment + * under the correct rule name along with the line and column number where the disable block comment * was encountered * * @param {Object} toggledRules - Contains the information about each rule enable @@ -58,7 +58,7 @@ var addDisableBlock = function (toggledRules, rules, block) { * Adds a globally disabled flag to the toggled rules globalEnable property including the line and column * that this comment was encountered on. * - * @param {Object} toggledRules - Contains the information about the globally disable comment + * @param {Object} toggledRules - Contains the information about the global disable comment encountered and and what line/column it occurred on * @param {number} line - The line number the disable appeared on * @param {number} column - The column number the disable appeared on @@ -72,7 +72,7 @@ var addDisableAll = function (toggledRules, line, column) { * Adds a globally enabled flag to the toggled rules globalEnable property including the line and column * that this comment was encountered on. * - * @param {Object} toggledRules - Contains the information about the globally enable comment + * @param {Object} toggledRules - Contains the information about the global enable comment encountered and and what line/column it occurred on * @param {number} line - The line number the enable appeared on * @param {number} column - The column number the enable appeared on @@ -145,7 +145,7 @@ var isBeforeOrSame = function (x, y, x2, y2) { }; /** - * Traverses the AST looking for sass-lint disable/enable comments and then building a Object representation + * Traverses the AST looking for sass-lint disable/enable comments and then builds an Object/node representation * of any it encounters * * @param {Object} ast - Gonzales PE abstract syntax tree @@ -184,7 +184,7 @@ module.exports.getToggledRules = function (ast) { addEnable(toggledRules, rules, comment.start.line, comment.start.column); break; case 'sass-lint:disable-block': - // TODO: not sure what the appropriate behavior is if there is no parent block; currently NPEs + // future ref: not sure what the appropriate behavior is if there is no parent block; currently NPEs addDisableBlock(toggledRules, rules, parent); break; case 'sass-lint:disable-all': @@ -210,12 +210,12 @@ module.exports.getToggledRules = function (ast) { }; /** - * Filters our rule results by checking the lint result and it's line/column against our + * Filters our rule results by checking the lint result and its line/column against our * toggledRules object to see whether we should still be reporting this lint. * * @param {Object} toggledRules - The toggledRules object containing all of our rule enable/disable information * - * @returns {Boolean} Whethere the current rule is disabled for this lint report + * @returns {Boolean} Whether the current rule is disabled for this lint report */ module.exports.isResultEnabled = function (toggledRules) { return function (ruleResult) { From d26b74bc49d420174bbfb10b3b60b7a9893d446a Mon Sep 17 00:00:00 2001 From: Dan Purdy Date: Wed, 2 Nov 2016 01:47:05 +0000 Subject: [PATCH 61/67] :mag: Add readme instructions --- README.md | 79 ++++++++++++++++++++++- docs/{options => }/toggle-rules-in-src.md | 0 2 files changed, 78 insertions(+), 1 deletion(-) rename docs/{options => }/toggle-rules-in-src.md (100%) diff --git a/README.md b/README.md index 5e43b27e..0d3cad35 100644 --- a/README.md +++ b/README.md @@ -82,7 +82,7 @@ If you want to configure options, set the rule to an array, where the first item Here is an example configuration of a rule, where we are specifying that breaking the [indentation rule](https://github.com/sasstools/sass-lint/blob/master/docs/rules/indentation.md) should be treated as an error (its severity set to two), and setting the `size` option of the rule to 2 spaces: ```yml -rules: +rules: indentation: - 2 - @@ -93,6 +93,83 @@ rules: --- +## Disabling Linters via Source + +Special comments can be used to disable and enable certain rules throughout your source files in a variety of scenarios. These can be useful when dealing with legacy code or with certain necessary code smells. You can read the documentation for this feature [here](https://github.com/sasstools/sass-lint/tree/master/docs/toggle-rules-in-src.md). + +Below are examples of how to use this feature: + + +### Disable a rule for the entire file + +```scss +// sass-lint:disable border-zero +p { + border: none; // No lint reported +} +``` + +### Disable more than 1 rule + +```scss +// sass-lint:disable border-zero, quotes +p { + border: none; // No lint reported + content: "hello"; // No lint reported +} +``` + +### Disable a rule for a single line + +```scss +p { + border: none; // sass-lint:disable-line border-zero +} +``` + +### Disable all lints within a block (and all contained blocks) + +```scss +p { + // sass-lint:disable-block border-zero + border: none; // No result reported +} + +a { + border: none; // Failing result reported +} +``` + +### Disable and enable again + +```scss +// sass-lint:disable border-zero +p { + border: none; // No result reported +} +// sass-lint:enable border-zero + +a { + border: none; // Failing result reported +} +``` + +### Disable/enable all linters + +```scss +// sass-lint:disable-all +p { + border: none; // No result reported +} +// sass-lint:enable-all + +a { + border: none; // Failing result reported +} +``` + +--- + ## CLI Sass Lint [`v1.1.0`](https://github.com/sasstools/sass-lint/releases/tag/v1.1.0) introduced the ability to run Sass Lint through a command line interface. See the [CLI Docs](https://github.com/sasstools/sass-lint/tree/master/docs/cli) for full documentation on how to use the CLI. diff --git a/docs/options/toggle-rules-in-src.md b/docs/toggle-rules-in-src.md similarity index 100% rename from docs/options/toggle-rules-in-src.md rename to docs/toggle-rules-in-src.md From 80ceebc041424626ca1be615a5153590bb8d3281 Mon Sep 17 00:00:00 2001 From: Dan Purdy Date: Wed, 2 Nov 2016 02:23:28 +0000 Subject: [PATCH 62/67] :bug: Prevent invalid conventions border-zero --- docs/rules/border-zero.md | 16 ++++++++++++++++ lib/rules/border-zero.js | 20 ++++++++++++++++++-- tests/rules/border-zero.js | 30 ++++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 2 deletions(-) diff --git a/docs/rules/border-zero.md b/docs/rules/border-zero.md index a1377b7f..dcd9e043 100644 --- a/docs/rules/border-zero.md +++ b/docs/rules/border-zero.md @@ -6,6 +6,8 @@ Rule `border-zero` will enforce whether one should use `0` or `none` when specif * `convention`: `'0'`/`'none'` (defaults to `0`) +> If an invalid convention is provided the rule will default back to `convention: '0'`. An extra warning/error will also be thrown on `line 1` `column 1` of a file with a lint issue to inform you of this fact. + ## Examples When `convention: '0'`, the following are allowed. When `convention: 'none'`, the following are disallowed: @@ -31,3 +33,17 @@ When `convention: 'none'`, the following are allowed. When `convention: '0'`, th border-left: none; } ``` + +### Invalid conventions + +When the invalid convention `convention: 'zero'` is supplied, the following are allowed as the rule defaults to `convention: '0'`. + +```scss +.foo { + border: none; +} + +.bar { + border-left: 0; +} +``` diff --git a/lib/rules/border-zero.js b/lib/rules/border-zero.js index 3f3d2b54..ab208902 100644 --- a/lib/rules/border-zero.js +++ b/lib/rules/border-zero.js @@ -3,6 +3,7 @@ var helpers = require('../helpers'); var borders = ['border', 'border-top', 'border-right', 'border-bottom', 'border-left']; +var allowedConventions = ['0', 'none']; module.exports = { 'name': 'border-zero', @@ -11,6 +12,11 @@ module.exports = { }, 'detect': function (ast, parser) { var result = []; + var userConvention = parser.options.convention.toString(); + var convention = allowedConventions.indexOf(userConvention) !== -1 + ? userConvention + : allowedConventions[0]; + var invalidConvention = convention !== userConvention; ast.traverseByType('declaration', function (declaration) { var isBorder = false; @@ -29,12 +35,22 @@ module.exports = { var node = item.content[0]; if (node.type === 'number' || node.type === 'ident') { if (node.content === '0' || node.content === 'none') { - if (parser.options.convention.toString() !== node.content) { + if (convention !== node.content) { + if (invalidConvention) { + invalidConvention = false; + result = helpers.addUnique(result, { + 'ruleId': parser.rule.name, + 'line': 1, + 'column': 1, + 'message': 'The border-zero convention `' + userConvention + ' in your config file is not valid. Defaulted to convention \'0\'', + 'severity': parser.severity + }); + } result = helpers.addUnique(result, { 'ruleId': parser.rule.name, 'line': node.start.line, 'column': node.start.column, - 'message': 'A value of `' + node.content + '` is not allowed. `' + parser.options.convention + '` must be used.', + 'message': 'A value of `' + node.content + '` is not allowed. `' + convention + '` must be used.', 'severity': parser.severity }); } diff --git a/tests/rules/border-zero.js b/tests/rules/border-zero.js index 21080c0b..1c026aa6 100644 --- a/tests/rules/border-zero.js +++ b/tests/rules/border-zero.js @@ -44,6 +44,21 @@ describe('border zero - scss', function () { done(); }); }); + + it('invalid convention [convention: \'zero\']', function (done) { + // defaults to convention 0 + lint.test(file, { + 'border-zero': [ + 1, + { + 'convention': 'zero' + } + ] + }, function (data) { + lint.assert.equal(4, data.warningCount); + done(); + }); + }); }); ////////////////////////////// @@ -88,4 +103,19 @@ describe('border zero - sass', function () { done(); }); }); + + it('invalid convention [convention: \'zero\']', function (done) { + // defaults to convention 0 + lint.test(file, { + 'border-zero': [ + 1, + { + 'convention': 'zero' + } + ] + }, function (data) { + lint.assert.equal(4, data.warningCount); + done(); + }); + }); }); From 31d26aa875c1e8198772e79c5e11aa6e6ed26e68 Mon Sep 17 00:00:00 2001 From: greenkeeperio-bot Date: Mon, 31 Oct 2016 15:20:38 -0400 Subject: [PATCH 63/67] chore(package): update eslint to version 3.9.1 https://greenkeeper.io/ --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index a9ce9644..952ec3ee 100644 --- a/package.json +++ b/package.json @@ -29,7 +29,7 @@ "homepage": "https://github.com/sasstools/sass-lint", "dependencies": { "commander": "^2.8.1", - "eslint": "^2.7.0", + "eslint": "^3.9.1", "front-matter": "2.1.0", "fs-extra": "^1.0.0", "glob": "^7.0.0", From 75f703d79e80b698f37fe7af3d0a0bef9e8cc33f Mon Sep 17 00:00:00 2001 From: Dan Purdy Date: Sun, 6 Nov 2016 14:33:46 +0000 Subject: [PATCH 64/67] :art: Fix lint errors --- index.js | 2 +- lib/rules/space-around-operator.js | 2 +- lib/rules/zero-unit.js | 18 +++++++++++++-- tests/main.js | 36 ++++++++++++++++-------------- 4 files changed, 37 insertions(+), 21 deletions(-) diff --git a/index.js b/index.js index 8f1fae84..f3ff5b5f 100644 --- a/index.js +++ b/index.js @@ -14,7 +14,7 @@ var slConfig = require('./lib/config'), var getToggledRules = ruleToggler.getToggledRules, isResultEnabled = ruleToggler.isResultEnabled; -var sassLint = function (config) { +var sassLint = function (config) { // eslint-disable-line no-unused-vars config = require('./lib/config')(config); return; }; diff --git a/lib/rules/space-around-operator.js b/lib/rules/space-around-operator.js index 8a4bc9c7..eb3d6db3 100644 --- a/lib/rules/space-around-operator.js +++ b/lib/rules/space-around-operator.js @@ -262,7 +262,7 @@ var checkSpacing = function (node, i, parent, parser, result) { } } } - return true; + return result; }; module.exports = { diff --git a/lib/rules/zero-unit.js b/lib/rules/zero-unit.js index 3505d35f..ac192960 100644 --- a/lib/rules/zero-unit.js +++ b/lib/rules/zero-unit.js @@ -2,8 +2,22 @@ var helpers = require('../helpers'); -var units = ['em', 'ex', 'ch', 'rem', 'vh', 'vw', 'vmin', 'vmax', - 'px', 'mm', 'cm', 'in', 'pt', 'pc']; +var units = [ + 'em', + 'ex', + 'ch', + 'rem', + 'vh', + 'vw', + 'vmin', + 'vmax', + 'px', + 'mm', + 'cm', + 'in', + 'pt', + 'pc' +]; module.exports = { 'name': 'zero-unit', diff --git a/tests/main.js b/tests/main.js index 05c7d9d6..28131df9 100644 --- a/tests/main.js +++ b/tests/main.js @@ -140,23 +140,25 @@ describe('sass lint', function () { // ============================================================================== it('should not try to blindly read and lint a directory', function (done) { var fileSpy = sinon.spy(lint, 'lintText'); - lintFiles('tests/dir-test/**/*.scss', {options: { - 'merge-default-rules': false - }, - rules: { - 'no-ids': 1 - }}, '', function (data) { - assert.equal(1, data[0].warningCount); - assert.equal(0, data[0].errorCount); - assert.equal(1, data[0].messages.length); - - assert(fileSpy.called); - assert(fileSpy.calledOnce); - assert(fileSpy.calledWithMatch({format: 'scss', filename: 'tests/dir-test/dir.scss/test.scss'})); - assert(fileSpy.neverCalledWithMatch({filename: 'tests/dir-test/dir.scss'})); - fileSpy.reset(); - done(); - }); + lintFiles('tests/dir-test/**/*.scss', { + options: { + 'merge-default-rules': false + }, + rules: { + 'no-ids': 1 + }}, '', function (data) { + assert.equal(1, data[0].warningCount); + assert.equal(0, data[0].errorCount); + assert.equal(1, data[0].messages.length); + + assert(fileSpy.called); + assert(fileSpy.calledOnce); + assert(fileSpy.calledWithMatch({format: 'scss', filename: 'tests/dir-test/dir.scss/test.scss'})); + assert(fileSpy.neverCalledWithMatch({filename: 'tests/dir-test/dir.scss'})); + fileSpy.reset(); + done(); + } + ); }); // ============================================================================== From a7c8833b820c5a1a4507f6c1aff8c0256a2962ab Mon Sep 17 00:00:00 2001 From: Dan Purdy Date: Sun, 6 Nov 2016 15:45:20 +0000 Subject: [PATCH 65/67] Prepare 1.10 --- CHANGELOG.md | 45 +++++++++++++++++++++++++++++++++++++++++++++ package.json | 2 +- 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e0c23689..bbb18818 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,50 @@ # Sass Lint Changelog +## v1.10.0 + +**November 6th, 2016** + +The 'you can ignore those bad habits again' update + +** :tada: DISABLE LINTERS :tada: ** +The ability to enable and disable linters on the fly has finally(!) been added [#677](https://github.com/sasstools/sass-lint/pull/677) [docs](https://github.com/sasstools/sass-lint/blob/master/docs/toggle-rules-in-src.md) + +A massive thank you to everyone who commented/contributed/reported and tested this feature this was very much a community effort here. An extra special thank you to +* [@donabrams](https://github.com/donabrams) + +For his initial hard work in getting this off the ground. There were lots of others who have fixed everything from test issues to AST issues to make this possible afterwards, so thanks to you too! + +**New Features** +* `max-warnings` which is available with the sass-lint CLI is now available as an option in your config file too [#857](https://github.com/sasstools/sass-lint/pull/857) +* **New Rule** `no-url-domains` rule [#846](https://github.com/sasstools/sass-lint/pull/846) [docs](https://github.com/sasstools/sass-lint/blob/master/docs/rules/no-url-domains.md) +* **New Rule** `max-line-length` rule was added [#840](https://github.com/sasstools/sass-lint/pull/840) [docs](https://github.com/sasstools/sass-lint/blob/master/docs/rules/max-line-length.md) +* **New Rule** `max-file-line-count` rule was added [#842](https://github.com/sasstools/sass-lint/pull/842) [docs](https://github.com/sasstools/sass-lint/blob/master/docs/rules/max-file-line-count.md) +* **New Rule** `declarations-before-nesting` rule was added [#866](https://github.com/sasstools/sass-lint/pull/866) [docs](https://github.com/sasstools/sass-lint/blob/master/docs/rules/declarations-before-nesting.md) + +**Fixes** +* Fixed an issue with an un handled error being thrown in certain circumstances for the `space-before-colon` rule [#894](https://github.com/sasstools/sass-lint/pull/894) +* Operators in variable names are now handled correctly for the `variable-name-format` rule [#903](https://github.com/sasstools/sass-lint/pull/903) +* Fixed an issue with string values in the `shorthand-values` rule [#848](https://github.com/sasstools/sass-lint/pull/848) +* Fixed an issue with valid strict BEM producing an error in the `*-name-format` rules [#892](https://github.com/sasstools/sass-lint/pull/892) +* Fixed an issue with non-string user conventions in the `border-zero` rule [#913](https://github.com/sasstools/sass-lint/pull/913) +* Fixed an issue where BOM markers in files were causing parse errors or random errors/warnings [#893](https://github.com/sasstools/sass-lint/pull/893) +* Fixed an issue with interpolates properties in the `no-duplicate-properties` rule [#915](https://github.com/sasstools/sass-lint/pull/915) +* Fixed a possible error with invalid user conventions in the `border-zero` rule [#926](https://github.com/sasstools/sass-lint/pull/926) + +**Changes** +* Node 0.10 and 0.12 are no longer officially supported by sass-lint. We've not deliberately broken these builds but we will no longer be testing against them either [#896](https://github.com/sasstools/sass-lint/issues/896) & [#924](https://github.com/sasstools/sass-lint/pull/924) +* In future the `no-url-protocols` rule will not lint domains in URL's for now a new flag is added to mimic this behaviour. The new `no-url-domains` rule can be used instead [#813](https://github.com/sasstools/sass-lint/issues/813) +* Front matter such as those present in Jekyll templates will now be ignored in all files before passing to the AST / Linting [897](https://github.com/sasstools/sass-lint/pull/897) +* Running the tests no longer required sass-lint development to be `npm-link`ed or globally installed. [#911](https://github.com/sasstools/sass-lint/pull/911) +* The concentric property list in `property-sort-order` was updated to reflect the latest release [#922](https://github.com/sasstools/sass-lint/pull/922) + +**Updates** +* AST fixes have arrived with version 3.4.7 of gonzales-pe [#906](https://github.com/sasstools/sass-lint/pull/906) +* Updated to the latest versions of many other packages + +**Documentation** +* The documentation around configuring a rule was tidied up and made clearer [#910](https://github.com/sasstools/sass-lint/pull/910) + ## v1.9.1 **August 25, 2016** diff --git a/package.json b/package.json index 952ec3ee..131bdd60 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "sass-lint", - "version": "1.9.1", + "version": "1.10.0", "description": "All Node Sass linter!", "main": "index.js", "scripts": { From 07fa64969e9c722101b2239e1436feafe16fae5a Mon Sep 17 00:00:00 2001 From: Dan Purdy Date: Sun, 6 Nov 2016 15:51:11 +0000 Subject: [PATCH 66/67] Add contributor thanks --- CHANGELOG.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index bbb18818..59df4184 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,18 @@ For his initial hard work in getting this off the ground. There were lots of oth **Documentation** * The documentation around configuring a rule was tidied up and made clearer [#910](https://github.com/sasstools/sass-lint/pull/910) +**Special thanks to** + +* [bgriffith](https://github.com/bgriffith) +* [donabrams](https://github.com/donabrams) +* [danpurdy](https://github.com/DanPurdy) +* [danwaz](https://github.com/danwaz) +* [lucasjahn](https://github.com/https://github.com/lucasjahn) +* [mrjamesriley](https://github.com/mrjamesriley) +* [notrobin](https://github.com/nottrobin) +* [onishiweb](https://github.com/onishiweb) +* [richarddewit](https://github.com/https://github.com/richarddewit) + ## v1.9.1 **August 25, 2016** From feea65db6a571ffc8c58844bcf2e9942425749bf Mon Sep 17 00:00:00 2001 From: Dan Purdy Date: Sun, 6 Nov 2016 15:53:32 +0000 Subject: [PATCH 67/67] Fix links --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 59df4184..ddd13e9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,11 +51,11 @@ For his initial hard work in getting this off the ground. There were lots of oth * [donabrams](https://github.com/donabrams) * [danpurdy](https://github.com/DanPurdy) * [danwaz](https://github.com/danwaz) -* [lucasjahn](https://github.com/https://github.com/lucasjahn) +* [lucasjahn](https://github.com/lucasjahn) * [mrjamesriley](https://github.com/mrjamesriley) * [notrobin](https://github.com/nottrobin) * [onishiweb](https://github.com/onishiweb) -* [richarddewit](https://github.com/https://github.com/richarddewit) +* [richarddewit](https://github.com/richarddewit) ## v1.9.1