Skip to content

Commit 3043872

Browse files
committed
Handle fake parent selectors when evaling
LibSass fully parses selectors during the parsing stage. This is different from Ruby Sass which parse selectors and string during the parsing stage, and lazily eval'ing when required. This difference causes some pathological selector issues. Our eager parsing of selectors has resulted in us hacking in fake `Parent_Selector` into `Sequence_Selector` during parsing. These fake `Parent_Selector` play havoc with `resolve_parent_refs` when within `@at-root` blocks. I spent a couple weeks going down the rabbit whole of refactoring our selector parsing to be lazy before bailing. Explicitly marking which `Parent_Selector` are fake during parsing allows us faithfully implement the `implicit_parent` flag in `resolve_parent_refs`. Fixes #2006 Fixes #2198 Spec sass/sass-spec#936
1 parent f891c7e commit 3043872

File tree

5 files changed

+59
-5
lines changed

5 files changed

+59
-5
lines changed

src/ast.cpp

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,12 +157,26 @@ namespace Sass {
157157
return false;
158158
}
159159

160+
bool SimpleSequence_Selector::has_real_parent_ref()
161+
{
162+
for (Simple_Selector* s : *this) {
163+
if (s && s->has_real_parent_ref()) return true;
164+
}
165+
return false;
166+
}
167+
160168
bool Sequence_Selector::has_parent_ref()
161169
{
162170
return (head() && head()->has_parent_ref()) ||
163171
(tail() && tail()->has_parent_ref());
164172
}
165173

