Support for @mixin, @include and @extend

pkg/csslib/lib/parser.dart

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 library csslib.parser;
 
 import 'dart:math' as math;
 
 import 'package:source_maps/span.dart' show SourceFile, Span, FileSpan;
 
@@ skipping 411 matching lines @@
             return null;
           }
         }
       } else if (isChecked) {
         _warning("Missing media feature in media expression", _makeSpan(start));
         return null;
       }
     }
   }
 
-  //  Directive grammar:
-  //
-  //  import:             '@import' [string | URI] media_list?
-  //  media:              '@media' media_query_list '{' ruleset '}'
-  //  page:               '@page' [':' IDENT]? '{' declarations '}'
-  //  stylet:             '@stylet' IDENT '{' ruleset '}'
-  //  media_query_list:   IDENT [',' IDENT]
-  //  keyframes:          '@-webkit-keyframes ...' (see grammar below).
-  //  font_face:          '@font-face' '{' declarations '}'
-  //  namespace:          '@namespace name url("xmlns")
-  //  host:               '@host '{' ruleset '}'
+  /**
+   * Directive grammar:
+   *
+   *  import:             '@import' [string | URI] media_list?
+   *  media:              '@media' media_query_list '{' ruleset '}'
+   *  page:               '@page' [':' IDENT]? '{' declarations '}'
+   *  stylet:             '@stylet' IDENT '{' ruleset '}'
+   *  media_query_list:   IDENT [',' IDENT]
+   *  keyframes:          '@-webkit-keyframes ...' (see grammar below).
+   *  font_face:          '@font-face' '{' declarations '}'
+   *  namespace:          '@namespace name url("xmlns")
+   *  host:               '@host '{' ruleset '}'
+   *  mixin:              '@mixin name [(args,...)] '{' declarations/ruleset '}'
+   *  include:            '@include name [(@arg,@arg1)]
+   *                      '@include name [(@arg...)]
+   *  content             '@content'
+   */
   processDirective() {

[nweiz, 2013/09/18 22:40:54]

Why is this (as well as many other productions) public?

[terry, 2013/10/09 03:40:33]

The parser should be a hidden class (_Parser).
On 2013/09/18 22:40:54, nweiz wrote:

     int start = _peekToken.start;
 
     var tokId = _peek();
     // Handle case for @ directive (where there's a whitespace between the @
     // sign and the directive name.  Technically, it's not valid grammar but
     // a number of CSS tests test for whitespace between @ and name.
     if (tokId == TokenKind.AT) {
       Token tok = _next();
       tokId = _peek();
       if (_peekIdentifier()) {
@@ skipping 287 matching lines @@
               namespaceUri = func.text;
               prefix = null;
             }
           } else {
             namespaceUri = processQuotedString(false);
           }
         }
 
         return new NamespaceDirective(prefix != null ? prefix.name : '',
             namespaceUri, _makeSpan(start));
+
+      case TokenKind.DIRECTIVE_MIXIN:
+        /* Stylet grammar:

[nweiz, 2013/09/18 22:40:54]

"Stylet" -> "Mixin"

[terry, 2013/10/09 03:40:33]

Done.

+         *
+         *  @mixin IDENT [(args,...)] '{'
+         *    ruleset | properties

[nweiz, 2013/09/18 22:40:54]

This should be "[ruleset | property | directive]*", or if you want to be more
correct "[ruleset | property | directive] (';' [ruleset | property |
directive])*" with the caveat that the semicolon isn't necessary if the
preceding production has a brace-delimited block.

[terry, 2013/10/09 03:40:33]

Done.

+         *  '}'
+         */
+        _next();
+
+        var name;
+        if (_peekIdentifier()) {

[Jennifer Messerly, 2013/09/18 06:44:15]

is identifier optional? it doesn't appear so from the grammar comment above

[terry, 2013/10/09 03:40:33]

It's not optional, it's required - changed.

On 2013/09/18 06:44:15, John Messerly wrote:

+          name = identifier();
+        }

[Jennifer Messerly, 2013/09/18 06:44:15]

TODO here about parsing the args?

[terry, 2013/10/09 03:40:33]

Done.

