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.

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

Silence compiler warnings in dependency callables with --quiet-deps #1650

Merged
merged 2 commits into from
Mar 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
## 1.49.10

* Quiet deps mode now silences compiler warnings in mixins and functions that
are defined in dependencies even if they're invoked from application
stylesheets.

## 1.49.9

### Embedded Sass
Expand Down
5 changes: 2 additions & 3 deletions lib/src/async_import_cache.dart
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,8 @@ class AsyncImportCache {
var resolvedUrl = baseUrl?.resolveUri(url) ?? url;
var canonicalUrl =
await _canonicalize(baseImporter, resolvedUrl, forImport);
if (canonicalUrl != null) {
return Tuple3(baseImporter, canonicalUrl, resolvedUrl);
}
if (canonicalUrl == null) return null;
return Tuple3(baseImporter, canonicalUrl, resolvedUrl);
});
if (relativeResult != null) return relativeResult;
}
Expand Down
9 changes: 8 additions & 1 deletion lib/src/callable/user_defined.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,14 @@ class UserDefinedCallable<E> implements Callable {
/// The environment in which this callable was declared.
final E environment;

/// Whether this callable was defined in a dependency.
///
/// That is, whether this was (transitively) loaded through a load path or
/// importer rather than relative to the entrypoint.
final bool inDependency;

String get name => declaration.name;

UserDefinedCallable(this.declaration, this.environment);
UserDefinedCallable(this.declaration, this.environment,
{required this.inDependency});
}
2 changes: 1 addition & 1 deletion lib/src/import_cache.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_import_cache.dart.
// See tool/grind/synchronize.dart for details.
//
// Checksum: 3e290e40f4576be99217ddfbd7a76c4d38721af1
// Checksum: cd71f3debc089cd05cd86e2eee32c2f10a05f489
//
// ignore_for_file: unused_import

Expand Down
26 changes: 20 additions & 6 deletions lib/src/visitor/async_evaluate.dart
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,9 @@ class _EvaluateVisitor
/// The human-readable name of the current stack frame.
var _member = "root stylesheet";

/// The innermost user-defined callable that's being invoked.
UserDefinedCallable<AsyncEnvironment>? _currentCallable;

/// The node for the innermost callable that's being invoked.
///
/// This is used to produce warnings for function calls. It's stored as an
Expand Down Expand Up @@ -1415,7 +1418,8 @@ class _EvaluateVisitor
}

Future<Value?> visitFunctionRule(FunctionRule node) async {
_environment.setFunction(UserDefinedCallable(node, _environment.closure()));
_environment.setFunction(UserDefinedCallable(node, _environment.closure(),
inDependency: _inDependency));
return null;
}

Expand Down Expand Up @@ -1704,8 +1708,9 @@ class _EvaluateVisitor
_stackTrace(node.spanWithoutContent));
}

var contentCallable = node.content.andThen(
(content) => UserDefinedCallable(content, _environment.closure()));
var contentCallable = node.content.andThen((content) =>
UserDefinedCallable(content, _environment.closure(),
inDependency: _inDependency));
await _runUserDefinedCallable(node.arguments, mixin, nodeWithSpan,
() async {
await _environment.withContent(contentCallable, () async {
Expand All @@ -1724,7 +1729,8 @@ class _EvaluateVisitor
}

Future<Value?> visitMixinRule(MixinRule node) async {
_environment.setMixin(UserDefinedCallable(node, _environment.closure()));
_environment.setMixin(UserDefinedCallable(node, _environment.closure(),
inDependency: _inDependency));
return null;
}

Expand Down Expand Up @@ -2428,7 +2434,9 @@ class _EvaluateVisitor
var name = callable.name;
if (name != "@content") name += "()";

return await _withStackFrame(name, nodeWithSpan, () {
var oldCallable = _currentCallable;
_currentCallable = callable;
var result = await _withStackFrame(name, nodeWithSpan, () {
// Add an extra closure() call so that modifications to the environment
// don't affect the underlying environment closure.
return _withEnvironment(callable.environment.closure(), () {
Expand Down Expand Up @@ -2493,6 +2501,8 @@ class _EvaluateVisitor
});
});
});
_currentCallable = oldCallable;
return result;
}

/// Evaluates [arguments] as applied to [callable].
Expand Down Expand Up @@ -3287,7 +3297,11 @@ class _EvaluateVisitor

/// Emits a warning with the given [message] about the given [span].
void _warn(String message, FileSpan span, {bool deprecation = false}) {
if (_quietDeps && _inDependency) return;
if (_quietDeps &&
(_inDependency || (_currentCallable?.inDependency ?? false))) {
return;
}

if (!_warningsEmitted.add(Tuple2(message, span))) return;
_logger.warn(message,
span: span, trace: _stackTrace(span), deprecation: deprecation);
Expand Down
28 changes: 21 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: d0af88db460da6528bdfeef34eb85baac00f9435
// Checksum: f11bdd289c888e0e0737bc96e63283bc8a332d9a
//
// ignore_for_file: unused_import

Expand Down Expand Up @@ -195,6 +195,9 @@ class _EvaluateVisitor
/// The human-readable name of the current stack frame.
var _member = "root stylesheet";

/// The innermost user-defined callable that's being invoked.
UserDefinedCallable<Environment>? _currentCallable;

/// The node for the innermost callable that's being invoked.
///
/// This is used to produce warnings for function calls. It's stored as an
Expand Down Expand Up @@ -1415,7 +1418,8 @@ class _EvaluateVisitor
}

Value? visitFunctionRule(FunctionRule node) {
_environment.setFunction(UserDefinedCallable(node, _environment.closure()));
_environment.setFunction(UserDefinedCallable(node, _environment.closure(),
inDependency: _inDependency));
return null;
}

Expand Down Expand Up @@ -1701,8 +1705,9 @@ class _EvaluateVisitor
_stackTrace(node.spanWithoutContent));
}