174+
bool Sequence_Selector::has_real_parent_ref()
175+
{
176+
return (head() && head()->has_real_parent_ref()) ||
177+
(tail() && tail()->has_real_parent_ref());
178+
}
179+
166180
bool Sequence_Selector::operator< (const Sequence_Selector& rhs) const
167181
{
168182
// const iterators for tails
@@ -1110,6 +1124,12 @@ namespace Sass {
11101124
Sequence_Selector* tail = this->tail();
11111125
SimpleSequence_Selector* head = this->head();
11121126

1127+
if (!this->has_real_parent_ref() && !implicit_parent) {
1128+
CommaSequence_Selector* retval = SASS_MEMORY_NEW(ctx.mem, CommaSequence_Selector, pstate());
1129+
*retval << this;
1130+
return retval;
1131+
}
1132+
11131133
// first resolve_parent_refs the tail (which may return an expanded list)
11141134
CommaSequence_Selector* tails = tail ? tail->resolve_parent_refs(ctx, parents, implicit_parent) : 0;
11151135

@@ -1396,6 +1416,14 @@ namespace Sass {
13961416
return false;
13971417
}
13981418

1419+
bool CommaSequence_Selector::has_real_parent_ref()
1420+
{
1421+
for (Sequence_Selector* s : *this) {
1422+
if (s && s->has_real_parent_ref()) return true;
1423+
}
1424+
return false;
1425+
}
1426+
13991427
bool Selector_Schema::has_parent_ref()
14001428
{
14011429
if (String_Schema* schema = dynamic_cast<String_Schema*>(contents())) {
@@ -1404,6 +1432,15 @@ namespace Sass {
14041432
return false;
14051433
}
14061434

1435+
bool Selector_Schema::has_real_parent_ref()
1436+
{
1437+
if (String_Schema* schema = dynamic_cast<String_Schema*>(contents())) {
1438+
Parent_Selector* p = dynamic_cast<Parent_Selector*>(schema->at(0));
1439+
return schema->length() > 0 && p != NULL && p->is_real_parent_ref();
1440+
}
1441+
return false;
1442+
}
1443+
14071444
void CommaSequence_Selector::adjust_after_pushing(Sequence_Selector* c)
14081445
{
14091446
// if (c->has_reference()) has_reference(true);

src/ast.hpp

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1897,6 +1897,9 @@ namespace Sass {
18971897
virtual bool has_parent_ref() {
18981898
return false;
18991899
}
1900+
virtual bool has_real_parent_ref() {
1901+
return false;
1902+
}
19001903
};
19011904
inline Selector::~Selector() { }
19021905

@@ -1912,6 +1915,7 @@ namespace Sass {
19121915
: Selector(pstate), contents_(c), at_root_(false)
19131916
{ }
19141917
virtual bool has_parent_ref();
1918+
virtual bool has_real_parent_ref();
19151919
virtual size_t hash() {
19161920
if (hash_ == 0) {
19171921
hash_combine(hash_, contents_->hash());
@@ -1990,6 +1994,7 @@ namespace Sass {
19901994
virtual ~Simple_Selector() = 0;
19911995
virtual SimpleSequence_Selector* unify_with(SimpleSequence_Selector*, Context&);
19921996
virtual bool has_parent_ref() { return false; };
1997+
virtual bool has_real_parent_ref() { return false; };
19931998
virtual bool is_pseudo_element() { return false; }
19941999
virtual bool is_pseudo_class() { return false; }
19952000

@@ -2012,11 +2017,14 @@ namespace Sass {
20122017
// inside strings in declarations (SimpleSequence_Selector).
20132018
// only one simple parent selector means the first case.
20142019
class Parent_Selector : public Simple_Selector {
2020+
ADD_PROPERTY(bool, real)
20152021
public:
2016-
Parent_Selector(ParserState pstate)
2017-
: Simple_Selector(pstate, "&")
2022+
Parent_Selector(ParserState pstate, bool r = true)
2023+
: Simple_Selector(pstate, "&"), real_(r)
20182024
{ /* has_reference(true); */ }
2025+
bool is_real_parent_ref() { return real(); };
20192026
virtual bool has_parent_ref() { return true; };
2027+
virtual bool has_real_parent_ref() { return is_real_parent_ref(); };
20202028
virtual unsigned long specificity()
20212029
{
20222030
return 0;
@@ -2225,6 +2233,11 @@ namespace Sass {
22252233
if (!selector()) return false;
22262234
return selector()->has_parent_ref();
22272235
}
2236+
virtual bool has_real_parent_ref() {
2237+
// if (has_reference()) return true;
2238+
if (!selector()) return false;
2239+
return selector()->has_real_parent_ref();
2240+
}
22282241
virtual bool has_wrapped_selector()
22292242
{
22302243
return true;
@@ -2283,6 +2296,7 @@ namespace Sass {
22832296
SimpleSequence_Selector* unify_with(SimpleSequence_Selector* rhs, Context& ctx);
22842297
// virtual Placeholder_Selector* find_placeholder();
22852298
virtual bool has_parent_ref();
2299+
virtual bool has_real_parent_ref();
22862300
Simple_Selector* base()
22872301
{
22882302
// Implement non-const in terms of const. Safe to const_cast since this method is non-const
@@ -2387,6 +2401,7 @@ namespace Sass {
23872401
// if ((h && h->has_placeholder()) || (t && t->has_placeholder())) has_placeholder(true);
23882402
}
23892403
virtual bool has_parent_ref();
2404+
virtual bool has_real_parent_ref();
23902405

23912406
Sequence_Selector* skip_empty_reference()
23922407
{
@@ -2429,7 +2444,7 @@ namespace Sass {
24292444
Sequence_Selector* innermost() { return last(); };
24302445

24312446
size_t length() const;
2432-
CommaSequence_Selector* resolve_parent_refs(Context& ctx, CommaSequence_Selector* parents, bool implicit_parent);
2447+
CommaSequence_Selector* resolve_parent_refs(Context& ctx, CommaSequence_Selector* parents, bool implicit_parent = true);
24332448
virtual bool is_superselector_of(SimpleSequence_Selector* sub, std::string wrapping = "");
24342449
virtual bool is_superselector_of(Sequence_Selector* sub, std::string wrapping = "");
24352450
virtual bool is_superselector_of(CommaSequence_Selector* sub, std::string wrapping = "");
@@ -2545,6 +2560,7 @@ namespace Sass {
25452560
// remove parent selector references
25462561
// basically unwraps parsed selectors
25472562
virtual bool has_parent_ref();
2563+
virtual bool has_real_parent_ref();
25482564
void remove_parent_selectors();
25492565
// virtual Placeholder_Selector* find_placeholder();
25502566
CommaSequence_Selector* resolve_parent_refs(Context& ctx, CommaSequence_Selector* parents, bool implicit_parent = true);

src/debugger.hpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ inline void debug_ast(AST_Node* node, std::string ind, Env* env)
107107
// if (selector->not_selector()) cerr << " [in_declaration]";
108108
std::cerr << " (" << pstate_source_position(node) << ")";
109109
std::cerr << " <" << selector->hash() << ">";
110+
std::cerr << " [" << (selector->is_real_parent_ref() ? "REAL" : "FAKE") << "]";
110111
std::cerr << " <" << prettyprint(selector->pstate().token.ws_before()) << ">" << std::endl;
111112
// debug_ast(selector->selector(), ind + "->", env);
112113

src/inspect.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -876,7 +876,7 @@ namespace Sass {
876876

877877
void Inspect::operator()(Parent_Selector* p)
878878
{
879-
append_string("&");
879+
if (p->is_real_parent_ref()) append_string("&");
880880
}
881881

882882
void Inspect::operator()(Placeholder_Selector* s)

src/parser.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -703,7 +703,7 @@ namespace Sass {
703703
// also skip adding parent ref if we only have refs
704704
if (!sel->has_parent_ref() && !in_at_root && !in_root) {
705705
// create the objects to wrap parent selector reference
706-
Parent_Selector* parent = SASS_MEMORY_NEW(ctx.mem, Parent_Selector, pstate);
706+
Parent_Selector* parent = SASS_MEMORY_NEW(ctx.mem, Parent_Selector, pstate, false);
707707
parent->media_block(last_media_block);
708708
SimpleSequence_Selector* head = SASS_MEMORY_NEW(ctx.mem, SimpleSequence_Selector, pstate);
709709
head->media_block(last_media_block);

0 commit comments

Comments
 (0)