Skip to content

Commit

Permalink
Fix the leftover-configuration check for @forward ... with (#1472)
Browse files Browse the repository at this point in the history
This check was previously checking whether *any* variables were left
in this configuration, which could include variables that were adopted
from outer configurations. This threw invalid errors when that outer
configuration would have been satisfied by another variable (or
forward) later in the file.

Closes sass/sass#1460
  • Loading branch information
nex3 authored Sep 9, 2021
1 parent 1672178 commit 33dab9f
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 16 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## 1.39.2

* Fix a bug where configuring with `@use ... with` would throw an error when
that variable was defined in a module that also contained `@forward ... with`.

## 1.39.1

* Partial fix for a bug where `@at-root` does not work properly in nested
Expand Down
2 changes: 2 additions & 0 deletions lib/src/configured_value.dart
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,6 @@ class ConfiguredValue {
/// variable prior to an `@import` of a file that contains a `@forward`.
ConfiguredValue.implicit(this.value, this.assignmentNode)
: configurationSpan = null;

String toString() => value.toString();
}
21 changes: 15 additions & 6 deletions lib/src/visitor/async_evaluate.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1326,12 +1326,21 @@ class _EvaluateVisitor
}, configuration: newConfiguration);

_removeUsedConfiguration(adjustedConfiguration, newConfiguration,
except: node.configuration.isEmpty
? const {}
: {
for (var variable in node.configuration)
if (!variable.isGuarded) variable.name
});
except: {
for (var variable in node.configuration)
if (!variable.isGuarded) variable.name
});

// Remove all the variables that weren't configured by this particular
// `@forward` before checking that the configuration is empty. Errors for
// outer `with` clauses will be thrown once those clauses finish
// executing.
var configuredVariables = {
for (var variable in node.configuration) variable.name
};
for (var name in newConfiguration.values.keys.toList()) {
if (!configuredVariables.contains(name)) newConfiguration.remove(name);
}

_assertConfigurationIsEmpty(newConfiguration);
} else {
Expand Down
23 changes: 16 additions & 7 deletions lib/src/visitor/evaluate.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
// DO NOT EDIT. This file was generated from async_evaluate.dart.
// See tool/grind/synchronize.dart for details.
//
// Checksum: 99e7cdfc43a20e2c0827753081db6dcbd914ec75
// Checksum: b73635829d711b9344e10836e865afe70ca55e77
//
// ignore_for_file: unused_import

Expand Down Expand Up @@ -1327,12 +1327,21 @@ class _EvaluateVisitor
}, configuration: newConfiguration);

_removeUsedConfiguration(adjustedConfiguration, newConfiguration,
except: node.configuration.isEmpty
? const {}
: {
for (var variable in node.configuration)
if (!variable.isGuarded) variable.name
});
except: {
for (var variable in node.configuration)
if (!variable.isGuarded) variable.name
});

// Remove all the variables that weren't configured by this particular
// `@forward` before checking that the configuration is empty. Errors for
// outer `with` clauses will be thrown once those clauses finish
// executing.
var configuredVariables = {
for (var variable in node.configuration) variable.name
};
for (var name in newConfiguration.values.keys.toList()) {
if (!configuredVariables.contains(name)) newConfiguration.remove(name);
}

_assertConfigurationIsEmpty(newConfiguration);
} else {
Expand Down
4 changes: 4 additions & 0 deletions pkg/sass_api/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 1.0.0-beta.8

* No user-visible changes.

## 1.0.0-beta.7

* No user-visible changes.
Expand Down
4 changes: 2 additions & 2 deletions pkg/sass_api/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@ name: sass_api
# Note: Every time we add a new Sass AST node, we need to bump the *major*
# version because it's a breaking change for anyone who's implementing the
# visitor interface(s).
version: 1.0.0-beta.7
version: 1.0.0-beta.8
description: Additional APIs for Dart Sass.
homepage: https://github.com/sass/dart-sass

environment:
sdk: '>=2.12.0 <3.0.0'

dependencies:
sass: 1.39.1
sass: 1.39.2

dependency_overrides:
sass: {path: ../..}
2 changes: 1 addition & 1 deletion pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: sass
version: 1.39.1
version: 1.39.2
description: A Sass implementation in Dart.
homepage: https://github.com/sass/dart-sass

Expand Down

0 comments on commit 33dab9f

Please sign in to comment.