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,17 +1,17 @@
-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 [Rework](https://github.com/reworkcss/rework) plugin to support `calc()`.  
+Particularly useful with the [rework-vars](https://github.com/reworkcss/rework-vars)
 
 ## Installation
 
 ```bash
 npm install rework-calc
 ```
 
-## Usage
+## Use
 
-An example of how to use `rework-calc`:
+As a Rework plugin:
 
 ```javascript
 var rework = require('rework'),
@@ -20,17 +20,15 @@ var rework = require('rework'),
 var css = rework(cssString).use(calc).toString();
 ```
 
-For available plugins see plugins section below.
+## Supported feature
 
-## calc() plugin
+This simply add `calc()` support, a feature to do simple calculations.
+This can be particularly useful with the [rework-vars](https://github.com/reworkcss/rework-vars) 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.
+**Note:** When multiple units are mixed together in the same expression, the `calc()` statement
+is left as is, to fallback to the CSS3 calc feature.
 
-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 {
@@ -70,10 +68,6 @@ Make sure the dev-dependencies are installed, and then run:
 npm test
 ```
 
-## Contributing
-
-Feel free to contribute!
-
 ## License
 
 MIT

lib/calc.js

@@ -1,85 +1,44 @@
-
 /**
- * Calculation Plugin
- *
- * Useful in combination with the [rework-vars](https://npmjs.org/package/rework-vars) plugin, e.g:
- *
- *   :root {
- *     var-base-font-size: 16px;
- *   }
- *   body {
- *     font-size: var(base-font-size);
- *   }
- *   h1 {
- *     font-size: calc(var(base-font-size) * 2);
- *   }
- *
- * Yields:
- *
- *   :root {
- *     var-base-font-size: 16px;
- *   }
- *   body {
- *     font-size: 16px;
- *   }
- *   h1 {
- *     font-size: 32px;
- *   }
- *
+ * Module dependencies.
  */
 
-module.exports = function (style) {
-    rules(style.rules);
-};
+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 calculations
+  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 calc() 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 +54,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 +77,25 @@ 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 calcStartIndex = value.indexOf(CALC_FUNC_IDENTIFIER + '(');
+  var calcRef = balanced('(', ')', value.substring(calcStartIndex));
+
+  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 +106,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

@@ -1,23 +1,33 @@
 {
   "name": "rework-calc",
   "version": "0.2.2",
-  "description": "Adding calc() support to rework",
-  "main": "index.js",
+  "description": "calc() support for Rework",
+  "dependencies": {
+    "balanced-match": "^0.1.0",
+    "rework-visit": "^1.0.0"
+  },
+  "devDependencies": {
+    "mocha": "~1.15.1",
+    "rework": "^1.0.0",
+    "chai": "~1.8.1"
+  },
+  "files": [
+    "index.js"
+  ],
   "scripts": {
-    "test": "./node_modules/.bin/mocha -R spec"
+    "test": "mocha --no-colors",
+    "watch": "mocha --slow 30 --reporter spec --watch"
   },
-  "repository": "git://github.com/rework/rework-calc.git",
+  "repository": {
+    "type": "git",
+    "url": "https://github.com/reworkcss/rework-calc.git"
+  },
+  "license": "MIT",
   "keywords": [
+    "css",
     "rework",
     "rework-plugins",
-    "css",
+    "calculation",
     "calc"
-  ],
-  "author": "Joakim Bengtson <[email protected]>",
-  "license": "MIT",
-  "devDependencies": {
-    "mocha": "~1.15.1",
-    "rework": "~0.18.3",
-    "chai": "~1.8.1"
-  }
+  ]
 }

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');
+  });
+});