Merge pull request #830 from sass/merge-master

Merge master into feature.use

.travis.yml

@@ -64,6 +64,9 @@ jobs:
     env: DART_CHANNEL=dev
   - <<: *dart-tests
     os: windows
+    # File system watching is extra flaky on Dart 2.5.0 on Windows (see
+    # dart-lang/sdk#38334).
+    env: DART_VERSION=2.4.1
   - <<: *dart-tests
     os: osx
 
@@ -110,10 +113,12 @@ jobs:
       (type IN (push, api)) AND (repo = sass/dart-sass) AND tag =~ ^\d+\.\d+\.\d+([+-].*)?$
     script: pub run grinder sanity-check-before-release
 
-  # Deploy Linux and Windows releases to GitHub. Mac OS releases are deployed in
-  # a later stage so that we can build application snapshots on Mac OS bots.
+  # Deploy Linux releases to GitHub. Mac OS releases are deployed in a later
+  # stage so that we can build application snapshots on Mac OS bots, and Windows
+  # releases are deployed later so they can use Dart 2.4.1 to work around
+  # dart-lang/sdk#38334.
   - stage: deploy 1
-    name: "GitHub: Windows and Linux"
+    name: "GitHub: Linux"
     if: *deploy-if
     env: &github-env
       - GITHUB_USER=sassbot
@@ -125,7 +130,7 @@ jobs:
     script: skip # Don't run tests
     deploy:
       provider: script
-      script: pub run grinder github-release github-linux github-windows
+      script: pub run grinder github-release github-linux
       skip_cleanup: true # Don't clean up the Dart SDK.
 
       # This causes the deploy to only be build when a tag is pushed. This
@@ -183,6 +188,9 @@ jobs:
   - name: Chocolatey
     if: *deploy-if
     env:
+      # File system watching is extra flaky on Dart 2.5.0 on Windows (see
+      # dart-lang/sdk#38334).
+      - DART_VERSION=2.4.1
       # CHOCO_TOKEN="..."
       - secure: "cW11kQYBBEElfVsc1pJfVEHOMYwt0ZK+9STZHwSPbAISlplIRnsimMN7TqCY2aLnkWXyUMU7DphIl9uQ86M4BT1bJopsHbapj27bFSlKWHlBSDB/xylFHywV41Yk5lMlr8DLMbsSzVahasyR34xS6HYIRlDpZ9TFiQuDQNJxQmqTZJg/FC+3nqCI7tyMKGkWc48ikTcmqDMHsG9CudG2u+Q3S9sLNXArh9T4tSnAyWkTvSrS05mvFx5tC83PcG9/VkioTId+VRSJchwTmCxDFDROrTikTXZMtYn8wMAQ2wQ34TQXNZMZ9uiHA6W0IuJV2EnYerJbqV2lrJq9xqZywKu6HW6i4GhrCvizALNFZx/N7s/10xuf3UcuWizYml/e0MYT+6t4ojTYBMKv+Cx+H2Y2Jdpvdn2ZAIl6LaU3pLw4OIPJ7aXjDwZd63MPxtwGwVLHbH7Zu+oUv1erIq5LtatuocGWipD8WdiMBQvyCuDRMowpLPoAbj+mevOf+xlY2Eym4tOXpxM7iY3lXFHROo5dQbhsARfVF9J1gl5PuYXvCjxqTfK/ef9t3ZoDbi57+yAJUWlZfWa5r1zKE8OS0pA8GfQRLom/Lt0wKVw4Xiofgolzd9pEHi4JpsYIQb8O+u1ACQU6nBCS87CGrQ+ylnzKfGUs0aW2K3gvbkg0LUg="
     script: skip
@@ -232,3 +240,26 @@ jobs:
       script: pub run grinder github-mac-os
       skip_cleanup: true
       on: {tags: true}
