Skip to content

Commit

Permalink
Merge pull request #1306 from sass/slash-separator
Browse files Browse the repository at this point in the history 
 Deprecate /-as-division and add replacements
  • Loading branch information
@nex3
nex3 committed May 18, 2021
2 parents 9caa0f3 + 3a5d28f commit 3849165
Show file tree
Hide file tree
Showing 19 changed files with 557 additions and 331 deletions.
15 changes: 15 additions & 0 deletions CHANGELOG.md
View file
@@ -1,5 +1,20 @@
## 1.33.0

* Deprecate the use of `/` for division. The new `math.div()` function should be
used instead. See [this page][] for details.

[this page]: https://sass-lang.com/documentation/breaking-changes/slash-div

* Add a `list.slash()` function that returns a slash-separated list.

* **Potentially breaking bug fix:** The heuristics around when potentially
slash-separated numbers are converted to slash-free numbers—for example, when
`1/2` will be printed as `0.5` rather than `1/2`—have been slightly expanded.
Previously, a number would be made slash-free if it was passed as an argument
to a *user-defined function*, but not to a *built-in function*. Now it will be
made slash-free in both cases. This is a behavioral change, but it's unlikely
to affect any real-world stylesheets.

### JS API

* The `this` context for importers now has a `fromImport` field, which is `true`
Expand Down
69 changes: 30 additions & 39 deletions lib/src/async_environment.dart
View file
Expand Up @@ -25,6 +25,11 @@ import 'utils.dart';
import 'value.dart';
import 'visitor/clone_css.dart';

// TODO(nweiz): This used to avoid tracking source spans for variables if source
// map generation was disabled. We always have to track them now to produce
// better warnings for /-as-division, but once those warnings are gone we should
// go back to tracking conditionally.

