Merge pull request #1342 from mgreter/feature/number-fixes

Fix error for multiplication of invalid units

Author: xzyfer

src/ast.cpp

@@ -1256,8 +1256,10 @@ namespace Sass {
       while (true) {
         r = u.find_first_of("*/", l);
         string unit(u.substr(l, r == string::npos ? r : r - l));
-        if (nominator) numerator_units_.push_back(unit);
-        else denominator_units_.push_back(unit);
+        if (!unit.empty()) {
+          if (nominator) numerator_units_.push_back(unit);
+          else denominator_units_.push_back(unit);
+        }
         if (r == string::npos) break;
         // ToDo: should error for multiple slashes
         // if (!nominator && u[r] == '/') error(...)
@@ -1287,7 +1289,7 @@ namespace Sass {
   bool Number::is_unitless()
   { return numerator_units_.empty() && denominator_units_.empty(); }
 
-  void Number::normalize(const string& prefered)
+  void Number::normalize(const string& prefered, bool strict)
   {
 
     // first make sure same units cancel each other out
@@ -1331,7 +1333,7 @@ namespace Sass {
         if (string_to_unit(nom) == UNKNOWN) continue;
         // we now have two convertable units
         // add factor for current conversion
-        factor *= conversion_factor(nom, denom);
+        factor *= conversion_factor(nom, denom, strict);
         // update nominator/denominator exponent
         -- exponents[nom]; ++ exponents[denom];
         // inner loop done
@@ -1352,8 +1354,10 @@ namespace Sass {
       {
         // opted to have these switches in the inner loop
         // makes it more readable and should not cost much
-        if (exp.second < 0) denominator_units_.push_back(exp.first);
-        else if (exp.second > 0) numerator_units_.push_back(exp.first);
+        if (!exp.first.empty()) {
+          if (exp.second < 0) denominator_units_.push_back(exp.first);
+          else if (exp.second > 0) numerator_units_.push_back(exp.first);
+        }
       }
     }
 
@@ -1363,14 +1367,14 @@ namespace Sass {
 
     // maybe convert to other unit
     // easier implemented on its own
-    try { convert(prefered); }
+    try { convert(prefered, strict); }
     catch (incompatibleUnits& err)
     { error(err.what(), pstate()); }
     catch (...) { throw; }
 
   }
 
-  void Number::convert(const string& prefered)
+  void Number::convert(const string& prefered, bool strict)
   {
     // abort if unit is empty
     if (prefered.empty()) return;
@@ -1405,7 +1409,7 @@ namespace Sass {
       if (string_to_unit(denom) == UNKNOWN) continue;
       // we now have two convertable units
       // add factor for current conversion
-      factor *= conversion_factor(denom, prefered);
+      factor *= conversion_factor(denom, prefered, strict);
       // update nominator/denominator exponent
       ++ exponents[denom]; -- exponents[prefered];
     }
@@ -1426,7 +1430,7 @@ namespace Sass {
       if (string_to_unit(nom) == UNKNOWN) continue;
       // we now have two convertable units
       // add factor for current conversion
-      factor *= conversion_factor(nom, prefered);
+      factor *= conversion_factor(nom, prefered, strict);
       // update nominator/denominator exponent
       -- exponents[nom]; ++ exponents[prefered];
     }
@@ -1444,8 +1448,10 @@ namespace Sass {
       {
         // opted to have these switches in the inner loop
         // makes it more readable and should not cost much
-        if (exp.second < 0) denominator_units_.push_back(exp.first);
-        else if (exp.second > 0) numerator_units_.push_back(exp.first);
+        if (!exp.first.empty()) {
+          if (exp.second < 0) denominator_units_.push_back(exp.first);
+          else if (exp.second > 0) numerator_units_.push_back(exp.first);
+        }
       }
     }
 

src/ast.hpp

@@ -1210,8 +1210,8 @@ namespace Sass {
     string unit() const;
 
     bool is_unitless();
-    void convert(const string& unit = "");
-    void normalize(const string& unit = "");
+    void convert(const string& unit = "", bool strict = false);
+    void normalize(const string& unit = "", bool strict = false);
     // useful for making one number compatible with another
     string find_convertible_unit() const;
 

src/eval.cpp

@@ -1120,7 +1120,8 @@ namespace Sass {
     }
 
     Number tmp(r);
-    tmp.normalize(l.find_convertible_unit());
+    bool strict = op != Sass_OP::MUL && op != Sass_OP::DIV;
+    tmp.normalize(l.find_convertible_unit(), strict);
     string l_unit(l.unit());
     string r_unit(tmp.unit());
     if (l_unit != r_unit && !l_unit.empty() && !r_unit.empty() &&

src/output.cpp

@@ -18,6 +18,24 @@ namespace Sass {
     return n->perform(this);
   }
 
+  void Output::operator()(Number* n)
+  {
+    // use values to_string facility
+    To_String to_string(ctx);
+    string res = n->perform(&to_string);
+    // check for a valid unit here
+    // includes result for reporting
+    if (n->numerator_units().size() > 1 ||
+        n->denominator_units().size() > 0 ||
+        (n->numerator_units().size() && n->numerator_units()[0].find_first_of('/') != string::npos) ||
+        (n->numerator_units().size() && n->numerator_units()[0].find_first_of('*') != string::npos)
+    ) {
+      error(res + " isn't a valid CSS value.", n->pstate());
+    }
+    // output the final token
+    append_token(res, n);
+  }
+
   void Output::operator()(Import* imp)
   {
     top_nodes.push_back(imp);

src/output.hpp

@@ -44,6 +44,7 @@ namespace Sass {
     virtual void operator()(Keyframe_Rule*);
     virtual void operator()(Import*);
     virtual void operator()(Comment*);
+    virtual void operator()(Number*);
     virtual void operator()(String_Quoted*);
     virtual void operator()(String_Constant*);
 

src/units.cpp

@@ -120,7 +120,7 @@ namespace Sass {
   }
 
   // throws incompatibleUnits exceptions
-  double conversion_factor(const string& s1, const string& s2)
+  double conversion_factor(const string& s1, const string& s2, bool strict)
   {
     // assert for same units
     if (s1 == s2) return 1;
@@ -135,7 +135,8 @@ namespace Sass {
     size_t i1 = u1 - t1;
     size_t i2 = u2 - t2;
     // error if units are not of the same group
-    if (t1 != t2) throw incompatibleUnits(u1, u2);
+    // don't error for multiplication and division
+    if (strict && t1 != t2) throw incompatibleUnits(u1, u2);
     // only process known units
     if (u1 != UNKNOWN && u2 != UNKNOWN) {
       switch (t1) {

src/units.hpp

@@ -67,7 +67,7 @@ namespace Sass {
   const char* unit_to_string(SassUnit unit);
   enum SassUnitType get_unit_type(SassUnit unit);
   // throws incompatibleUnits exceptions
-  double conversion_factor(const string&, const string&);
+  double conversion_factor(const string&, const string&, bool = true);
 
   class incompatibleUnits: public exception
   {