Merge pull request #2267 from mgreter/bugfix/error-messages

Adjust a few error messages and cleanups
sass · Dec 29, 2016 · 380a85b · 380a85b
2 parents 2a9f386 + 5fb20cd
commit 380a85b
Show file tree

Hide file tree

Showing 13 changed files with 41 additions and 145 deletions.
diff --git a/script/ci-install-compiler b/script/ci-install-compiler
@@ -2,3 +2,5 @@
 
 gem install minitest
 gem install minitap
+
+pip install --user 'requests[security]'
diff --git a/script/ci-report-coverage b/script/ci-report-coverage
@@ -3,7 +3,8 @@
 if [ "x$COVERAGE" = "xyes" ]; then
 
   # exclude some directories from profiling (.libs is from autotools)
-  export EXCLUDE_COVERAGE="--exclude src/sassc
+  export EXCLUDE_COVERAGE="--exclude plugins
+                           --exclude sassc/sassc.c
                            --exclude src/sass-spec
                            --exclude src/.libs
                            --exclude src/debug.hpp

diff --git a/src/ast.cpp b/src/ast.cpp
@@ -342,15 +342,15 @@ namespace Sass {
   {
     if (Selector_List_Ptr_Const sl = dynamic_cast<Selector_List_Ptr_Const>(this)) return *sl == rhs;
     if (Simple_Selector_Ptr_Const sp = dynamic_cast<Simple_Selector_Ptr_Const>(this)) return *sp == rhs;
-    throw "invalid selector base classes to compare";
+    throw std::runtime_error("invalid selector base classes to compare");
     return false;
   }
 
   bool Selector::operator< (const Selector& rhs) const
   {
     if (Selector_List_Ptr_Const sl = dynamic_cast<Selector_List_Ptr_Const>(this)) return *sl < rhs;
     if (Simple_Selector_Ptr_Const sp = dynamic_cast<Simple_Selector_Ptr_Const>(this)) return *sp < rhs;
-    throw "invalid selector base classes to compare";
+    throw std::runtime_error("invalid selector base classes to compare");
     return false;
   }
 

diff --git a/src/ast.hpp b/src/ast.hpp
@@ -924,7 +924,6 @@ namespace Sass {
   typedef Environment<AST_Node_Obj> Env;
   typedef const char* Signature;
   typedef Expression_Ptr (*Native_Function)(Env&, Env&, Context&, Signature, ParserState, Backtrace*, std::vector<Selector_List_Obj>);
-  typedef const char* Signature;
   class Definition : public Has_Block {
   public:
     enum Type { MIXIN, FUNCTION };
@@ -2293,12 +2292,6 @@ namespace Sass {
     virtual void set_media_block(Media_Block_Ptr mb) {
       media_block(mb);
     }
-    virtual bool has_wrapped_selector() {
-      return false;
-    }
-    virtual bool has_placeholder() {
-      return false;
-    }
     virtual bool has_parent_ref() {
       return false;
     }
@@ -2414,6 +2407,10 @@ namespace Sass {
       return name_ == "*";
     }
 
+    virtual bool has_placeholder() {
+      return false;
+    }
+
     virtual ~Simple_Selector() = 0;
     virtual Compound_Selector_Ptr unify_with(Compound_Selector_Ptr, Context&);
     virtual bool has_parent_ref() { return false; };
@@ -2697,10 +2694,6 @@ namespace Sass {
       if (!selector()) return false;
       return selector()->has_real_parent_ref();
     }
-    virtual bool has_wrapped_selector()
-    {
-      return true;
-    }
     virtual unsigned long specificity()
     {
       return selector_ ? selector_->specificity() : 0;
@@ -2797,15 +2790,6 @@ namespace Sass {
       return sum;
     }
 
-    virtual bool has_wrapped_selector()
-    {
-      if (length() == 0) return false;
-      if (Simple_Selector_Obj ss = elements().front()) {
-        if (ss->has_wrapped_selector()) return true;
-      }
-      return false;
-    }
-
     virtual bool has_placeholder()
     {
       if (length() == 0) return false;
@@ -2939,11 +2923,6 @@ namespace Sass {
       if (tail_) tail_->set_media_block(mb);
       if (head_) head_->set_media_block(mb);
     }
-    virtual bool has_wrapped_selector() {
-      if (head_ && head_->has_wrapped_selector()) return true;
-      if (tail_ && tail_->has_wrapped_selector()) return true;
-      return false;
-    }
     virtual bool has_placeholder() {
       if (head_ && head_->has_placeholder()) return true;
       if (tail_ && tail_->has_placeholder()) return true;
@@ -3061,12 +3040,6 @@ namespace Sass {
         cs->set_media_block(mb);
       }
     }
-    virtual bool has_wrapped_selector() {
-      for (Complex_Selector_Obj cs : elements()) {
-        if (cs->has_wrapped_selector()) return true;
-      }
-      return false;
-    }
     virtual bool has_placeholder() {
       for (Complex_Selector_Obj cs : elements()) {
         if (cs->has_placeholder()) return true;

diff --git a/src/context.cpp b/src/context.cpp
@@ -562,7 +562,7 @@ namespace Sass {
     }
 
     // abort early if no content could be loaded (various reasons)
-    if (!contents) throw "File to read not found or unreadable: " + input_path;
+    if (!contents) throw std::runtime_error("File to read not found or unreadable: " + input_path);
 
     // store entry path
     entry_path = abs_path;

diff --git a/src/eval.cpp b/src/eval.cpp
@@ -54,11 +54,6 @@ namespace Sass {
   { }
   Eval::~Eval() { }
 
-  Context& Eval::context()
-  {
-    return ctx;
-  }
-
   Env* Eval::environment()
   {
     return exp.environment();
@@ -180,8 +175,8 @@ namespace Sass {
     // check if units are valid for sequence
     if (sass_start->unit() != sass_end->unit()) {
       std::stringstream msg; msg << "Incompatible units: '"
-        << sass_start->unit() << "' and '"
-        << sass_end->unit() << "'.";
+        << sass_end->unit() << "' and '"
+        << sass_start->unit() << "'.";
       error(msg.str(), low->pstate(), backtrace());
     }
     double start = sass_start->value();
@@ -925,7 +920,7 @@ namespace Sass {
       } else if (sass_value_get_tag(c_val) == SASS_WARNING) {
         error("warning in C function " + c->name() + ": " + sass_warning_get_message(c_val), c->pstate(), backtrace());
       }
-      result = cval_to_astnode(c_val, ctx, backtrace(), c->pstate());
+      result = cval_to_astnode(c_val, backtrace(), c->pstate());
 
       exp.backtrace_stack.pop_back();
       sass_delete_value(c_args);
@@ -1590,7 +1585,7 @@ namespace Sass {
     return SASS_MEMORY_NEW(String_Constant, pstate ? *pstate : lhs.pstate(), lstr + sep + rstr);
   }
 
-  Expression_Ptr cval_to_astnode(union Sass_Value* v, Context& ctx, Backtrace* backtrace, ParserState pstate)
+  Expression_Ptr cval_to_astnode(union Sass_Value* v, Backtrace* backtrace, ParserState pstate)
   {
     using std::strlen;
     using std::strcpy;
@@ -1615,16 +1610,16 @@ namespace Sass {
       case SASS_LIST: {
         List_Ptr l = SASS_MEMORY_NEW(List, pstate, sass_list_get_length(v), sass_list_get_separator(v));
         for (size_t i = 0, L = sass_list_get_length(v); i < L; ++i) {
-          l->append(cval_to_astnode(sass_list_get_value(v, i), ctx, backtrace, pstate));
+          l->append(cval_to_astnode(sass_list_get_value(v, i), backtrace, pstate));
         }
         e = l;
       } break;
       case SASS_MAP: {
         Map_Ptr m = SASS_MEMORY_NEW(Map, pstate);
         for (size_t i = 0, L = sass_map_get_length(v); i < L; ++i) {
           *m << std::make_pair(
-            cval_to_astnode(sass_map_get_key(v, i), ctx, backtrace, pstate),
-            cval_to_astnode(sass_map_get_value(v, i), ctx, backtrace, pstate));
+            cval_to_astnode(sass_map_get_key(v, i), backtrace, pstate),
+            cval_to_astnode(sass_map_get_value(v, i), backtrace, pstate));
         }
         e = m;
       } break;

diff --git a/src/eval.hpp b/src/eval.hpp
@@ -27,9 +27,8 @@ namespace Sass {
     bool is_in_comment;
 
     Env* environment();
-    Context& context();
-    Selector_List_Obj selector();
     Backtrace* backtrace();
+    Selector_List_Obj selector();
 
     // for evaluating function bodies
     Expression_Ptr operator()(Block_Ptr);
@@ -103,7 +102,7 @@ namespace Sass {
 
   };
 
-  Expression_Ptr cval_to_astnode(union Sass_Value* v, Context& ctx, Backtrace* backtrace, ParserState pstate = ParserState("[AST]"));
+  Expression_Ptr cval_to_astnode(union Sass_Value* v, Backtrace* backtrace, ParserState pstate = ParserState("[AST]"));
 
 }
 

diff --git a/src/extend.cpp b/src/extend.cpp
@@ -1627,11 +1627,11 @@ namespace Sass {
       SourcesSet debugSet;
       debugSet = pNewSelector->sources();
       if (debugSet.size() > 0) {
-        throw "The new selector should start with no sources. Something needs to be cloned to fix this.";
+        throw std::runtime_error("The new selector should start with no sources. Something needs to be cloned to fix this.");
       }
       debugSet = pExtComplexSelector->sources();
       if (debugSet.size() > 0) {
-        throw "The extension selector from our subset map should not have sources. These will bleed to the new selector. Something needs to be cloned to fix this.";
+        throw std::runtime_error("The extension selector from our subset map should not have sources. These will bleed to the new selector. Something needs to be cloned to fix this.");
       }
 #endif
 

diff --git a/src/functions.cpp b/src/functions.cpp
@@ -170,23 +170,6 @@ namespace Sass {
       return Parser::parse_selector(exp_src.c_str(), ctx);
     }
 
-    template <>
-    Complex_Selector_Obj get_arg_sel(const std::string& argname, Env& env, Signature sig, ParserState pstate, Backtrace* backtrace, Context& ctx) {
-      Expression_Obj exp = ARG(argname, Expression);
-      if (exp->concrete_type() == Expression::NULL_VAL) {
-        std::stringstream msg;
-        msg << argname << ": null is not a valid selector: it must be a string,\n";
-        msg << "a list of strings, or a list of lists of strings for `" << function_name(sig) << "'";
-        error(msg.str(), pstate);
-      }
-      if (String_Constant_Ptr str = SASS_MEMORY_CAST(String_Constant, exp)) {
-        str->quote_mark(0);
-      }
-      std::string exp_src = exp->to_string(ctx.c_options) + "{";
-      Selector_List_Obj sel_list = Parser::parse_selector(exp_src.c_str(), ctx);
-      return (sel_list->length() > 0) ? &sel_list->first() : 0;
-    }
-
     template <>
     Compound_Selector_Obj get_arg_sel(const std::string& argname, Env& env, Signature sig, ParserState pstate, Backtrace* backtrace, Context& ctx) {
       Expression_Obj exp = ARG(argname, Expression);
@@ -702,7 +685,7 @@ namespace Sass {
       bool hsl = h || s || l;
 
       if (rgb && hsl) {
-        error("cannot specify both RGB and HSL values for `adjust-color`", pstate);
+        error("Cannot specify HSL and RGB values for a color at the same time for `adjust-color'", pstate);
       }
       if (rgb) {
         double rr = r ? ARGR("$red",   Number, -255, 255)->value() : 0;
@@ -736,7 +719,7 @@ namespace Sass {
                                color->b(),
                                color->a() + (a ? a->value() : 0));
       }
-      error("not enough arguments for `adjust-color`", pstate);
+      error("not enough arguments for `adjust-color'", pstate);
       // unreachable
       return color;
     }
@@ -757,7 +740,7 @@ namespace Sass {
       bool hsl = h || s || l;
 
       if (rgb && hsl) {
-        error("cannot specify both RGB and HSL values for `scale-color`", pstate);
+        error("Cannot specify HSL and RGB values for a color at the same time for `scale-color'", pstate);
       }
       if (rgb) {
         double rscale = (r ? ARGR("$red",   Number, -100.0, 100.0)->value() : 0.0) / 100.0;
@@ -792,7 +775,7 @@ namespace Sass {
                                color->b(),
                                color->a() + ascale * (ascale > 0.0 ? 1.0 - color->a() : color->a()));
       }
-      error("not enough arguments for `scale-color`", pstate);
+      error("not enough arguments for `scale-color'", pstate);
       // unreachable
       return color;
     }
@@ -813,7 +796,7 @@ namespace Sass {
       bool hsl = h || s || l;
 
       if (rgb && hsl) {
-        error("cannot specify both RGB and HSL values for `change-color`", pstate);
+        error("Cannot specify HSL and RGB values for a color at the same time for `change-color'", pstate);
       }
       if (rgb) {
         return SASS_MEMORY_NEW(Color,
@@ -840,7 +823,7 @@ namespace Sass {
                                color->b(),
                                alpha);
       }
-      error("not enough arguments for `change-color`", pstate);
+      error("not enough arguments for `change-color'", pstate);
       // unreachable
       return color;
     }
@@ -1201,13 +1184,13 @@ namespace Sass {
         double v = l->value();
         if (v < 1) {
           stringstream err;
-          err << "$limit " << v << " must be greater than or equal to 1 for `random`";
+          err << "$limit " << v << " must be greater than or equal to 1 for `random'";
           error(err.str(), pstate);
         }
         bool eq_int = std::fabs(trunc(v) - v) < NUMBER_EPSILON;
         if (!eq_int) {
           stringstream err;
-          err << "Expected $limit to be an integer but got `" << v << "` for `random`";
+          err << "Expected $limit to be an integer but got " << v << " for `random'";
           error(err.str(), pstate);
         }
         std::uniform_real_distribution<> distributor(1, v + 1);
@@ -1771,7 +1754,7 @@ namespace Sass {
 
       // Not enough parameters
       if( arglist->length() == 0 )
-        error("$selectors: At least one selector must be passed", pstate);
+        error("$selectors: At least one selector must be passed for `selector-nest'", pstate);
 
       // Parse args into vector of selectors
       std::vector<Selector_List_Obj> parsedSelectors;
@@ -1824,7 +1807,7 @@ namespace Sass {
 
       // Not enough parameters
       if( arglist->length() == 0 )
-        error("$selectors: At least one selector must be passed", pstate);
+        error("$selectors: At least one selector must be passed for `selector-append'", pstate);
 
       // Parse args into vector of selectors
       std::vector<Selector_List_Obj> parsedSelectors;
@@ -1873,22 +1856,22 @@ namespace Sass {
 
             // Must be a simple sequence
             if( childSeq->combinator() != Complex_Selector::Combinator::ANCESTOR_OF ) {
-              std::string msg("Can't append  `");
+              std::string msg("Can't append \"");
               msg += childSeq->to_string();
-              msg += "` to `";
+              msg += "\" to \"";
               msg += parentSeqClone->to_string();
-              msg += "`";
+              msg += "\" for `selector-append'";
               error(msg, pstate, backtrace);
             }
 
             // Cannot be a Universal selector
             Element_Selector_Obj pType = SASS_MEMORY_CAST(Element_Selector, childSeq->head()->first());
             if(pType && pType->name() == "*") {
-              std::string msg("Can't append  `");
+              std::string msg("Can't append \"");
               msg += childSeq->to_string();
-              msg += "` to `";
+              msg += "\" to \"";
               msg += parentSeqClone->to_string();
-              msg += "`";
+              msg += "\" for `selector-append'";
               error(msg, pstate, backtrace);
             }
 

diff --git a/src/parser.cpp b/src/parser.cpp
@@ -1016,16 +1016,6 @@ namespace Sass {
   {
     Expression_Obj key = parse_list();
     List_Obj map = SASS_MEMORY_NEW(List, pstate, 0, SASS_HASH);
-    if (String_Quoted_Ptr str = SASS_MEMORY_CAST(String_Quoted, key)) {
-      if (!str->quote_mark() && !str->is_delayed()) {
-        if (Color_Ptr_Const col = name_to_color(str->value())) {
-          Color_Ptr c = SASS_MEMORY_NEW(Color, col); // copy
-          c->pstate(str->pstate());
-          c->disp(str->value());
-          key = c;
-        }
-      }
-    }
 
     // it's not a map so return the lexed value as a list value
     if (!lex_css< exactly<':'> >())
@@ -1043,16 +1033,6 @@ namespace Sass {
       { break; }
 
       Expression_Obj key = parse_space_list();
-      if (String_Quoted_Ptr str = SASS_MEMORY_CAST(String_Quoted, key)) {
-        if (!str->quote_mark() && !str->is_delayed()) {
-          if (Color_Ptr_Const col = name_to_color(str->value())) {
-            Color_Ptr c = SASS_MEMORY_NEW(Color, col); // copy
-            c->pstate(str->pstate());
-            c->disp(str->value());
-            key = c;
-          }
-        }
-      }
 
       if (!(lex< exactly<':'> >()))
       { css_error("Invalid CSS", " after ", ": expected \":\", was "); }