+
+        _eat(TokenKind.LBRACE);
+
+        List<TreeNode> productions = [];
+        List<TreeNode> declarations = [];
+        var mixinDirective;
+
+        start = _peekToken.start;
+        while (!_maybeEat(TokenKind.END_OF_FILE)) {
+          var directive = processDirective();

[Jennifer Messerly, 2013/09/18 06:44:15]

is it possible to reuse any existing parser code here?

from the grammar snippet I was expecting something a bit simpler.

[terry, 2013/10/09 03:40:33]

Was able to simplify a bit but this is really handling special cases for mixins
both declarations and top-level stuff which are very different.  The parsing of
the decl vs ruleset is of course re-used code.

On 2013/09/18 06:44:15, John Messerly wrote:

+          if (directive != null) {
+            productions.add(directive);
+            _maybeEat(TokenKind.SEMICOLON);

[nweiz, 2013/09/18 22:40:54]

Why are you using [_maybeEat] here? Any number of successive semicolons should
be allowed between children of a directive, not just one. Also, the following is
invalid (due to the lack of a semicolon between the two sub-directives), where
it looks like it would be allowed by this code: "@mixin foo {@some-directive
@another-directive}".

The algorithm that Sass uses for this sort of parsing looks like this:

    while consume(';') or last child has child nodes:
      last child <- process block child

where a block child could be empty.

[terry, 2013/10/09 03:40:33]

Okay, I fixed it.

On 2013/09/18 22:40:54, nweiz wrote:

+            continue;
+          }
+          var selectorGroup = _nestedSelector();
+          if (selectorGroup != null) {
+            // Nested selector so process as a ruleset.
+            var ruleset = processRuleSet(selectorGroup);
+            if (ruleset == null) {
+              mixinDirective = new MixinRulesetDirective(name.name, [], false,
+                  productions, _makeSpan(start));
+              break;

[nweiz, 2013/09/18 22:40:54]

Why are you breaking here? Mixins can contain more than one ruleset, or rulesets
mixed in with other types of children.

[terry, 2013/10/09 03:40:33]

Was able to eliminate the whole nestedSelector chunk of code - no longer
relevant (plus not used anyway).
On 2013/09/18 22:40:54, nweiz wrote:

+            }
+            productions.add(ruleset);
+          }
+
+          var declGroup = processDeclarations(checkBrace: false);
+
+          var decls = [];
+          if (declGroup.declarations.any((decl) {
+            return decl is Declaration && decl is! IncludeMixinAtDeclaration;
+            })) {

[nweiz, 2013/09/18 22:40:54]

Style nit: if you're going to write this if statement like this, it should be
indented as so:

    if (declGroup.declarations.any((decl) {
          return decl is Declaration && decl is! IncludeMixinAtDeclaration;
        })) {

The baseline indentation for a continued expression is +4 spaces, and the body
of the closure should be +2 spaces in addition to that.

That said, it's probably cleaner and more readable if you move the expression
into a variable rather than using it inline in the if statement.

[terry, 2013/10/09 03:40:33]

Ok, changed the indentation.
On 2013/09/18 22:40:54, nweiz wrote:

+            var newDecls = [];
+            productions.forEach((include) {
+              // If declGroup has items that are declarations then we assume
+              // this mixin is a declaration mixin not a top-level mixin.

[nweiz, 2013/09/18 22:40:54]

This isn't a distinction that should exist.

[terry, 2013/10/09 03:40:33]

Warnings are returned if a mixin has both decls and rulesets e.g.,

@mixin a {
  foo { color: red; }
  background-color: blue;
}

@include a;

gives a warning instead of magically losing the background-color.

On 2013/09/18 22:40:54, nweiz wrote:

+              if (include is IncludeDirective) {
+                newDecls.add(new IncludeMixinAtDeclaration(include, include.span));

[Jennifer Messerly, 2013/09/18 06:44:15]

long line

[terry, 2013/10/09 03:40:33]

Done.

+              } else {
+                print("Error mixing of top-level vs declarations mixins");
+              }
+            });
+            declGroup.declarations.insertAll(0, newDecls);
+            productions = [];
+          } else {
+            // Declarations are just @includes make it a list of productions
+            // no a declaration group.

[nweiz, 2013/09/18 22:40:54]

If I interpreted this sentence correctly, it should be "Declarations are just
@includes. Make it a list of productions, not a declaration group."

[terry, 2013/10/09 03:40:33]

Done.

+            declGroup.declarations.forEach((decl) {

[Jennifer Messerly, 2013/09/18 06:44:15]

I would probably just use a normal for-in loop here

[terry, 2013/10/09 03:40:33]

Done.

+              productions.add(decl is IncludeMixinAtDeclaration ?
+                  decl.include : decl);
+            });
+            declGroup.declarations.clear();
+          }
+
+          if (declGroup.declarations.isNotEmpty) {
+            if (productions.isEmpty) {
+              mixinDirective = new MixinDeclarationDirective(name.name, [],
+                  false, declGroup, _makeSpan(start));
+              break;
+            } else {
+              declGroup.declarations.forEach((decl) {
+                if (decl is IncludeMixinAtDeclaration) {

[Jennifer Messerly, 2013/09/18 06:44:15]

this looks similar to:

   productions.add(decl is IncludeMixinAtDeclaration ? decl.include : decl);

... above, maybe use the same style in both places?

[terry, 2013/10/09 03:40:33]

Done.

+                  productions.add(decl.include);
+                } else {
+                  productions.add(decl);
+                }
+              });
+            }
+          } else {
+            mixinDirective = new MixinRulesetDirective(name.name, [], false,
+                productions, _makeSpan(start));
+            break;
+          }
+        }
+
+        _eat(TokenKind.RBRACE);
+
+        return mixinDirective;
+
+      case TokenKind.DIRECTIVE_INCLUDE:
+        return processInclude( _makeSpan(start));
+
+      case TokenKind.DIRECTIVE_CONTENT:
+        // TODO(terry): TBD

