Skip to content

Commit

Permalink
Add an analysis option allowing linter exceptions to be propagated.
Browse files Browse the repository at this point in the history
This will be used in internal builds to ensure that an exception
occuring during linting fails the build; this should help us be more
confident in the robustness of the linter code.

Change-Id: I4eaa47044b32b45433a67cc919ad047663dc771c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/208141
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Paul Berry <[email protected]>
  • Loading branch information
stereotype441 authored and [email protected] committed Jul 30, 2021
1 parent d135395 commit b13dc22
Show file tree
Hide file tree
Showing 8 changed files with 34 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class AnalysisOptionsGenerator extends YamlCompletionGenerator {
AnalyzerOptions.chromeOsManifestChecks: EmptyProducer(),
}),
AnalyzerOptions.plugins: EmptyProducer(),
AnalyzerOptions.propagateLinterExceptions: EmptyProducer(),
AnalyzerOptions.strong_mode: MapProducer({
AnalyzerOptions.declarationCasts: EmptyProducer(),
AnalyzerOptions.implicitCasts: EmptyProducer(),
Expand Down
6 changes: 5 additions & 1 deletion pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import 'package:analyzer/src/context/source.dart';
import 'package:analyzer/src/dart/analysis/file_state.dart';
import 'package:analyzer/src/dart/analysis/testing_data.dart';
import 'package:analyzer/src/dart/ast/ast.dart';
import 'package:analyzer/src/dart/ast/utilities.dart';
import 'package:analyzer/src/dart/constant/compute.dart';
import 'package:analyzer/src/dart/constant/constant_verifier.dart';
import 'package:analyzer/src/dart/constant/evaluation.dart';
Expand Down Expand Up @@ -351,7 +352,10 @@ class LibraryAnalyzer {
}

// Run lints that handle specific node types.
unit.accept(LinterVisitor(nodeRegistry));
unit.accept(LinterVisitor(
nodeRegistry,
LinterExceptionHandler(_analysisOptions.propagateLinterExceptions)
.logException));
}

void _computeVerifyErrors(FileState file, CompilationUnit unit) {
Expand Down
9 changes: 9 additions & 0 deletions pkg/analyzer/lib/src/dart/ast/utilities.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1306,6 +1306,12 @@ class DeferredLibraryReferenceDetector extends RecursiveAstVisitor<void> {
///
/// Clients may not extend, implement or mix-in this class.
class LinterExceptionHandler {
/// Indicates whether linter exceptions should be propagated to the caller (by
/// re-throwing them)
final bool propagateLinterExceptions;

LinterExceptionHandler(this.propagateLinterExceptions);

/// A method that can be passed to the `LinterVisitor` constructor to handle
/// exceptions that occur during linting.
void logException(
Expand All @@ -1326,6 +1332,9 @@ class LinterExceptionHandler {
// TODO(39284): should this exception be silent?
AnalysisEngine.instance.instrumentationService.logException(
SilentException(buffer.toString(), exception, stackTrace));
if (propagateLinterExceptions) {
throw exception;
}
}
}

Expand Down
5 changes: 4 additions & 1 deletion pkg/analyzer/lib/src/dart/micro/library_analyzer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,10 @@ class LibraryAnalyzer {
}

// Run lints that handle specific node types.
unit.accept(LinterVisitor(nodeRegistry));
unit.accept(LinterVisitor(
nodeRegistry,
LinterExceptionHandler(_analysisOptions.propagateLinterExceptions)
.logException));
}

void _computeVerifyErrors(FileState file, CompilationUnit unit) {
Expand Down
6 changes: 6 additions & 0 deletions pkg/analyzer/lib/src/generated/engine.dart
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,10 @@ class AnalysisOptionsImpl implements AnalysisOptions {
/// This option is experimental and subject to change.
bool implicitDynamic = true;

/// Indicates whether linter exceptions should be propagated to the caller (by
/// re-throwing them)
bool propagateLinterExceptions = false;

/// A flag indicating whether inference failures are allowed, off by default.
///
/// This option is experimental and subject to change.
Expand Down Expand Up @@ -319,6 +323,7 @@ class AnalysisOptionsImpl implements AnalysisOptions {
if (options is AnalysisOptionsImpl) {
implicitCasts = options.implicitCasts;
implicitDynamic = options.implicitDynamic;
propagateLinterExceptions = options.propagateLinterExceptions;
strictInference = options.strictInference;
strictRawTypes = options.strictRawTypes;
}
Expand Down Expand Up @@ -382,6 +387,7 @@ class AnalysisOptionsImpl implements AnalysisOptions {
// Append boolean flags.
buffer.addBool(implicitCasts);
buffer.addBool(implicitDynamic);
buffer.addBool(propagateLinterExceptions);
buffer.addBool(strictInference);
buffer.addBool(strictRawTypes);
buffer.addBool(useFastaParser);
Expand Down
2 changes: 1 addition & 1 deletion pkg/analyzer/lib/src/lint/linter_visitor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class LinterVisitor extends RecursiveAstVisitor<void> {

LinterVisitor(this.registry, [LintRuleExceptionHandler? exceptionHandler])
: exceptionHandler =
exceptionHandler ?? LinterExceptionHandler().logException;
exceptionHandler ?? LinterExceptionHandler(true).logException;

@override
void visitAdjacentStrings(AdjacentStrings node) {
Expand Down
6 changes: 6 additions & 0 deletions pkg/analyzer/lib/src/task/options.dart
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,8 @@ class AnalyzerOptions {
/// Ways to say `include`.
static const List<String> includeSynonyms = ['include', 'true'];

static const String propagateLinterExceptions = 'propagate-linter-exceptions';

/// Ways to say `true` or `false`.
static const List<String> trueOrFalse = ['true', 'false'];

Expand All @@ -163,6 +165,7 @@ class AnalyzerOptions {
language,
optionalChecks,
plugins,
propagateLinterExceptions,
strong_mode,
];

Expand Down Expand Up @@ -805,6 +808,9 @@ class _OptionsProcessor {
if (feature == AnalyzerOptions.implicitDynamic) {
options.implicitDynamic = boolValue;
}
if (feature == AnalyzerOptions.propagateLinterExceptions) {
options.propagateLinterExceptions = boolValue;
}
}
}

Expand Down
2 changes: 2 additions & 0 deletions pkg/analyzer/test/src/dart/analysis/context_builder_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,8 @@ environment:
);
expect(actual.implicitCasts, expected.implicitCasts);
expect(actual.implicitDynamic, expected.implicitDynamic);
expect(
actual.propagateLinterExceptions, expected.propagateLinterExceptions);
expect(actual.strictInference, expected.strictInference);
expect(actual.strictRawTypes, expected.strictRawTypes);
}
Expand Down

0 comments on commit b13dc22

Please sign in to comment.