From d19b2b0349eb6771698117cc0016874d6a676cd4 Mon Sep 17 00:00:00 2001 From: Konstantin Shcheglov Date: Thu, 16 Jan 2020 22:55:49 +0000 Subject: [PATCH] Use NullableDereferenceVerifier in resolver instead of ErrorVerifier. Change-Id: Ib360bd9091656e04820879052a04e6ee23c495d7 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/132161 Commit-Queue: Konstantin Shcheglov Reviewed-by: Brian Wilkerson Reviewed-by: Paul Berry --- .../assignment_expression_resolver.dart | 6 +++ ...nction_expression_invocation_resolver.dart | 6 +++ .../resolver/method_invocation_resolver.dart | 3 +- .../error/nullable_dereference_verifier.dart | 48 ------------------- .../lib/src/generated/error_verifier.dart | 24 +--------- pkg/analyzer/lib/src/generated/resolver.dart | 21 +++++++- .../invalid_use_of_null_value_test.dart | 1 + 7 files changed, 35 insertions(+), 74 deletions(-) diff --git a/pkg/analyzer/lib/src/dart/resolver/assignment_expression_resolver.dart b/pkg/analyzer/lib/src/dart/resolver/assignment_expression_resolver.dart index e1d0634110f32..a9cb1f38c9918 100644 --- a/pkg/analyzer/lib/src/dart/resolver/assignment_expression_resolver.dart +++ b/pkg/analyzer/lib/src/dart/resolver/assignment_expression_resolver.dart @@ -16,6 +16,7 @@ import 'package:analyzer/src/dart/resolver/invocation_inference_helper.dart'; import 'package:analyzer/src/dart/resolver/resolution_result.dart'; import 'package:analyzer/src/dart/resolver/type_property_resolver.dart'; import 'package:analyzer/src/error/codes.dart'; +import 'package:analyzer/src/error/nullable_dereference_verifier.dart'; import 'package:analyzer/src/generated/element_type_provider.dart'; import 'package:analyzer/src/generated/resolver.dart'; import 'package:analyzer/src/generated/type_system.dart'; @@ -49,6 +50,9 @@ class AssignmentExpressionResolver { bool get _isNonNullableByDefault => _typeSystem.isNonNullableByDefault; + NullableDereferenceVerifier get _nullableDereferenceVerifier => + _resolver.nullableDereferenceVerifier; + TypeProvider get _typeProvider => _resolver.typeProvider; TypeSystemImpl get _typeSystem => _resolver.typeSystem; @@ -66,6 +70,8 @@ class AssignmentExpressionResolver { if (operator == TokenType.EQ || operator == TokenType.QUESTION_QUESTION_EQ) { InferenceContext.setType(right, left.staticType); + } else { + _nullableDereferenceVerifier.expression(left); } right?.accept(_resolver); diff --git a/pkg/analyzer/lib/src/dart/resolver/function_expression_invocation_resolver.dart b/pkg/analyzer/lib/src/dart/resolver/function_expression_invocation_resolver.dart index 516298a8f5d30..1db222a0b26a5 100644 --- a/pkg/analyzer/lib/src/dart/resolver/function_expression_invocation_resolver.dart +++ b/pkg/analyzer/lib/src/dart/resolver/function_expression_invocation_resolver.dart @@ -12,6 +12,7 @@ import 'package:analyzer/src/dart/resolver/extension_member_resolver.dart'; import 'package:analyzer/src/dart/resolver/invocation_inference_helper.dart'; import 'package:analyzer/src/dart/resolver/type_property_resolver.dart'; import 'package:analyzer/src/error/codes.dart'; +import 'package:analyzer/src/error/nullable_dereference_verifier.dart'; import 'package:analyzer/src/generated/element_type_provider.dart'; import 'package:analyzer/src/generated/resolver.dart'; import 'package:meta/meta.dart'; @@ -35,6 +36,9 @@ class FunctionExpressionInvocationResolver { ExtensionMemberResolver get _extensionResolver => _resolver.extensionResolver; + NullableDereferenceVerifier get _nullableDereferenceVerifier => + _resolver.nullableDereferenceVerifier; + void resolve(FunctionExpressionInvocationImpl node) { var function = node.function; @@ -43,6 +47,8 @@ class FunctionExpressionInvocationResolver { return; } + _nullableDereferenceVerifier.expression(function); + var receiverType = function.staticType; if (receiverType is FunctionType) { _resolve(node, receiverType); diff --git a/pkg/analyzer/lib/src/dart/resolver/method_invocation_resolver.dart b/pkg/analyzer/lib/src/dart/resolver/method_invocation_resolver.dart index 92b843406c57a..2aac48dc2a8a5 100644 --- a/pkg/analyzer/lib/src/dart/resolver/method_invocation_resolver.dart +++ b/pkg/analyzer/lib/src/dart/resolver/method_invocation_resolver.dart @@ -422,8 +422,7 @@ class MethodInvocationResolver { InterfaceType receiverType, SimpleIdentifier nameNode, String name) { if (_isCoreFunction(receiverType) && name == FunctionElement.CALL_METHOD_NAME) { - _resolver.nullableDereferenceVerifier - .methodInvocation(receiver, receiverType, name); + _resolver.nullableDereferenceVerifier.expression(receiver); _setDynamicResolution(node); return; } diff --git a/pkg/analyzer/lib/src/error/nullable_dereference_verifier.dart b/pkg/analyzer/lib/src/error/nullable_dereference_verifier.dart index 36dff58241b1d..d37a1abc0a4a6 100644 --- a/pkg/analyzer/lib/src/error/nullable_dereference_verifier.dart +++ b/pkg/analyzer/lib/src/error/nullable_dereference_verifier.dart @@ -12,18 +12,6 @@ import 'package:meta/meta.dart'; /// Helper for checking potentially nullable dereferences. class NullableDereferenceVerifier { - /// Properties on the object class which are safe to call on nullable types. - /// - /// Note that this must include tear-offs. - /// - /// TODO(mfairhurst): Calculate these fields rather than hard-code them. - static const _objectPropertyNames = { - 'hashCode', - 'runtimeType', - 'noSuchMethod', - 'toString', - }; - final TypeSystemImpl _typeSystem; final ErrorReporter _errorReporter; @@ -41,42 +29,6 @@ class NullableDereferenceVerifier { return _check(expression, expression.staticType); } - void implicitThis(AstNode errorNode, DartType thisType) { - if (!_typeSystem.isNonNullableByDefault) { - return; - } - - _check(errorNode, thisType); - } - - void methodInvocation( - Expression receiver, - DartType receiverType, - String methodName, - ) { - if (!_typeSystem.isNonNullableByDefault) { - return; - } - - if (methodName == 'toString' || methodName == 'noSuchMethod') { - return; - } - - _check(receiver, receiverType); - } - - void propertyAccess(AstNode errorNode, DartType receiverType, String name) { - if (!_typeSystem.isNonNullableByDefault) { - return; - } - - if (_objectPropertyNames.contains(name)) { - return; - } - - _check(errorNode, receiverType); - } - void report(AstNode errorNode, DartType receiverType) { var errorCode = receiverType == _typeSystem.typeProvider.nullType ? StaticWarningCode.INVALID_USE_OF_NULL_VALUE diff --git a/pkg/analyzer/lib/src/generated/error_verifier.dart b/pkg/analyzer/lib/src/generated/error_verifier.dart index a0aef3d4146ed..2e5f765b8ee54 100644 --- a/pkg/analyzer/lib/src/generated/error_verifier.dart +++ b/pkg/analyzer/lib/src/generated/error_verifier.dart @@ -25,7 +25,6 @@ import 'package:analyzer/src/error/codes.dart'; import 'package:analyzer/src/error/constructor_fields_verifier.dart'; import 'package:analyzer/src/error/duplicate_definition_verifier.dart'; import 'package:analyzer/src/error/literal_element_verifier.dart'; -import 'package:analyzer/src/error/nullable_dereference_verifier.dart'; import 'package:analyzer/src/error/required_parameters_verifier.dart'; import 'package:analyzer/src/error/type_arguments_verifier.dart'; import 'package:analyzer/src/generated/element_resolver.dart'; @@ -255,7 +254,6 @@ class ErrorVerifier extends RecursiveAstVisitor { final DuplicateDefinitionVerifier _duplicateDefinitionVerifier; TypeArgumentsVerifier _typeArgumentsVerifier; ConstructorFieldsVerifier _constructorFieldsVerifier; - NullableDereferenceVerifier _nullableDereferenceVerifier; /** * Initialize a newly created error verifier. @@ -286,10 +284,6 @@ class ErrorVerifier extends RecursiveAstVisitor { typeSystem: _typeSystem, errorReporter: _errorReporter, ); - _nullableDereferenceVerifier = NullableDereferenceVerifier( - typeSystem: _typeSystem, - errorReporter: _errorReporter, - ); } /** @@ -351,7 +345,6 @@ class ErrorVerifier extends RecursiveAstVisitor { _checkForInvalidAssignment(lhs, rhs); } else { _checkForArgumentTypeNotAssignableForArgument(rhs); - _nullableDereferenceVerifier.expression(lhs); } _checkForAssignmentToFinal(lhs); super.visitAssignmentExpression(node); @@ -798,8 +791,7 @@ class ErrorVerifier extends RecursiveAstVisitor { } DartType expressionType = functionExpression.staticType; - if (!_nullableDereferenceVerifier.expression(functionExpression) && - !_checkForUseOfVoidResult(functionExpression) && + if (!_checkForUseOfVoidResult(functionExpression) && !_checkForUseOfNever(functionExpression) && node.staticElement == null && !_isFunctionType(expressionType)) { @@ -809,7 +801,6 @@ class ErrorVerifier extends RecursiveAstVisitor { } else if (expressionType is FunctionType) { _typeArgumentsVerifier.checkFunctionExpressionInvocation(node); } - _nullableDereferenceVerifier.expression(functionExpression); _requiredParametersVerifier.visitFunctionExpressionInvocation(node); super.visitFunctionExpressionInvocation(node); } @@ -989,7 +980,6 @@ class ErrorVerifier extends RecursiveAstVisitor { _checkForUnnecessaryNullAware(target, node.operator); } else { _checkForUnqualifiedReferenceToNonLocalStaticMember(methodName); - _nullableDereferenceVerifier.expression(node.function); } _typeArgumentsVerifier.checkMethodInvocation(node); _requiredParametersVerifier.visitMethodInvocation(node); @@ -1169,9 +1159,7 @@ class ErrorVerifier extends RecursiveAstVisitor { @override void visitSpreadElement(SpreadElement node) { - if (!node.isNullAware) { - _nullableDereferenceVerifier.expression(node.expression); - } else { + if (node.isNullAware) { _checkForUnnecessaryNullAware(node.expression, node.spreadOperator); } super.visitSpreadElement(node); @@ -1217,7 +1205,6 @@ class ErrorVerifier extends RecursiveAstVisitor { @override void visitThrowExpression(ThrowExpression node) { _checkForConstEvalThrowsException(node); - _nullableDereferenceVerifier.expression(node.expression); _checkForUseOfVoidResult(node.expression); super.visitThrowExpression(node); } @@ -1320,9 +1307,6 @@ class ErrorVerifier extends RecursiveAstVisitor { void visitYieldStatement(YieldStatement node) { if (_inGenerator) { _checkForYieldOfInvalidType(node.expression, node.star != null); - if (node.star != null) { - _nullableDereferenceVerifier.expression(node.expression); - } } else { CompileTimeErrorCode errorCode; if (node.star != null) { @@ -2412,10 +2396,6 @@ class ErrorVerifier extends RecursiveAstVisitor { * information in the for-each part. */ bool _checkForEachParts(ForEachParts node, SimpleIdentifier variable) { - if (_nullableDereferenceVerifier.expression(node.iterable)) { - return false; - } - if (_checkForUseOfVoidResult(node.iterable)) { return false; } diff --git a/pkg/analyzer/lib/src/generated/resolver.dart b/pkg/analyzer/lib/src/generated/resolver.dart index 1408eb663ceb6..44da39d4cedf8 100644 --- a/pkg/analyzer/lib/src/generated/resolver.dart +++ b/pkg/analyzer/lib/src/generated/resolver.dart @@ -1179,9 +1179,11 @@ class ResolverVisitor extends ScopedVisitor { // cannot be in scope while visiting the iterator. // iterable?.accept(this); - // Note: the iterable could have been rewritten so grab it from - // forLoopParts again. + // Note: the iterable could have been rewritten so grab it again. iterable = forLoopParts.iterable; + + nullableDereferenceVerifier.expression(iterable); + loopVariable?.accept(this); var elementType = typeAnalyzer.computeForEachElementType( iterable, node.awaitKeyword != null); @@ -1655,6 +1657,15 @@ class ResolverVisitor extends ScopedVisitor { super.visitSimpleIdentifier(node); } + @override + void visitSpreadElement(SpreadElement node) { + super.visitSpreadElement(node); + + if (!node.isNullAware) { + nullableDereferenceVerifier.expression(node.expression); + } + } + @override void visitSuperConstructorInvocation(SuperConstructorInvocation node) { // @@ -1716,6 +1727,7 @@ class ResolverVisitor extends ScopedVisitor { @override void visitThrowExpression(ThrowExpression node) { super.visitThrowExpression(node); + nullableDereferenceVerifier.expression(node.expression); _flowAnalysis?.flow?.handleExit(); } @@ -1865,6 +1877,11 @@ class ResolverVisitor extends ScopedVisitor { InferenceContext.setType(e, type); } super.visitYieldStatement(node); + + if (node.star != null) { + nullableDereferenceVerifier.expression(node.expression); + } + DartType type = e?.staticType; if (type != null && isGenerator) { // If this just a yield, then we just pass on the element type diff --git a/pkg/analyzer/test/src/diagnostics/invalid_use_of_null_value_test.dart b/pkg/analyzer/test/src/diagnostics/invalid_use_of_null_value_test.dart index aa815e10edbda..51d5179169d1b 100644 --- a/pkg/analyzer/test/src/diagnostics/invalid_use_of_null_value_test.dart +++ b/pkg/analyzer/test/src/diagnostics/invalid_use_of_null_value_test.dart @@ -65,6 +65,7 @@ m() { } ''', [ error(HintCode.UNUSED_LOCAL_VARIABLE, 27, 1), + error(StaticTypeWarningCode.FOR_IN_OF_INVALID_TYPE, 32, 1), error(StaticWarningCode.INVALID_USE_OF_NULL_VALUE, 32, 1), ]); }