Skip to content

Commit 0c1370c

Browse files
committed
Allow errors to refer to multiple source spans at once
This makes error messages for issues like configuring a module after it's loaded much clearer.
1 parent ca63b1b commit 0c1370c

File tree

8 files changed

+447
-117
lines changed

8 files changed

+447
-117
lines changed

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
## 1.24.5
2+
3+
* Highlight contextually-relevant sections of the stylesheet in error messages,
4+
rather than only highlighting the section where the error was detected.
5+
16
## 1.24.4
27

38
### JavaScript API

lib/src/async_environment.dart

Lines changed: 73 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -35,16 +35,32 @@ class AsyncEnvironment {
3535
Map<String, Module> get modules => UnmodifiableMapView(_modules);
3636
final Map<String, Module> _modules;
3737

38+
/// A map from module namespaces to the nodes whose spans indicate where those
39+
/// modules were originally loaded.
40+
final Map<String, AstNode> _namespaceNodes;
41+
3842
/// The namespaceless modules used in the current scope.
3943
///
4044
/// This is `null` if there are no namespaceless modules.
4145
Set<Module> _globalModules;
4246

47+
/// A map from modules in [_globalModules] to the nodes whose spans
48+
/// indicate where those modules were originally loaded.
49+
///
50+
/// This is `null` if there are no namespaceless modules.
51+
Map<Module, AstNode> _globalModuleNodes;
52+
4353
/// The modules forwarded by this module.
4454
///
4555
/// This is `null` if there are no forwarded modules.
4656
List<Module> _forwardedModules;
4757

58+
/// A map from modules in [_forwardedModules] to the nodes whose spans
59+
/// indicate where those modules were originally forwarded.
60+
///
61+
/// This is `null` if there are no forwarded modules.
62+
Map<Module, AstNode> _forwardedModuleNodes;
63+
4864
/// Modules forwarded by nested imports at each lexical scope level *beneath
4965
/// the global scope*.
5066
///
@@ -136,8 +152,11 @@ class AsyncEnvironment {
136152
/// If [sourceMap] is `true`, this tracks variables' source locations
137153
AsyncEnvironment({bool sourceMap = false})
138154
: _modules = {},
155+
_namespaceNodes = {},
139156
_globalModules = null,
157+
_globalModuleNodes = null,
140158
_forwardedModules = null,
159+
_forwardedModuleNodes = null,
141160
_nestedForwardedModules = null,
142161
_allModules = [],
143162
_variables = [{}],
@@ -150,8 +169,11 @@ class AsyncEnvironment {
150169

151170
AsyncEnvironment._(
152171
this._modules,
172+
this._namespaceNodes,
153173
this._globalModules,
174+
this._globalModuleNodes,
154175
this._forwardedModules,
176+
this._forwardedModuleNodes,
155177
this._nestedForwardedModules,
156178
this._allModules,
157179
this._variables,
@@ -174,8 +196,11 @@ class AsyncEnvironment {
174196
/// when the closure was created will be reflected.
175197
AsyncEnvironment closure() => AsyncEnvironment._(
176198
_modules,
199+
_namespaceNodes,
177200
_globalModules,
201+
_globalModuleNodes,
178202
_forwardedModules,
203+
_forwardedModuleNodes,
179204
_nestedForwardedModules,
180205
_allModules,
181206
_variables.toList(),
@@ -190,6 +215,9 @@ class AsyncEnvironment {
190215
/// functions, and mixins, but not its modules.
191216
AsyncEnvironment global() => AsyncEnvironment._(
192217
{},
218+
{},
219+
null,
220+
null,
193221
null,
194222
null,
195223
null,
@@ -202,16 +230,20 @@ class AsyncEnvironment {
202230

203231
/// Adds [module] to the set of modules visible in this environment.
204232
///
233+
/// [nodeWithSpan]'s span is used to report any errors with the module.
234+
///
205235
/// If [namespace] is passed, the module is made available under that
206236
/// namespace.
207237
///
208-
/// Throws a [SassScriptException] if there's already a module with the given
238+
/// Throws a [SassException] if there's already a module with the given
209239
/// [namespace], or if [namespace] is `null` and [module] defines a variable
210240
/// with the same name as a variable defined in this environment.
211-
void addModule(Module module, {String namespace}) {
241+
void addModule(Module module, AstNode nodeWithSpan, {String namespace}) {
212242
if (namespace == null) {
213243
_globalModules ??= {};
244+
_globalModuleNodes ??= {};
214245
_globalModules.add(module);
246+
_globalModuleNodes[module] = nodeWithSpan;
215247
_allModules.add(module);
216248

217249
for (var name in _variables.first.keys) {
@@ -223,11 +255,14 @@ class AsyncEnvironment {
223255
}
224256
} else {
225257
if (_modules.containsKey(namespace)) {
226-
throw SassScriptException(
227-
"There's already a module with namespace \"$namespace\".");
258+
throw MultiSpanSassScriptException(
259+
"There's already a module with namespace \"$namespace\".",
260+
"new @use",
261+
{_namespaceNodes[namespace].span: "original @use"});
228262
}
229263

230264
_modules[namespace] = module;
265+
_namespaceNodes[namespace] = nodeWithSpan;
231266
_allModules.add(module);
232267
}
233268
}
@@ -236,12 +271,15 @@ class AsyncEnvironment {
236271
/// defined in this module, according to the modifications defined by [rule].
237272
void forwardModule(Module module, ForwardRule rule) {
238273
_forwardedModules ??= [];
274+
_forwardedModuleNodes ??= {};
239275

240276
var view = ForwardedModuleView(module, rule);
241277
for (var other in _forwardedModules) {
242-
_assertNoConflicts(view.variables, other.variables, "variable", other);
243-
_assertNoConflicts(view.functions, other.functions, "function", other);
244-
_assertNoConflicts(view.mixins, other.mixins, "mixin", other);
278+
_assertNoConflicts(
279+
view.variables, other.variables, "variable", other, rule);
280+
_assertNoConflicts(
281+
view.functions, other.functions, "function", other, rule);
282+
_assertNoConflicts(view.mixins, other.mixins, "mixin", other, rule);
245283
}
246284

247285
// Add the original module to [_allModules] (rather than the
@@ -250,14 +288,19 @@ class AsyncEnvironment {
250288
// CSS, not for the members they expose.
251289
_allModules.add(module);
252290
_forwardedModules.add(view);
291+
_forwardedModuleNodes[view] = rule;
253292
}
254293

255294
/// Throws a [SassScriptException] if [newMembers] has any keys that overlap
256295
/// with [oldMembers].
257296
///
258-
/// The [type] and [oldModule] is used for error reporting.
259-
void _assertNoConflicts(Map<String, Object> newMembers,
260-
Map<String, Object> oldMembers, String type, Module oldModule) {
297+
/// The [type], [other], [newModuleNodeWithSpan] are used for error reporting.
298+
void _assertNoConflicts(
299+
Map<String, Object> newMembers,
300+
Map<String, Object> oldMembers,
301+
String type,
302+
Module other,
303+
AstNode newModuleNodeWithSpan) {
261304
Map<String, Object> smaller;
262305
Map<String, Object> larger;
263306
if (newMembers.length < oldMembers.length) {
@@ -271,9 +314,10 @@ class AsyncEnvironment {
271314
for (var name in smaller.keys) {
272315
if (larger.containsKey(name)) {
273316
if (type == "variable") name = "\$$name";
274-
throw SassScriptException(
275-
'Module ${p.prettyUri(oldModule.url)} and the new module both '
276-
'forward a $type named $name.');
317+
throw MultiSpanSassScriptException(
318+
'Two forwarded modules both define a $type named $name.',
319+
"new @forward",
320+
{_forwardedModuleNodes[other].span: "original @forward"});
277321
}
278322
}
279323
}
@@ -288,7 +332,9 @@ class AsyncEnvironment {
288332
if (forwarded == null) return;
289333

290334
_globalModules ??= {};
335+
_globalModuleNodes ??= {};
291336
_forwardedModules ??= [];
337+
_forwardedModuleNodes ??= {};
292338

293339
var forwardedVariableNames =
294340
forwarded.expand((module) => module.variables.keys).toSet();
@@ -308,6 +354,7 @@ class AsyncEnvironment {
308354
if (shadowed != null) {
309355
_globalModules.remove(module);
310356
_globalModules.add(shadowed);
357+
_globalModuleNodes[shadowed] = _globalModuleNodes.remove(module);
311358
}
312359
}
313360
for (var i = 0; i < _forwardedModules.length; i++) {
@@ -316,11 +363,17 @@ class AsyncEnvironment {
316363
variables: forwardedVariableNames,
317364
mixins: forwardedMixinNames,
318365
functions: forwardedFunctionNames);
319-
if (shadowed != null) _forwardedModules[i] = shadowed;
366+
if (shadowed != null) {
367+
_forwardedModules[i] = shadowed;
368+
_forwardedModuleNodes[shadowed] =
369+
_forwardedModuleNodes.remove(module);
370+
}
320371
}
321372

322373
_globalModules.addAll(forwarded);
374+
_globalModuleNodes.addAll(module._environment._forwardedModuleNodes);
323375
_forwardedModules.addAll(forwarded);
376+
_forwardedModuleNodes.addAll(module._environment._forwardedModuleNodes);
324377
} else {
325378
_nestedForwardedModules ??=
326379
List.generate(_variables.length - 1, (_) => []);
@@ -795,11 +848,12 @@ class AsyncEnvironment {
795848
if (valueInModule == null) continue;
796849

797850
if (value != null) {
798-
throw SassScriptException(
799-
'This $type is available from multiple global modules:\n' +
800-
bulletedList(_globalModules
801-
.where((module) => callback(module) != null)
802-
.map((module) => p.prettyUri(module.url))));
851+
throw MultiSpanSassScriptException(
852+
'This $type is available from multiple global modules.',
853+
'$type use', {
854+
for (var entry in _globalModuleNodes.entries)
855+
if (callback(entry.key) != null) entry.value.span: 'includes $type'
856+
});
803857
}
804858

805859
value = valueInModule;

0 commit comments

Comments
 (0)