From: Jakub Pawlowicz <contact@jakubpawlowicz.com>
Date: Tue, 31 May 2016 06:59:00 +0000 (+0200)
Subject: Fixes #768 - invalid border-radius property.
X-Git-Url: https://git.ndcode.org/public/gitweb.cgi?a=commitdiff_plain;h=4bcb61c1b43bb631cda2f43b16427eec43b7936e;p=clean-css.git

Fixes #768 - invalid border-radius property.

Whenever we encounter an invalid property there should be a
InvalidPropertyError exception thrown and property reported as `unused`.

Currently warnings are reported without any further details like line
number, but such feature is due in #657.
---

diff --git a/History.md b/History.md
index d330c711..4ab38f1f 100644
--- a/History.md
+++ b/History.md
@@ -9,6 +9,7 @@
 * Fixed issue [#751](https://github.com/jakubpawlowicz/clean-css/issues/751) - stringifying CSS variables.
 * Fixed issue [#763](https://github.com/jakubpawlowicz/clean-css/issues/763) - data URI SVG and quoting.
 * Fixed issue [#765](https://github.com/jakubpawlowicz/clean-css/issues/765) - two values of border-radius.
+* Fixed issue [#768](https://github.com/jakubpawlowicz/clean-css/issues/768) - invalid border-radius property.
 
 [3.4.13 / 2016-05-23](https://github.com/jakubpawlowicz/clean-css/compare/v3.4.12...v3.4.13)
 ==================
diff --git a/lib/clean.js b/lib/clean.js
index 9f966a82..4838c8d2 100644
--- a/lib/clean.js
+++ b/lib/clean.js
@@ -225,7 +225,7 @@ function minify(context, data) {
   simpleOptimize(tokens, options);
 
   if (options.advanced)
-    advancedOptimize(tokens, options, context.validator, true);
+    advancedOptimize(tokens, options, context, true);
 
   return stringify(tokens, options, restoreEscapes, context.inputSourceMapTracker);
 }
diff --git a/lib/properties/break-up.js b/lib/properties/break-up.js
index c5dfc414..6657b7a7 100644
--- a/lib/properties/break-up.js
+++ b/lib/properties/break-up.js
@@ -1,4 +1,5 @@
 var wrapSingle = require('./wrap-for-optimizing').single;
+var InvalidPropertyError = require('./invalid-property-error');
 
 var split = require('../utils/split');
 var MULTIPLEX_SEPARATOR = ',';
@@ -131,6 +132,10 @@ function borderRadius(property, compactable) {
     }
   }
 
+  if (splitAt === 0 || splitAt === values.length - 1) {
+    throw new InvalidPropertyError('Invalid border-radius value.');
+  }
+
   var target = _wrapDefault(property.name, property, compactable);
   target.value = splitAt > -1 ?
     values.slice(0, splitAt) :
diff --git a/lib/properties/invalid-property-error.js b/lib/properties/invalid-property-error.js
new file mode 100644
index 00000000..86d5b5f9
--- /dev/null
+++ b/lib/properties/invalid-property-error.js
@@ -0,0 +1,10 @@
+function InvalidPropertyError(message) {
+  this.name = 'InvalidPropertyError';
+  this.message = message;
+  this.stack = (new Error()).stack;
+}
+
+InvalidPropertyError.prototype = Object.create(Error.prototype);
+InvalidPropertyError.prototype.constructor = InvalidPropertyError;
+
+module.exports = InvalidPropertyError;
diff --git a/lib/properties/optimizer.js b/lib/properties/optimizer.js
index d244ad8e..855b7f9d 100644
--- a/lib/properties/optimizer.js
+++ b/lib/properties/optimizer.js
@@ -189,15 +189,18 @@ function _optimize(properties, mergeAdjacent, aggressiveMerging, validator) {
   }
 }
 
-function optimize(selector, properties, mergeAdjacent, withCompacting, options, validator) {
+function optimize(selector, properties, mergeAdjacent, withCompacting, options, context) {
+  var validator = context.validator;
+  var warnings = context.warnings;
+
   var _properties = wrapForOptimizing(properties);
-  populateComponents(_properties, validator);
+  populateComponents(_properties, validator, warnings);
   _optimize(_properties, mergeAdjacent, options.aggressiveMerging, validator);
 
   for (var i = 0, l = _properties.length; i < l; i++) {
     var _property = _properties[i];
     if (_property.variable && _property.block)
-      optimize(selector, _property.value[0], mergeAdjacent, withCompacting, options, validator);
+      optimize(selector, _property.value[0], mergeAdjacent, withCompacting, options, context);
   }
 
   if (withCompacting && options.shorthandCompacting) {
diff --git a/lib/properties/populate-components.js b/lib/properties/populate-components.js
index 30930714..dd8700cc 100644
--- a/lib/properties/populate-components.js
+++ b/lib/properties/populate-components.js
@@ -1,6 +1,7 @@
 var compactable = require('./compactable');
+var InvalidPropertyError = require('./invalid-property-error');
 
-function populateComponents(properties, validator) {
+function populateComponents(properties, validator, warnings) {
   for (var i = properties.length - 1; i >= 0; i--) {
     var property = properties[i];
     var descriptor = compactable[property.name];
@@ -8,7 +9,17 @@ function populateComponents(properties, validator) {
     if (descriptor && descriptor.shorthand) {
       property.shorthand = true;
       property.dirty = true;
-      property.components = descriptor.breakUp(property, compactable, validator);
+
+      try {
+        property.components = descriptor.breakUp(property, compactable, validator);
+      } catch (e) {
+        if (e instanceof InvalidPropertyError) {
+          property.components = []; // this will set property.unused to true below
+          warnings.push(e.message);
+        } else {
+          throw e;
+        }
+      }
 
       if (property.components.length > 0)
         property.multiplex = property.components[0].multiplex;
diff --git a/lib/properties/shorthand-compactor.js b/lib/properties/shorthand-compactor.js
index 1cd48f6e..9be76236 100644
--- a/lib/properties/shorthand-compactor.js
+++ b/lib/properties/shorthand-compactor.js
@@ -42,7 +42,7 @@ function replaceWithShorthand(properties, candidateComponents, name, sourceMaps,
   newProperty.shorthand = true;
   newProperty.dirty = true;
 
-  populateComponents([newProperty], validator);
+  populateComponents([newProperty], validator, []);
 
   for (var i = 0, l = descriptor.components.length; i < l; i++) {
     var component = candidateComponents[descriptor.components[i]];
diff --git a/lib/selectors/advanced.js b/lib/selectors/advanced.js
index f03b11fa..38630e6a 100644
--- a/lib/selectors/advanced.js
+++ b/lib/selectors/advanced.js
@@ -31,52 +31,52 @@ function removeEmpty(tokens) {
   }
 }
 
-function recursivelyOptimizeBlocks(tokens, options, validator) {
+function recursivelyOptimizeBlocks(tokens, options, context) {
   for (var i = 0, l = tokens.length; i < l; i++) {
     var token = tokens[i];
 
     if (token[0] == 'block') {
       var isKeyframes = /@(-moz-|-o-|-webkit-)?keyframes/.test(token[1][0]);
-      optimize(token[2], options, validator, !isKeyframes);
+      optimize(token[2], options, context, !isKeyframes);
     }
   }
 }
 
-function recursivelyOptimizeProperties(tokens, options, validator) {
+function recursivelyOptimizeProperties(tokens, options, context) {
   for (var i = 0, l = tokens.length; i < l; i++) {
     var token = tokens[i];
 
     switch (token[0]) {
       case 'selector':
-        optimizeProperties(token[1], token[2], false, true, options, validator);
+        optimizeProperties(token[1], token[2], false, true, options, context);
         break;
       case 'block':
-        recursivelyOptimizeProperties(token[2], options, validator);
+        recursivelyOptimizeProperties(token[2], options, context);
     }
   }
 }
 
-function optimize(tokens, options, validator, withRestructuring) {
-  recursivelyOptimizeBlocks(tokens, options, validator);
-  recursivelyOptimizeProperties(tokens, options, validator);
+function optimize(tokens, options, context, withRestructuring) {
+  recursivelyOptimizeBlocks(tokens, options, context);
+  recursivelyOptimizeProperties(tokens, options, context);
 
   removeDuplicates(tokens);
-  mergeAdjacent(tokens, options, validator);
-  reduceNonAdjacent(tokens, options, validator);
+  mergeAdjacent(tokens, options, context);
+  reduceNonAdjacent(tokens, options, context);
 
-  mergeNonAdjacentBySelector(tokens, options, validator);
+  mergeNonAdjacentBySelector(tokens, options, context);
   mergeNonAdjacentByBody(tokens, options);
 
   if (options.restructuring && withRestructuring) {
     restructure(tokens, options);
-    mergeAdjacent(tokens, options, validator);
+    mergeAdjacent(tokens, options, context);
   }
 
   if (options.mediaMerging) {
     removeDuplicateMediaQueries(tokens);
     var reduced = mergeMediaQueries(tokens);
     for (var i = reduced.length - 1; i >= 0; i--) {
-      optimize(reduced[i][2], options, validator, false);
+      optimize(reduced[i][2], options, context, false);
     }
   }
 
diff --git a/lib/selectors/merge-adjacent.js b/lib/selectors/merge-adjacent.js
index 1485707c..3de1c1fd 100644
--- a/lib/selectors/merge-adjacent.js
+++ b/lib/selectors/merge-adjacent.js
@@ -5,7 +5,7 @@ var stringifySelectors = require('../stringifier/one-time').selectors;
 var cleanUpSelectors = require('./clean-up').selectors;
 var isSpecial = require('./is-special');
 
-function mergeAdjacent(tokens, options, validator) {
+function mergeAdjacent(tokens, options, context) {
   var lastToken = [null, [], []];
   var adjacentSpace = options.compatibility.selectors.adjacentSpace;
 
@@ -20,7 +20,7 @@ function mergeAdjacent(tokens, options, validator) {
     if (lastToken[0] == 'selector' && stringifySelectors(token[1]) == stringifySelectors(lastToken[1])) {
       var joinAt = [lastToken[2].length];
       Array.prototype.push.apply(lastToken[2], token[2]);
-      optimizeProperties(token[1], lastToken[2], joinAt, true, options, validator);
+      optimizeProperties(token[1], lastToken[2], joinAt, true, options, context);
       token[2] = [];
     } else if (lastToken[0] == 'selector' && stringifyBody(token[2]) == stringifyBody(lastToken[2]) &&
         !isSpecial(options, stringifySelectors(token[1])) && !isSpecial(options, stringifySelectors(lastToken[1]))) {
diff --git a/lib/selectors/merge-non-adjacent-by-selector.js b/lib/selectors/merge-non-adjacent-by-selector.js
index 28924b94..bd1c1b9c 100644
--- a/lib/selectors/merge-non-adjacent-by-selector.js
+++ b/lib/selectors/merge-non-adjacent-by-selector.js
@@ -3,7 +3,7 @@ var stringifySelectors = require('../stringifier/one-time').selectors;
 var extractProperties = require('./extractor');
 var canReorder = require('./reorderable').canReorder;
 
-function mergeNonAdjacentBySelector(tokens, options, validator) {
+function mergeNonAdjacentBySelector(tokens, options, context) {
   var allSelectors = {};
   var repeatedSelectors = [];
   var i;
@@ -66,7 +66,7 @@ function mergeNonAdjacentBySelector(tokens, options, validator) {
           Array.prototype.push.apply(target[2], moved[2]);
         }
 
-        optimizeProperties(target[1], target[2], joinAt, true, options, validator);
+        optimizeProperties(target[1], target[2], joinAt, true, options, context);
         moved[2] = [];
       }
     }
diff --git a/lib/selectors/reduce-non-adjacent.js b/lib/selectors/reduce-non-adjacent.js
index 0bb4c31a..7a2d1813 100644
--- a/lib/selectors/reduce-non-adjacent.js
+++ b/lib/selectors/reduce-non-adjacent.js
@@ -4,7 +4,7 @@ var stringifySelectors = require('../stringifier/one-time').selectors;
 var isSpecial = require('./is-special');
 var cloneArray = require('../utils/clone-array');
 
-function reduceNonAdjacent(tokens, options, validator) {
+function reduceNonAdjacent(tokens, options, context) {
   var candidates = {};
   var repeated = [];
 
@@ -40,8 +40,8 @@ function reduceNonAdjacent(tokens, options, validator) {
     }
   }
 
-  reduceSimpleNonAdjacentCases(tokens, repeated, candidates, options, validator);
-  reduceComplexNonAdjacentCases(tokens, candidates, options, validator);
+  reduceSimpleNonAdjacentCases(tokens, repeated, candidates, options, context);
+  reduceComplexNonAdjacentCases(tokens, candidates, options, context);
 }
 
 function wrappedSelectorsFrom(list) {
@@ -54,7 +54,7 @@ function wrappedSelectorsFrom(list) {
   return wrapped;
 }
 
-function reduceSimpleNonAdjacentCases(tokens, repeated, candidates, options, validator) {
+function reduceSimpleNonAdjacentCases(tokens, repeated, candidates, options, context) {
   function filterOut(idx, bodies) {
     return data[idx].isPartial && bodies.length === 0;
   }
@@ -71,11 +71,11 @@ function reduceSimpleNonAdjacentCases(tokens, repeated, candidates, options, val
     reduceSelector(tokens, selector, data, {
       filterOut: filterOut,
       callback: reduceBody
-    }, options, validator);
+    }, options, context);
   }
 }
 
-function reduceComplexNonAdjacentCases(tokens, candidates, options, validator) {
+function reduceComplexNonAdjacentCases(tokens, candidates, options, context) {
   var localContext = {};
 
   function filterOut(idx) {
@@ -116,7 +116,7 @@ function reduceComplexNonAdjacentCases(tokens, candidates, options, validator) {
       reduceSelector(tokens, selector, data, {
         filterOut: filterOut,
         callback: collectReducedBodies
-      }, options, validator);
+      }, options, context);
 
       if (stringifyBody(reducedBodies[reducedBodies.length - 1]) != stringifyBody(reducedBodies[0]))
         continue allSelectors;
@@ -126,7 +126,7 @@ function reduceComplexNonAdjacentCases(tokens, candidates, options, validator) {
   }
 }
 
-function reduceSelector(tokens, selector, data, context, options, validator) {
+function reduceSelector(tokens, selector, data, context, options, outerContext) {
   var bodies = [];
   var bodiesAsList = [];
   var joinsAt = [];
@@ -150,7 +150,7 @@ function reduceSelector(tokens, selector, data, context, options, validator) {
       joinsAt.push((joinsAt[j - 1] || 0) + bodiesAsList[j].length);
   }
 
-  optimizeProperties(selector, bodies, joinsAt, false, options, validator);
+  optimizeProperties(selector, bodies, joinsAt, false, options, outerContext);
 
   var processedCount = processedTokens.length;
   var propertyIdx = bodies.length - 1;
diff --git a/test/properties/break-up-test.js b/test/properties/break-up-test.js
index a0c8cea8..4aed5987 100644
--- a/test/properties/break-up-test.js
+++ b/test/properties/break-up-test.js
@@ -11,7 +11,7 @@ var breakUp = require('../../lib/properties/break-up');
 function _breakUp(properties) {
   var validator = new Validator(new Compatibility().toOptions());
   var wrapped = wrapForOptimizing(properties);
-  populateComponents(wrapped, validator);
+  populateComponents(wrapped, validator, []);
 
   return wrapped[0].components;
 }
@@ -374,6 +374,22 @@ vows.describe(breakUp)
           assert.equal(components[3].name, '-webkit-border-bottom-left-radius');
           assert.deepEqual(components[3].value, [['1px'], ['4px']]);
         }
+      },
+      'with missing vertical value': {
+        'topic': function () {
+          return _breakUp([[['border-radius'], ['0px'], ['/']]]);
+        },
+        'has no components': function (components) {
+          assert.lengthOf(components, 0);
+        }
+      },
+      'with missing horizontal value': {
+        'topic': function () {
+          return _breakUp([[['border-radius'], ['/'], ['0px']]]);
+        },
+        'has no components': function (components) {
+          assert.lengthOf(components, 0);
+        }
       }
     },
     'four values': {
diff --git a/test/properties/longhand-overriding-test.js b/test/properties/longhand-overriding-test.js
index 80c71412..fef5949f 100644
--- a/test/properties/longhand-overriding-test.js
+++ b/test/properties/longhand-overriding-test.js
@@ -17,7 +17,7 @@ function _optimize(source) {
 
   var compatibility = new Compatibility().toOptions();
   var validator = new Validator(compatibility);
-  optimize(tokens[0][1], tokens[0][2], false, true, { compatibility: compatibility, aggressiveMerging: true, shorthandCompacting: true }, validator);
+  optimize(tokens[0][1], tokens[0][2], false, true, { compatibility: compatibility, aggressiveMerging: true, shorthandCompacting: true }, { validator: validator });
 
   return tokens[0][2];
 }
diff --git a/test/properties/optimizer-test.js b/test/properties/optimizer-test.js
index 3f646ca8..1cf25a29 100644
--- a/test/properties/optimizer-test.js
+++ b/test/properties/optimizer-test.js
@@ -18,7 +18,7 @@ function _optimize(source, mergeAdjacent, aggressiveMerging, compatibilityOption
     warnings: []
   });
 
-  optimize(tokens[0][1], tokens[0][2], mergeAdjacent, true, { compatibility: compatibility, aggressiveMerging: aggressiveMerging }, validator);
+  optimize(tokens[0][1], tokens[0][2], mergeAdjacent, true, { compatibility: compatibility, aggressiveMerging: aggressiveMerging }, { validator: validator });
 
   return tokens[0][2];
 }
diff --git a/test/properties/override-compacting-test.js b/test/properties/override-compacting-test.js
index f0b51855..d3126db1 100644
--- a/test/properties/override-compacting-test.js
+++ b/test/properties/override-compacting-test.js
@@ -22,7 +22,7 @@ function _optimize(source, compatibility, aggressiveMerging) {
     compatibility: compatibility,
     shorthandCompacting: true
   };
-  optimize(tokens[0][1], tokens[0][2], false, true, options, validator);
+  optimize(tokens[0][1], tokens[0][2], false, true, options, { validator: validator });
 
   return tokens[0][2];
 }
diff --git a/test/properties/shorthand-compacting-source-maps-test.js b/test/properties/shorthand-compacting-source-maps-test.js
index 9bd8d9be..b32306d5 100644
--- a/test/properties/shorthand-compacting-source-maps-test.js
+++ b/test/properties/shorthand-compacting-source-maps-test.js
@@ -33,7 +33,7 @@ function _optimize(source) {
     sourceMap: true,
     shorthandCompacting: true
   };
-  optimize(tokens[0][1], tokens[0][2], false, true, options, validator);
+  optimize(tokens[0][1], tokens[0][2], false, true, options, { validator: validator });
 
   return tokens[0][2];
 }
diff --git a/test/properties/shorthand-compacting-test.js b/test/properties/shorthand-compacting-test.js
index 4dc63a07..f66af73f 100644
--- a/test/properties/shorthand-compacting-test.js
+++ b/test/properties/shorthand-compacting-test.js
@@ -22,7 +22,7 @@ function _optimize(source) {
     compatibility: compatibility,
     shorthandCompacting: true
   };
-  optimize(tokens[0][1], tokens[0][2], false, true, options, validator);
+  optimize(tokens[0][1], tokens[0][2], false, true, options, { validator: validator });
 
   return tokens[0][2];
 }