Add support for SASS_PATH (#514)

Closes #512

CHANGELOG.md

@@ -3,6 +3,10 @@
 * Add support for interpolation in at-rule names. See [the
   proposal][at-rule-interpolation] for details.
 
+* Add paths from the `SASS_PATH` environment variable to the load paths in the
+  command-line interface, Dart API, and JS API. These load paths are checked
+  just after the load paths explicitly passed by the user.
+
 [at-rule-interpolation]: https://github.com/sass/language/blob/master/accepted/at-rule-interpolation.md
 
 ### JavaScript API

lib/sass.dart

@@ -44,6 +44,9 @@ export 'src/visitor/serialize.dart' show OutputStyle;
 /// * Each load path in [loadPaths]. Note that this is a shorthand for adding
 ///   [FilesystemImporter]s to [importers].
 ///
+/// * Each load path specified in the `SASS_PATH` environment variable, which
+///   should be semicolon-separated on Windows and colon-separated elsewhere.
+///
 /// * `package:` resolution using [packageResolver], which is a
 ///   [`SyncPackageResolver`][] from the `package_resolver` package. Note that
 ///   this is a shorthand for adding a [PackageImporter] to [importers].
@@ -114,6 +117,9 @@ String compile(String path,
 /// * Each load path in [loadPaths]. Note that this is a shorthand for adding
 ///   [FilesystemImporter]s to [importers].
 ///
+/// * Each load path specified in the `SASS_PATH` environment variable, which
+///   should be semicolon-separated on Windows and colon-separated elsewhere.
+///
 /// * `package:` resolution using [packageResolver], which is a
 ///   [`SyncPackageResolver`][] from the `package_resolver` package. Note that
 ///   this is a shorthand for adding a [PackageImporter] to [importers].

lib/src/async_import_cache.dart

@@ -10,6 +10,7 @@ import 'package:tuple/tuple.dart';
 
 import 'ast/sass.dart';
 import 'importer.dart';
+import 'io.dart';
 import 'logger.dart';
 import 'sync_package_resolver.dart';
 import 'utils.dart'; // ignore: unused_import
@@ -45,6 +46,9 @@ class AsyncImportCache {
   /// * Each load path in [loadPaths]. Note that this is a shorthand for adding
   ///   [FilesystemImporter]s to [importers].
   ///
+  /// * Each load path specified in the `SASS_PATH` environment variable, which
+  ///   should be semicolon-separated on Windows and colon-separated elsewhere.
+  ///
   /// * `package:` resolution using [packageResolver], which is a
   ///   [`SyncPackageResolver`][] from the `package_resolver` package. Note that
   ///   this is a shorthand for adding a [PackageImporter] to [importers].
@@ -64,12 +68,22 @@ class AsyncImportCache {
   static List<AsyncImporter> _toImporters(Iterable<AsyncImporter> importers,
       Iterable<String> loadPaths, SyncPackageResolver packageResolver) {
     var list = importers?.toList() ?? [];
+
     if (loadPaths != null) {
       list.addAll(loadPaths.map((path) => new FilesystemImporter(path)));
     }
+
+    var sassPath = getEnvironmentVariable('SASS_PATH');
+    if (sassPath != null) {
+      list.addAll(sassPath
+          .split(isWindows ? ';' : ':')
+          .map((path) => new FilesystemImporter(path)));
+    }
+
     if (packageResolver != null) {
       list.add(new PackageImporter(packageResolver));
     }
+
     return list;
   }
 

lib/src/import_cache.dart

@@ -5,7 +5,7 @@
 // DO NOT EDIT. This file was generated from async_import_cache.dart.
 // See tool/synchronize.dart for details.
 //
-// Checksum: 57c42546fb8e0b68e29ea841ba106ee99127bede
+// Checksum: 3fd08a2b9ad7226a0e3b34057eede0595b6d5aa4
 
 import 'package:collection/collection.dart';
 import 'package:path/path.dart' as p;
@@ -13,6 +13,7 @@ import 'package:tuple/tuple.dart';
 
 import 'ast/sass.dart';
 import 'importer.dart';
+import 'io.dart';
 import 'logger.dart';
 import 'sync_package_resolver.dart';
 import 'utils.dart'; // ignore: unused_import
@@ -48,6 +49,9 @@ class ImportCache {
   /// * Each load path in [loadPaths]. Note that this is a shorthand for adding
   ///   [FilesystemImporter]s to [importers].
   ///
+  /// * Each load path specified in the `SASS_PATH` environment variable, which
+  ///   should be semicolon-separated on Windows and colon-separated elsewhere.
+  ///
   /// * `package:` resolution using [packageResolver], which is a
   ///   [`SyncPackageResolver`][] from the `package_resolver` package. Note that
   ///   this is a shorthand for adding a [PackageImporter] to [importers].
@@ -67,12 +71,22 @@ class ImportCache {
   static List<Importer> _toImporters(Iterable<Importer> importers,
       Iterable<String> loadPaths, SyncPackageResolver packageResolver) {
     var list = importers?.toList() ?? [];
+
     if (loadPaths != null) {
       list.addAll(loadPaths.map((path) => new FilesystemImporter(path)));
     }
+
+    var sassPath = getEnvironmentVariable('SASS_PATH');
+    if (sassPath != null) {
+      list.addAll(sassPath
+          .split(isWindows ? ';' : ':')
+          .map((path) => new FilesystemImporter(path)));
+    }
+
     if (packageResolver != null) {
       list.add(new PackageImporter(packageResolver));
     }
+
     return list;
   }
 

lib/src/importer/node/implementation.dart

@@ -36,6 +36,7 @@ import '../utils.dart';
 ///   1. Filesystem imports relative to the base file.
 ///   2. Filesystem imports relative to the working directory.
 ///   3. Filesystem imports relative to an `includePaths` path.
+///   3. Filesystem imports relative to a `SASS_PATH` path.
 ///   4. Custom importer imports.
 class NodeImporter {
   /// The `this` context in which importer functions are invoked.
@@ -48,9 +49,18 @@ class NodeImporter {
   final List<JSFunction> _importers;
 
   NodeImporter(this._context, Iterable<String> includePaths, Iterable importers)
-      : _includePaths = new List.unmodifiable(includePaths),
+      : _includePaths = new List.unmodifiable(_addSassPath(includePaths)),
         _importers = new List.unmodifiable(importers);
 
+  /// Returns [includePaths] followed by any paths loaded from the `SASS_PATH`
+  /// environment variable.
+  static Iterable<String> _addSassPath(Iterable<String> includePaths) sync* {
+    yield* includePaths;
+    var sassPath = getEnvironmentVariable("SASS_PATH");
+    if (sassPath == null) return;
+    yield* sassPath.split(isWindows ? ';' : ':');
+  }
+
   /// Loads the stylesheet at [url].
   ///
   /// The [previous] URL is the URL of the stylesheet in which the import

lib/src/io/interface.dart

@@ -83,6 +83,10 @@ Iterable<String> listDir(String path) => null;
 /// Returns the modification time of the file at [path].
 DateTime modificationTime(String path) => null;
 
+/// Returns the value of the environment variable with the given [name], or
+/// `null` if it's not set.
+String getEnvironmentVariable(String name) => null;
+
 /// Gets and sets the exit code that the process will use when it exits.
 int exitCode;
 

lib/src/io/node.dart

@@ -4,6 +4,7 @@
 
 import 'dart:async';
 import 'dart:convert';
+import 'dart:js_util';
 
 import 'package:js/js.dart';
 import 'package:path/path.dart' as p;
@@ -59,6 +60,7 @@ class _SystemError {
 @JS()
 class _Process {
   external String get platform;
+  external Object get env;
   external String cwd();
 }
 
@@ -204,6 +206,9 @@ DateTime modificationTime(String path) => _systemErrorToFileSystemException(
     () => new DateTime.fromMillisecondsSinceEpoch(
         _fs.statSync(path).mtime.getTime()));
 
+String getEnvironmentVariable(String name) =>
+    getProperty(_process.env, name) as String;
+
 /// Runs callback and converts any [_SystemError]s it throws into
 /// [FileSystemException]s.
 T _systemErrorToFileSystemException<T>(T callback()) {

lib/src/io/vm.dart

@@ -83,6 +83,9 @@ DateTime modificationTime(String path) {
   return stat.modified;
 }
 
+String getEnvironmentVariable(String name) =>
+    io.Platform.environment['SASS_PATH'];
+
 Future<Stream<WatchEvent>> watchDir(String path, {bool poll: false}) async {
   var watcher =
       poll ? new PollingDirectoryWatcher(path) : new DirectoryWatcher(path);

test/cli/dart_test.dart

@@ -45,7 +45,8 @@ void ensureSnapshotUpToDate() {
   }
 }
 
-Future<TestProcess> runSass(Iterable<String> arguments) {
+Future<TestProcess> runSass(Iterable<String> arguments,
+    {Map<String, String> environment}) {
   var executable = _snapshotPaths.firstWhere(
       (path) => new File(path).existsSync(),
       orElse: () => p.absolute("bin/sass.dart"));
@@ -61,5 +62,6 @@ Future<TestProcess> runSass(Iterable<String> arguments) {
         ..add(executable)
         ..addAll(arguments),
       workingDirectory: d.sandbox,
+      environment: environment,
       description: "sass");
 }

test/cli/node_test.dart

@@ -30,6 +30,10 @@ void main() {
   });
 }
 
-Future<TestProcess> runSass(Iterable<String> arguments) => TestProcess.start(
-    "node", [p.absolute("build/npm/sass.js")]..addAll(arguments),
-    workingDirectory: d.sandbox, description: "sass");
+Future<TestProcess> runSass(Iterable<String> arguments,
+        {Map<String, String> environment}) =>
+    TestProcess.start(
+        "node", [p.absolute("build/npm/sass.js")]..addAll(arguments),
+        workingDirectory: d.sandbox,
+        environment: environment,
+        description: "sass");

test/cli/shared.dart

@@ -3,6 +3,7 @@
 // https://opensource.org/licenses/MIT.
 
 import 'dart:async';
+import 'dart:io';
 
 import 'package:path/path.dart' as p;
 import 'package:test/test.dart';
@@ -10,12 +11,16 @@ import 'package:test_descriptor/test_descriptor.dart' as d;
 import 'package:test_process/test_process.dart';
 
 /// Defines test that are shared between the Dart and Node.js CLI test suites.
-void sharedTests(Future<TestProcess> runSass(Iterable<String> arguments)) {
+void sharedTests(
+    Future<TestProcess> runSass(Iterable<String> arguments,
+        {Map<String, String> environment})) {
   /// Runs the executable on [arguments] plus an output file, then verifies that
   /// the contents of the output file match [expected].
-  Future expectCompiles(List<String> arguments, expected) async {
+  Future expectCompiles(List<String> arguments, expected,
+      {Map<String, String> environment}) async {
     var sass = await runSass(
-        arguments.toList()..add("out.css")..add("--no-source-map"));
+        arguments.toList()..add("out.css")..add("--no-source-map"),
+        environment: environment);
     await sass.shouldExit(0);
     await d.file("out.css", expected).validate();
   }
@@ -123,6 +128,21 @@ void sharedTests(Future<TestProcess> runSass(Iterable<String> arguments)) {
           equalsIgnoringWhitespace("a { b: c; }"));
     });
 
+    test("from SASS_PATH", () async {
+      await d.file("test.scss", """
+        @import 'test2';
+        @import 'test3';
+      """).create();
+
+      await d.dir("dir2", [d.file("test2.scss", "a {b: c}")]).create();
+      await d.dir("dir3", [d.file("test3.scss", "x {y: z}")]).create();
+
+      var separator = Platform.isWindows ? ';' : ':';
+      await expectCompiles(
+          ["test.scss"], equalsIgnoringWhitespace("a { b: c; } x { y: z; }"),
+          environment: {"SASS_PATH": "dir2${separator}dir3"});
+    });
+
     // Regression test for #369
     test("from within a directory, relative to a file on the load path",
         () async {
@@ -161,6 +181,29 @@ void sharedTests(Future<TestProcess> runSass(Iterable<String> arguments)) {
           equalsIgnoringWhitespace("x { y: z; }"));
     });
 
+    test("from the load path in preference to from SASS_PATH", () async {
+      await d.file("test.scss", "@import 'test2'").create();
+
+      await d.dir("dir1", [d.file("test2.scss", "a {b: c}")]).create();
+      await d.dir("dir2", [d.file("test2.scss", "x {y: z}")]).create();
+
+      await expectCompiles(["--load-path", "dir2", "test.scss"],
+          equalsIgnoringWhitespace("x { y: z; }"),
+          environment: {"SASS_PATH": "dir1"});
+    });
+
+    test("in SASS_PATH order", () async {
+      await d.file("test.scss", "@import 'test2'").create();
+
+      await d.dir("dir1", [d.file("test2.scss", "a {b: c}")]).create();
+      await d.dir("dir2", [d.file("test2.scss", "x {y: z}")]).create();
+
+      var separator = Platform.isWindows ? ';' : ':';
+      await expectCompiles(
+          ["test.scss"], equalsIgnoringWhitespace("x { y: z; }"),
+          environment: {"SASS_PATH": "dir2${separator}dir3"});
+    });
+
     // Regression test for an internal Google issue.
     test("multiple times from different load paths", () async {
       await d.file("test.scss", """

test/dart_api_test.dart

@@ -13,6 +13,8 @@ import 'package:sass/sass.dart';
 import 'package:sass/src/exception.dart';
 
 main() {
+  // TODO(nweiz): test SASS_PATH when dart-lang/sdk#28160 is fixed.
+
   group("importers", () {
     test("is used to resolve imports", () async {
       await d.dir("subdir", [d.file("subtest.scss", "a {b: c}")]).create();

test/node_api/utils.dart

@@ -15,6 +15,9 @@ import 'package:sass/src/node/function.dart';
 import '../hybrid.dart';
 import 'api.dart';
 
+@JS('process.env')
+external Object get _environment;
+
 String sandbox;
 
 void useSandbox() {
@@ -97,3 +100,8 @@ void runTestInSandbox() {
   chdir(sandbox);
   addTearDown(() => chdir(oldWorkingDirectory));
 }
+
+/// Sets the environment variable [name] to [value] within this process.
+void setEnvironmentVariable(String name, String value) {
+  setProperty(_environment, name, value);
+}

test/node_api_test.dart

@@ -12,6 +12,7 @@ import 'package:path/path.dart' as p;
 import 'package:test/test.dart';
 
 import 'package:sass/src/node/utils.dart';
+import 'package:sass/src/io.dart';
 
 import 'ensure_npm_package.dart';
 import 'hybrid.dart';
@@ -95,6 +96,45 @@ void main() {
           equalsIgnoringWhitespace('a { b: c; }'));
     });
 
+    test("supports SASS_PATH", () async {
+      await createDirectory(p.join(sandbox, 'dir1'));
+      await createDirectory(p.join(sandbox, 'dir2'));
+      await writeTextFile(p.join(sandbox, 'dir1', 'test1.scss'), 'a {b: c}');
+      await writeTextFile(p.join(sandbox, 'dir2', 'test2.scss'), 'x {y: z}');
+
+      var separator = isWindows ? ';' : ':';
+      setEnvironmentVariable("SASS_PATH",
+          p.join(sandbox, 'dir1') + separator + p.join(sandbox, 'dir2'));
+
+      try {
+        expect(renderSync(new RenderOptions(data: """
+              @import 'test1';
+              @import 'test2';
+            """)), equalsIgnoringWhitespace('a { b: c; } x { y: z; }'));
+      } finally {
+        setEnvironmentVariable("SASS_PATH", null);
+      }
+    });
+
+    test("load path takes precedence over SASS_PATH", () async {
+      await createDirectory(p.join(sandbox, 'dir1'));
+      await createDirectory(p.join(sandbox, 'dir2'));
+      await writeTextFile(p.join(sandbox, 'dir1', 'test.scss'), 'a {b: c}');
+      await writeTextFile(p.join(sandbox, 'dir2', 'test.scss'), 'x {y: z}');
+
+      setEnvironmentVariable("SASS_PATH", p.join(sandbox, 'dir1'));
+
+      try {
+        expect(
+            renderSync(new RenderOptions(
+                data: "@import 'test'",
+                includePaths: [p.join(sandbox, 'dir2')])),
+            equalsIgnoringWhitespace('x { y: z; }'));
+      } finally {
+        setEnvironmentVariable("SASS_PATH", null);
+      }
+    });
+
     // Regression test for #314
     test(
         "a file imported through a relative load path supports relative "