Skip to content

Commit

Permalink
Improves disallowAttributeConcatenation and disallowStringConcatenati…
Browse files Browse the repository at this point in the history
…on to remove false positives
  • Loading branch information
Ben Edwards committed Feb 25, 2016
1 parent 34900cc commit b9b3497
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 11 deletions.
8 changes: 5 additions & 3 deletions lib/rules/disallow-attribute-concatenation.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,11 @@ module.exports.prototype =

, lint: function (file, errors) {

var regex = /\+(?=([^']*'[^']*')*[^']*$)/

file.addErrorForAllLinesByType('attribute', regex, true, errors, 'Attribute concatenation must not be used')
file.iterateTokensByType('attribute', function (token) {
if (utils.concatenationRegex.test(token.val)) {
errors.add('Attribute concatenation must not be used', token.line)
}
})

}
}
10 changes: 6 additions & 4 deletions lib/rules/disallow-string-concatenation.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,13 @@ module.exports.prototype =

, lint: function (file, errors) {

var regex = /['"]\s*\+|\+\s*['"]/

file.addErrorForAllLinesByFilter(function (token) {
file.iterateTokensByFilter(function (token) {
return (token.type === 'code' && token.buffer)
}, regex, true, errors, 'String concatenation must not be used')
}, function (token) {
if (utils.concatenationRegex.test(token.val)) {
errors.add('String concatenation must not be used', token.line)
}
})

}
}
2 changes: 2 additions & 0 deletions lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,5 @@ exports.validateTrueOptions = function (name, options) {
}

exports.htmlTagBoundaryTypes = [ ':', 'newline', 'indent', 'outdent', 'eos' ]

exports.concatenationRegex = /.['"]\s*\+|\+\s*['"]./
10 changes: 6 additions & 4 deletions test/rules/disallow-attribute-concatenation.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,16 @@ function createTest (linter, fixturesPath) {
it('should report attribute concatenation', function () {
assert.equal(linter.checkString('a(href=\'text \' + title) Link').length, 1)
assert.equal(linter.checkString('a(href=title + \'text \') Link').length, 1)
assert.equal(linter.checkString('a(href=title + "text ") Link').length, 1)
assert.equal(linter.checkString('a(href=title + title)').length, 0)
assert.equal(linter.checkString('img(src=\'logo.png\', alt=\'+G Logo\')').length, 0)
assert.equal(linter.checkString('img(src="logo.png", alt="G+ Logo")').length, 0)
assert.equal(linter.checkString('img(src=\'logo.png\', alt=\'G Logo+\')').length, 0)
assert.equal(linter.checkString('img(src=\'logo.png\', alt=\'+\')').length, 0)
})

it('should not report attribute interpolation', function () {
assert.equal(linter.checkString('a(href=\'#{title}\') Link').length, 0)
assert.equal(linter.checkString('img(src=\'logo.png\', alt=\'+G Logo\')').length, 0)
assert.equal(linter.checkString('img(src=\'logo.png\', alt=\'G+ Logo\')').length, 0)
assert.equal(linter.checkString('img(src=\'logo.png\', alt=\'G Logo+\')').length, 0)
assert.equal(linter.checkString('img(src=\'logo.png\', alt=\'+\')').length, 0)
})

it('should report multiple errors found in file', function () {
Expand Down
3 changes: 3 additions & 0 deletions test/rules/disallow-string-concatenation.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ function createTest (linter, fixturesPath) {

it('should report string concatenation', function () {
assert.equal(linter.checkString('h1= title + \'text\'').length, 1)
assert.equal(linter.checkString('h1(class="test" + "test")= title + \'text\'').length, 1)
assert.equal(linter.checkString('h1= \'text+\'').length, 0)
assert.equal(linter.checkString('h1= test + test').length, 0)
})

it('should not report string interpolation', function () {
Expand Down

0 comments on commit b9b3497

Please sign in to comment.