Silence compiler warnings in dependency callables with --quiet-deps (#…

…1650) Closes #1560
sass · Mar 14, 2022 · 3abcc20 · 3abcc20
1 parent 2baa2f6
commit 3abcc20
Show file tree

Hide file tree

Showing 8 changed files with 185 additions and 19 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -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

diff --git a/lib/src/async_import_cache.dart b/lib/src/async_import_cache.dart
@@ -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;
     }

diff --git a/lib/src/callable/user_defined.dart b/lib/src/callable/user_defined.dart
@@ -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});
 }
diff --git a/lib/src/import_cache.dart b/lib/src/import_cache.dart
@@ -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
 

diff --git a/lib/src/visitor/async_evaluate.dart b/lib/src/visitor/async_evaluate.dart
@@ -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
@@ -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;
   }
 
@@ -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 {
@@ -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;
   }
 
@@ -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(), () {
@@ -2493,6 +2501,8 @@ class _EvaluateVisitor
         });
       });
     });
+    _currentCallable = oldCallable;
+    return result;
   }
 
   /// Evaluates [arguments] as applied to [callable].
@@ -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);

diff --git a/lib/src/visitor/evaluate.dart b/lib/src/visitor/evaluate.dart
@@ -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
 
@@ -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
@@ -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;
   }
 
@@ -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(() {
@@ -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;
   }
 
@@ -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(), () {
@@ -2478,6 +2486,8 @@ class _EvaluateVisitor
         });
       });
     });
+    _currentCallable = oldCallable;
+    return result;
   }
 
   /// Evaluates [arguments] as applied to [callable].
@@ -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);

diff --git a/pubspec.yaml b/pubspec.yaml
@@ -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
 

diff --git a/test/cli/shared.dart b/test/cli/shared.dart
@@ -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", () {