Skip to content

Commit 67c4e1b

Browse files
nex3nex3
authored
Don't throw conflict errors for identical members (#947)
See sass/sass#2820 Closes #946
1 parent 9676cc9 commit 67c4e1b

File tree

8 files changed

+112
-39
lines changed

8 files changed

+112
-39
lines changed

‎CHANGELOG.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1-
## 1.25.2
1+
## 1.26.0
2+
3+
* Don't throw errors if the exact same member is loaded or forwarded from
4+
multiple modules at the same time.
25

36
* Fix a bug where, under extremely rare circumstances, a valid variable could
47
become unassigned.

‎lib/src/async_environment.dart

Lines changed: 39 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -276,10 +276,11 @@ class AsyncEnvironment {
276276
var view = ForwardedModuleView(module, rule);
277277
for (var other in _forwardedModules) {
278278
_assertNoConflicts(
279-
view.variables, other.variables, "variable", other, rule);
279+
view.variables, other.variables, module, other, "variable", rule);
280280
_assertNoConflicts(
281-
view.functions, other.functions, "function", other, rule);
282-
_assertNoConflicts(view.mixins, other.mixins, "mixin", other, rule);
281+
view.functions, other.functions, module, other, "function", rule);
282+
_assertNoConflicts(
283+
view.mixins, other.mixins, module, other, "mixin", rule);
283284
}
284285

285286
// Add the original module to [_allModules] (rather than the
@@ -291,15 +292,16 @@ class AsyncEnvironment {
291292
_forwardedModuleNodes[view] = rule;
292293
}
293294