+
+  - name: "GitHub: Windows"
+    if: *deploy-if
+    env:
+      # We can't re-use the github-env alias here because we also need to
+      # override DART_VERSION.
+      #
+      # File system watching is extra flaky on Dart 2.5.0 on Windows (see
+      # dart-lang/sdk#38334).
+      - DART_VERSION=2.4.1
+      - GITHUB_USER=sassbot
+      # GITHUB_AUTH="..."
+      #
+      # Note that this overrides the read-only auth token that's set for all
+      # builds.
+      - secure: "AAP74aT+8SQmwGeHrCsZ7GgppvCCkDAZXszivocMy3Fi9gfMCLABBCh67pGINJX4VlLW7ftPF3xivlvgGu+e4ncXz9m9jIPZ9Iza3cW5jCnCgyRGZD98gwabIDFWiv4X9V2xnJA2p1ZuYBf8Sh3TTipUFBKMjlnxVxYkIOTud4rUss/htFhxVA/oFTo0ThTZwXuxJ+GRGTM4PcuHPJvPf18iRPs2AHFV6ZP51xgc3AsXC6Zyom5EJeX0yGj9zWQ0XCjnuFdGsI6G9jmkrmqgAXuUipgqAn0tjxPYp9R/1HqnBLD3Zbrvyi5pCiSFclU6CS6kTDbefzPOc5+zrnlkaolVeF8tQ+EhZiZqtLnpLYUz9bgknoFUapUN4N0R36sKBStdRv54+sMeoOzpQ8ep3PeZW5nWbak12wcrDx38ToWs6hQ4ycb0SQDZZatHsASpSu2nX8HwzZSDAZmsAdB+epPmgA0CBjWVG1ycmVnT6l3OopUmbaY3pXBNzFUXq5Fcd7Q39/MfrmHpyxSc3QVf8xNtUx9ggYtK0Kwx6dgykhNMVzFGZRVyQgwpaiyDqgMGEU2GQzzcJhgKo9+y1fDtdfj/cctmvJ2Fo1fkk+DMkEPUHGOVo6uKFnartky9iLm1WiHDMruJ6SIOJzAnb+TMBWQTSwI+F4wyEiRVR8Zv4uA="
+
+    script: skip
+    deploy:
+      provider: script
+      script: pub run grinder github-windows
+      skip_cleanup: true
+      on: {tags: true}

CHANGELOG.md

@@ -1,3 +1,26 @@
+## 1.22.12
+
+* **Potentially breaking bug fix:** character sequences consisting of two or
+  more hyphens followed by a number (such as `--123`), or two or more hyphens on
+  their own (such as `--`), are now parsed as identifiers [in accordance with
+  the CSS spec][ident-token-diagram].
+
+  [ident-token-diagram]: https://drafts.csswg.org/css-syntax-3/#ident-token-diagram
+
+  The sequence `--` was previously parsed as multiple applications of the `-`
+  operator. Since this is unlikely to be used intentionally in practice, we
+  consider this bug fix safe.
+
+### Command-Line Interface
+
+* Fix a bug where changes in `.css` files would be ignored in `--watch` mode.
+
+### JavaScript API
+
+* Allow underscore-separated custom functions to be defined.
+
+* Improve the performance of Node.js compilation involving many `@import`s.
+
 ## 1.22.11
 
 * Don't try to load unquoted plain-CSS indented-syntax imports.

lib/src/executable/watch.dart

