Replace buggy selector tree rewriting with 2-pass compilation

leafo · Aug 1, 2015 · 609c085 · 609c085
1 parent be55b9e
commit 609c085
Show file tree

Hide file tree

Showing 5 changed files with 82 additions and 57 deletions.
diff --git a/src/Compiler.php b/src/Compiler.php
@@ -119,6 +119,7 @@ class Compiler
     private $storeEnv;
     private $charsetSeen;
     private $stderr;
+    private $shouldEvaluate;
 
     /**
      * Compile scss
@@ -509,82 +510,87 @@ protected function compileComment($block)
         $this->scope->children[] = $out;
     }
 
-    // joins together .classes and #ids
-    protected function flattenSelectorSingle($single)
+    protected function evalSelectors($selectors)
     {
-        $joined = array();
-        foreach ($single as $part) {
-            if (empty($joined) ||
-                ! is_string($part) ||
-                preg_match('/[\[.:#%]/', $part)
-            ) {
-                $joined[] = $part;
-                continue;
-            }
+        $this->shouldEvaluate = false;
 
-            if (is_array(end($joined))) {
-                $joined[] = $part;
-            } else {
-                $joined[count($joined) - 1] .= $part;
+        $selectors = array_map(array($this, 'evalSelector'), $selectors);
+
+        // after evaluating interpolates, we might need a second pass
+        if ($this->shouldEvaluate) {
+            $buffer = $this->collapseSelectors($selectors);
+            $parser = new Parser(__METHOD__, false);
+
+            if ($parser->parseSelector($buffer, $newSelectors)) {
+                $selectors = array_map(array($this, 'evalSelector'), $newSelectors);
             }
         }
 
-        return $joined;
+        return $selectors;
     }
 
-    // replaces all the interpolates
     protected function evalSelector($selector)
     {
         return array_map(array($this, 'evalSelectorPart'), $selector);
     }
 
-    protected function evalSelectors($selectors)
+    // replaces all the interpolates, stripping quotes
+    protected function evalSelectorPart($part)
     {
-        $selectors = array_map(array($this, 'evalSelector'), $selectors);
-
-        $newSelectors = array();
-
-        foreach ($selectors as $selector) {
-            if (is_array($selector[0][0])) {
-                $newSelectors[] = $selector;
-            } elseif (strpos($selector[0][0], ',') === false) {
-                if ($selector[0][0][0] === '&') {
-                    $selector = array(array(array('self'), substr($selector[0][0], 1)));
-                }
+        foreach ($part as &$p) {
+            if (is_array($p) && ($p[0] === 'interpolate' || $p[0] === 'string')) {
+                $p = $this->compileValue($p);
 
-                $newSelectors[] = $selector;
-            } else {
-                foreach (array_map(function ($s) { return trim($s, " \t\n\r\0\x0b'\""); }, explode(',', $selector[0][0])) as $newSelectorPart) {
-                    if ($newSelectorPart[0] === '&') {
-                        $newSelectors[] = array(array(array('self'), substr($newSelectorPart, 1)));
-                    } else {
-                        $newSelectors[] = array(array($newSelectorPart));
-                    }
+                // force re-evaluation
+                if (strpos($p, '&') !== false || strpos($p, ',') !== false) {
+                    $this->shouldEvaluate = true;
                 }
+            } elseif (is_string($p) && strlen($p) >= 2 &&
+                ($first = $p[0]) && ($first === '"' || $first === "'") &&
+                substr($p, -1) === $first
+            ) {
+                $p = substr($p, 1, -1);
             }
         }
 
-        return $newSelectors;
+        return $this->flattenSelectorSingle($part);
     }
 
-    protected function evalSelectorPart($piece)
+    protected function collapseSelectors($selectors)
     {
-        foreach ($piece as &$p) {
-            if (! is_array($p)) {
+        $output = '';
+
+        array_walk_recursive(
+            $selectors,
+            function ($value, $key) use (&$output) {
+                $output .= $value;
+            }
+        );
+
+        return $output;
+    }
+
+    // joins together .classes and #ids
+    protected function flattenSelectorSingle($single)
+    {
+        $joined = array();
+        foreach ($single as $part) {
+            if (empty($joined) ||
+                ! is_string($part) ||
+                preg_match('/[\[.:#%]/', $part)
+            ) {
+                $joined[] = $part;
                 continue;
             }
 
-            switch ($p[0]) {
-                case 'interpolate':
-                    $p = $this->compileValue($p);
-                    break;
-                case 'string':
-                    $p = $this->compileValue($p);
-                    break;
+            if (is_array(end($joined))) {
+                $joined[] = $part;
+            } else {
+                $joined[count($joined) - 1] .= $part;
             }
         }
 
-        return $this->flattenSelectorSingle($piece);
+        return $joined;
     }
 
     // compiles to string
@@ -918,8 +924,8 @@ protected function compileChild($child, $out)
 
                 foreach ($selectors as $sel) {
                     // only use the first one
-                    $sel = current($this->evalSelector($sel));
-                    $this->pushExtends($sel, $out->selectors);
+                    $result = $this->evalSelectors(array($sel));
+                    $this->pushExtends(current($result[0]), $out->selectors);
                 }
                 break;
             case 'if':

diff --git a/src/Parser.php b/src/Parser.php
@@ -170,6 +170,25 @@ public function parseValue($buffer, &$out)
         return $this->valueList($out);
     }
 
+    /**
+     * Parse a selector or selector list
+     *
+     * @param string $buffer
+     * @param string $out
+     *
+     * @return boolean
+     */
+    public function parseSelector($buffer, &$out)
+    {
+        $this->count           = 0;
+        $this->env             = null;
+        $this->inParens        = false;
+        $this->eatWhiteDefault = true;
+        $this->buffer          = (string) $buffer;
+
+        return $this->selectors($out);
+    }
+
     /**
      * Parse a single chunk off the head of the buffer and append it to the
      * current parse environment.

diff --git a/tests/outputs/scss_css.css b/tests/outputs/scss_css.css
@@ -174,7 +174,7 @@ foo {
 12px {
   a: b; }
 
-"foo" {
+foo {
   a: b; }
 
 foo {

diff --git a/tests/outputs/selectors.css b/tests/outputs/selectors.css
@@ -363,7 +363,7 @@ span a, p a, div a {
 
 .parent.self1, .parent.self2 {
   content: "should match .parent.self1, .parent.self2"; }
-.self1 .parent {
-  content: "should match .self1 .parent"; }
-.parent + .parent {
-  content: "should match .parent + .parent"; }
+  .self1 .parent {
+    content: "should match .self1 .parent"; }
+  .parent + .parent {
+    content: "should match .parent + .parent"; }
diff --git a/tests/outputs_numbered/scss_css.css b/tests/outputs_numbered/scss_css.css
@@ -185,7 +185,7 @@ foo {
 12px {
   a: b; }
 /* line 195, inputs/scss_css.scss */
-"foo" {
+foo {
   a: b; }
 /* line 199, inputs/scss_css.scss */
 foo {
-Original file line number
+Diff line change
@@ Expand Up / @@ -174,7 +174,7 @@ foo { @@
 px {
       a: b; }
-    "foo" {
+    foo {
       a: b; }
     foo {
@@ Expand Down @@