From 6d6fab6cb68b6b99285f2384ef1882fe92d0fe36 Mon Sep 17 00:00:00 2001 From: xzyfer Date: Sun, 1 May 2016 21:39:26 +1000 Subject: [PATCH 1/2] Error when declarations are printed without ruleset This is introduces a vastly reduced `CheckNesting` visitor from Ruby Sass as discussed in #2061. Doing this properly will require some small changes to the parser, and probably eval, which currently try to do some nesting checking of their own. This is just first step to properly introducing the proper nesting checking. Closes #2061 Fixes #1732 --- Makefile.conf | 1 + src/check_nesting.cpp | 66 +++++++++++++++++++++++++++++++++++++ src/check_nesting.hpp | 38 +++++++++++++++++++++ src/context.cpp | 6 ++++ win/libsass.targets | 2 ++ win/libsass.vcxproj.filters | 6 ++++ 6 files changed, 119 insertions(+) create mode 100644 src/check_nesting.cpp create mode 100644 src/check_nesting.hpp diff --git a/Makefile.conf b/Makefile.conf index 1cb598208d..2fa75560a4 100644 --- a/Makefile.conf +++ b/Makefile.conf @@ -4,6 +4,7 @@ SOURCES = \ ast.cpp \ base64vlq.cpp \ bind.cpp \ + check_nesting.cpp \ color_maps.cpp \ constants.cpp \ context.cpp \ diff --git a/src/check_nesting.cpp b/src/check_nesting.cpp new file mode 100644 index 0000000000..8dbfcd6c7e --- /dev/null +++ b/src/check_nesting.cpp @@ -0,0 +1,66 @@ +#include "sass.hpp" +#include + +#include "check_nesting.hpp" +#include "context.hpp" +// #include "debugger.hpp" + +namespace Sass { + + CheckNesting::CheckNesting(Context& ctx) + : ctx(ctx), + parent_stack(std::vector()) + { } + + AST_Node* CheckNesting::parent() + { + if (parent_stack.size() > 0) + return parent_stack.back(); + return 0; + } + + Statement* CheckNesting::operator()(Block* b) + { + parent_stack.push_back(b); + append_block(b); + parent_stack.pop_back(); + return b; + } + + Statement* CheckNesting::operator()(Declaration* d) + { + if (!is_valid_prop_parent(parent())) { + throw Exception::InvalidSass(d->pstate(), "Properties are only allowed " + "within rules, directives, mixin includes, or other properties."); + } + return static_cast(d); + } + + Statement* CheckNesting::fallback_impl(AST_Node* n) + { + return static_cast(n); + } + + bool CheckNesting::is_valid_prop_parent(AST_Node* p) + { + if (Definition* def = dynamic_cast(p)) { + return def->type() == Definition::MIXIN; + } + + return dynamic_cast(p) || + dynamic_cast(p) || + dynamic_cast(p) || + dynamic_cast(p) || + dynamic_cast(p); + } + + void CheckNesting::append_block(Block* b) + { + for (size_t i = 0, L = b->length(); i < L; ++i) { + Statement* ith = (*b)[i]->perform(this); + if (ith) { + (*b)[i] = ith; + } + } + } +} diff --git a/src/check_nesting.hpp b/src/check_nesting.hpp new file mode 100644 index 0000000000..cd8bcd09c9 --- /dev/null +++ b/src/check_nesting.hpp @@ -0,0 +1,38 @@ +#ifndef SASS_CHECK_NESTING_H +#define SASS_CHECK_NESTING_H + +#include "ast.hpp" +#include "context.hpp" +#include "operation.hpp" + +namespace Sass { + + typedef Environment Env; + + class CheckNesting : public Operation_CRTP { + + Context& ctx; + std::vector block_stack; + std::vector parent_stack; + + AST_Node* parent(); + + Statement* fallback_impl(AST_Node* n); + + public: + CheckNesting(Context&); + ~CheckNesting() { } + + Statement* operator()(Block*); + Statement* operator()(Declaration*); + + template + Statement* fallback(U x) { return fallback_impl(x); } + + bool is_valid_prop_parent(AST_Node*); + void append_block(Block*); + }; + +} + +#endif diff --git a/src/context.cpp b/src/context.cpp index 3135908cbb..7b365ff5ef 100644 --- a/src/context.cpp +++ b/src/context.cpp @@ -18,6 +18,7 @@ #include "output.hpp" #include "expand.hpp" #include "eval.hpp" +#include "check_nesting.hpp" #include "cssize.hpp" #include "listize.hpp" #include "extend.hpp" @@ -648,8 +649,13 @@ namespace Sass { // create crtp visitor objects Expand expand(*this, &global, &backtrace); Cssize cssize(*this, &backtrace); + CheckNesting check_nesting(*this); + // check nesting + root = root->perform(&check_nesting)->block(); // expand and eval the tree root = root->perform(&expand)->block(); + // check nesting + root = root->perform(&check_nesting)->block(); // merge and bubble certain rules root = root->perform(&cssize)->block(); // should we extend something? diff --git a/win/libsass.targets b/win/libsass.targets index b783e4de1e..bbc7ad6879 100644 --- a/win/libsass.targets +++ b/win/libsass.targets @@ -18,6 +18,7 @@ + @@ -71,6 +72,7 @@ + diff --git a/win/libsass.vcxproj.filters b/win/libsass.vcxproj.filters index ee6c1b9cb9..eb8e3625b6 100644 --- a/win/libsass.vcxproj.filters +++ b/win/libsass.vcxproj.filters @@ -66,6 +66,9 @@ Headers + + Headers + Headers @@ -224,6 +227,9 @@ Sources + + Sources + Sources From c75f24d8979f81081a46561c716d192b4aad04dc Mon Sep 17 00:00:00 2001 From: xzyfer Date: Mon, 2 May 2016 20:35:21 +1000 Subject: [PATCH 2/2] wip --- src/check_nesting.cpp | 16 +++++----------- src/check_nesting.hpp | 1 - 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/src/check_nesting.cpp b/src/check_nesting.cpp index 8dbfcd6c7e..1ab468ab41 100644 --- a/src/check_nesting.cpp +++ b/src/check_nesting.cpp @@ -22,7 +22,11 @@ namespace Sass { Statement* CheckNesting::operator()(Block* b) { parent_stack.push_back(b); - append_block(b); + + for (auto n : *b) { + n->perform(this); + } + parent_stack.pop_back(); return b; } @@ -53,14 +57,4 @@ namespace Sass { dynamic_cast(p) || dynamic_cast(p); } - - void CheckNesting::append_block(Block* b) - { - for (size_t i = 0, L = b->length(); i < L; ++i) { - Statement* ith = (*b)[i]->perform(this); - if (ith) { - (*b)[i] = ith; - } - } - } } diff --git a/src/check_nesting.hpp b/src/check_nesting.hpp index cd8bcd09c9..7f41dbc7a5 100644 --- a/src/check_nesting.hpp +++ b/src/check_nesting.hpp @@ -30,7 +30,6 @@ namespace Sass { Statement* fallback(U x) { return fallback_impl(x); } bool is_valid_prop_parent(AST_Node*); - void append_block(Block*); }; }