/// The lexical environment in which Sass is executed.
///
/// This tracks lexically-scoped information, such as variables, functions, and
Expand Down Expand Up @@ -77,12 +82,10 @@ class AsyncEnvironment {

/// The nodes where each variable in [_variables] was defined.
///
/// This is `null` if source mapping is disabled.
///
/// This stores [AstNode]s rather than [FileSpan]s so it can avoid calling
/// [AstNode.span] if the span isn't required, since some nodes need to do
/// real work to manufacture a source span.
final List<Map<String, AstNode>>? _variableNodes;
final List<Map<String, AstNode>> _variableNodes;

/// A map of variable names to their indices in [_variables].
///
Expand Down Expand Up @@ -145,7 +148,7 @@ class AsyncEnvironment {
/// Creates an [AsyncEnvironment].
///
/// If [sourceMap] is `true`, this tracks variables' source locations
AsyncEnvironment({bool sourceMap = false})
AsyncEnvironment()
: _modules = {},
_namespaceNodes = {},
_globalModules = {},
Expand All @@ -155,7 +158,7 @@ class AsyncEnvironment {
_nestedForwardedModules = null,
_allModules = [],
_variables = [{}],
_variableNodes = sourceMap ? [{}] : null,
_variableNodes = [{}],
_variableIndices = {},
_functions = [{}],
_functionIndices = {},
Expand Down Expand Up @@ -199,7 +202,7 @@ class AsyncEnvironment {
_nestedForwardedModules,
_allModules,
_variables.toList(),
_variableNodes?.toList(),
_variableNodes.toList(),
_functions.toList(),
_mixins.toList(),
_content);
Expand All @@ -218,7 +221,7 @@ class AsyncEnvironment {
null,
[],
_variables.toList(),
_variableNodes?.toList(),
_variableNodes.toList(),
_functions.toList(),
_mixins.toList(),
_content);
Expand Down Expand Up @@ -406,7 +409,7 @@ class AsyncEnvironment {
for (var variable in forwardedVariableNames) {
_variableIndices.remove(variable);
_variables.last.remove(variable);
_variableNodes?.last.remove(variable);
_variableNodes.last.remove(variable);
}
for (var function in forwardedFunctionNames) {
_functionIndices.remove(function);
Expand Down Expand Up @@ -468,25 +471,18 @@ class AsyncEnvironment {
/// required, since some nodes need to do real work to manufacture a source
/// span.
AstNode? getVariableNode(String name, {String? namespace}) {
var variableNodes = _variableNodes;
if (variableNodes == null) {
throw StateError(
"getVariableNodes() should only be called if sourceMap = true was "
"passed in.");
}

if (namespace != null) return _getModule(namespace).variableNodes![name];
if (namespace != null) return _getModule(namespace).variableNodes[name];

if (_lastVariableName == name) {
return variableNodes[_lastVariableIndex!][name] ??
return _variableNodes[_lastVariableIndex!][name] ??
_getVariableNodeFromGlobalModule(name);
}

var index = _variableIndices[name];
if (index != null) {
_lastVariableName = name;
_lastVariableIndex = index;
return variableNodes[index][name] ??
return _variableNodes[index][name] ??
_getVariableNodeFromGlobalModule(name);
}

Expand All @@ -496,7 +492,8 @@ class AsyncEnvironment {
_lastVariableName = name;
_lastVariableIndex = index;
_variableIndices[name] = index;
return variableNodes[index][name] ?? _getVariableNodeFromGlobalModule(name);
return _variableNodes[index][name] ??
_getVariableNodeFromGlobalModule(name);
}

/// Returns the node for the variable named [name] from a namespaceless
Expand All @@ -511,7 +508,7 @@ class AsyncEnvironment {
// We don't need to worry about multiple modules defining the same variable,
// because that's already been checked by [getVariable].
for (var module in _globalModules) {
var value = module.variableNodes![name];
var value = module.variableNodes[name];
if (value != null) return value;
}
return null;
Expand Down Expand Up @@ -559,7 +556,7 @@ class AsyncEnvironment {
/// defined with the given namespace, if no variable with the given [name] is
/// defined in module with the given namespace, or if no [namespace] is passed
/// and multiple global modules define variables named [name].
void setVariable(String name, Value value, AstNode? nodeWithSpan,
void setVariable(String name, Value value, AstNode nodeWithSpan,
{String? namespace, bool global = false}) {
if (namespace != null) {
_getModule(namespace).setVariable(name, value, nodeWithSpan);
Expand Down Expand Up @@ -587,7 +584,7 @@ class AsyncEnvironment {
}

_variables.first[name] = value;
if (nodeWithSpan != null) _variableNodes?.first[name] = nodeWithSpan;
_variableNodes.first[name] = nodeWithSpan;
return;
}

Expand Down Expand Up @@ -617,7 +614,7 @@ class AsyncEnvironment {
_lastVariableName = name;
_lastVariableIndex = index;
_variables[index][name] = value;
_variableNodes?[index][name] = nodeWithSpan!;
_variableNodes[index][name] = nodeWithSpan;
}

/// Sets the variable named [name] to [value], associated with
Expand All @@ -629,15 +626,13 @@ class AsyncEnvironment {
/// This takes an [AstNode] rather than a [FileSpan] so it can avoid calling
/// [AstNode.span] if the span isn't required, since some nodes need to do
/// real work to manufacture a source span.
void setLocalVariable(String name, Value value, AstNode? nodeWithSpan) {
void setLocalVariable(String name, Value value, AstNode nodeWithSpan) {
var index = _variables.length - 1;
_lastVariableName = name;
_lastVariableIndex = index;
_variableIndices[name] = index;
_variables[index][name] = value;
if (nodeWithSpan != null) {
_variableNodes?[index][name] = nodeWithSpan;
}
_variableNodes[index][name] = nodeWithSpan;
}

/// Returns the value of the function named [name], optionally with the given
Expand Down Expand Up @@ -789,7 +784,7 @@ class AsyncEnvironment {
_inSemiGlobalScope = semiGlobal;

_variables.add({});
_variableNodes?.add({});
_variableNodes.add({});
_functions.add({});
_mixins.add({});
_nestedForwardedModules?.add([]);
Expand Down Expand Up @@ -818,12 +813,12 @@ class AsyncEnvironment {
var configuration = <String, ConfiguredValue>{};
for (var i = 0; i < _variables.length; i++) {
var values = _variables[i];
var nodes = _variableNodes?[i] ?? <String, AstNode>{};
var nodes = _variableNodes[i];
for (var entry in values.entries) {
// Implicit configurations are never invalid, making [configurationSpan]
// unnecessary, so we pass null here to avoid having to compute it.
configuration[entry.key] =
ConfiguredValue.implicit(entry.value, nodes[entry.key]);
ConfiguredValue.implicit(entry.value, nodes[entry.key]!);
}
}
return Configuration.implicit(configuration);
Expand Down Expand Up @@ -921,7 +916,7 @@ class _EnvironmentModule implements Module {

final List<Module> upstream;
final Map<String, Value> variables;
final Map<String, AstNode>? variableNodes;
final Map<String, AstNode> variableNodes;
final Map<String, AsyncCallable> functions;
final Map<String, AsyncCallable> mixins;
final ExtensionStore extensionStore;
Expand Down Expand Up @@ -951,10 +946,8 @@ class _EnvironmentModule implements Module {
_makeModulesByVariable(forwarded),
_memberMap(environment._variables.first,
forwarded.map((module) => module.variables)),
environment._variableNodes.andThen((nodes) => _memberMap(
nodes.first,
// dart-lang/sdk#45348
forwarded!.map((module) => module.variableNodes!))),
_memberMap(environment._variableNodes.first,
forwarded.map((module) => module.variableNodes)),
_memberMap(environment._functions.first,
forwarded.map((module) => module.functions)),
_memberMap(environment._mixins.first,
Expand Down Expand Up @@ -1017,7 +1010,7 @@ class _EnvironmentModule implements Module {
required this.transitivelyContainsExtensions})
: upstream = _environment._allModules;

void setVariable(String name, Value value, AstNode? nodeWithSpan) {
void setVariable(String name, Value value, AstNode nodeWithSpan) {
var module = _modulesByVariable[name];
if (module != null) {
module.setVariable(name, value, nodeWithSpan);
Expand All @@ -1029,9 +1022,7 @@ class _EnvironmentModule implements Module {
}

_environment._variables.first[name] = value;
if (nodeWithSpan != null) {
_environment._variableNodes?.first[name] = nodeWithSpan;
}
_environment._variableNodes.first[name] = nodeWithSpan;
return;
}

Expand Down
4 changes: 1 addition & 3 deletions lib/src/configured_value.dart
View file
Expand Up @@ -17,9 +17,7 @@ class ConfiguredValue {
final FileSpan? configurationSpan;

/// The [AstNode] where the variable's value originated.
///
/// This is used to generate source maps.
final AstNode? assignmentNode;
final AstNode assignmentNode;

/// Creates a variable value that's been configured explicitly with a `with`
/// clause.
Expand Down

0 comments on commit 3849165

Please sign in to comment.