Do variable index adjustments when accessing variables, not nodes

Doing this for variable *nodes* was breaking in extremely specific
circumstances:

* A null node is passed into setVariable() and, if source maps are
  enabled, recorded in _variableNodes.

* Later on, getVariableNode() is called for that variable. Because
  that variable's node is null, it calls
  _getVariableNodeFromGlobalModule() even though the variable is
  actually defined.

* _getVariableNodeFromGlobalModule() sets _lastVariableIndex to 0 on
  the assumption that the variable is undefined, which turns out to be
  incorrect in this specific case.

The next commit will fix the issue where a null node can be passed
into setVariable() when source maps are enabled, but it makes more
sense to assign the variable index to 0 in the variable access anyway,
since it happens first and isn't skipped when source maps are
disabled.

Author: nex3

lib/src/async_environment.dart

@@ -419,7 +419,12 @@ class AsyncEnvironment {
     }
 
     index = _variableIndex(name);
-    if (index == null) return _getVariableFromGlobalModule(name);
+    if (index == null) {
+      // There isn't a real variable defined as this index, but it will cause
+      // [getVariable] to short-circuit and get to this function faster next
+      // time the variable is accessed.
+      return _getVariableFromGlobalModule(name);
+    }
 
     _lastVariableName = name;
     _lastVariableIndex = index;
@@ -476,12 +481,6 @@ class AsyncEnvironment {
   /// required, since some nodes need to do real work to manufacture a source
   /// span.
   AstNode _getVariableNodeFromGlobalModule(String name) {
-    // There isn't a real variable defined as this index, but it will cause
-    // [getVariable] to short-circuit and get to this function faster next time
-    // the variable is accessed.
-    _lastVariableName = name;
-    _lastVariableIndex = 0;
-
     if (_globalModules == null) return null;
 
     // We don't need to worry about multiple modules defining the same variable,

lib/src/environment.dart

@@ -5,7 +5,7 @@
 // DO NOT EDIT. This file was generated from async_environment.dart.
 // See tool/grind/synchronize.dart for details.
 //
-// Checksum: f02d694ae04ee7fb2219d7930049480eda1fd734
+// Checksum: 76e7c2e929fc7d19390987069ff3a8c4b3fadfc5
 //
 // ignore_for_file: unused_import
 
@@ -426,7 +426,12 @@ class Environment {
     }
 
     index = _variableIndex(name);
-    if (index == null) return _getVariableFromGlobalModule(name);
+    if (index == null) {
+      // There isn't a real variable defined as this index, but it will cause
+      // [getVariable] to short-circuit and get to this function faster next
+      // time the variable is accessed.
+      return _getVariableFromGlobalModule(name);
+    }
 
     _lastVariableName = name;
     _lastVariableIndex = index;
@@ -483,12 +488,6 @@ class Environment {
   /// required, since some nodes need to do real work to manufacture a source
   /// span.
   AstNode _getVariableNodeFromGlobalModule(String name) {
-    // There isn't a real variable defined as this index, but it will cause
-    // [getVariable] to short-circuit and get to this function faster next time
-    // the variable is accessed.
-    _lastVariableName = name;
-    _lastVariableIndex = 0;
-
     if (_globalModules == null) return null;
 
     // We don't need to worry about multiple modules defining the same variable,

test/cli/shared.dart

@@ -538,4 +538,33 @@ void sharedTests(
       });
     });
   });
+
+  test("doesn't unassign variables", () async {
+    // This is a regression test for one of the strangest errors I've ever
+    // encountered. Every bit of what's going on was necessary to reproduce it,
+    // *including* running with source maps enabled, which is why it's here and
+    // not in sass-spec.
+    await d.file("input.scss", "a {@import 'downstream'}").create();
+    await d.file("_downstream.scss", r"""
+      @import 'midstream';
+
+      $b: $c;
+      @mixin d($_) {}
+      @include d($b);
+      e {f: $b}
+    """).create();
+    await d.file("_midstream.scss", "@forward 'upstream'").create();
+    await d.file("_upstream.scss", r"$c: g").create();
+
+    var sass = await runSass(["input.scss", "output.css"]);
+    await sass.shouldExit(0);
+
+    await d.file("output.css", equalsIgnoringWhitespace("""
+      a e {
+        f: g;
+      }
+
+      /*# sourceMappingURL=output.css.map */
+    """)).validate();
+  });
 }