Factor out a function to load modules (#693)

This will allow us to re-use logic for @forward. It also fixes some
usability issues where incorrect or duplicated spans were being used
for @use errors.

lib/src/visitor/async_evaluate.dart

@@ -175,7 +175,11 @@ class _EvaluateVisitor
   /// imports, it contains the URL passed to the `@import`.
   final _includedFiles = Set<String>();
 
-  final _activeImports = Set<Uri>();
+  /// The set of canonical URLs for modules (or imported files) that are
+  /// currently being evaluated.
+  ///
+  /// This is used to ensure that we don't get into an infinite load loop.
+  final _activeModules = Set<Uri>();
 
   /// The dynamic call stack representing function invocations, mixin
   /// invocations, and imports surrounding the current context.
@@ -331,6 +335,7 @@ class _EvaluateVisitor
   Future<EvaluateResult> run(AsyncImporter importer, Stylesheet node) async {
     var url = node.span?.sourceUrl;
     if (url != null) {
+      _activeModules.add(url);
       if (_asNodeSass) {
         if (url.scheme == 'file') {
           _includedFiles.add(p.fromUri(url));
@@ -356,6 +361,33 @@ class _EvaluateVisitor
     return expression.accept(this);
   }
 
+  /// Loads the module at [url] and passes it to [callback].
+  ///
+  /// The [stackFrame] and [nodeForSpan] are used for the name and location of
+  /// the stack frame in which the new module is executed.
+  Future<void> _loadModule(Uri url, String stackFrame, AstNode nodeForSpan,
+      void callback(Module module)) async {
+    var result = await inUseRuleAsync(
+        () => _loadStylesheet(url.toString(), nodeForSpan.span));
+    var importer = result.item1;
+    var stylesheet = result.item2;
+
+    var canonicalUrl = stylesheet.span.sourceUrl;
+    if (_activeModules.contains(canonicalUrl)) {
+      throw _exception(
+          "This module is currently being loaded.", nodeForSpan.span);
+    }
+    _activeModules.add(canonicalUrl);
+
+    var module = await _withStackFrame(
+        stackFrame, nodeForSpan, () => _execute(importer, stylesheet));
+    try {
+      return _addExceptionSpan(nodeForSpan, () => callback(module));
+    } finally {
+      _activeModules.remove(canonicalUrl);
+    }
+  }
+
   /// Executes [stylesheet], loaded by [importer], to produce a module.
   Future<Module> _execute(AsyncImporter importer, Stylesheet stylesheet) {
     var url = stylesheet.span.sourceUrl;
@@ -363,7 +395,6 @@ class _EvaluateVisitor
       var environment = AsyncEnvironment(sourceMap: _sourceMap);
       CssStylesheet css;
       var extender = Extender();
-      _activeImports.add(url);
       await _withEnvironment(environment, () async {
         var oldImporter = _importer;
         var oldStylesheet = _stylesheet;
@@ -411,7 +442,6 @@ class _EvaluateVisitor
         _atRootExcludingStyleRule = oldAtRootExcludingStyleRule;
         _inKeyframes = oldInKeyframes;
       });
-      _activeImports.remove(url);
 
       return environment.toModule(css, extender);
     });
@@ -985,11 +1015,11 @@ class _EvaluateVisitor
     var stylesheet = result.item2;
 
     var url = stylesheet.span.sourceUrl;
-    if (!_activeImports.add(url)) {
+    if (!_activeModules.add(url)) {
       throw _exception("This file is already being loaded.", import.span);
     }
 
-    _activeImports.add(url);
+    _activeModules.add(url);
 
     // TODO(nweiz): If [stylesheet] contains no `@use` rules, just evaluate it
     // directly in [_root] rather than making a new stylesheet.
@@ -1041,7 +1071,7 @@ class _EvaluateVisitor
       child.accept(visitor);
     }
 
-    _activeImports.remove(url);
+    _activeModules.remove(url);
   }
 
   /// Loads the [Stylesheet] identified by [url], or throws a
@@ -1427,21 +1457,8 @@ class _EvaluateVisitor
   }
 
   Future<Value> visitUseRule(UseRule node) async {
-    var result = await inUseRuleAsync(
+    await _loadModule(node.url, "@use", node, (module) {
-        () => _loadStylesheet(node.url.toString(), node.span));
+      _environment.addModule(module, namespace: node.namespace);
-    var importer = result.item1;
-    var stylesheet = result.item2;
-
-    var url = stylesheet.span.sourceUrl;
-    if (_activeImports.contains(url)) {
-      throw _exception("This module is currently being loaded.", node.span);
-    }
-
-    await _withStackFrame("@use", stylesheet, () {
-      return _addExceptionSpanAsync(node, () async {
-        _environment.addModule(await _execute(importer, stylesheet),
-            namespace: node.namespace);
-      });
     });
 
     return null;

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: 2bd6f8c74ca546e481cce450f4f982344ab8a611
+// Checksum: 36af5d91812c44a7f77ef0b7f580fa9fb8520ad0
 //
 // ignore_for_file: unused_import
 
@@ -184,7 +184,11 @@ class _EvaluateVisitor
   /// imports, it contains the URL passed to the `@import`.
   final _includedFiles = Set<String>();
 
-  final _activeImports = Set<Uri>();
+  /// The set of canonical URLs for modules (or imported files) that are
+  /// currently being evaluated.
+  ///
+  /// This is used to ensure that we don't get into an infinite load loop.
+  final _activeModules = Set<Uri>();
 
   /// The dynamic call stack representing function invocations, mixin
   /// invocations, and imports surrounding the current context.
@@ -339,6 +343,7 @@ class _EvaluateVisitor
   EvaluateResult run(Importer importer, Stylesheet node) {
     var url = node.span?.sourceUrl;
     if (url != null) {
+      _activeModules.add(url);
       if (_asNodeSass) {
         if (url.scheme == 'file') {
           _includedFiles.add(p.fromUri(url));
@@ -363,6 +368,33 @@ class _EvaluateVisitor
     return expression.accept(this);
   }
 
+  /// Loads the module at [url] and passes it to [callback].
+  ///
+  /// The [stackFrame] and [nodeForSpan] are used for the name and location of
+  /// the stack frame in which the new module is executed.
+  void _loadModule(Uri url, String stackFrame, AstNode nodeForSpan,
+      void callback(Module<Callable> module)) {
+    var result =
+        inUseRule(() => _loadStylesheet(url.toString(), nodeForSpan.span));
+    var importer = result.item1;
+    var stylesheet = result.item2;
+
+    var canonicalUrl = stylesheet.span.sourceUrl;
+    if (_activeModules.contains(canonicalUrl)) {
+      throw _exception(
+          "This module is currently being loaded.", nodeForSpan.span);
+    }
+    _activeModules.add(canonicalUrl);
+
+    var module = _withStackFrame(
+        stackFrame, nodeForSpan, () => _execute(importer, stylesheet));
+    try {
+      return _addExceptionSpan(nodeForSpan, () => callback(module));
+    } finally {
+      _activeModules.remove(canonicalUrl);
+    }
+  }
+
   /// Executes [stylesheet], loaded by [importer], to produce a module.
   Module<Callable> _execute(Importer importer, Stylesheet stylesheet) {
     var url = stylesheet.span.sourceUrl;
@@ -370,7 +402,6 @@ class _EvaluateVisitor
       var environment = Environment(sourceMap: _sourceMap);
       CssStylesheet css;
       var extender = Extender();
-      _activeImports.add(url);
       _withEnvironment(environment, () {
         var oldImporter = _importer;
         var oldStylesheet = _stylesheet;
@@ -418,7 +449,6 @@ class _EvaluateVisitor
         _atRootExcludingStyleRule = oldAtRootExcludingStyleRule;
         _inKeyframes = oldInKeyframes;
       });
-      _activeImports.remove(url);
 
       return environment.toModule(css, extender);
     });
@@ -987,11 +1017,11 @@ class _EvaluateVisitor
     var stylesheet = result.item2;
 
     var url = stylesheet.span.sourceUrl;
-    if (!_activeImports.add(url)) {
+    if (!_activeModules.add(url)) {
       throw _exception("This file is already being loaded.", import.span);
     }
 
-    _activeImports.add(url);
+    _activeModules.add(url);
 
     // TODO(nweiz): If [stylesheet] contains no `@use` rules, just evaluate it
     // directly in [_root] rather than making a new stylesheet.
@@ -1043,7 +1073,7 @@ class _EvaluateVisitor
       child.accept(visitor);
     }
 
-    _activeImports.remove(url);
+    _activeModules.remove(url);
   }
 
   /// Loads the [Stylesheet] identified by [url], or throws a
@@ -1422,21 +1452,8 @@ class _EvaluateVisitor
   }
 
   Value visitUseRule(UseRule node) {
-    var result =
+    _loadModule(node.url, "@use", node, (module) {
-        inUseRule(() => _loadStylesheet(node.url.toString(), node.span));
+      _environment.addModule(module, namespace: node.namespace);
-    var importer = result.item1;
-    var stylesheet = result.item2;
-
-    var url = stylesheet.span.sourceUrl;
-    if (_activeImports.contains(url)) {
-      throw _exception("This module is currently being loaded.", node.span);
-    }
-
-    _withStackFrame("@use", stylesheet, () {
-      return _addExceptionSpan(node, () {
-        _environment.addModule(_execute(importer, stylesheet),
-            namespace: node.namespace);
-      });
     });
 
     return null;