From 16f181660c0246bfbe30694315646ea2b87853ba Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Fri, 21 May 2021 22:40:30 -0700 Subject: [PATCH] Only omit 5 deprecation warnings per feature by default (#1327) Closes #1323 --- CHANGELOG.md | 9 ++++ lib/sass.dart | 16 ++++++ lib/src/async_compile.dart | 19 ++++++- lib/src/compile.dart | 21 ++++++-- lib/src/executable/compile_stylesheet.dart | 4 ++ lib/src/executable/options.dart | 5 ++ lib/src/logger/terse.dart | 58 ++++++++++++++++++++++ lib/src/parse/scss.dart | 3 +- lib/src/parse/stylesheet.dart | 11 ++-- lib/src/visitor/async_evaluate.dart | 25 ++++++---- lib/src/visitor/evaluate.dart | 27 ++++++---- pubspec.yaml | 2 +- test/cli/shared.dart | 46 +++++++++++++++++ 13 files changed, 214 insertions(+), 32 deletions(-) create mode 100644 lib/src/logger/terse.dart diff --git a/CHANGELOG.md b/CHANGELOG.md index 814bf151c..2f3618899 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,17 +2,26 @@ * Don't emit the same warning in the same location multiple times. +* Cap deprecation warnings at 5 per feature by default. + ### Command Line Interface * Add a `--quiet-deps` flag which silences compiler warnings from stylesheets loaded through `--load-path`s. +* Add a `--verbose` flag which causes the compiler to emit all deprecation + warnings, not just 5 per feature. + ### Dart API * Add a `quietDeps` argument to `compile()`, `compileString()`, `compileAsync()`, and `compileStringAsync()` which silences compiler warnings from stylesheets loaded through importers, load paths, and `package:` URLs. +* Add a `verbose` argument to `compile()`, `compileString()`, `compileAsync()`, + and `compileStringAsync()` which causes the compiler to emit all deprecation + warnings, not just 5 per feature. + ## 1.33.0 * Deprecate the use of `/` for division. The new `math.div()` function should be diff --git a/lib/sass.dart b/lib/sass.dart index 3d56252b7..c52b7187a 100644 --- a/lib/sass.dart +++ b/lib/sass.dart @@ -64,6 +64,10 @@ export 'src/warn.dart' show warn; /// If [quietDeps] is `true`, this will silence compiler warnings emitted for /// stylesheets loaded through [importers], [loadPaths], or [packageConfig]. /// +/// By default, once a deprecation warning for a given feature is printed five +/// times, further warnings for that feature are silenced. If [verbose] is true, +/// all deprecation warnings are printed instead. +/// /// If [sourceMap] is passed, it's passed a [SingleMapping] that indicates which /// sections of the source file(s) correspond to which in the resulting CSS. /// It's called immediately before this method returns, and only if compilation @@ -98,6 +102,7 @@ String compile(String path, Iterable? functions, OutputStyle? style, bool quietDeps = false, + bool verbose = false, void sourceMap(SingleMapping map)?, bool charset = true}) { logger ??= Logger.stderr(color: color); @@ -111,6 +116,7 @@ String compile(String path, functions: functions, style: style, quietDeps: quietDeps, + verbose: verbose, sourceMap: sourceMap != null, charset: charset); result.sourceMap.andThen(sourceMap); @@ -158,6 +164,10 @@ String compile(String path, /// If [quietDeps] is `true`, this will silence compiler warnings emitted for /// stylesheets loaded through [importers], [loadPaths], or [packageConfig]. /// +/// By default, once a deprecation warning for a given feature is printed five +/// times, further warnings for that feature are silenced. If [verbose] is true, +/// all deprecation warnings are printed instead. +/// /// If [sourceMap] is passed, it's passed a [SingleMapping] that indicates which /// sections of the source file(s) correspond to which in the resulting CSS. /// It's called immediately before this method returns, and only if compilation @@ -195,6 +205,7 @@ String compileString(String source, Importer? importer, Object? url, bool quietDeps = false, + bool verbose = false, void sourceMap(SingleMapping map)?, bool charset = true, @Deprecated("Use syntax instead.") bool indented = false}) { @@ -212,6 +223,7 @@ String compileString(String source, importer: importer, url: url, quietDeps: quietDeps, + verbose: verbose, sourceMap: sourceMap != null, charset: charset); result.sourceMap.andThen(sourceMap); @@ -232,6 +244,7 @@ Future compileAsync(String path, Iterable? functions, OutputStyle? style, bool quietDeps = false, + bool verbose = false, void sourceMap(SingleMapping map)?}) async { logger ??= Logger.stderr(color: color); var result = await c.compileAsync(path, @@ -244,6 +257,7 @@ Future compileAsync(String path, functions: functions, style: style, quietDeps: quietDeps, + verbose: verbose, sourceMap: sourceMap != null); result.sourceMap.andThen(sourceMap); return result.css; @@ -266,6 +280,7 @@ Future compileStringAsync(String source, AsyncImporter? importer, Object? url, bool quietDeps = false, + bool verbose = false, void sourceMap(SingleMapping map)?, bool charset = true, @Deprecated("Use syntax instead.") bool indented = false}) async { @@ -283,6 +298,7 @@ Future compileStringAsync(String source, importer: importer, url: url, quietDeps: quietDeps, + verbose: verbose, sourceMap: sourceMap != null, charset: charset); result.sourceMap.andThen(sourceMap); diff --git a/lib/src/async_compile.dart b/lib/src/async_compile.dart index f8ba229e0..0cae4f8e7 100644 --- a/lib/src/async_compile.dart +++ b/lib/src/async_compile.dart @@ -15,6 +15,7 @@ import 'importer.dart'; import 'importer/node.dart'; import 'io.dart'; import 'logger.dart'; +import 'logger/terse.dart'; import 'syntax.dart'; import 'utils.dart'; import 'visitor/async_evaluate.dart'; @@ -35,8 +36,12 @@ Future compileAsync(String path, int? indentWidth, LineFeed? lineFeed, bool quietDeps = false, + bool verbose = false, bool sourceMap = false, bool charset = true}) async { + TerseLogger? terseLogger; + if (!verbose) logger = terseLogger = TerseLogger(logger ?? Logger.stderr()); + // If the syntax is different than the importer would default to, we have to // parse the file manually and we can't store it in the cache. Stylesheet? stylesheet; @@ -52,7 +57,7 @@ Future compileAsync(String path, url: p.toUri(path), logger: logger); } - return await _compileStylesheet( + var result = await _compileStylesheet( stylesheet, logger, importCache, @@ -66,6 +71,9 @@ Future compileAsync(String path, quietDeps, sourceMap, charset); + + terseLogger?.summarize(node: nodeImporter != null); + return result; } /// Like [compileStringAsync] in `lib/sass.dart`, but provides more options to @@ -87,12 +95,16 @@ Future compileStringAsync(String source, LineFeed? lineFeed, Object? url, bool quietDeps = false, + bool verbose = false, bool sourceMap = false, bool charset = true}) async { + TerseLogger? terseLogger; + if (!verbose) logger = terseLogger = TerseLogger(logger ?? Logger.stderr()); + var stylesheet = Stylesheet.parse(source, syntax ?? Syntax.scss, url: url, logger: logger); - return _compileStylesheet( + var result = await _compileStylesheet( stylesheet, logger, importCache, @@ -106,6 +118,9 @@ Future compileStringAsync(String source, quietDeps, sourceMap, charset); + + terseLogger?.summarize(node: nodeImporter != null); + return result; } /// Compiles [stylesheet] and returns its result. diff --git a/lib/src/compile.dart b/lib/src/compile.dart index 51c505c2c..adc072807 100644 --- a/lib/src/compile.dart +++ b/lib/src/compile.dart @@ -5,7 +5,7 @@ // DO NOT EDIT. This file was generated from async_compile.dart. // See tool/grind/synchronize.dart for details. // -// Checksum: bdf01f7ff8eea0efafa6c7c93920caf26e324f4e +// Checksum: 8e813f2ead6e78899ce820e279983278809a7ea5 // // ignore_for_file: unused_import @@ -25,6 +25,7 @@ import 'importer.dart'; import 'importer/node.dart'; import 'io.dart'; import 'logger.dart'; +import 'logger/terse.dart'; import 'syntax.dart'; import 'utils.dart'; import 'visitor/evaluate.dart'; @@ -45,8 +46,12 @@ CompileResult compile(String path, int? indentWidth, LineFeed? lineFeed, bool quietDeps = false, + bool verbose = false, bool sourceMap = false, bool charset = true}) { + TerseLogger? terseLogger; + if (!verbose) logger = terseLogger = TerseLogger(logger ?? Logger.stderr()); + // If the syntax is different than the importer would default to, we have to // parse the file manually and we can't store it in the cache. Stylesheet? stylesheet; @@ -62,7 +67,7 @@ CompileResult compile(String path, url: p.toUri(path), logger: logger); } - return _compileStylesheet( + var result = _compileStylesheet( stylesheet, logger, importCache, @@ -76,6 +81,9 @@ CompileResult compile(String path, quietDeps, sourceMap, charset); + + terseLogger?.summarize(node: nodeImporter != null); + return result; } /// Like [compileString] in `lib/sass.dart`, but provides more options to @@ -97,12 +105,16 @@ CompileResult compileString(String source, LineFeed? lineFeed, Object? url, bool quietDeps = false, + bool verbose = false, bool sourceMap = false, bool charset = true}) { + TerseLogger? terseLogger; + if (!verbose) logger = terseLogger = TerseLogger(logger ?? Logger.stderr()); + var stylesheet = Stylesheet.parse(source, syntax ?? Syntax.scss, url: url, logger: logger); - return _compileStylesheet( + var result = _compileStylesheet( stylesheet, logger, importCache, @@ -116,6 +128,9 @@ CompileResult compileString(String source, quietDeps, sourceMap, charset); + + terseLogger?.summarize(node: nodeImporter != null); + return result; } /// Compiles [stylesheet] and returns its result. diff --git a/lib/src/executable/compile_stylesheet.dart b/lib/src/executable/compile_stylesheet.dart index a1f8cebdc..23a90121a 100644 --- a/lib/src/executable/compile_stylesheet.dart +++ b/lib/src/executable/compile_stylesheet.dart @@ -69,6 +69,7 @@ Future compileStylesheet(ExecutableOptions options, StylesheetGraph graph, importer: FilesystemImporter('.'), style: options.style, quietDeps: options.quietDeps, + verbose: options.verbose, sourceMap: options.emitSourceMap, charset: options.charset) : await compileAsync(source, @@ -77,6 +78,7 @@ Future compileStylesheet(ExecutableOptions options, StylesheetGraph graph, importCache: importCache, style: options.style, quietDeps: options.quietDeps, + verbose: options.verbose, sourceMap: options.emitSourceMap, charset: options.charset); } else { @@ -88,6 +90,7 @@ Future compileStylesheet(ExecutableOptions options, StylesheetGraph graph, importer: FilesystemImporter('.'), style: options.style, quietDeps: options.quietDeps, + verbose: options.verbose, sourceMap: options.emitSourceMap, charset: options.charset) : compile(source, @@ -96,6 +99,7 @@ Future compileStylesheet(ExecutableOptions options, StylesheetGraph graph, importCache: graph.importCache, style: options.style, quietDeps: options.quietDeps, + verbose: options.verbose, sourceMap: options.emitSourceMap, charset: options.charset); } diff --git a/lib/src/executable/options.dart b/lib/src/executable/options.dart index e3b0fed27..8687c0d7e 100644 --- a/lib/src/executable/options.dart +++ b/lib/src/executable/options.dart @@ -102,6 +102,8 @@ class ExecutableOptions { ..addFlag('quiet-deps', help: "Don't print compiler warnings from dependencies.\n" "Stylesheets imported through load paths count as dependencies.") + ..addFlag('verbose', + help: "Print all deprecation warnings even when they're repetitive.") ..addFlag('trace', help: 'Print full Dart stack traces for exceptions.') ..addFlag('help', abbr: 'h', help: 'Print this usage information.', negatable: false) @@ -172,6 +174,9 @@ class ExecutableOptions { /// Whether to silence warnings in dependencies. bool get quietDeps => _options['quiet-deps'] as bool; + /// Whether to emit all repetitive deprecation warnings. + bool get verbose => _options['verbose'] as bool; + /// The logger to use to emit messages from Sass. Logger get logger => quiet ? Logger.quiet : Logger.stderr(color: color); diff --git a/lib/src/logger/terse.dart b/lib/src/logger/terse.dart new file mode 100644 index 000000000..83258da15 --- /dev/null +++ b/lib/src/logger/terse.dart @@ -0,0 +1,58 @@ +// Copyright 2018 Google Inc. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +import 'package:collection/collection.dart'; +import 'package:source_span/source_span.dart'; +import 'package:stack_trace/stack_trace.dart'; + +import '../logger.dart'; + +/// The maximum number of repetitions of the same warning [TerseLogger] will +/// emit before hiding the rest. +const _maxRepetitions = 5; + +/// A logger that wraps an inner logger to omit repeated deprecation warnings. +/// +/// A warning is considered "repeated" if the first paragraph is the same as +/// another warning that's already been emitted. +class TerseLogger implements Logger { + /// A map from the first paragraph of a warning to the number of times this + /// logger has emitted a warning with that line. + final _warningCounts = {}; + + final Logger _inner; + + TerseLogger(this._inner); + + void warn(String message, + {FileSpan? span, Trace? trace, bool deprecation = false}) { + if (deprecation) { + var firstParagraph = message.split("\n\n").first; + var count = _warningCounts[firstParagraph] = + (_warningCounts[firstParagraph] ?? 0) + 1; + if (count > _maxRepetitions) return; + } + + _inner.warn(message, span: span, trace: trace, deprecation: deprecation); + } + + void debug(String message, SourceSpan span) => _inner.debug(message, span); + + /// Prints a warning indicating the number of deprecation warnings that were + /// omitted. + /// + /// The [node] flag indicates whether this is running in Node.js mode, in + /// which case it doesn't mention "verbose mode" because the Node API doesn't + /// support that. + void summarize({required bool node}) { + var total = _warningCounts.values + .where((count) => count > _maxRepetitions) + .map((count) => count - _maxRepetitions) + .sum; + if (total > 0) { + _inner.warn("$total repetitive deprecation warnings omitted." + + (node ? "" : "\nRun in verbose mode to see all warnings.")); + } + } +} diff --git a/lib/src/parse/scss.dart b/lib/src/parse/scss.dart index a03b7e6f5..c961045f6 100644 --- a/lib/src/parse/scss.dart +++ b/lib/src/parse/scss.dart @@ -48,7 +48,8 @@ class ScssParser extends StylesheetParser { logger.warn( '@elseif is deprecated and will not be supported in future Sass ' 'versions.\n' - 'Use "@else if" instead.', + '\n' + 'Recommendation: @else if', span: scanner.spanFrom(beforeAt), deprecation: true); scanner.position -= 2; diff --git a/lib/src/parse/stylesheet.dart b/lib/src/parse/stylesheet.dart index 72bff4deb..024598cbe 100644 --- a/lib/src/parse/stylesheet.dart +++ b/lib/src/parse/stylesheet.dart @@ -1334,10 +1334,13 @@ abstract class StylesheetParser extends Parser { var value = buffer.interpolation(scanner.spanFrom(valueStart)); return _withChildren(_statement, start, (children, span) { if (needsDeprecationWarning) { - logger.warn(""" -@-moz-document is deprecated and support will be removed from Sass in a future -release. For details, see http://bit.ly/moz-document. -""", span: span, deprecation: true); + logger.warn( + "@-moz-document is deprecated and support will be removed in Dart " + "Sass 2.0.0.\n" + "\n" + "For details, see http://bit.ly/moz-document.", + span: span, + deprecation: true); } return AtRule(name, span, value: value, children: children); diff --git a/lib/src/visitor/async_evaluate.dart b/lib/src/visitor/async_evaluate.dart index 9320baa0a..d3aad9d84 100644 --- a/lib/src/visitor/async_evaluate.dart +++ b/lib/src/visitor/async_evaluate.dart @@ -436,8 +436,10 @@ class _EvaluateVisitor if (function is SassString) { warn( - "Passing a string to call() is deprecated and will be illegal\n" - "in Dart Sass 2.0.0. Use call(get-function($function)) instead.", + "Passing a string to call() is deprecated and will be illegal in " + "Dart Sass 2.0.0.\n" + "\n" + "Recommendation: call(get-function($function))", deprecation: true); var callableNode = _callableNode!; @@ -1972,14 +1974,17 @@ class _EvaluateVisitor if (node.isGlobal && !_environment.globalVariableExists(node.name)) { _warn( _environment.atRoot - ? "As of Dart Sass 2.0.0, !global assignments won't be able to\n" - "declare new variables. Since this assignment is at the root " - "of the stylesheet,\n" - "the !global flag is unnecessary and can safely be removed." - : "As of Dart Sass 2.0.0, !global assignments won't be able to\n" - "declare new variables. Consider adding " - "`${node.originalName}: null` at the root of the\n" - "stylesheet.", + ? "As of Dart Sass 2.0.0, !global assignments won't be able to " + "declare new variables.\n" + "\n" + "Since this assignment is at the root of the stylesheet, the " + "!global flag is\n" + "unnecessary and can safely be removed." + : "As of Dart Sass 2.0.0, !global assignments won't be able to " + "declare new variables.\n" + "\n" + "Recommendation: add `${node.originalName}: null` at the " + "stylesheet root.", node.span, deprecation: true); } diff --git a/lib/src/visitor/evaluate.dart b/lib/src/visitor/evaluate.dart index fb53ea99d..f61b28444 100644 --- 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: 1d5f3cb78c4567e19f106241ee67c70f4ae01df1 +// Checksum: 24b9012f1cf8908b2cbde11cd10974113d4c8163 // // ignore_for_file: unused_import @@ -443,8 +443,10 @@ class _EvaluateVisitor if (function is SassString) { warn( - "Passing a string to call() is deprecated and will be illegal\n" - "in Dart Sass 2.0.0. Use call(get-function($function)) instead.", + "Passing a string to call() is deprecated and will be illegal in " + "Dart Sass 2.0.0.\n" + "\n" + "Recommendation: call(get-function($function))", deprecation: true); var callableNode = _callableNode!; @@ -1963,14 +1965,17 @@ class _EvaluateVisitor if (node.isGlobal && !_environment.globalVariableExists(node.name)) { _warn( _environment.atRoot - ? "As of Dart Sass 2.0.0, !global assignments won't be able to\n" - "declare new variables. Since this assignment is at the root " - "of the stylesheet,\n" - "the !global flag is unnecessary and can safely be removed." - : "As of Dart Sass 2.0.0, !global assignments won't be able to\n" - "declare new variables. Consider adding " - "`${node.originalName}: null` at the root of the\n" - "stylesheet.", + ? "As of Dart Sass 2.0.0, !global assignments won't be able to " + "declare new variables.\n" + "\n" + "Since this assignment is at the root of the stylesheet, the " + "!global flag is\n" + "unnecessary and can safely be removed." + : "As of Dart Sass 2.0.0, !global assignments won't be able to " + "declare new variables.\n" + "\n" + "Recommendation: add `${node.originalName}: null` at the " + "stylesheet root.", node.span, deprecation: true); } diff --git a/pubspec.yaml b/pubspec.yaml index 4d3b98afa..159a963e7 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: sass -version: 1.34.0-dev +version: 1.34.0 description: A Sass implementation in Dart. author: Sass Team homepage: https://github.com/sass/dart-sass diff --git a/test/cli/shared.dart b/test/cli/shared.dart index 6e6d15ea4..5625eeab5 100644 --- a/test/cli/shared.dart +++ b/test/cli/shared.dart @@ -483,6 +483,52 @@ void sharedTests( }); }); + group("with a bunch of deprecation warnings", () { + setUp(() async { + await d.file("test.scss", r""" + $_: call("inspect", null); + $_: call("rgb", 0, 0, 0); + $_: call("nth", null, 1); + $_: call("join", null, null); + $_: call("if", true, 1, 2); + $_: call("hsl", 0, 100%, 100%); + + $_: 1/2; + $_: 1/3; + $_: 1/4; + $_: 1/5; + $_: 1/6; + $_: 1/7; + """).create(); + }); + + test("without --verbose, only prints five", () async { + var sass = await runSass(["test.scss"]); + expect(sass.stderr, + emitsInOrder(List.filled(5, emitsThrough(contains("call()"))))); + expect(sass.stderr, neverEmits(contains("call()"))); + + expect(sass.stderr, + emitsInOrder(List.filled(5, emitsThrough(contains("math.div"))))); + expect(sass.stderr, neverEmits(contains("math.div()"))); + + expect(sass.stderr, + emitsThrough(contains("2 repetitive deprecation warnings omitted."))); + }); + + test("with --verbose, prints all", () async { + var sass = await runSass(["--verbose", "test.scss"]); + expect(sass.stderr, + neverEmits(contains("2 repetitive deprecation warnings omitted."))); + + expect(sass.stderr, + emitsInOrder(List.filled(6, emitsThrough(contains("call()"))))); + + expect(sass.stderr, + emitsInOrder(List.filled(6, emitsThrough(contains("math.div"))))); + }); + }); + group("with --charset", () { test("doesn't emit @charset for a pure-ASCII stylesheet", () async { await d.file("test.scss", "a {b: c}").create();