Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Produce better stack traces for syntax errors loaded via import/use #722

Merged
merged 2 commits into from
Jun 19, 2019
Merged

Produce better stack traces for syntax errors loaded via import/use #722

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f6822d5
Produce better stack traces for syntax errors loaded via import/use
nex3 Jun 13, 2019
8e6364e
Update a test
nex3 Jun 17, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
## 1.22.0

* Produce better stack traces when importing a file that contains a syntax
error.

### Dart API

* Add a `Value.realNull` getter, which returns Dart's `null` if the value is
Expand Down
134 changes: 71 additions & 63 deletions lib/src/visitor/async_evaluate.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -439,25 +439,31 @@ class _EvaluateVisitor
return;
}

var result = await inUseRuleAsync(
() => _loadStylesheet(url.toString(), nodeForSpan.span));
var importer = result.item1;
var stylesheet = result.item2;
await _withStackFrame(stackFrame, nodeForSpan, () async {
var result = await inUseRuleAsync(
() => _loadStylesheet(url.toString(), nodeForSpan.span));
var importer = result.item1;
var stylesheet = result.item2;

var canonicalUrl = stylesheet.span.sourceUrl;
if (_activeModules.contains(canonicalUrl)) {
throw _exception("Module loop: this module is already being loaded.");
}
_activeModules.add(canonicalUrl);

var canonicalUrl = stylesheet.span.sourceUrl;
if (_activeModules.contains(canonicalUrl)) {
throw _exception("Module loop: this module is already being loaded.",
nodeForSpan.span);
}
_activeModules.add(canonicalUrl);
Module module;
try {
module = await _execute(importer, stylesheet);
} finally {
_activeModules.remove(canonicalUrl);
}

var module = await _withStackFrame(
stackFrame, nodeForSpan, () => _execute(importer, stylesheet));
try {
await _addExceptionSpan(nodeForSpan, () => callback(module));
} finally {
_activeModules.remove(canonicalUrl);
}
try {
await callback(module);
} on SassScriptException catch (error) {
throw _exception(error.message);
}
});
}

/// Executes [stylesheet], loaded by [importer], to produce a module.
Expand Down Expand Up @@ -1089,25 +1095,25 @@ class _EvaluateVisitor
}

/// Adds the stylesheet imported by [import] to the current document.
Future<void> _visitDynamicImport(DynamicImport import) async {
var result = await _loadStylesheet(import.url, import.span);
var importer = result.item1;
var stylesheet = result.item2;

var url = stylesheet.span.sourceUrl;
if (!_activeModules.add(url)) {
throw _exception("This file is already being loaded.", import.span);
}
Future<void> _visitDynamicImport(DynamicImport import) {
return _withStackFrame("@import", import, () async {
var result = await _loadStylesheet(import.url, import.span);
var importer = result.item1;
var stylesheet = result.item2;

var url = stylesheet.span.sourceUrl;
if (!_activeModules.add(url)) {
throw _exception("This file is already being loaded.");
}

_activeModules.add(url);
_activeModules.add(url);

// TODO(nweiz): If [stylesheet] contains no `@use` or `@forward` rules, just
// evaluate it directly in [_root] rather than making a new
// [ModifiableCssStylesheet] and manually copying members.
// TODO(nweiz): If [stylesheet] contains no `@use` or `@forward` rules, just
// evaluate it directly in [_root] rather than making a new
// [ModifiableCssStylesheet] and manually copying members.

List<ModifiableCssNode> children;
var environment = _environment.global();
await _withStackFrame("@import", import, () async {
List<ModifiableCssNode> children;
var environment = _environment.global();
await _withEnvironment(environment, () async {
var oldImporter = _importer;
var oldStylesheet = _stylesheet;
Expand All @@ -1132,30 +1138,30 @@ class _EvaluateVisitor
_endOfImports = oldEndOfImports;
_outOfOrderImports = oldOutOfOrderImports;
});
});

// Create a dummy module with empty CSS and no extensions to make forwarded
// members available in the current import context and to combine all the
// CSS from modules used by [stylesheet].
var module = environment.toModule(
CssStylesheet(const [], stylesheet.span), Extender.empty);
_environment.importForwards(module);

if (module.transitivelyContainsCss) {
// If any transitively used module contains extensions, we need to clone
// all modules' CSS. Otherwise, it's possible that they'll be used or
// imported from another location that shouldn't have the same extensions
// applied.
await _combineCss(module, clone: module.transitivelyContainsExtensions)
.accept(this);
}
// Create a dummy module with empty CSS and no extensions to make forwarded
// members available in the current import context and to combine all the
// CSS from modules used by [stylesheet].
var module = environment.toModule(
CssStylesheet(const [], stylesheet.span), Extender.empty);
_environment.importForwards(module);

if (module.transitivelyContainsCss) {
// If any transitively used module contains extensions, we need to clone
// all modules' CSS. Otherwise, it's possible that they'll be used or
// imported from another location that shouldn't have the same extensions
// applied.
await _combineCss(module, clone: module.transitivelyContainsExtensions)
.accept(this);
}

var visitor = _ImportedCssVisitor(this);
for (var child in children) {
child.accept(visitor);
}
var visitor = _ImportedCssVisitor(this);
for (var child in children) {
child.accept(visitor);
}

_activeModules.remove(url);
_activeModules.remove(url);
});
}

/// Loads the [Stylesheet] identified by [url], or throws a
Expand Down Expand Up @@ -1183,16 +1189,15 @@ class _EvaluateVisitor
throw "Can't find stylesheet to import.";
}
} on SassException catch (error) {
var frames = [...error.trace.frames, ..._stackTrace(span).frames];
throw SassRuntimeException(error.message, error.span, Trace(frames));
throw _exception(error.message, error.span);
} catch (error) {
String message;
try {
message = error.message as String;
} catch (_) {
message = error.toString();
}
throw _exception(message, span);
throw _exception(message);
} finally {
_importSpan = null;
}
Expand Down Expand Up @@ -2522,11 +2527,11 @@ class _EvaluateVisitor

/// Returns a stack trace at the current point.
///
/// [span] is the current location, used for the bottom-most stack frame.
Trace _stackTrace(FileSpan span) {
/// If [span] is passed, it's used for the innermost stack frame.
Trace _stackTrace([FileSpan span]) {
var frames = [
..._stack.map((tuple) => _stackFrame(tuple.item1, tuple.item2.span)),
_stackFrame(_member, span)
if (span != null) _stackFrame(_member, span)
];
return Trace(frames.reversed);
}
Expand All @@ -2536,9 +2541,12 @@ class _EvaluateVisitor
_logger.warn(message,
span: span, trace: _stackTrace(span), deprecation: deprecation);

/// Throws a [SassRuntimeException] with the given [message] and [span].
SassRuntimeException _exception(String message, FileSpan span) =>
SassRuntimeException(message, span, _stackTrace(span));
/// Throws a [SassRuntimeException] with the given [message].
///
/// If [span] is passed, it's used for the innermost stack frame.
SassRuntimeException _exception(String message, [FileSpan span]) =>
SassRuntimeException(
message, span ?? _stack.last.item2.span, _stackTrace(span));

/// Runs [callback], and adjusts any [SassFormatException] to be within
/// [nodeWithSpan]'s source span.
Expand Down
Loading