Skip to content

Commit

Permalink
Only emit each warning once per source location
Browse files Browse the repository at this point in the history
  • Loading branch information
nex3 committed May 21, 2021
1 parent 112aaa6 commit 829835e
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 19 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 1.33.1

* Don't emit the same warning in the same location multiple times.

## 1.33.0

* Deprecate the use of `/` for division. The new `math.div()` function should be
Expand Down
30 changes: 21 additions & 9 deletions lib/src/visitor/async_evaluate.dart
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,13 @@ class _EvaluateVisitor
/// The logger to use to print warnings.
final Logger _logger;

/// A set of message/location pairs for warnings that have been emitted via
/// [_warn].
///
/// We only want to emit one warning per location, to avoid blowing up users'
/// consoles with redundant warnings.
final _warningsEmitted = <Tuple2<String, SourceSpan>>{};

/// Whether to track source map information.
final bool _sourceMap;

Expand Down Expand Up @@ -1936,7 +1943,7 @@ class _EvaluateVisitor
}

if (node.isGlobal && !_environment.globalVariableExists(node.name)) {
_logger.warn(
_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 "
Expand All @@ -1946,8 +1953,7 @@ class _EvaluateVisitor
"declare new variables. Consider adding "
"`${node.originalName}: null` at the root of the\n"
"stylesheet.",
span: node.span,
trace: _stackTrace(node.span),
node.span,
deprecation: true);
}

Expand Down Expand Up @@ -1987,9 +1993,13 @@ class _EvaluateVisitor
Future<Value?> visitWarnRule(WarnRule node) async {
var value =
await _addExceptionSpanAsync(node, () => node.expression.accept(this));
_logger.warn(
value is SassString ? value.text : _serialize(value, node.expression),
trace: _stackTrace(node.span));
var message =
value is SassString ? value.text : _serialize(value, node.expression);

// Don't use [_warn] because we don't want to pass the span to the logger.
if (_warningsEmitted.add(Tuple2(message, node.span))) {
_warn(message, node.span);
}
return null;
}

Expand Down Expand Up @@ -3087,9 +3097,11 @@ class _EvaluateVisitor
}

/// Emits a warning with the given [message] about the given [span].
void _warn(String message, FileSpan span, {bool deprecation = false}) =>
_logger.warn(message,
span: span, trace: _stackTrace(span), deprecation: deprecation);
void _warn(String message, FileSpan span, {bool deprecation = false}) {
if (!_warningsEmitted.add(Tuple2(message, span))) return;
_logger.warn(message,
span: span, trace: _stackTrace(span), deprecation: deprecation);
}

/// Returns a [SassRuntimeException] with the given [message].
///
Expand Down
32 changes: 22 additions & 10 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: 1fc6b9e6018eba3ac520464abb56e686f4cb9886
// Checksum: 75235f66937f2acdfb9d433bd25f821c63d5ff53
//
// ignore_for_file: unused_import

Expand Down Expand Up @@ -156,6 +156,13 @@ class _EvaluateVisitor
/// The logger to use to print warnings.
final Logger _logger;

/// A set of message/location pairs for warnings that have been emitted via
/// [_warn].
///
/// We only want to emit one warning per location, to avoid blowing up users'
/// consoles with redundant warnings.
final _warningsEmitted = <Tuple2<String, SourceSpan>>{};

/// Whether to track source map information.
final bool _sourceMap;

Expand Down Expand Up @@ -1928,7 +1935,7 @@ class _EvaluateVisitor
}

if (node.isGlobal && !_environment.globalVariableExists(node.name)) {
_logger.warn(
_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 "
Expand All @@ -1938,8 +1945,7 @@ class _EvaluateVisitor
"declare new variables. Consider adding "
"`${node.originalName}: null` at the root of the\n"
"stylesheet.",
span: node.span,
trace: _stackTrace(node.span),
node.span,
deprecation: true);
}

Expand Down Expand Up @@ -1977,9 +1983,13 @@ class _EvaluateVisitor

Value? visitWarnRule(WarnRule node) {
var value = _addExceptionSpan(node, () => node.expression.accept(this));
_logger.warn(
value is SassString ? value.text : _serialize(value, node.expression),
trace: _stackTrace(node.span));
var message =
value is SassString ? value.text : _serialize(value, node.expression);

// Don't use [_warn] because we don't want to pass the span to the logger.
if (_warningsEmitted.add(Tuple2(message, node.span))) {
_warn(message, node.span);
}
return null;
}

Expand Down Expand Up @@ -3058,9 +3068,11 @@ class _EvaluateVisitor
}

/// Emits a warning with the given [message] about the given [span].
void _warn(String message, FileSpan span, {bool deprecation = false}) =>
_logger.warn(message,
span: span, trace: _stackTrace(span), deprecation: deprecation);
void _warn(String message, FileSpan span, {bool deprecation = false}) {
if (!_warningsEmitted.add(Tuple2(message, span))) return;
_logger.warn(message,
span: span, trace: _stackTrace(span), deprecation: deprecation);
}

/// Returns a [SassRuntimeException] with the given [message].
///
Expand Down
28 changes: 28 additions & 0 deletions test/dart_api/logger_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

@TestOn('vm')

import 'dart:async';

import 'package:test/test.dart';
import 'package:source_span/source_span.dart';
import 'package:stack_trace/stack_trace.dart';
Expand All @@ -29,6 +31,32 @@ void main() {
}));
});

test("passes multiple messages with different locations to the logger", () {
var controller = StreamController<String>();
compileString('''
@mixin foo {@warn heck}
@include foo;
@warn heck;
''',
logger: _TestLogger.withWarn((message,
{span, trace, deprecation = false}) =>
controller.add(message)));

expect(controller.stream, emitsInOrder(["heck", "heck"]));
});

test("only passes a message with a single location to the logger once", () {
var count = 0;
compileString(r'''
@for $i from 1 to 5 {
@warn heck;
}
''',
logger: _TestLogger.withWarn(
(message, {span, trace, deprecation = false}) => count++));
expect(count, equals(1));
});

test("stringifies the argument", () {
var mustBeCalled = expectAsync0(() {});
compileString('@warn #abc', logger:
Expand Down

0 comments on commit 829835e

Please sign in to comment.