Skip to content

Commit

Permalink
Merge pull request #673 from DanPurdy/feature/fix-shorthand-values
Browse files Browse the repository at this point in the history 
Fix shorthand values
  • Loading branch information
@bgriffith
bgriffith committed May 4, 2016
2 parents e5010bd + 32a121a commit 15ab86b
Show file tree
Hide file tree
Showing 4 changed files with 277 additions and 73 deletions.
164 changes: 107 additions & 57 deletions lib/rules/shorthand-values.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@ var shortVals = [
'padding'
];

/**
* Checks to see if a series of values can be condensed down to a singular value
*
* @param {array} value - The array of values to check
* @param {array} allowed - The parser options to specify the levels allowed to condense to
* @returns {boolean} Whether the values can be condensed to a singular value
*/
var condenseToOne = function (value, allowed) {
if (allowed.indexOf(1) !== -1 && value.length > 1) {
for (var i = 1; i < value.length; i++) {
Expand All @@ -23,6 +30,13 @@ var condenseToOne = function (value, allowed) {
return false;
};

/**
* Checks to see if a series of values can be condensed down to two values
*
* @param {array} value - The array of values to check
* @param {array} allowed - The parser options to specify the levels allowed to condense to
* @returns {boolean} Whether the values can be condensed to two values
*/
var condenseToTwo = function (value, allowed) {
if (allowed.indexOf(2) !== -1 && value.length > 2) {
if ((value[0] === value[2] && value[1] === value[3]) || (value[0] === value[2] && !value[3] && value[0] !== value[1])) {
Expand All @@ -32,6 +46,13 @@ var condenseToTwo = function (value, allowed) {
return false;
};

/**
* Checks to see if a series of values can be condensed down to three values
*
* @param {array} value - The array of values to check
* @param {array} allowed - The parser options to specify the levels allowed to condense to
* @returns {boolean} Whether the values can be condensed to three values
*/
var condenseToThree = function (value, allowed) {
if (allowed.indexOf(3) !== -1 && value.length > 3) {
if (value[1] === value[3] ) {
Expand All @@ -41,6 +62,72 @@ var condenseToThree = function (value, allowed) {
return false;
};

/**
* Used to scan property values and create a string representation of the values to display
*
* @param {Object} node - The current node
* @returns {string} A string reconstruction of the current properties value
*/
var scanValue = function (node) {
var curValue = [];
var fullVal = '';
node.forEach(function (val) {
// add to our value string depending on node type
if (val.is('dimension')) {
val.forEach(function (el) {
fullVal += el.content;
});
}

else if (val.is('percentage')) {
val.forEach(function (el) {
fullVal += el.content + '%';
});
}

else if (val.is('interpolation')) {
fullVal += '#{' + scanValue(val.content) + '}';
}

else if (val.is('operator') || val.is('ident') || val.is('number') || val.is('unaryOperator')) {
fullVal += val.content;
}

else if (val.is('variable')) {
val.forEach(function (el) {
fullVal += '$' + el.content;
});
}

else if (val.is('function')) {

var func = val.first('ident'),
args = '';

val.forEach('arguments', function (arg) {
args = scanValue(arg).join(' ');
});

fullVal = func + '(' + args + ')';
}

else if (val.is('space')) {
// This is a non value character such as a space
// We want to start another value here
curValue.push(fullVal);

// reset the value string for the next iteration
fullVal = '';
}
});

if (fullVal !== '') {
// The last dimension in a value will not be followed by a character so we push here
curValue.push(fullVal);
}
return curValue;
};

module.exports = {
'name': 'shorthand-values',
'defaults': {
Expand Down Expand Up @@ -71,70 +158,33 @@ module.exports = {
var value = [];

if (item.is('value')) {
var node = item.content,
fullVal = '';
var node = item.content;

// Build each value into an array of strings with value and type
node.forEach(function (val) {
// add to our value string depending on node type
if (val.is('dimension')) {
val.forEach(function (el) {
fullVal += el.content;
});
}
value = scanValue(node);

else if (val.is('percentage')) {
val.forEach(function (el) {
fullVal += el.content + '%';
});
}
if (value.length <= 4 && value.length >= 1) {
var output = [];

else if (val.is('operator') || val.is('ident') || val.is('number') || val.is('unaryOperator')) {
fullVal += val.content;
// check which values can condense
if (condenseToOne(value, parser.options['allowed-shorthands'])) {
output = [value[0]];
}

else if (val.is('variable')) {
val.forEach(function (el) {
fullVal += '$' + el.content;
});
else if (condenseToTwo(value, parser.options['allowed-shorthands'])) {
output = [value[0], value[1]];
}

else if (val.is('space')) {
// This is a non value character such as a space
// We want to start another value here
value.push(fullVal);

// reset the value string for the next iteration
fullVal = '';
else if (condenseToThree(value, parser.options['allowed-shorthands'])) {
output = [value[0], value[1], value[2]];
}
});
if (fullVal !== '') {
// The last dimension in a value will not be followed by a character so we push here
value.push(fullVal);

if (value.length <= 4 && value.length >= 1) {
var output = [];

// check which values can condense
if (condenseToOne(value, parser.options['allowed-shorthands'])) {
output = [value[0]];
}
else if (condenseToTwo(value, parser.options['allowed-shorthands'])) {
output = [value[0], value[1]];
}
else if (condenseToThree(value, parser.options['allowed-shorthands'])) {
output = [value[0], value[1], value[2]];
}

if (output.length) {
result = helpers.addUnique(result, {
'ruleId': parser.rule.name,
'line': item.start.line,
'column': item.start.column,
'message': 'Property `' + property + '` should be written more concisely as `' + output.join(' ') + '` instead of `' + value.join(' ') + '`',
'severity': parser.severity
});
}

if (output.length) {
result = helpers.addUnique(result, {
'ruleId': parser.rule.name,
'line': item.start.line,
'column': item.start.column,
'message': 'Property `' + property + '` should be written more concisely as `' + output.join(' ') + '` instead of `' + value.join(' ') + '`',
'severity': parser.severity
});
}
}
}
Expand Down
32 changes: 16 additions & 16 deletions tests/rules/shorthand-values.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ describe('shorthand values - scss', function () {
lint.test(file, {
'shorthand-values': 1
}, function (data) {
lint.assert.equal(54, data.warningCount);
lint.assert.equal(64, data.warningCount);
done();
});
});
Expand All @@ -28,7 +28,7 @@ describe('shorthand values - scss', function () {
}
]
}, function (data) {
lint.assert.equal(23, data.warningCount);
lint.assert.equal(27, data.warningCount);
done();
});
});
Expand All @@ -44,7 +44,7 @@ describe('shorthand values - scss', function () {
}
]
}, function (data) {
lint.assert.equal(26, data.warningCount);
lint.assert.equal(32, data.warningCount);
done();
});
});
Expand All @@ -60,7 +60,7 @@ describe('shorthand values - scss', function () {
}
]
}, function (data) {
lint.assert.equal(32, data.warningCount);
lint.assert.equal(38, data.warningCount);
done();
});
});
Expand Down Expand Up @@ -92,7 +92,7 @@ describe('shorthand values - scss', function () {
}
]
}, function (data) {
lint.assert.equal(40, data.warningCount);
lint.assert.equal(48, data.warningCount);
done();
});
});
Expand All @@ -109,7 +109,7 @@ describe('shorthand values - scss', function () {
}
]
}, function (data) {
lint.assert.equal(46, data.warningCount);
lint.assert.equal(54, data.warningCount);
done();
});
});
Expand All @@ -126,7 +126,7 @@ describe('shorthand values - scss', function () {
}
]
}, function (data) {
lint.assert.equal(40, data.warningCount);
lint.assert.equal(48, data.warningCount);
done();
});
});
Expand All @@ -144,7 +144,7 @@ describe('shorthand values - scss', function () {
}
]
}, function (data) {
lint.assert.equal(54, data.warningCount);
lint.assert.equal(64, data.warningCount);
done();
});
});
Expand All @@ -161,7 +161,7 @@ describe('shorthand values - sass', function () {
lint.test(file, {
'shorthand-values': 1
}, function (data) {
lint.assert.equal(54, data.warningCount);
lint.assert.equal(64, data.warningCount);
done();
});
});
Expand All @@ -177,7 +177,7 @@ describe('shorthand values - sass', function () {
}
]
}, function (data) {
lint.assert.equal(23, data.warningCount);
lint.assert.equal(27, data.warningCount);
done();
});
});
Expand All @@ -193,7 +193,7 @@ describe('shorthand values - sass', function () {
}
]
}, function (data) {
lint.assert.equal(26, data.warningCount);
lint.assert.equal(32, data.warningCount);
done();
});
});
Expand All @@ -209,7 +209,7 @@ describe('shorthand values - sass', function () {
}
]
}, function (data) {
lint.assert.equal(32, data.warningCount);
lint.assert.equal(38, data.warningCount);
done();
});
});
Expand Down Expand Up @@ -241,7 +241,7 @@ describe('shorthand values - sass', function () {
}
]
}, function (data) {
lint.assert.equal(40, data.warningCount);
lint.assert.equal(48, data.warningCount);
done();
});
});
Expand All @@ -258,7 +258,7 @@ describe('shorthand values - sass', function () {
}
]
}, function (data) {
lint.assert.equal(46, data.warningCount);
lint.assert.equal(54, data.warningCount);
done();
});
});
Expand All @@ -275,7 +275,7 @@ describe('shorthand values - sass', function () {
}
]
}, function (data) {
lint.assert.equal(40, data.warningCount);
lint.assert.equal(48, data.warningCount);
done();
});
});
Expand All @@ -293,7 +293,7 @@ describe('shorthand values - sass', function () {
}
]
}, function (data) {
lint.assert.equal(54, data.warningCount);
lint.assert.equal(64, data.warningCount);
done();
});
});
Expand Down
Loading

0 comments on commit 15ab86b

Please sign in to comment.