var contentCallable = node.content.andThen(
(content) => UserDefinedCallable(content, _environment.closure()));
var contentCallable = node.content.andThen((content) =>
UserDefinedCallable(content, _environment.closure(),
inDependency: _inDependency));
_runUserDefinedCallable(node.arguments, mixin, nodeWithSpan, () {
_environment.withContent(contentCallable, () {
_environment.asMixin(() {
Expand All @@ -1720,7 +1725,8 @@ class _EvaluateVisitor
}

Value? visitMixinRule(MixinRule node) {
_environment.setMixin(UserDefinedCallable(node, _environment.closure()));
_environment.setMixin(UserDefinedCallable(node, _environment.closure(),
inDependency: _inDependency));
return null;
}

Expand Down Expand Up @@ -2414,7 +2420,9 @@ class _EvaluateVisitor
var name = callable.name;
if (name != "@content") name += "()";

return _withStackFrame(name, nodeWithSpan, () {
var oldCallable = _currentCallable;
_currentCallable = callable;
var result = _withStackFrame(name, nodeWithSpan, () {
// Add an extra closure() call so that modifications to the environment
// don't affect the underlying environment closure.
return _withEnvironment(callable.environment.closure(), () {
Expand Down Expand Up @@ -2478,6 +2486,8 @@ class _EvaluateVisitor
});
});
});
_currentCallable = oldCallable;
return result;
}

/// Evaluates [arguments] as applied to [callable].
Expand Down Expand Up @@ -3257,7 +3267,11 @@ class _EvaluateVisitor

/// Emits a warning with the given [message] about the given [span].
void _warn(String message, FileSpan span, {bool deprecation = false}) {
if (_quietDeps && _inDependency) return;
if (_quietDeps &&
(_inDependency || (_currentCallable?.inDependency ?? false))) {
return;
}

if (!_warningsEmitted.add(Tuple2(message, span))) return;
_logger.warn(message,
span: span, trace: _stackTrace(span), deprecation: deprecation);
Expand Down
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.49.9
version: 1.49.10-dev
description: A Sass implementation in Dart.
homepage: https://github.com/sass/dart-sass

Expand Down
126 changes: 126 additions & 0 deletions test/cli/shared.dart
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,132 @@ void sharedTests(
await sass.shouldExit(0);
});
});

group("in dependency callables", () {
group("(mixin)", () {
test("emits @warn", () async {
await d.file("test.scss", """
@use 'other';
@include other.foo;
""").create();

await d.dir("dir", [
d.file("_other.scss", """
@mixin foo {
@warn heck;
}
""")
]).create();

var sass = await runSass(["--quiet-deps", "-I", "dir", "test.scss"]);
expect(sass.stderr, emitsThrough(contains("heck")));
await sass.shouldExit(0);
});

test("emits @debug", () async {
await d.file("test.scss", """
@use 'other';
@include other.foo;
""").create();

await d.dir("dir", [
d.file("_other.scss", """
@mixin foo {
@debug heck;
}
""")
]).create();

var sass = await runSass(["--quiet-deps", "-I", "dir", "test.scss"]);
expect(sass.stderr, emitsThrough(contains("heck")));
await sass.shouldExit(0);
});

test("doesn't emit runner warnings", () async {
await d.file("test.scss", """
@use 'other';
@include other.foo;
""").create();

await d.dir("dir", [
d.file("_other.scss", """
@mixin foo {
#{blue} {x: y}
}
""")
]).create();
await d.file("test.scss", "@use 'other'").create();
await d.dir("dir", [d.file("_other.scss", "")]).create();

var sass = await runSass(["--quiet-deps", "-I", "dir", "test.scss"]);
expect(sass.stderr, emitsDone);
await sass.shouldExit(0);
});
});

group("(function)", () {
test("emits @warn", () async {
await d.file("test.scss", r"""
@use 'other';
$_: other.foo();
""").create();

await d.dir("dir", [
d.file("_other.scss", """
@function foo() {
@warn heck;
@return null;
}
""")
]).create();

var sass = await runSass(["--quiet-deps", "-I", "dir", "test.scss"]);
expect(sass.stderr, emitsThrough(contains("heck")));
await sass.shouldExit(0);
});

test("emits @debug", () async {
await d.file("test.scss", r"""
@use 'other';
$_: other.foo();
""").create();

await d.dir("dir", [
d.file("_other.scss", """
@function foo() {
@debug heck;
@return null;
}
""")
]).create();

var sass = await runSass(["--quiet-deps", "-I", "dir", "test.scss"]);
expect(sass.stderr, emitsThrough(contains("heck")));
await sass.shouldExit(0);
});

test("doesn't emit runner warnings", () async {
await d.file("test.scss", r"""
@use 'other';
$_: other.foo();
""").create();

await d.dir("dir", [
d.file("_other.scss", """
@function foo() {
@return #{blue};
}
""")
]).create();
await d.file("test.scss", "@use 'other'").create();
await d.dir("dir", [d.file("_other.scss", "")]).create();

var sass = await runSass(["--quiet-deps", "-I", "dir", "test.scss"]);
expect(sass.stderr, emitsDone);
await sass.shouldExit(0);
});
});
});
});

group("with a bunch of deprecation warnings", () {
Expand Down