Move DEAD_NULL_COALESCE reporting to ErrorVerifier.

So, it is reported even when hints are disabled.

Change-Id: Id2df8faa5421fac00e01755a70aedf3d3f570f10
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/135492
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
This commit is contained in:
Konstantin Shcheglov 2020-02-13 19:42:48 +00:00 committed by commit-bot@chromium.org
parent 93bb4b0f1b
commit ed705e8c84
6 changed files with 50 additions and 37 deletions

View file

@ -10,14 +10,12 @@ import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/dart/element/type.dart';
import 'package:analyzer/error/error.dart';
import 'package:analyzer/error/listener.dart';
import 'package:analyzer/src/dart/element/type.dart';
import 'package:analyzer/src/dart/resolver/exit_detector.dart';
import 'package:analyzer/src/dart/resolver/scope.dart';
import 'package:analyzer/src/error/codes.dart';
import 'package:analyzer/src/generated/constant.dart';
import 'package:analyzer/src/generated/resolver.dart';
import 'package:analyzer/src/generated/type_system.dart';
import 'package:analyzer/src/task/strong/checker.dart';
/// A visitor that finds dead code and unused labels.
class DeadCodeVerifier extends RecursiveAstVisitor<void> {
@ -30,9 +28,6 @@ class DeadCodeVerifier extends RecursiveAstVisitor<void> {
/// The object used to track the usage of labels within a given label scope.
_LabelTracker labelTracker;
/// Is `true` if the library being analyzed is non-nullable by default.
final bool _isNonNullableByDefault;
/// Initialize a newly created dead code verifier that will report dead code
/// to the given [errorReporter] and will use the given [typeSystem] if one is
/// provided.
@ -44,25 +39,13 @@ class DeadCodeVerifier extends RecursiveAstVisitor<void> {
isNonNullableByDefault: false,
strictInference: false,
typeProvider: null,
),
_isNonNullableByDefault = featureSet.isEnabled(Feature.non_nullable);
@override
void visitAssignmentExpression(AssignmentExpression node) {
TokenType operatorType = node.operator.type;
if (operatorType == TokenType.QUESTION_QUESTION_EQ) {
_checkForDeadNullCoalesce(
getReadType(node.leftHandSide), node.rightHandSide);
}
super.visitAssignmentExpression(node);
}
);
@override
void visitBinaryExpression(BinaryExpression node) {
Token operator = node.operator;
bool isAmpAmp = operator.type == TokenType.AMPERSAND_AMPERSAND;
bool isBarBar = operator.type == TokenType.BAR_BAR;
bool isQuestionQuestion = operator.type == TokenType.QUESTION_QUESTION;
if (isAmpAmp || isBarBar) {
Expression lhsCondition = node.leftOperand;
if (!_isDebugConstant(lhsCondition)) {
@ -105,9 +88,6 @@ class DeadCodeVerifier extends RecursiveAstVisitor<void> {
// return null;
// }
// }
} else if (isQuestionQuestion) {
_checkForDeadNullCoalesce(
getReadType(node.leftOperand), node.rightOperand);
}
super.visitBinaryExpression(node);
}
@ -394,17 +374,6 @@ class DeadCodeVerifier extends RecursiveAstVisitor<void> {
}
}
void _checkForDeadNullCoalesce(TypeImpl lhsType, Expression rhs) {
if (!_isNonNullableByDefault) return;
if (_typeSystem.isStrictlyNonNullable(lhsType)) {
_errorReporter.reportErrorForNode(
StaticWarningCode.DEAD_NULL_COALESCE,
rhs,
);
}
}
/// Given some list of [statements], loop through the list searching for dead
/// statements. If [allowMandated] is true, then allow dead statements that
/// are mandated by the language spec. This allows for a final break,

View file

@ -35,6 +35,7 @@ import 'package:analyzer/src/generated/java_engine.dart';
import 'package:analyzer/src/generated/parser.dart' show ParserErrorCode;
import 'package:analyzer/src/generated/resolver.dart';
import 'package:analyzer/src/generated/sdk.dart' show DartSdk, SdkLibrary;
import 'package:analyzer/src/task/strong/checker.dart';
/**
* A visitor used to traverse an AST structure looking for additional errors and
@ -342,6 +343,10 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
} else {
_checkForArgumentTypeNotAssignableForArgument(rhs);
}
if (operatorType == TokenType.QUESTION_QUESTION_EQ) {
_checkForDeadNullCoalesce(
getReadType(node.leftHandSide), node.rightHandSide);
}
_checkForAssignmentToFinal(lhs);
super.visitAssignmentExpression(node);
}
@ -371,6 +376,11 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
_checkForArgumentTypeNotAssignableForArgument(node.rightOperand);
}
if (type == TokenType.QUESTION_QUESTION) {
_checkForDeadNullCoalesce(
getReadType(node.leftOperand), node.rightOperand);
}
_checkForUseOfVoidResult(node.leftOperand);
super.visitBinaryExpression(node);
@ -2310,6 +2320,17 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
}
}
void _checkForDeadNullCoalesce(TypeImpl lhsType, Expression rhs) {
if (!_isNonNullableByDefault) return;
if (_typeSystem.isStrictlyNonNullable(lhsType)) {
_errorReporter.reportErrorForNode(
StaticWarningCode.DEAD_NULL_COALESCE,
rhs,
);
}
}
/**
* Verify that the given default formal [parameter] is not part of a function
* typed parameter.

View file

@ -27,3 +27,8 @@ ERROR|STATIC_WARNING|NEW_WITH_UNDEFINED_CONSTRUCTOR|lib/collection/collection.da
ERROR|STATIC_WARNING|NEW_WITH_UNDEFINED_CONSTRUCTOR|lib/collection/collection.dart|3026|37|5|The class 'List' doesn't have a constructor named 'empty'.
ERROR|STATIC_WARNING|NEW_WITH_UNDEFINED_CONSTRUCTOR|lib/internal/internal.dart|1205|37|5|The class 'List' doesn't have a constructor named 'empty'.
ERROR|STATIC_WARNING|NEW_WITH_UNDEFINED_CONSTRUCTOR|lib/internal/internal.dart|1663|52|5|The class 'List' doesn't have a constructor named 'empty'.
WARNING|STATIC_WARNING|DEAD_NULL_COALESCE|lib/_http/http.dart|1463|39|5|The left operand can't be null, so the right operand is never executed.
WARNING|STATIC_WARNING|DEAD_NULL_COALESCE|lib/_http/http.dart|8345|60|5|The left operand can't be null, so the right operand is never executed.
WARNING|STATIC_WARNING|DEAD_NULL_COALESCE|lib/_http/http.dart|9272|54|5|The left operand can't be null, so the right operand is never executed.
WARNING|STATIC_WARNING|DEAD_NULL_COALESCE|lib/html/dart2js/html_dart2js.dart|4075|25|2|The left operand can't be null, so the right operand is never executed.
WARNING|STATIC_WARNING|DEAD_NULL_COALESCE|lib/io/io.dart|9167|16|1|The left operand can't be null, so the right operand is never executed.

View file

@ -1 +1,7 @@
WARNING|STATIC_WARNING|DEAD_NULL_COALESCE|lib/_http/http.dart|1463|39|5|The left operand can't be null, so the right operand is never executed.
WARNING|STATIC_WARNING|DEAD_NULL_COALESCE|lib/_http/http.dart|8345|60|5|The left operand can't be null, so the right operand is never executed.
WARNING|STATIC_WARNING|DEAD_NULL_COALESCE|lib/_http/http.dart|9272|54|5|The left operand can't be null, so the right operand is never executed.
WARNING|STATIC_WARNING|DEAD_NULL_COALESCE|lib/collection/collection.dart|1076|46|13|The left operand can't be null, so the right operand is never executed.
WARNING|STATIC_WARNING|DEAD_NULL_COALESCE|lib/developer/developer.dart|332|25|23|The left operand can't be null, so the right operand is never executed.
WARNING|STATIC_WARNING|DEAD_NULL_COALESCE|lib/html/dart2js/html_dart2js.dart|4075|25|2|The left operand can't be null, so the right operand is never executed.
WARNING|STATIC_WARNING|DEAD_NULL_COALESCE|lib/io/io.dart|9167|16|1|The left operand can't be null, so the right operand is never executed.

View file

@ -13,10 +13,20 @@ void f1(
int? nullableInt,
int nonNullInt,
) {
(nullableInt ?? nonNullInt) + 1; //# 00: ok
(nullableInt ?? nullableInt) + 1; //# 01: compile-time error
(nonNullInt ?? nullableInt) + 1; //# 02: compile-time error
(nonNullInt ?? nonNullInt) + 1; //# 03: ok
(nullableInt ?? nonNullInt) + 1;
(nullableInt ?? nullableInt) + 1;
//^^^^^^^^^^^^^^^^^^^^^^^^^^^^
// [analyzer] STATIC_WARNING.UNCHECKED_USE_OF_NULLABLE_VALUE
// [cfe] unspecified
(nonNullInt ?? nullableInt) + 1;
// ^^^^^^^^^^^
// [analyzer] STATIC_WARNING.DEAD_NULL_COALESCE
//^^^^^^^^^^^^^^^^^^^^^^^^^^^
// [analyzer] STATIC_WARNING.UNCHECKED_USE_OF_NULLABLE_VALUE
// [cfe] unspecified
(nonNullInt ?? nonNullInt) + 1;
// ^^^^^^^^^^
// [analyzer] STATIC_WARNING.DEAD_NULL_COALESCE
}
// TODO(mfairhurst) add cases with type parameter types

View file

@ -37,6 +37,8 @@ main() {
// { a is bool ?? true : 3 } is parsed as a map literal { ((a is bool) ?? true) : 3 }.
a = true;
var x5 = {a is bool ?? true : 3};
// ^^^^
// [analyzer] STATIC_WARNING.DEAD_NULL_COALESCE
Expect.type<Map<dynamic, dynamic>>(x5);
Map<dynamic, dynamic> y5 = x5;
}