Merge pull request #103 from sass/fixes

Fix more bugs.

CHANGELOG.md

@@ -14,6 +14,10 @@
 
 * Warn about using named colors in interpolation.
 
+* Don't emit loud comments in functions.
+
+* Detect import loops.
+
 * Fix `@import` with a `supports()` clause.
 
 * Forbid functions named "and", "or", and "not".
@@ -41,6 +45,16 @@
 
 * Allow whitespace between `=` and the mixin name in the indented syntax.
 
+* Fix some slash division edge cases.
+
+* Fix `not` when used like a function.
+
+* Fix attribute selectors with single-character values.
+
+* Fix some bugs with the `call()` function.
+
+* Properly handle a backslash followed by a CRLF sequence in a quoted string.
+
 ## 1.0.0-alpha.8
 
 * Add the `content-exists()` function.

lib/src/ast/sass/expression/value.dart

@@ -16,9 +16,9 @@ class ValueExpression implements Expression {
   /// The embedded value.
   final Value value;
 
-  FileSpan get span => null;
+  final FileSpan span;
 
-  ValueExpression(this.value);
+  ValueExpression(this.value, [this.span]);
 
   /*=T*/ accept/*<T>*/(ExpressionVisitor/*<T>*/ visitor) =>
       visitor.visitValueExpression(this);

lib/src/parse/sass.dart

@@ -39,7 +39,15 @@ class SassParser extends StylesheetParser {
   SassParser(String contents, {url, bool color: false})
       : super(contents, url: url, color: color);
 
-  void expectStatementSeparator() {
+  void expectStatementSeparator([String name]) {
+    if (!atEndOfStatement()) scanner.expectChar($lf);
+    if (_peekIndentation() <= currentIndentation) return;
+    scanner.error(
+        "Nothing may be indented ${name == null ? 'here' : 'beneath a $name'}.",
+        position: _nextIndentationEnd.position);
+  }
+
+  void expectSemicolon(String name) {
     if (!atEndOfStatement()) scanner.expectChar($lf);
   }
 

lib/src/parse/scss.dart

@@ -17,7 +17,7 @@ class ScssParser extends StylesheetParser {
   ScssParser(String contents, {url, bool color: false})
       : super(contents, url: url, color: color);
 
-  void expectStatementSeparator() {
+  void expectStatementSeparator([String name]) {
     whitespaceWithoutComments();
     if (scanner.isDone) return;
     var next = scanner.peekChar();

lib/src/parse/stylesheet.dart

@@ -131,8 +131,7 @@ abstract class StylesheetParser extends Parser {
       whitespace();
     }
 
-    expectStatementSeparator();
-
+    expectStatementSeparator("variable declaration");
     return new VariableDeclaration(name, expression, scanner.spanFrom(start),
         guarded: guarded, global: global);
   }
@@ -230,10 +229,7 @@ abstract class StylesheetParser extends Parser {
     var name = nameBuffer.interpolation(scanner.spanFrom(start));
     if (name.initialPlain.startsWith('--')) {
       var value = _interpolatedDeclarationValue();
-      if (!atEndOfStatement()) {
-        if (!indented) scanner.expectChar($semicolon);
-        scanner.error("Expected newline.");
-      }
+      expectStatementSeparator("custom property");
       return new Declaration(name, scanner.spanFrom(start), value: value);
     }
 
@@ -489,6 +485,7 @@ abstract class StylesheetParser extends Parser {
   ContentRule _contentRule(LineScannerState start) {
     if (_inMixin) {
       _mixinHasContent = true;
+      expectStatementSeparator("@content rule");
       return new ContentRule(scanner.spanFrom(start));
     }
 
@@ -502,7 +499,7 @@ abstract class StylesheetParser extends Parser {
   /// [start] should point before the `@`.
   DebugRule _debugRule(LineScannerState start) {
     var expression = _expression();
-    expectStatementSeparator();
+    expectStatementSeparator("@debug rule");
     return new DebugRule(expression, scanner.spanFrom(start));
   }
 
@@ -537,7 +534,7 @@ abstract class StylesheetParser extends Parser {
   /// [start] should point before the `@`.
   ErrorRule _errorRule(LineScannerState start) {
     var expression = _expression();
-    expectStatementSeparator();
+    expectStatementSeparator("@error rule");
     return new ErrorRule(expression, scanner.spanFrom(start));
   }
 
@@ -548,7 +545,7 @@ abstract class StylesheetParser extends Parser {
     var value = _almostAnyValue();
     var optional = scanner.scanChar($exclamation);
     if (optional) expectIdentifier("optional");
-    expectStatementSeparator();
+    expectStatementSeparator("@extend rule");
     return new ExtendRule(value, scanner.spanFrom(start), optional: optional);
   }
 
@@ -669,7 +666,7 @@ abstract class StylesheetParser extends Parser {
       imports.add(_importArgument(start));
       whitespace();
     } while (scanner.scanChar($comma));
-    expectStatementSeparator();
+    expectStatementSeparator("@import rule");
 
     return new ImportRule(imports, scanner.spanFrom(start));
   }
@@ -877,7 +874,7 @@ abstract class StylesheetParser extends Parser {
   /// [start] should point before the `@`.
   ReturnRule _returnRule(LineScannerState start) {
     var expression = _expression();
-    expectStatementSeparator();
+    expectStatementSeparator("@return rule");
     return new ReturnRule(expression, scanner.spanFrom(start));
   }
 
@@ -896,7 +893,7 @@ abstract class StylesheetParser extends Parser {
   /// [start] should point before the `@`.
   WarnRule _warnRule(LineScannerState start) {
     var expression = _expression();
-    expectStatementSeparator();
+    expectStatementSeparator("@warn rule");
     return new WarnRule(expression, scanner.spanFrom(start));
   }
 
@@ -990,8 +987,6 @@ abstract class StylesheetParser extends Parser {
   /// invocations don't allow the Microsoft-style `=` operator at the top level,
   /// but function invocations do.
   ArgumentInvocation _argumentInvocation({bool mixin: false}) {
-    var wasInParentheses = _inParentheses;
-    _inParentheses = true;
     var start = scanner.state;
     scanner.expectChar($lparen);
     whitespace();
@@ -1034,7 +1029,6 @@ abstract class StylesheetParser extends Parser {
     }
     scanner.expectChar($rparen);
 
-    _inParentheses = wasInParentheses;
     return new ArgumentInvocation(positional, named, scanner.spanFrom(start),
         rest: rest, keywordRest: keywordRest);
   }
@@ -1892,9 +1886,11 @@ abstract class StylesheetParser extends Parser {
       } else if (next == null || isNewline(next)) {
         scanner.error("Expected ${new String.fromCharCode(quote)}.");
       } else if (next == $backslash) {
-        if (isNewline(scanner.peekChar(1))) {
+        var second = scanner.peekChar(1);
+        if (isNewline(second)) {
           scanner.readChar();
           scanner.readChar();
+          if (second == $cr) scanner.scanChar($lf);
         } else {
           buffer.writeCharCode(escapeCharacter());
         }
@@ -1923,6 +1919,10 @@ abstract class StylesheetParser extends Parser {
         var invocation = _argumentInvocation();
         return new IfExpression(
             invocation, spanForList([identifier, invocation]));
+      } else if (plain == "not") {
+        whitespace();
+        return new UnaryOperationExpression(
+            UnaryOperator.not, _singleExpression(), identifier.span);
       }
 
       var lower = plain.toLowerCase();
@@ -1930,10 +1930,6 @@ abstract class StylesheetParser extends Parser {
         switch (plain) {
           case "false":
             return new BooleanExpression(false, identifier.span);
-          case "not":
-            whitespace();
-            return new UnaryOperationExpression(
-                UnaryOperator.not, _singleExpression(), identifier.span);
           case "null":
             return new NullExpression(identifier.span);
           case "true":
@@ -2644,8 +2640,10 @@ abstract class StylesheetParser extends Parser {
   /// Asserts that the scanner is positioned before a statement separator, or at
   /// the end of a list of statements.
   ///
+  /// If the [name] of the parent rule is passed, it's used for error reporting.
+  ///
   /// This consumes whitespace, but nothing else, including comments.
-  void expectStatementSeparator();
+  void expectStatementSeparator([String name]);
 
   /// Whether the scanner is positioned at the end of a statement.
   bool atEndOfStatement();

lib/src/visitor/perform.dart

@@ -5,6 +5,7 @@
 import 'dart:math' as math;
 
 import 'package:charcode/charcode.dart';
+import 'package:collection/collection.dart';
 import 'package:path/path.dart' as p;
 import 'package:source_span/source_span.dart';
 import 'package:stack_trace/stack_trace.dart';
@@ -82,6 +83,9 @@ class _PerformVisitor
   /// This is used to provide `call()` with a span.
   FileSpan _callableSpan;
 
+  /// Whether we're currently executing a function.
+  var _inFunction = false;
+
   /// Whether we're currently building the output of an unknown at rule.
   var _inUnknownAtRule = false;
 
@@ -118,6 +122,8 @@ class _PerformVisitor
   /// multiple times.
   final _importedFiles = <String, Stylesheet>{};
 
+  final _activeImports = new Set<Uri>();
+
   /// The extender that handles extensions for this perform run.
   final _extender = new Extender();
 
@@ -158,7 +164,14 @@ class _PerformVisitor
       var args = arguments[1] as SassArgumentList;
 
       var invocation = new ArgumentInvocation([], {}, _callableSpan,
-          rest: new ValueExpression(args));
+          rest: new ValueExpression(args, _callableSpan),
+          keywordRest: args.keywords.isEmpty
+              ? null
+              : new ValueExpression(
+                  new SassMap(mapMap(args.keywords,
+                      key: (String key, Value _) => new SassString(key),
+                      value: (String _, Value value) => value)),
+                  _callableSpan));
 
       if (function is SassString) {
         warn(
@@ -179,6 +192,7 @@ class _PerformVisitor
   }
 
   CssStylesheet run(Stylesheet node) {
+    if (node.span != null) _activeImports.add(node.span.sourceUrl);
     visitStylesheet(node);
     return _root;
   }
@@ -395,8 +409,8 @@ class _PerformVisitor
   Value visitEachRule(EachRule node) {
     var list = node.list.accept(this);
     var setVariables = node.variables.length == 1
-        ? (Value value) =>
-            _environment.setLocalVariable(node.variables.first, value)
+        ? (Value value) => _environment.setLocalVariable(
+            node.variables.first, value.withoutSlash())
         : (Value value) => _setMultipleVariables(node.variables, value);
     return _environment.scope(() {
       return _handleReturn/*<Value>*/(list.asList, (element) {
@@ -413,7 +427,7 @@ class _PerformVisitor
     var list = value.asList;
     var minLength = math.min(variables.length, list.length);
     for (var i = 0; i < minLength; i++) {
-      _environment.setLocalVariable(variables[i], list[i]);
+      _environment.setLocalVariable(variables[i], list[i].withoutSlash());
     }
     for (var i = minLength; i < variables.length; i++) {
       _environment.setLocalVariable(variables[i], sassNull);
@@ -550,6 +564,13 @@ class _PerformVisitor
   /// Adds the stylesheet imported by [import] to the current document.
   void _visitDynamicImport(DynamicImport import) {
     var stylesheet = _loadImport(import);
+
+    var url = stylesheet.span.sourceUrl;
+    if (_activeImports.contains(url)) {
+      throw _exception("This file is already being imported.", import.span);
+    }
+
+    _activeImports.add(url);
     _withStackFrame("@import", import.span, () {
       _withEnvironment(_environment.global(), () {
         for (var statement in stylesheet.children) {
@@ -557,6 +578,7 @@ class _PerformVisitor
         }
       });
     });
+    _activeImports.remove(url);
   }
 
   /// Loads the [Stylesheet] imported by [import], or throws a
@@ -683,6 +705,8 @@ class _PerformVisitor
   }
 
   Value visitLoudComment(LoudComment node) {
+    if (_inFunction) return null;
+
     // Comments are allowed to appear between CSS imports.
     if (_parent == _root && _endOfImports == _root.children.length) {
       _endOfImports++;
@@ -1029,7 +1053,11 @@ class _PerformVisitor
         (plainName == null ? null : _environment.getFunction(plainName)) ??
             new PlainCssCallable(_performInterpolation(node.name));
 
-    return _runFunctionCallable(node.arguments, function, node.span);
+    var oldInFunction = _inFunction;
+    _inFunction = true;
+    var result = _runFunctionCallable(node.arguments, function, node.span);
+    _inFunction = oldInFunction;
+    return result;
   }
 
   /// Evaluates the arguments in [arguments] as applied to [callable], and
@@ -1054,15 +1082,14 @@ class _PerformVisitor
           var minLength = math.min(positional.length, declaredArguments.length);
           for (var i = 0; i < minLength; i++) {
             _environment.setLocalVariable(
-                declaredArguments[i].name, positional[i]);
+                declaredArguments[i].name, positional[i].withoutSlash());
           }
 
           for (var i = positional.length; i < declaredArguments.length; i++) {
             var argument = declaredArguments[i];
-            _environment.setLocalVariable(
-                argument.name,
-                named.remove(argument.name) ??
-                    argument.defaultValue?.accept(this));
+            var value = named.remove(argument.name) ??
+                argument.defaultValue?.accept(this);
+            _environment.setLocalVariable(argument.name, value?.withoutSlash());
           }
 
           SassArgumentList argumentList;

lib/src/visitor/serialize.dart

@@ -837,9 +837,11 @@ class _SerializeCssVisitor
     var scanner = new StringScanner(text);
     while (scanner.scanChar($dash)) {}
 
+    if (scanner.isDone) return false;
     var first = scanner.readChar();
-    if (first == null) return false;
+
     if (isNameStart(first)) {
+      if (scanner.isDone) return true;
       scanner.readChar();
     } else if (first == $backslash) {
       if (!_consumeEscape(scanner)) return false;
@@ -878,6 +880,7 @@ class _SerializeCssVisitor
       }
       if (isWhitespace(scanner.peekChar())) scanner.readChar();
     } else {
+      if (scanner.isDone) return false;
       scanner.readChar();
     }
 

pubspec.yaml

@@ -13,7 +13,7 @@ environment:
 dependencies:
   args: "^0.13.0"
   charcode: "^1.1.0"
-  collection: "^1.1.0"
+  collection: "^1.8.0"
   path: "^1.0.0"
   source_span: "^1.3.0"
   string_scanner: ">=0.1.5 <2.0.0"