Small refactoring + error reporting

Just a PR following my stupid
#3

I’ve refactored the code a little bit (& also use rework-visit which is
more bullet proof) like rework-vars.
I quickly refactor tests to make it more understandable.
I add some error reporting like we have in rework-vars.

Btw, maybe we can expand the use of balanced-match to rewrite other
parts of the plugin.

README.md

@@ -1,7 +1,6 @@
-rework-calc
-===================
+# rework-calc [![Build Status](https://travis-ci.org/reworkcss/rework-calc.png)](https://travis-ci.org/reworkcss/rework-calc)
 
-A `calc()` plugin for the CSS Preprocessor [rework](https://github.com/visionmedia/rework).
+A `calc()` plugin for the CSS Preprocessor [rework](https://github.com/reworkcss/rework).
 
 ## Installation
 
@@ -25,12 +24,12 @@ For available plugins see plugins section below.
 ## calc() plugin
 
 Add calculations support. A feature to do simple calculations, and can be
-particularly useful together with the [rework-vars](https://npmjs.org/package/rework-vars) plugin.
+particularly useful together with the [rework-vars](https://github.com/reworkcss/rework-vars) plugin.
 
 When multiple units are mixed together in the same expression, the calc() statement
 is left as is, to fallback to the CSS3 Calc feature.
 
-**Example** (with rework-vars enabled as well):
+**Example** (with [rework-vars](https://github.com/reworkcss/rework-vars) enabled as well):
 
 ```css
 :root {

lib/calc.js

@@ -1,24 +1,20 @@
-
 /**
  * Calculation Plugin
  *
  * Useful in combination with the [rework-vars](https://npmjs.org/package/rework-vars) plugin, e.g:
  *
  *   :root {
- *     var-base-font-size: 16px;
+ *     --base-font-size: 16px;
  *   }
  *   body {
- *     font-size: var(base-font-size);
+ *     font-size: var(--base-font-size);
  *   }
  *   h1 {
- *     font-size: calc(var(base-font-size) * 2);
+ *     font-size: calc(var(--base-font-size) * 2);
  *   }
  *
  * Yields:
  *
- *   :root {
- *     var-base-font-size: 16px;
- *   }
  *   body {
  *     font-size: 16px;
  *   }
@@ -28,58 +24,47 @@
  *
  */
 
-module.exports = function (style) {
-    rules(style.rules);
-};
+/**
+ * Module dependencies.
+ */
+
+var balanced = require('balanced-match');
+var visit = require('rework-visit');
 
 /**
- * Constants
+ * Constants.
  */
 var DEFAULT_UNIT = 'px',
-    EXPRESSION_METHOD_NAME =  'calc',
+    CALC_FUNC_IDENTIFIER =  'calc',
+
     EXPRESSION_OPT_VENDOR_PREFIX = '(\\-[a-z]+\\-)?',
-    EXPRESSION_METHOD_REGEXP = EXPRESSION_OPT_VENDOR_PREFIX + EXPRESSION_METHOD_NAME,
+    EXPRESSION_METHOD_REGEXP = EXPRESSION_OPT_VENDOR_PREFIX + CALC_FUNC_IDENTIFIER,
     EXPRESSION_REGEXP = '\\b' + EXPRESSION_METHOD_REGEXP + '\\(';
 
 /**
- * Visit all rules
- *
- * @param {Array} arr Array with css rules
- * @api private
+ * Module export.
  */
-function rules(arr) {
-    arr.forEach(function (rule) {
-        if (rule.rules) rules(rule.rules);
-        if (rule.declarations) visit(rule.declarations);
-    });
-}
 
-/**
- * Visit all declarations (in a rule)
- *
- * @param {Array} declarations
- * @api private
- */
-function visit(declarations) {
-    declarations.forEach(function (decl) {
-        if (!hasExpressions(decl.value)) return;
+module.exports = function calc(style) {
+  // resolve variables
+  visit(style, function (declarations, node) {
+    var decl;
+    var resolvedValue;
+    var value;
 
-        var expressions = getExpressionsFromValue(decl.value);
+    for (var i = 0; i < declarations.length; i++) {
+      decl = declarations[i];
+      value = decl.value;
 
-        evaluateAndApplyExpressions(expressions, decl);
-    });
-}
+      // skip comments
+      if (decl.type !== 'declaration') continue;
+      // skip values that don't contain variable functions
+      if (!value || value.indexOf(CALC_FUNC_IDENTIFIER + '(') === -1) continue;
 
-/**
- * Checks if a value contains an expression
- *
- * @param {String} value
- * @returns {Boolean}
- * @api private
- */
-function hasExpressions(value) {
-    return (new RegExp(EXPRESSION_REGEXP)).exec(value);
-}
+      decl.value = resolveValue(value);
+    }
+  });
+};
 
 /**
  * Parses expressions in a value
@@ -95,7 +80,7 @@ function getExpressionsFromValue(value) {
 
     // Parse value and extract expressions:
     for (var i = 0; i < value.length; i++) {
-        if (value[i] == '(' && value.slice(i - 4, i) == EXPRESSION_METHOD_NAME && !start) {
+        if (value[i] == '(' && value.slice(i - 4, i) == CALC_FUNC_IDENTIFIER && !start) {
             start = i;
             parentheses++;
         } else if (value[i] == '(' && start !== null) {
@@ -118,19 +103,24 @@ function getExpressionsFromValue(value) {
  * @param {Object} declaration
  * @api private
  */
-function evaluateAndApplyExpressions(expressions, declaration) {
-    expressions.forEach(function (expression) {
-        var result = evaluateExpression(expression);
-
-        if (!result) return;
-
-        // Insert the evaluated value:
-        var expRegexp = new RegExp(
-                                EXPRESSION_METHOD_REGEXP + '\\(' +
-                                escapeExp(expression) + '\\)'
-                            );
-        declaration.value = declaration.value.replace(expRegexp, result);
-    });
+function resolveValue(value) {
+  var balancedParens = balanced('(', ')', value);
+  var calcRef = balanced(CALC_FUNC_IDENTIFIER + '(', ')', value);
+
+  if (!balancedParens) throw new Error('rework-calc: missing closing ")" in the value "' + value + '"');
+  if (!calcRef || calcRef.body === '') throw new Error('rework-calc: calc() must contain a non-whitespace string');
+
+  getExpressionsFromValue(value).forEach(function (expression) {
+    var result = evaluateExpression(expression);
+
+    if (!result) return;
+
+    // Insert the evaluated value:
+    var expRegexp = new RegExp(EXPRESSION_METHOD_REGEXP + '\\(' + escapeExp(expression) + '\\)');
+    value = value.replace(expRegexp, result);
+  });
+
+  return value
 }
 
 /**
@@ -141,7 +131,7 @@ function evaluateAndApplyExpressions(expressions, declaration) {
  * @api private
  */
 function evaluateExpression (expression) {
-    var originalExpression = EXPRESSION_METHOD_NAME + '(' + expression + ')';
+    var originalExpression = CALC_FUNC_IDENTIFIER + '(' + expression + ')';
 
     // Remove method names for possible nested expressions:
     expression = expression.replace(new RegExp(EXPRESSION_REGEXP, 'g'), '(');

package.json

@@ -19,5 +19,9 @@
     "mocha": "~1.15.1",
     "rework": "~0.18.3",
     "chai": "~1.8.1"
+  },
+  "dependencies": {
+    "balanced-match": "^0.1.0",
+    "rework-visit": "^1.0.0"
   }
 }

test/fixtures/substitution-empty.css

@@ -0,0 +1,3 @@
+div {
+  width: calc();
+}

test/fixtures/substitution-malformed.css

@@ -0,0 +1,4 @@
+div {
+  /* missing closing ')' */
+  width: calc(10px - 5px;
+}

test/test.js

@@ -0,0 +1,49 @@
+var calc = require('../index'),
+    rework = require('rework'),
+    expect = require('chai').expect,
+    read = require('fs').readFileSync;
+
+function fixture(name){
+  return read('test/fixtures/' + name + '.css', 'utf8').trim();
+}
+
+function compareFixtures(name){
+  return expect(
+    rework(fixture(name + '.in'))
+    .use(calc)
+    .toString().trim()
+  ).to.equal(fixture(name + '.out'));
+}
+
+describe('rework-calc', function() {
+  it('throws an error when a calc function is empty', function () {
+    var output = function () {
+      return rework(fixture('substitution-empty')).use(calc).toString();
+    };
+    expect(output).to.Throw(Error, 'rework-calc: calc() must contain a non-whitespace string');
+  });
+
+  it('throws an error when a calc function is malformed', function () {
+    var output = function () {
+      return rework(fixture('substitution-malformed')).use(calc).toString();
+    };
+    expect(output).to.Throw(Error, 'rework-calc: missing closing ")" in the value "calc(10px - 5px"');
+  });
+
+
+  it('should calculate expressions with only one unit involved', function() {
+    compareFixtures('calc');
+  });
+
+  it('should calculate expressions with percents correctly', function () {
+    compareFixtures('calc-percent');
+  });
+
+  it('should use CSS3 Calc function as fallback for expressions with multiple units', function () {
+    compareFixtures('calc-complex');
+  });
+
+  it('should handle vendor prefixed expressions', function () {
+    compareFixtures('calc-prefix');
+  });
+});