Skip to content

Commit

Permalink
Extract BoolExpressionVerifier.
Browse files Browse the repository at this point in the history
Mostly because it also reports UNCHECKED_USE_OF_NULLABLE_VALUE, and
we want to report it during resolution, to use for migration.

Change-Id: I89f988d9be34a70dfe0e590a5fede2696423c89c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/132029
Reviewed-by: Paul Berry <paulberry@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
  • Loading branch information
scheglov authored and commit-bot@chromium.org committed Jan 16, 2020
1 parent 445c084 commit 356fa11
Show file tree
Hide file tree
Showing 5 changed files with 146 additions and 111 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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));

Expand Down
96 changes: 96 additions & 0 deletions pkg/analyzer/lib/src/error/bool_expression_verifier.dart
Original file line number Diff line number Diff line change
@@ -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;
}
}
97 changes: 1 addition & 96 deletions pkg/analyzer/lib/src/generated/error_verifier.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -342,20 +341,6 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
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;
Expand Down Expand Up @@ -536,12 +521,6 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
_featureSet = null;
}

@override
void visitConditionalExpression(ConditionalExpression node) {
_checkForNonBoolCondition(node.condition);
super.visitConditionalExpression(node);
}

@override
void visitConstructorDeclaration(ConstructorDeclaration node) {
ExecutableElement outerFunction = _enclosingFunction;
Expand Down Expand Up @@ -607,12 +586,6 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
super.visitDefaultFormalParameter(node);
}

@override
void visitDoStatement(DoStatement node) {
_checkForNonBoolCondition(node.condition);
super.visitDoStatement(node);
}

@override
void visitEnumDeclaration(EnumDeclaration node) {
ClassElement outerEnum = _enclosingEnum;
Expand Down Expand Up @@ -751,23 +724,12 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {

@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;
Expand Down Expand Up @@ -893,18 +855,6 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
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);
Expand Down Expand Up @@ -1124,9 +1074,7 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
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);
}
Expand Down Expand Up @@ -1362,12 +1310,6 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
_isInLateLocalVariable.removeLast();
}

@override
void visitWhileStatement(WhileStatement node) {
_checkForNonBoolCondition(node.condition);
super.visitWhileStatement(node);
}

@override
void visitWithClause(WithClause node) {
node.mixinTypes.forEach(_checkForImplicitDynamicType);
Expand Down Expand Up @@ -3957,43 +3899,6 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
[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`
Expand Down
Loading

0 comments on commit 356fa11

Please sign in to comment.