294-
/// Throws a [SassScriptException] if [newMembers] has any keys that overlap
295-
/// with [oldMembers].
295+
/// Throws a [SassScriptException] if [newMembers] from [newModule] has any
296+
/// keys that overlap with [oldMembers] from [oldModule].
296297
///
297-
/// The [type], [other], [newModuleNodeWithSpan] are used for error reporting.
298+
/// The [type] and [newModuleNodeWithSpan] are used for error reporting.
298299
void _assertNoConflicts(
299300
Map<String, Object> newMembers,
300301
Map<String, Object> oldMembers,
302+
Module newModule,
303+
Module oldModule,
301304
String type,
302-
Module other,
303305
AstNode newModuleNodeWithSpan) {
304306
Map<String, Object> smaller;
305307
Map<String, Object> larger;
@@ -312,13 +314,17 @@ class AsyncEnvironment {
312314
}
313315

314316
for (var name in smaller.keys) {
315-
if (larger.containsKey(name)) {
316-
if (type == "variable") name = "\$$name";
317-
throw MultiSpanSassScriptException(
318-
'Two forwarded modules both define a $type named $name.',
319-
"new @forward",
320-
{_forwardedModuleNodes[other].span: "original @forward"});
317+
if (!larger.containsKey(name)) continue;
318+
if (newModule.variableIdentity(name) ==
319+
oldModule.variableIdentity(name)) {
320+
continue;
321321
}
322+
323+
if (type == "variable") name = "\$$name";
324+
throw MultiSpanSassScriptException(
325+
'Two forwarded modules both define a $type named $name.',
326+
"new @forward",
327+
{_forwardedModuleNodes[oldModule].span: "original @forward"});
322328
}
323329
}
324330

@@ -436,7 +442,7 @@ class AsyncEnvironment {
436442
/// module, or `null` if no such variable is declared in any namespaceless
437443
/// module.
438444
Value _getVariableFromGlobalModule(String name) =>
439-
_fromOneModule("variable", (module) => module.variables[name]);
445+
_fromOneModule(name, "variable", (module) => module.variables[name]);
440446

441447
/// Returns the node for the variable named [name], or `null` if no such
442448
/// variable is declared.
@@ -553,7 +559,7 @@ class AsyncEnvironment {
553559
// If this module doesn't already contain a variable named [name], try
554560
// setting it in a global module.
555561
if (!_variables.first.containsKey(name) && _globalModules != null) {
556-
var moduleWithName = _fromOneModule("variable",
562+
var moduleWithName = _fromOneModule(name, "variable",
557563
(module) => module.variables.containsKey(name) ? module : null);
558564
if (moduleWithName != null) {
559565
moduleWithName.setVariable(name, value, nodeWithSpan);
@@ -636,7 +642,7 @@ class AsyncEnvironment {
636642
/// module, or `null` if no such function is declared in any namespaceless
637643
/// module.
638644
AsyncCallable _getFunctionFromGlobalModule(String name) =>
639-
_fromOneModule("function", (module) => module.functions[name]);
645+
_fromOneModule(name, "function", (module) => module.functions[name]);
640646

641647
/// Returns the index of the last map in [_functions] that has a [name] key,
642648
/// or `null` if none exists.
@@ -685,7 +691,7 @@ class AsyncEnvironment {
685691
/// module, or `null` if no such mixin is declared in any namespaceless
686692
/// module.
687693
AsyncCallable _getMixinFromGlobalModule(String name) =>
688-
_fromOneModule("mixin", (module) => module.mixins[name]);
694+
_fromOneModule(name, "mixin", (module) => module.mixins[name]);
689695

690696
/// Returns the index of the last map in [_mixins] that has a [name] key, or
691697
/// `null` if none exists.
@@ -841,9 +847,11 @@ class AsyncEnvironment {
841847
/// Returns `null` if [callback] returns `null` for all modules. Throws an
842848
/// error if [callback] returns non-`null` for more than one module.
843849
///
850+
/// The [name] is the name of the member being looked up.
851+
///
844852
/// The [type] should be the singular name of the value type being returned.
845853
/// It's used to format an appropriate error message.
846-
T _fromOneModule<T>(String type, T callback(Module module)) {
854+
T _fromOneModule<T>(String name, String type, T callback(Module module)) {
847855
if (_nestedForwardedModules != null) {
848856
for (var modules in _nestedForwardedModules.reversed) {
849857
for (var module in modules.reversed) {
@@ -856,10 +864,16 @@ class AsyncEnvironment {
856864
if (_globalModules == null) return null;
857865

858866
T value;
867+
Object identity;
859868
for (var module in _globalModules) {
860869
var valueInModule = callback(module);
861870
if (valueInModule == null) continue;
862871

872+
var identityFromModule = valueInModule is AsyncCallable
873+
? valueInModule
874+
: module.variableIdentity(name);
875+
if (identityFromModule == identity) continue;
876+
863877
if (value != null) {
864878
throw MultiSpanSassScriptException(
865879
'This $type is available from multiple global modules.',
@@ -870,6 +884,7 @@ class AsyncEnvironment {
870884
}
871885

872886
value = valueInModule;
887+
identity = identityFromModule;
873888
}
874889
return value;
875890
}
@@ -994,6 +1009,12 @@ class _EnvironmentModule implements Module {
9941009
return;
9951010
}
9961011

1012+
Object variableIdentity(String name) {
1013+
assert(variables.containsKey(name));
1014+
var module = _modulesByVariable[name];
1015+
return module == null ? this : module.variableIdentity(name);
1016+
}
1017+
9971018
Module cloneCss() {
9981019
if (css.children.isEmpty) return this;
9991020

‎lib/src/environment.dart

Lines changed: 41 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
// DO NOT EDIT. This file was generated from async_environment.dart.
66
// See tool/grind/synchronize.dart for details.
77
//
8-
// Checksum: b5212ffc7c50a8e7e436b25c7c16eb2996da2def
8+
// Checksum: 6666522945667f7f041530ee545444b7b40cfc80
99
//
1010
// ignore_for_file: unused_import
1111

@@ -283,10 +283,11 @@ class Environment {
283283
var view = ForwardedModuleView(module, rule);
284284
for (var other in _forwardedModules) {
285285
_assertNoConflicts(
286-
view.variables, other.variables, "variable", other, rule);
286+
view.variables, other.variables, module, other, "variable", rule);
287287
_assertNoConflicts(
288-
view.functions, other.functions, "function", other, rule);
289-
_assertNoConflicts(view.mixins, other.mixins, "mixin", other, rule);
288+
view.functions, other.functions, module, other, "function", rule);
289+
_assertNoConflicts(
290+
view.mixins, other.mixins, module, other, "mixin", rule);
290291
}
291292

292293
// Add the original module to [_allModules] (rather than the
@@ -298,15 +299,16 @@ class Environment {
298299
_forwardedModuleNodes[view] = rule;
299300
}
300301

301-
/// Throws a [SassScriptException] if [newMembers] has any keys that overlap
302-
/// with [oldMembers].
302+
/// Throws a [SassScriptException] if [newMembers] from [newModule] has any
303+
/// keys that overlap with [oldMembers] from [oldModule].
303304
///
304-
/// The [type], [other], [newModuleNodeWithSpan] are used for error reporting.
305+
/// The [type] and [newModuleNodeWithSpan] are used for error reporting.
305306
void _assertNoConflicts(
306307
Map<String, Object> newMembers,
307308
Map<String, Object> oldMembers,
309+
Module<Callable> newModule,
310+
Module<Callable> oldModule,
308311
String type,
309-
Module<Callable> other,
310312
AstNode newModuleNodeWithSpan) {
311313
Map<String, Object> smaller;
312314
Map<String, Object> larger;
@@ -319,13 +321,17 @@ class Environment {
319321
}
320322

321323
for (var name in smaller.keys) {
322-
if (larger.containsKey(name)) {
323-
if (type == "variable") name = "\$$name";
324-
throw MultiSpanSassScriptException(
325-
'Two forwarded modules both define a $type named $name.',
326-
"new @forward",
327-
{_forwardedModuleNodes[other].span: "original @forward"});
324+
if (!larger.containsKey(name)) continue;
325+
if (newModule.variableIdentity(name) ==
326+
oldModule.variableIdentity(name)) {
327+
continue;
328328
}
329+
330+
if (type == "variable") name = "\$$name";
331+
throw MultiSpanSassScriptException(
332+
'Two forwarded modules both define a $type named $name.',
333+
"new @forward",
334+
{_forwardedModuleNodes[oldModule].span: "original @forward"});
329335
}
330336
}
331337

@@ -443,7 +449,7 @@ class Environment {
443449
/// module, or `null` if no such variable is declared in any namespaceless
444450
/// module.
445451
Value _getVariableFromGlobalModule(String name) =>
446-
_fromOneModule("variable", (module) => module.variables[name]);
452+
_fromOneModule(name, "variable", (module) => module.variables[name]);
447453

448454
/// Returns the node for the variable named [name], or `null` if no such
449455
/// variable is declared.
@@ -560,7 +566,7 @@ class Environment {
560566
// If this module doesn't already contain a variable named [name], try
561567
// setting it in a global module.
562568
if (!_variables.first.containsKey(name) && _globalModules != null) {
563-
var moduleWithName = _fromOneModule("variable",
569+
var moduleWithName = _fromOneModule(name, "variable",
564570
(module) => module.variables.containsKey(name) ? module : null);
565571
if (moduleWithName != null) {
566572
moduleWithName.setVariable(name, value, nodeWithSpan);
@@ -643,7 +649,7 @@ class Environment {
643649
/// module, or `null` if no such function is declared in any namespaceless
644650
/// module.
645651
Callable _getFunctionFromGlobalModule(String name) =>
646-
_fromOneModule("function", (module) => module.functions[name]);
652+
_fromOneModule(name, "function", (module) => module.functions[name]);
647653

648654
/// Returns the index of the last map in [_functions] that has a [name] key,
649655
/// or `null` if none exists.
@@ -692,7 +698,7 @@ class Environment {
692698
/// module, or `null` if no such mixin is declared in any namespaceless
693699
/// module.
694700
Callable _getMixinFromGlobalModule(String name) =>
695-
_fromOneModule("mixin", (module) => module.mixins[name]);
701+
_fromOneModule(name, "mixin", (module) => module.mixins[name]);
696702

697703
/// Returns the index of the last map in [_mixins] that has a [name] key, or
698704
/// `null` if none exists.
@@ -846,9 +852,12 @@ class Environment {
846852
/// Returns `null` if [callback] returns `null` for all modules. Throws an
847853
/// error if [callback] returns non-`null` for more than one module.
848854
///
855+
/// The [name] is the name of the member being looked up.
856+
///
849857
/// The [type] should be the singular name of the value type being returned.
850858
/// It's used to format an appropriate error message.
851-
T _fromOneModule<T>(String type, T callback(Module<Callable> module)) {
859+
T _fromOneModule<T>(
860+
String name, String type, T callback(Module<Callable> module)) {
852861
if (_nestedForwardedModules != null) {
853862
for (var modules in _nestedForwardedModules.reversed) {
854863
for (var module in modules.reversed) {
@@ -861,10 +870,16 @@ class Environment {
861870
if (_globalModules == null) return null;
862871

863872
T value;
873+
Object identity;
864874
for (var module in _globalModules) {
865875
var valueInModule = callback(module);
866876
if (valueInModule == null) continue;
867877

878+
var identityFromModule = valueInModule is Callable
879+
? valueInModule
880+
: module.variableIdentity(name);
881+
if (identityFromModule == identity) continue;
882+
868883
if (value != null) {
869884
throw MultiSpanSassScriptException(
870885
'This $type is available from multiple global modules.',
@@ -875,6 +890,7 @@ class Environment {
875890
}
876891

877892
value = valueInModule;
893+
identity = identityFromModule;
878894
}
879895
return value;
880896
}
@@ -1000,6 +1016,12 @@ class _EnvironmentModule implements Module<Callable> {
10001016
return;
10011017
}
10021018

1019+
Object variableIdentity(String name) {
1020+
assert(variables.containsKey(name));
1021+
var module = _modulesByVariable[name];
1022+
return module == null ? this : module.variableIdentity(name);
1023+
}
1024+
10031025
Module<Callable> cloneCss() {
10041026
if (css.children.isEmpty) return this;
10051027

‎lib/src/module.dart

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,12 @@ abstract class Module<T extends AsyncCallable> {
7373
/// named [name].
7474
void setVariable(String name, Value value, AstNode nodeWithSpan);
7575

76+
/// Returns an opaque object that will be equal to another
77+
/// `variableIdentity()` return value for the same name in another module if
78+
/// and only if both modules expose identical definitions of the variable in
79+
/// question, as defined by the Sass spec.
80+
Object variableIdentity(String name);
81+
7682
/// Creates a copy of this module with new [css] and [extender].
7783
Module<T> cloneCss();
7884
}

‎lib/src/module/built_in.dart

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,5 +49,10 @@ class BuiltInModule<T extends AsyncCallable> implements Module<T> {
4949
throw SassScriptException("Cannot modify built-in variable.");
5050
}
5151

52+
Object variableIdentity(String name) {
53+
assert(variables.containsKey(name));
54+
return this;
55+
}
56+
5257
Module<T> cloneCss() => this;
5358
}

‎lib/src/module/forwarded_view.dart

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,5 +91,16 @@ class ForwardedModuleView<T extends AsyncCallable> implements Module<T> {
9191
return _inner.setVariable(name, value, nodeWithSpan);
9292
}
9393

94+
Object variableIdentity(String name) {
95+
assert(variables.containsKey(name));
96+
97+
if (_rule.prefix != null) {
98+
assert(name.startsWith(_rule.prefix));
99+
name = name.substring(_rule.prefix.length);
100+
}
101+
102+
return _inner.variableIdentity(name);
103+
}
104+
94105
Module<T> cloneCss() => ForwardedModuleView(_inner.cloneCss(), _rule);
95106
}

‎lib/src/module/shadowed_view.dart

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,11 @@ class ShadowedModuleView<T extends AsyncCallable> implements Module<T> {
7474
}
7575
}
7676

77+
Object variableIdentity(String name) {
78+
assert(variables.containsKey(name));
79+
return _inner.variableIdentity(name);
80+
}
81+
7782
Module<T> cloneCss() => ShadowedModuleView._(
7883
_inner.cloneCss(), variables, variableNodes, functions, mixins);
7984
}

‎pubspec.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
name: sass
2-
version: 1.25.2-dev
2+
version: 1.26.0-dev
33
description: A Sass implementation in Dart.
44
author: Sass Team
55
homepage: https://github.com/sass/dart-sass

0 commit comments

Comments
 (0)