diff --git a/pkg/analyzer/lib/src/dart/resolver/prefix_expression_resolver.dart b/pkg/analyzer/lib/src/dart/resolver/prefix_expression_resolver.dart index 533cd5a7e74c5..7281a90288528 100644 --- a/pkg/analyzer/lib/src/dart/resolver/prefix_expression_resolver.dart +++ b/pkg/analyzer/lib/src/dart/resolver/prefix_expression_resolver.dart @@ -56,7 +56,7 @@ class PrefixExpressionResolver { void resolve(PrefixExpressionImpl node) { var operator = node.operator.type; if (operator == TokenType.BANG) { - _resolveNegation(node, node.operand); + _resolveNegation(node); return; } @@ -257,10 +257,14 @@ class PrefixExpressionResolver { } } - void _resolveNegation(PrefixExpressionImpl node, Expression operand) { + void _resolveNegation(PrefixExpressionImpl node) { + var operand = node.operand; InferenceContext.setType(operand, _typeProvider.boolType); operand.accept(_resolver); + operand = node.operand; + + _resolver.boolExpressionVerifier.checkForNonBoolNegationExpression(operand); _recordStaticType(node, _nonNullable(_typeProvider.boolType)); diff --git a/pkg/analyzer/lib/src/error/bool_expression_verifier.dart b/pkg/analyzer/lib/src/error/bool_expression_verifier.dart new file mode 100644 index 0000000000000..6ded2f9b3ab99 --- /dev/null +++ b/pkg/analyzer/lib/src/error/bool_expression_verifier.dart @@ -0,0 +1,96 @@ +// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'package:analyzer/dart/ast/ast.dart'; +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/error/codes.dart'; +import 'package:analyzer/src/generated/resolver.dart'; +import 'package:meta/meta.dart'; + +/// Helper for verifying expression that should be of type bool. +class BoolExpressionVerifier { + final TypeSystemImpl _typeSystem; + final ErrorReporter _errorReporter; + + final ClassElement _boolElement; + final InterfaceType _boolType; + + BoolExpressionVerifier({ + @required TypeSystemImpl typeSystem, + @required ErrorReporter errorReporter, + }) : _typeSystem = typeSystem, + _errorReporter = errorReporter, + _boolElement = typeSystem.typeProvider.boolElement, + _boolType = typeSystem.typeProvider.boolType; + + /// Check to ensure that the [condition] is of type bool, are. Otherwise an + /// error is reported on the expression. + /// + /// See [StaticTypeWarningCode.NON_BOOL_CONDITION]. + void checkForNonBoolCondition(Expression condition) { + checkForNonBoolExpression( + condition, + errorCode: StaticTypeWarningCode.NON_BOOL_CONDITION, + ); + } + + /// Verify that the given [expression] is of type 'bool', and report + /// [errorCode] if not, or a nullability error if its improperly nullable. + void checkForNonBoolExpression(Expression expression, + {@required ErrorCode errorCode}) { + var type = expression.staticType; + if (!_checkForUseOfVoidResult(expression) && + !_typeSystem.isAssignableTo(type, _boolType)) { + if (type.element == _boolElement) { + _errorReporter.reportErrorForNode( + StaticWarningCode.UNCHECKED_USE_OF_NULLABLE_VALUE, + expression, + ); + } else { + _errorReporter.reportErrorForNode(errorCode, expression); + } + } + } + + /// Checks to ensure that the given [expression] is assignable to bool. + void checkForNonBoolNegationExpression(Expression expression) { + checkForNonBoolExpression( + expression, + errorCode: StaticTypeWarningCode.NON_BOOL_NEGATION_EXPRESSION, + ); + } + + /** + * Check for situations where the result of a method or function is used, when + * it returns 'void'. Or, in rare cases, when other types of expressions are + * void, such as identifiers. + * + * TODO(scheglov) Move this in a separate verifier. + */ + bool _checkForUseOfVoidResult(Expression expression) { + if (expression == null || + !identical(expression.staticType, VoidTypeImpl.instance)) { + return false; + } + + if (expression is MethodInvocation) { + SimpleIdentifier methodName = expression.methodName; + _errorReporter.reportErrorForNode( + StaticWarningCode.USE_OF_VOID_RESULT, + methodName, + ); + } else { + _errorReporter.reportErrorForNode( + StaticWarningCode.USE_OF_VOID_RESULT, + expression, + ); + } + + return true; + } +} diff --git a/pkg/analyzer/lib/src/generated/error_verifier.dart b/pkg/analyzer/lib/src/generated/error_verifier.dart index 3e5c1a8e760cc..a0aef3d4146ed 100644 --- a/pkg/analyzer/lib/src/generated/error_verifier.dart +++ b/pkg/analyzer/lib/src/generated/error_verifier.dart @@ -34,7 +34,6 @@ 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:meta/meta.dart'; /** * A visitor used to traverse an AST structure looking for additional errors and @@ -342,20 +341,6 @@ class ErrorVerifier extends RecursiveAstVisitor { super.visitAsExpression(node); } - @override - void visitAssertInitializer(AssertInitializer node) { - _checkForNonBoolExpression(node.condition, - errorCode: StaticTypeWarningCode.NON_BOOL_EXPRESSION); - super.visitAssertInitializer(node); - } - - @override - void visitAssertStatement(AssertStatement node) { - _checkForNonBoolExpression(node.condition, - errorCode: StaticTypeWarningCode.NON_BOOL_EXPRESSION); - super.visitAssertStatement(node); - } - @override void visitAssignmentExpression(AssignmentExpression node) { TokenType operatorType = node.operator.type; @@ -536,12 +521,6 @@ class ErrorVerifier extends RecursiveAstVisitor { _featureSet = null; } - @override - void visitConditionalExpression(ConditionalExpression node) { - _checkForNonBoolCondition(node.condition); - super.visitConditionalExpression(node); - } - @override void visitConstructorDeclaration(ConstructorDeclaration node) { ExecutableElement outerFunction = _enclosingFunction; @@ -607,12 +586,6 @@ class ErrorVerifier extends RecursiveAstVisitor { super.visitDefaultFormalParameter(node); } - @override - void visitDoStatement(DoStatement node) { - _checkForNonBoolCondition(node.condition); - super.visitDoStatement(node); - } - @override void visitEnumDeclaration(EnumDeclaration node) { ClassElement outerEnum = _enclosingEnum; @@ -751,23 +724,12 @@ class ErrorVerifier extends RecursiveAstVisitor { @override void visitForPartsWithDeclarations(ForPartsWithDeclarations node) { - if (node.condition != null) { - _checkForNonBoolCondition(node.condition); - } if (node.variables != null) { _duplicateDefinitionVerifier.checkForVariables(node.variables); } super.visitForPartsWithDeclarations(node); } - @override - void visitForPartsWithExpression(ForPartsWithExpression node) { - if (node.condition != null) { - _checkForNonBoolCondition(node.condition); - } - super.visitForPartsWithExpression(node); - } - @override void visitFunctionDeclaration(FunctionDeclaration node) { ExecutableElement functionElement = node.declaredElement; @@ -893,18 +855,6 @@ class ErrorVerifier extends RecursiveAstVisitor { super.visitGenericTypeAlias(node); } - @override - void visitIfElement(IfElement node) { - _checkForNonBoolCondition(node.condition); - super.visitIfElement(node); - } - - @override - void visitIfStatement(IfStatement node) { - _checkForNonBoolCondition(node.condition); - super.visitIfStatement(node); - } - @override void visitImplementsClause(ImplementsClause node) { node.interfaces.forEach(_checkForImplicitDynamicType); @@ -1124,9 +1074,7 @@ class ErrorVerifier extends RecursiveAstVisitor { void visitPrefixExpression(PrefixExpression node) { TokenType operatorType = node.operator.type; Expression operand = node.operand; - if (operatorType == TokenType.BANG) { - _checkForNonBoolNegationExpression(operand); - } else { + if (operatorType != TokenType.BANG) { if (operatorType.isIncrementOperator) { _checkForAssignmentToFinal(operand); } @@ -1362,12 +1310,6 @@ class ErrorVerifier extends RecursiveAstVisitor { _isInLateLocalVariable.removeLast(); } - @override - void visitWhileStatement(WhileStatement node) { - _checkForNonBoolCondition(node.condition); - super.visitWhileStatement(node); - } - @override void visitWithClause(WithClause node) { node.mixinTypes.forEach(_checkForImplicitDynamicType); @@ -3957,43 +3899,6 @@ class ErrorVerifier extends RecursiveAstVisitor { [superType, _enclosingClass.displayName]); } - /** - * Check to ensure that the [condition] is of type bool, are. Otherwise an - * error is reported on the expression. - * - * See [StaticTypeWarningCode.NON_BOOL_CONDITION]. - */ - void _checkForNonBoolCondition(Expression condition) { - _checkForNonBoolExpression(condition, - errorCode: StaticTypeWarningCode.NON_BOOL_CONDITION); - } - - /** - * Verify that the given [expression] is of type 'bool', and report - * [errorCode] if not, or a nullability error if its improperly nullable. - */ - void _checkForNonBoolExpression(Expression expression, - {@required ErrorCode errorCode}) { - DartType type = getStaticType(expression); - if (!_checkForUseOfVoidResult(expression) && - !_typeSystem.isAssignableTo(type, _boolType)) { - if (type.element == _boolType.element) { - _errorReporter.reportErrorForNode( - StaticWarningCode.UNCHECKED_USE_OF_NULLABLE_VALUE, expression); - } else { - _errorReporter.reportErrorForNode(errorCode, expression); - } - } - } - - /** - * Checks to ensure that the given [expression] is assignable to bool. - */ - void _checkForNonBoolNegationExpression(Expression expression) { - _checkForNonBoolExpression(expression, - errorCode: StaticTypeWarningCode.NON_BOOL_NEGATION_EXPRESSION); - } - /** * Verify the given map [literal] either: * * has `const modifier` diff --git a/pkg/analyzer/lib/src/generated/resolver.dart b/pkg/analyzer/lib/src/generated/resolver.dart index 54325b71030f6..1408eb663ceb6 100644 --- a/pkg/analyzer/lib/src/generated/resolver.dart +++ b/pkg/analyzer/lib/src/generated/resolver.dart @@ -34,6 +34,7 @@ import 'package:analyzer/src/dart/resolver/prefix_expression_resolver.dart'; import 'package:analyzer/src/dart/resolver/scope.dart'; import 'package:analyzer/src/dart/resolver/type_property_resolver.dart'; import 'package:analyzer/src/dart/resolver/typed_literal_resolver.dart'; +import 'package:analyzer/src/error/bool_expression_verifier.dart'; import 'package:analyzer/src/error/codes.dart'; import 'package:analyzer/src/error/nullable_dereference_verifier.dart'; import 'package:analyzer/src/generated/collection_element_provider.dart'; @@ -200,6 +201,9 @@ class ResolverVisitor extends ScopedVisitor { final CollectionElementProvider _collectionElementProvider; + /// Helper for checking expression that should have the `bool` type. + BoolExpressionVerifier boolExpressionVerifier; + /// Helper for checking potentially nullable dereferences. NullableDereferenceVerifier nullableDereferenceVerifier; @@ -331,6 +335,10 @@ class ResolverVisitor extends ScopedVisitor { super(definingLibrary, source, typeProvider, errorListener, nameScope: nameScope) { this._promoteManager = TypePromotionManager(typeSystem); + this.boolExpressionVerifier = BoolExpressionVerifier( + typeSystem: typeSystem, + errorReporter: errorReporter, + ); this.nullableDereferenceVerifier = NullableDereferenceVerifier( typeSystem: typeSystem, errorReporter: errorReporter, @@ -631,6 +639,10 @@ class ResolverVisitor extends ScopedVisitor { InferenceContext.setType(node.condition, typeProvider.boolType); _flowAnalysis?.flow?.assert_begin(); node.condition?.accept(this); + boolExpressionVerifier.checkForNonBoolExpression( + node.condition, + errorCode: StaticTypeWarningCode.NON_BOOL_EXPRESSION, + ); _flowAnalysis?.flow?.assert_afterCondition(node.condition); node.message?.accept(this); _flowAnalysis?.flow?.assert_end(); @@ -641,6 +653,10 @@ class ResolverVisitor extends ScopedVisitor { InferenceContext.setType(node.condition, typeProvider.boolType); _flowAnalysis?.flow?.assert_begin(); node.condition?.accept(this); + boolExpressionVerifier.checkForNonBoolExpression( + node.condition, + errorCode: StaticTypeWarningCode.NON_BOOL_EXPRESSION, + ); _flowAnalysis?.flow?.assert_afterCondition(node.condition); node.message?.accept(this); _flowAnalysis?.flow?.assert_end(); @@ -768,6 +784,7 @@ class ResolverVisitor extends ScopedVisitor { // TODO(scheglov) Do we need these checks for null? condition?.accept(this); + boolExpressionVerifier.checkForNonBoolCondition(node.condition); Expression thenExpression = node.thenExpression; InferenceContext.setTypeFromNode(thenExpression, node); @@ -910,6 +927,7 @@ class ResolverVisitor extends ScopedVisitor { _flowAnalysis?.flow?.doStatement_conditionBegin(); condition.accept(this); + boolExpressionVerifier.checkForNonBoolCondition(node.condition); _flowAnalysis?.flow?.doStatement_end(node.condition); } @@ -1023,7 +1041,10 @@ class ResolverVisitor extends ScopedVisitor { var condition = forLoopParts.condition; InferenceContext.setType(condition, typeProvider.boolType); _flowAnalysis?.for_conditionBegin(node, condition); - condition?.accept(this); + if (condition != null) { + condition.accept(this); + boolExpressionVerifier.checkForNonBoolCondition(condition); + } _flowAnalysis?.for_bodyBegin(node, condition); node.body?.accept(this); _flowAnalysis?.flow?.for_updaterBegin(); @@ -1108,6 +1129,7 @@ class ResolverVisitor extends ScopedVisitor { _flowAnalysis?.for_conditionBegin(node, condition); if (condition != null) { condition.accept(this); + boolExpressionVerifier.checkForNonBoolCondition(condition); } _flowAnalysis?.for_bodyBegin(node, condition); @@ -1324,6 +1346,8 @@ class ResolverVisitor extends ScopedVisitor { // TODO(scheglov) Do we need these checks for null? condition?.accept(this); + boolExpressionVerifier.checkForNonBoolCondition(node.condition); + CollectionElement thenElement = node.thenElement; if (_flowAnalysis != null) { _flowAnalysis.flow.ifStatement_thenBegin(condition); @@ -1359,6 +1383,8 @@ class ResolverVisitor extends ScopedVisitor { InferenceContext.setType(condition, typeProvider.boolType); condition?.accept(this); + boolExpressionVerifier.checkForNonBoolCondition(node.condition); + Statement thenStatement = node.thenStatement; if (_flowAnalysis != null) { _flowAnalysis.flow.ifStatement_thenBegin(condition); @@ -1801,6 +1827,8 @@ class ResolverVisitor extends ScopedVisitor { _flowAnalysis?.flow?.whileStatement_conditionBegin(node); condition?.accept(this); + boolExpressionVerifier.checkForNonBoolCondition(node.condition); + Statement body = node.body; if (body != null) { _flowAnalysis?.flow?.whileStatement_bodyBegin(node, condition); diff --git a/pkg/analyzer/test/src/diagnostics/non_bool_condition_test.dart b/pkg/analyzer/test/src/diagnostics/non_bool_condition_test.dart index 927bb08f5e2b9..d002a4da7c41c 100644 --- a/pkg/analyzer/test/src/diagnostics/non_bool_condition_test.dart +++ b/pkg/analyzer/test/src/diagnostics/non_bool_condition_test.dart @@ -19,19 +19,20 @@ main() { @reflectiveTest class NonBoolConditionTest extends DriverResolutionTest { + test_forElement() async { + await assertErrorsInCode(''' +var v = [for (; 0;) 1]; +''', [ + error(StaticTypeWarningCode.NON_BOOL_CONDITION, 16, 1), + ]); + } + test_ifElement() async { - await assertErrorsInCode( - ''' -const c = [if (3) 1]; -''', - analysisOptions.experimentStatus.constant_update_2018 - ? [ - error(StaticTypeWarningCode.NON_BOOL_CONDITION, 15, 1), - ] - : [ - error(CompileTimeErrorCode.NON_CONSTANT_LIST_ELEMENT, 11, 8), - error(StaticTypeWarningCode.NON_BOOL_CONDITION, 15, 1), - ]); + await assertErrorsInCode(''' +var v = [if (3) 1]; +''', [ + error(StaticTypeWarningCode.NON_BOOL_CONDITION, 13, 1), + ]); } } @@ -40,6 +41,7 @@ class NonBoolConditionTest_NNBD extends DriverResolutionTest { @override AnalysisOptionsImpl get analysisOptions => AnalysisOptionsImpl()..enabledExperiments = [EnableString.non_nullable]; + test_if_null() async { await assertErrorsInCode(r''' m() {