@@ -122,7 +122,9 @@ class _Watcher {
   Future<void> watch(MultiDirWatcher watcher) async {
     await for (var event in _debounceEvents(watcher.events)) {
       var extension = p.extension(event.path);
-      if (extension != '.sass' && extension != '.scss') continue;
+      if (extension != '.sass' && extension != '.scss' && extension != '.css') {
+        continue;
+      }
 
       switch (event.type) {
         case ChangeType.MODIFY:

lib/src/io/node.dart

@@ -156,23 +156,39 @@ String _cleanErrorMessage(_SystemError error) {
 }
 
 bool fileExists(String path) {
-  try {
-    return _fs.statSync(path).isFile();
-  } catch (error) {
-    var systemError = error as _SystemError;
-    if (systemError.code == 'ENOENT') return false;
-    rethrow;
-  }
+  return _systemErrorToFileSystemException(() {
+    // `existsSync()` is faster than `statSync()`, but it doesn't clarify
+    // whether the entity in question is a file or a directory. Since false
+    // negatives are much more common than false positives, it works out in our
+    // favor to check this first.
+    if (!_fs.existsSync(path)) return false;
+
+    try {
+      return _fs.statSync(path).isFile();
+    } catch (error) {
+      var systemError = error as _SystemError;
+      if (systemError.code == 'ENOENT') return false;
+      rethrow;
+    }
+  });
 }
 
 bool dirExists(String path) {
-  try {
-    return _fs.statSync(path).isDirectory();
-  } catch (error) {
-    var systemError = error as _SystemError;
-    if (systemError.code == 'ENOENT') return false;
-    rethrow;
-  }
+  return _systemErrorToFileSystemException(() {
+    // `existsSync()` is faster than `statSync()`, but it doesn't clarify
+    // whether the entity in question is a file or a directory. Since false
+    // negatives are much more common than false positives, it works out in our
+    // favor to check this first.
+    if (!_fs.existsSync(path)) return false;
+
+    try {
+      return _fs.statSync(path).isDirectory();
+    } catch (error) {
+      var systemError = error as _SystemError;
+      if (systemError.code == 'ENOENT') return false;
+      rethrow;
+    }
+  });
 }
 
 void ensureDir(String path) {

lib/src/parse/parser.dart

@@ -147,12 +147,18 @@ class Parser {
   @protected
   String identifier({bool normalize = false, bool unit = false}) {
     // NOTE: this logic is largely duplicated in
-    // StylesheetParser._interpolatedIdentifier and isIdentifier in utils.dart.
-    // Most changes here should be mirrored there.
+    // StylesheetParser.interpolatedIdentifier. Most changes here should be
+    // mirrored there.
 
     var text = StringBuffer();
-    while (scanner.scanChar($dash)) {
+    if (scanner.scanChar($dash)) {
       text.writeCharCode($dash);
+
+      if (scanner.scanChar($dash)) {
+        text.writeCharCode($dash);
+        _identifierBody(text, normalize: normalize, unit: unit);
+        return text.toString();
+      }
     }
 
     var first = scanner.peekChar();
@@ -580,11 +586,7 @@ class Parser {
 
     var second = scanner.peekChar(forward + 1);
     if (second == null) return false;
-    if (isNameStart(second) || second == $backslash) return true;
-    if (second != $dash) return false;
-
-    var third = scanner.peekChar(forward + 2);
-    return third != null && isNameStart(third);
+    return isNameStart(second) || second == $backslash || second == $dash;
   }
 
   /// Returns whether the scanner is immediately before a sequence of characters

lib/src/parse/stylesheet.dart

@@ -3102,8 +3102,14 @@ relase. For details, see http://bit.ly/moz-document.
     var start = scanner.state;
     var buffer = InterpolationBuffer();
 
-    while (scanner.scanChar($dash)) {
+    if (scanner.scanChar($dash)) {
       buffer.writeCharCode($dash);
+
+      if (scanner.scanChar($dash)) {
+        buffer.writeCharCode($dash);
+        _interpolatedIdentifierBody(buffer);
+        return buffer.interpolation(scanner.spanFrom(start));
+      }
     }
 
     var first = scanner.peekChar();
@@ -3119,6 +3125,13 @@ relase. For details, see http://bit.ly/moz-document.
       scanner.error("Expected identifier.");
     }
 
+    _interpolatedIdentifierBody(buffer);
+    return buffer.interpolation(scanner.spanFrom(start));
+  }
+
+  /// Consumes a chunk of a possibly-interpolated CSS identifier after the name
+  /// start, and adds the contents to the [buffer] buffer.
+  void _interpolatedIdentifierBody(InterpolationBuffer buffer) {
     while (true) {
       var next = scanner.peekChar();
       if (next == null) {
@@ -3136,8 +3149,6 @@ relase. For details, see http://bit.ly/moz-document.
         break;
       }
     }
-
-    return buffer.interpolation(scanner.spanFrom(start));
   }
 
   /// Consumes interpolation.
@@ -3384,15 +3395,8 @@ relase. For details, see http://bit.ly/moz-document.
     if (first != $dash) return false;
     var second = scanner.peekChar(1);
     if (second == null) return false;
-    if (isNameStart(second) || second == $backslash) return true;
-
     if (second == $hash) return scanner.peekChar(2) == $lbrace;
-    if (second != $dash) return false;
-
-    var third = scanner.peekChar(2);
-    if (third == null) return false;
-    if (third == $hash) return scanner.peekChar(3) == $lbrace;
-    return isNameStart(third);
+    return isNameStart(second) || second == $backslash || second == $dash;
   }
 
   /// Returns whether the scanner is immediately before a sequence of characters

lib/src/visitor/async_evaluate.dart

@@ -442,7 +442,7 @@ class _EvaluateVisitor
 
     functions = [...?functions, ...globalFunctions, ...metaFunctions];
     for (var function in functions) {
-      _builtInFunctions[function.name] = function;
+      _builtInFunctions[function.name.replaceAll("_", "-")] = function;
     }
   }
 

lib/src/visitor/evaluate.dart

@@ -5,7 +5,7 @@
 // DO NOT EDIT. This file was generated from async_evaluate.dart.
 // See tool/grind/synchronize.dart for details.
 //
-// Checksum: 5c9f270ef574f9c6062421ed1866af3d07672b46
+// Checksum: 3fc19891432af3cebdc0f36730e57cbbf672d959
 //
 // ignore_for_file: unused_import
 
@@ -448,7 +448,7 @@ class _EvaluateVisitor
 
     functions = [...?functions, ...globalFunctions, ...metaFunctions];
     for (var function in functions) {
-      _builtInFunctions[function.name] = function;
+      _builtInFunctions[function.name.replaceAll("_", "-")] = function;
     }
   }
 

lib/src/visitor/recursive_statement.dart

@@ -23,7 +23,7 @@ import 'interface/statement.dart';
 /// The default implementation of the visit methods all return `null`.
 abstract class RecursiveStatementVisitor<T> implements StatementVisitor<T> {
   T visitAtRootRule(AtRootRule node) {
-    visitInterpolation(node.query);
+    if (node.query != null) visitInterpolation(node.query);
     return visitChildren(node);
   }
 
@@ -47,7 +47,7 @@ abstract class RecursiveStatementVisitor<T> implements StatementVisitor<T> {
 
   T visitDeclaration(Declaration node) {
     visitInterpolation(node.name);
-    visitExpression(node.value);
+    if (node.value != null) visitExpression(node.value);
     return node.children == null ? null : visitChildren(node);
   }
 

pubspec.yaml

@@ -1,5 +1,5 @@
 name: sass
-version: 1.22.11
+version: 1.22.12
 description: A Sass implementation in Dart.
 author: Sass Team
 homepage: https://github.com/sass/dart-sass

test/cli/dart/watch_test.dart

@@ -5,6 +5,10 @@
 // OS X's modification time reporting is flaky, so we skip these tests on it.
 @TestOn('vm && !mac-os')
 
+// File watching is inherently flaky at the OS level. To mitigate this, we do a
+// few retries when the tests fail.
+@Retry(3)
+
 import 'package:test/test.dart';
 
 import '../dart_test.dart';

test/cli/node/watch_test.dart

@@ -6,6 +6,10 @@
 @TestOn('vm && !mac-os')
 @Tags(['node'])
 
+// File watching is inherently flaky at the OS level. To mitigate this, we do a
+// few retries when the tests fail.
+@Retry(3)
+
 import 'package:test/test.dart';
 
 import '../../ensure_npm_package.dart';

test/cli/shared/watch.dart

@@ -499,6 +499,26 @@ void sharedTests(Future<TestProcess> runSass(Iterable<String> arguments)) {
             ]).validate();
           });
         });
+
+        // Regression test for #806
+        test("with a .css extension", () async {
+          await d.file("test.css", "a {b: c}").create();
+
+          var sass = await watch(["test.css:out.css"]);
+          await expectLater(
+              sass.stdout, emits('Compiled test.css to out.css.'));
+          await expectLater(sass.stdout, _watchingForChanges);
+          await tickIfPoll();
+
+          await d.file("test.css", "x {y: z}").create();
+          await expectLater(
+              sass.stdout, emits('Compiled test.css to out.css.'));
+          await sass.kill();
+
+          await d
+              .file("out.css", equalsIgnoringWhitespace("x { y: z; }"))
+              .validate();
+        });
       });
 
       group("doesn't recompile the watched file", () {

test/dart_api/function_test.dart

@@ -130,4 +130,28 @@ main() {
 
     expect(css, equalsIgnoringWhitespace("a { b: 1; }"));
   });
+
+  group("are dash-normalized", () {
+    test("when defined with dashes", () {
+      expect(
+          compileString('a {b: foo_bar()}', functions: [
+            Callable("foo-bar", "", expectAsync1((arguments) {
+              expect(arguments, isEmpty);
+              return sassNull;
+            }))
+          ]),
+          isEmpty);
+    });
+
+    test("when defined with underscores", () {
+      expect(
+          compileString('a {b: foo-bar()}', functions: [
+            Callable("foo_bar", "", expectAsync1((arguments) {
+              expect(arguments, isEmpty);
+              return sassNull;
+            }))
+          ]),
+          isEmpty);
+    });
+  });
 }