Skip to content

Commit

Permalink
Use NullableDereferenceVerifier in resolver instead of ErrorVerifier.
Browse files Browse the repository at this point in the history
Change-Id: Ib360bd9091656e04820879052a04e6ee23c495d7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/132161
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
  • Loading branch information
scheglov authored and commit-bot@chromium.org committed Jan 16, 2020
1 parent 93b40dc commit d19b2b0
Show file tree
Hide file tree
Showing 7 changed files with 35 additions and 74 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -35,6 +36,9 @@ class FunctionExpressionInvocationResolver {

ExtensionMemberResolver get _extensionResolver => _resolver.extensionResolver;

NullableDereferenceVerifier get _nullableDereferenceVerifier =>
_resolver.nullableDereferenceVerifier;

void resolve(FunctionExpressionInvocationImpl node) {
var function = node.function;

Expand All @@ -43,6 +47,8 @@ class FunctionExpressionInvocationResolver {
return;
}

_nullableDereferenceVerifier.expression(function);

var receiverType = function.staticType;
if (receiverType is FunctionType) {
_resolve(node, receiverType);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
48 changes: 0 additions & 48 deletions pkg/analyzer/lib/src/error/nullable_dereference_verifier.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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
Expand Down
24 changes: 2 additions & 22 deletions pkg/analyzer/lib/src/generated/error_verifier.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -255,7 +254,6 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
final DuplicateDefinitionVerifier _duplicateDefinitionVerifier;
TypeArgumentsVerifier _typeArgumentsVerifier;
ConstructorFieldsVerifier _constructorFieldsVerifier;
NullableDereferenceVerifier _nullableDereferenceVerifier;

/**
* Initialize a newly created error verifier.
Expand Down Expand Up @@ -286,10 +284,6 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
typeSystem: _typeSystem,
errorReporter: _errorReporter,
);
_nullableDereferenceVerifier = NullableDereferenceVerifier(
typeSystem: _typeSystem,
errorReporter: _errorReporter,
);
}

/**
Expand Down Expand Up @@ -351,7 +345,6 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
_checkForInvalidAssignment(lhs, rhs);
} else {
_checkForArgumentTypeNotAssignableForArgument(rhs);
_nullableDereferenceVerifier.expression(lhs);
}
_checkForAssignmentToFinal(lhs);
super.visitAssignmentExpression(node);
Expand Down Expand Up @@ -798,8 +791,7 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
}

DartType expressionType = functionExpression.staticType;
if (!_nullableDereferenceVerifier.expression(functionExpression) &&
!_checkForUseOfVoidResult(functionExpression) &&
if (!_checkForUseOfVoidResult(functionExpression) &&
!_checkForUseOfNever(functionExpression) &&
node.staticElement == null &&
!_isFunctionType(expressionType)) {
Expand All @@ -809,7 +801,6 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
} else if (expressionType is FunctionType) {
_typeArgumentsVerifier.checkFunctionExpressionInvocation(node);
}
_nullableDereferenceVerifier.expression(functionExpression);
_requiredParametersVerifier.visitFunctionExpressionInvocation(node);
super.visitFunctionExpressionInvocation(node);
}
Expand Down Expand Up @@ -989,7 +980,6 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
_checkForUnnecessaryNullAware(target, node.operator);
} else {
_checkForUnqualifiedReferenceToNonLocalStaticMember(methodName);
_nullableDereferenceVerifier.expression(node.function);
}
_typeArgumentsVerifier.checkMethodInvocation(node);
_requiredParametersVerifier.visitMethodInvocation(node);
Expand Down Expand Up @@ -1169,9 +1159,7 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {

@override
void visitSpreadElement(SpreadElement node) {
if (!node.isNullAware) {
_nullableDereferenceVerifier.expression(node.expression);
} else {
if (node.isNullAware) {
_checkForUnnecessaryNullAware(node.expression, node.spreadOperator);
}
super.visitSpreadElement(node);
Expand Down Expand Up @@ -1217,7 +1205,6 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
@override
void visitThrowExpression(ThrowExpression node) {
_checkForConstEvalThrowsException(node);
_nullableDereferenceVerifier.expression(node.expression);
_checkForUseOfVoidResult(node.expression);
super.visitThrowExpression(node);
}
Expand Down Expand Up @@ -1320,9 +1307,6 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
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) {
Expand Down Expand Up @@ -2412,10 +2396,6 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
* 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;
}
Expand Down
21 changes: 19 additions & 2 deletions pkg/analyzer/lib/src/generated/resolver.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
//
Expand Down Expand Up @@ -1716,6 +1727,7 @@ class ResolverVisitor extends ScopedVisitor {
@override
void visitThrowExpression(ThrowExpression node) {
super.visitThrowExpression(node);
nullableDereferenceVerifier.expression(node.expression);
_flowAnalysis?.flow?.handleExit();
}

Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),
]);
}
Expand Down

0 comments on commit d19b2b0

Please sign in to comment.