From 1f709b84df670019c90ff3e1dc1a1a0aae569314 Mon Sep 17 00:00:00 2001 From: Marcel Greter Date: Sat, 11 Jul 2015 14:59:22 +0200 Subject: [PATCH] Fix error for multiplication of invalid units Fixes minor bug when parsing numbers like `2/px` --- src/ast.cpp | 30 ++++++++++++++++++------------ src/ast.hpp | 4 ++-- src/eval.cpp | 3 ++- src/output.cpp | 18 ++++++++++++++++++ src/output.hpp | 1 + src/units.cpp | 5 +++-- src/units.hpp | 2 +- 7 files changed, 45 insertions(+), 18 deletions(-) diff --git a/src/ast.cpp b/src/ast.cpp index 2d31f39d9..06d85d97c 100644 --- a/src/ast.cpp +++ b/src/ast.cpp @@ -960,8 +960,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(...) @@ -991,7 +993,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 @@ -1035,7 +1037,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 @@ -1056,8 +1058,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); + } } } @@ -1067,14 +1071,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; @@ -1109,7 +1113,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]; } @@ -1130,7 +1134,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]; } @@ -1148,8 +1152,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); + } } } diff --git a/src/ast.hpp b/src/ast.hpp index 28348ac1a..1d0791bf2 100644 --- a/src/ast.hpp +++ b/src/ast.hpp @@ -1193,8 +1193,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; diff --git a/src/eval.cpp b/src/eval.cpp index 5abbb8f66..4c6b2a7b0 100644 --- a/src/eval.cpp +++ b/src/eval.cpp @@ -1118,7 +1118,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() && diff --git a/src/output.cpp b/src/output.cpp index 14ceaaeca..57d650c8a 100644 --- a/src/output.cpp +++ b/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); diff --git a/src/output.hpp b/src/output.hpp index 2a3b97820..46ee0ac2f 100644 --- a/src/output.hpp +++ b/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*); diff --git a/src/units.cpp b/src/units.cpp index 982f2bde7..07a20e112 100644 --- a/src/units.cpp +++ b/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) { diff --git a/src/units.hpp b/src/units.hpp index aea7ecbd2..5f081faf4 100644 --- a/src/units.hpp +++ b/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 {