[Jennifer Messerly, 2013/09/18 06:44:15]

issue warning about not implemented feature?

[terry, 2013/10/09 03:40:33]

Done.

     }
   }
 
+  IncludeDirective processInclude(Span span, [bool eatSemiColon = true]) {

[Jennifer Messerly, 2013/09/18 06:44:15]

for bools it's kind of nice to use named arguments like:

{bool eatSemicolon: true}

... a tad more readable at the call site

[terry, 2013/10/09 03:40:33]

Done.

+    /* Stylet grammar:

[nweiz, 2013/09/18 22:40:54]

"Stylet" -> "Include"

[terry, 2013/10/09 03:40:33]

Done.

+    *
+     *  @include IDENT [(args,...)];
+     */
+    _next();
+
+    var name;
+    if (_peekIdentifier()) {

[nweiz, 2013/09/18 22:40:54]

As in John's comment for @mixin, this shouldn't be optional.

[terry, 2013/10/09 03:40:33]

Right fixed and handled parameters too.
On 2013/09/18 22:40:54, nweiz wrote:

+      name = identifier();
+    }
+
+    if (eatSemiColon) {
+      _maybeEat(TokenKind.SEMICOLON);
+    }
+
+    // TODO(terry): Handle parameters to mixin.
+
+    return new IncludeDirective(name.name, [], span);
+  }
+
   RuleSet processRuleSet([SelectorGroup selectorGroup]) {
     if (selectorGroup == null) {
       selectorGroup = processSelectorGroup();
     }
     if (selectorGroup != null) {
       return new RuleSet(selectorGroup, processDeclarations(),
           selectorGroup.span);
     }
   }
 
@@ skipping 38 matching lines @@
       messages = oldMessages;
       return null;
     } else {
       // Remember any messages from look ahead.
       oldMessages.mergeMessages(messages);
       messages = oldMessages;
       return selGroup;
     }
   }
 
-  processDeclarations({bool checkBrace: true}) {
+  DeclarationGroup processDeclarations({bool checkBrace: true}) {
     int start = _peekToken.start;
 
     if (checkBrace) _eat(TokenKind.LBRACE);
 
     List decls = [];
     List dartStyles = [];             // List of latest styles exposed to Dart.
 
     do {
       var selectorGroup = _nestedSelector();
       while (selectorGroup != null) {
@@ skipping 532 matching lines @@
       }
 
       _eat(TokenKind.RBRACK);
 
       return new AttributeSelector(attrName, op, value, _makeSpan(start));
     }
   }
 
   //  Declaration grammar:
   //
   //  declaration:  property ':' expr prio?

[nweiz, 2013/09/18 22:40:54]

Shouldn't this include variables and now @include as well?

[terry, 2013/10/09 03:40:33]

It does.  See VAR_DEFINITION and DIRECTIVE_INCLUDE in this function.

On 2013/09/18 22:40:54, nweiz wrote:

   //
   //  property:  IDENT [or IE hacks]
   //  prio:      !important
   //  expr:      (see processExpr)
   //
   // Here are the ugly IE hacks we need to support:
   //   property: expr prio? \9; - IE8 and below property, /9 before semi-colon
   //   *IDENT                   - IE7 or below
   //   _IDENT                   - IE6 property (automatically a valid ident)
   //
@@ skipping 28 matching lines @@
     } else if (_peekToken.kind == TokenKind.VAR_DEFINITION) {
       _next();
       var definedName;
       if (_peekIdentifier()) definedName = identifier();
 
       _eat(TokenKind.COLON);
 
       Expressions exprs = processExpr();
 
       decl = new VarDefinition(definedName, exprs, _makeSpan(start));
+    } else if (_peekToken.kind == TokenKind.DIRECTIVE_INCLUDE) {
+      // @include mixinName in the declaration area.
+      var span = _makeSpan(start);
+      var include = processInclude(span, false);
+      decl = new IncludeMixinAtDeclaration(include, span);
     }
 
     return decl;
   }
 
   /** List of styles exposed to the Dart UI framework. */
   static const int _fontPartFont= 0;
   static const int _fontPartVariant = 1;
   static const int _fontPartWeight = 2;
   static const int _fontPartSize = 3;
@@ skipping 976 matching lines @@
 
     if (replace != null && result == null) {
       result = new StringBuffer(text.substring(0, i));
     }
 
     if (result != null) result.write(replace != null ? replace : text[i]);
   }
 
   return result == null ? text : result.toString();
 }