diff --git a/pkg/_fe_analyzer_shared/test/flow_analysis/nullability/data/null_aware_access.dart b/pkg/_fe_analyzer_shared/test/flow_analysis/nullability/data/null_aware_access.dart index d50373b6001..44c192f05cc 100644 --- a/pkg/_fe_analyzer_shared/test/flow_analysis/nullability/data/null_aware_access.dart +++ b/pkg/_fe_analyzer_shared/test/flow_analysis/nullability/data/null_aware_access.dart @@ -15,7 +15,7 @@ class C { class D { void set setter(value) {} - D? operator [](index) => this; + C? operator [](index) => C(); void operator []=(index, value) {} D get getterSetter => this; void set getterSetter(value) {} 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 089148aefc3..60d5fb6e924 100644 --- a/pkg/analyzer/lib/src/dart/resolver/method_invocation_resolver.dart +++ b/pkg/analyzer/lib/src/dart/resolver/method_invocation_resolver.dart @@ -148,7 +148,8 @@ class MethodInvocationResolver { } if (receiverType is InterfaceType) { - _resolveReceiverInterfaceType(node, receiverType, nameNode, name); + _resolveReceiverInterfaceType( + node, receiver, receiverType, nameNode, name); return; } @@ -509,16 +510,20 @@ class MethodInvocationResolver { ); } - void _resolveReceiverInterfaceType(MethodInvocation node, + void _resolveReceiverInterfaceType(MethodInvocation node, Expression receiver, InterfaceType receiverType, SimpleIdentifier nameNode, String name) { if (_isCoreFunction(receiverType) && name == FunctionElement.CALL_METHOD_NAME) { + _resolver.nullableDereferenceVerifier + .methodInvocation(receiver, receiverType, name); _setDynamicResolution(node); return; } var target = _inheritance.getMember(receiverType, _currentName); if (target != null) { + _resolver.nullableDereferenceVerifier + .methodInvocation(receiver, receiverType, name); target = _resolver.toLegacyElement(target); nameNode.staticElement = target; if (target is PropertyAccessorElement) { diff --git a/pkg/analyzer/lib/src/error/nullable_dereference_verifier.dart b/pkg/analyzer/lib/src/error/nullable_dereference_verifier.dart new file mode 100644 index 00000000000..22d16d95966 --- /dev/null +++ b/pkg/analyzer/lib/src/error/nullable_dereference_verifier.dart @@ -0,0 +1,71 @@ +// Copyright (c) 2019, 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/type.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'; + +/// 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; + + NullableDereferenceVerifier(this._typeSystem, this._errorReporter); + + void methodInvocation( + Expression receiver, + DartType receiverType, + String methodName, + ) { + if (!_typeSystem.isNonNullableByDefault) { + return; + } + + if (methodName == 'toString' || methodName == 'noSuchMethod') { + return; + } + + _check(receiver, receiverType); + } + + void propertyAccess(Expression receiver, DartType receiverType, String name) { + if (!_typeSystem.isNonNullableByDefault) { + return; + } + + if (_objectPropertyNames.contains(name)) { + return; + } + + _check(receiver, receiverType); + } + + /// If the [receiverType] is potentially nullable, report it. + void _check(Expression receiver, DartType receiverType) { + if (identical(receiverType, DynamicTypeImpl.instance) || + !_typeSystem.isPotentiallyNullable(receiverType)) { + return; + } + + var errorCode = receiverType == _typeSystem.typeProvider.nullType + ? StaticWarningCode.INVALID_USE_OF_NULL_VALUE + : StaticWarningCode.UNCHECKED_USE_OF_NULLABLE_VALUE; + _errorReporter.reportErrorForNode(errorCode, receiver); + } +} diff --git a/pkg/analyzer/lib/src/generated/element_resolver.dart b/pkg/analyzer/lib/src/generated/element_resolver.dart index ba4892573f1..d83bc139673 100644 --- a/pkg/analyzer/lib/src/generated/element_resolver.dart +++ b/pkg/analyzer/lib/src/generated/element_resolver.dart @@ -175,8 +175,9 @@ class ElementResolver extends SimpleAstVisitor { String methodName = operatorType.lexeme; // TODO(brianwilkerson) Change the [methodNameNode] from the left hand // side to the operator. - var result = _newPropertyResolver() - .resolve(leftHandSide, staticType, methodName, leftHandSide); + var result = _newPropertyResolver().resolve( + leftHandSide, staticType, methodName, leftHandSide, + isNullAware: false); node.staticElement = result.getter; if (_shouldReportInvalidMember(staticType, result)) { _recordUndefinedToken( @@ -490,8 +491,9 @@ class ElementResolver extends SimpleAstVisitor { if (target is ExtensionOverride) { result = _extensionResolver.getOverrideMember(target, getterMethodName); } else { - result = _newPropertyResolver() - .resolve(target, targetType, getterMethodName, target); + result = _newPropertyResolver().resolve( + target, targetType, getterMethodName, target, + isNullAware: node.isNullAware); } bool isInGetterContext = node.inGetterContext(); @@ -563,7 +565,7 @@ class ElementResolver extends SimpleAstVisitor { String methodName = _getPostfixOperator(node); DartType staticType = _getStaticType(operand); var result = _newPropertyResolver() - .resolve(operand, staticType, methodName, operand); + .resolve(operand, staticType, methodName, operand, isNullAware: false); node.staticElement = result.getter; if (_shouldReportInvalidMember(staticType, result)) { if (operand is SuperExpression) { @@ -686,8 +688,9 @@ class ElementResolver extends SimpleAstVisitor { return; } DartType staticType = _getStaticType(operand, read: true); - var result = _newPropertyResolver() - .resolve(operand, staticType, methodName, operand); + var result = _newPropertyResolver().resolve( + operand, staticType, methodName, operand, + isNullAware: false); node.staticElement = result.getter; if (_shouldReportInvalidMember(staticType, result)) { if (operand is SuperExpression) { @@ -888,7 +891,8 @@ class ElementResolver extends SimpleAstVisitor { enclosingClass != null) { InterfaceType enclosingType = enclosingClass.thisType; var propertyResolver = _newPropertyResolver(); - propertyResolver.resolve(null, enclosingType, node.name, node); + propertyResolver.resolve(null, enclosingType, node.name, node, + isNullAware: false); node.auxiliaryElements = AuxiliaryElements( propertyResolver.result.getter, ); @@ -1153,7 +1157,8 @@ class ElementResolver extends SimpleAstVisitor { } else if (invokeType is InterfaceType) { var propertyResolver = _newPropertyResolver(); propertyResolver.resolve(null, invokeType, - FunctionElement.CALL_METHOD_NAME, invocation.function); + FunctionElement.CALL_METHOD_NAME, invocation.function, + isNullAware: false); ExecutableElement callMethod = propertyResolver.result.getter; invocation.staticElement = callMethod; parameterizableType = _elementTypeProvider.safeExecutableType(callMethod); @@ -1488,8 +1493,9 @@ class ElementResolver extends SimpleAstVisitor { return; } DartType leftType = _getStaticType(leftOperand); - ResolutionResult result = _newPropertyResolver() - .resolve(leftOperand, leftType, methodName, node); + ResolutionResult result = _newPropertyResolver().resolve( + leftOperand, leftType, methodName, node, + isNullAware: methodName == '=='); node.staticElement = result.getter; node.staticInvokeType = @@ -1779,8 +1785,9 @@ class ElementResolver extends SimpleAstVisitor { return; } - var result = _newPropertyResolver() - .resolve(target, staticType, propertyName.name, propertyName); + var result = _newPropertyResolver().resolve( + target, staticType, propertyName.name, propertyName, + isNullAware: isNullAware); if (propertyName.inGetterContext()) { var shouldReportUndefinedGetter = false; @@ -1855,7 +1862,8 @@ class ElementResolver extends SimpleAstVisitor { if (enclosingClass != null) { var propertyResolver = _newPropertyResolver(); propertyResolver.resolve( - null, enclosingClass.thisType, identifier.name, identifier); + null, enclosingClass.thisType, identifier.name, identifier, + isNullAware: false); setter = propertyResolver.result.setter; } } @@ -1893,7 +1901,8 @@ class ElementResolver extends SimpleAstVisitor { if (element == null && enclosingType != null) { var propertyResolver = _newPropertyResolver(); propertyResolver.resolve( - null, enclosingType, identifier.name, identifier); + null, enclosingType, identifier.name, identifier, + isNullAware: false); if (identifier.inSetterContext() || identifier.parent is CommentReference) { element = propertyResolver.result.setter; @@ -2071,8 +2080,9 @@ class _PropertyResolver { Expression target, DartType type, String name, - Expression errorNode, - ) { + Expression errorNode, { + @required bool isNullAware, + }) { type = _resolveTypeParameter(type); ExecutableElement typeGetter; @@ -2121,6 +2131,10 @@ class _PropertyResolver { result = ResolutionResult(getter: typeGetter, setter: typeSetter); } + if (!isNullAware && result.isSingle) { + _resolver.nullableDereferenceVerifier.propertyAccess(target, type, name); + } + if (result.isNone) { result = _extensionResolver.findExtension(type, name, errorNode); } diff --git a/pkg/analyzer/lib/src/generated/error_verifier.dart b/pkg/analyzer/lib/src/generated/error_verifier.dart index 6745b7f626d..82139188fba 100644 --- a/pkg/analyzer/lib/src/generated/error_verifier.dart +++ b/pkg/analyzer/lib/src/generated/error_verifier.dart @@ -38,20 +38,6 @@ import 'package:meta/meta.dart'; * warnings not covered by the parser and resolver. */ class ErrorVerifier extends RecursiveAstVisitor { - /** - * 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 final _objectPropertyNames = { - 'hashCode', - 'runtimeType', - 'noSuchMethod', - 'toString', - }; - /** * The error reporter by which errors will be reported. */ @@ -415,7 +401,6 @@ class ErrorVerifier extends RecursiveAstVisitor { promoteParameterToNullable: true); } else if (type != TokenType.QUESTION_QUESTION) { _checkForArgumentTypeNotAssignableForArgument(node.rightOperand); - _checkForNullableDereference(node.leftOperand); } else { _checkForArgumentTypeNotAssignableForArgument(node.rightOperand); } @@ -932,8 +917,6 @@ class ErrorVerifier extends RecursiveAstVisitor { node.realTarget, node.period ?? node.leftBracket, ); - } else { - _checkForNullableDereference(node.realTarget); } super.visitIndexExpression(node); } @@ -1040,11 +1023,6 @@ class ErrorVerifier extends RecursiveAstVisitor { _typeArgumentsVerifier.checkMethodInvocation(node); _checkForNullableDereference(methodName); _requiredParametersVerifier.visitMethodInvocation(node); - if (!node.isNullAware && - methodName.name != 'toString' && - methodName.name != 'noSuchMethod') { - _checkForNullableDereference(target); - } super.visitMethodInvocation(node); } @@ -1106,7 +1084,6 @@ class ErrorVerifier extends RecursiveAstVisitor { } else { _checkForAssignmentToFinal(node.operand); _checkForIntNotAssignable(node.operand); - _checkForNullableDereference(node.operand); } super.visitPostfixExpression(node); } @@ -1120,11 +1097,6 @@ class ErrorVerifier extends RecursiveAstVisitor { _checkForStaticAccessToInstanceMember(typeReference, name); _checkForInstanceAccessToStaticMember(typeReference, node.prefix, name); } - String property = node.identifier.name; - if (node.staticElement is ExecutableElement && - !_objectPropertyNames.contains(property)) { - _checkForNullableDereference(node.prefix); - } super.visitPrefixedIdentifier(node); } @@ -1138,7 +1110,6 @@ class ErrorVerifier extends RecursiveAstVisitor { if (operatorType.isIncrementOperator) { _checkForAssignmentToFinal(operand); } - _checkForNullableDereference(operand); _checkForUseOfVoidResult(operand); _checkForIntNotAssignable(operand); } @@ -1153,10 +1124,6 @@ class ErrorVerifier extends RecursiveAstVisitor { _checkForStaticAccessToInstanceMember(typeReference, propertyName); _checkForInstanceAccessToStaticMember( typeReference, node.target, propertyName); - if (!node.isNullAware && - !_objectPropertyNames.contains(propertyName.name)) { - _checkForNullableDereference(target); - } _checkForUnnecessaryNullAware(target, node.operator); super.visitPropertyAccess(node); } diff --git a/pkg/analyzer/lib/src/generated/resolver.dart b/pkg/analyzer/lib/src/generated/resolver.dart index 9b81585f335..e8dc6d76ef9 100644 --- a/pkg/analyzer/lib/src/generated/resolver.dart +++ b/pkg/analyzer/lib/src/generated/resolver.dart @@ -30,6 +30,7 @@ import 'package:analyzer/src/dart/resolver/method_invocation_resolver.dart'; import 'package:analyzer/src/dart/resolver/scope.dart'; import 'package:analyzer/src/diagnostic/diagnostic_factory.dart'; import 'package:analyzer/src/error/codes.dart'; +import 'package:analyzer/src/error/nullable_dereference_verifier.dart'; import 'package:analyzer/src/generated/constant.dart'; import 'package:analyzer/src/generated/element_resolver.dart'; import 'package:analyzer/src/generated/element_type_provider.dart'; @@ -464,6 +465,9 @@ class ResolverVisitor extends ScopedVisitor { final ElementTypeProvider _elementTypeProvider; + /// Helper for checking potentially nullable dereferences. + NullableDereferenceVerifier nullableDereferenceVerifier; + /// Helper for extension method resolution. ExtensionMemberResolver extensionResolver; @@ -579,6 +583,10 @@ class ResolverVisitor extends ScopedVisitor { super(definingLibrary, source, typeProvider, errorListener, nameScope: nameScope) { this._promoteManager = TypePromotionManager(typeSystem); + this.nullableDereferenceVerifier = NullableDereferenceVerifier( + typeSystem, + errorReporter, + ); this.extensionResolver = ExtensionMemberResolver(this); this.elementResolver = ElementResolver(this, reportConstEvaluationErrors: reportConstEvaluationErrors, diff --git a/pkg/analyzer/test/src/dart/resolution/extension_method_test.dart b/pkg/analyzer/test/src/dart/resolution/extension_method_test.dart index 0e5f82d2a47..05f40f2da18 100644 --- a/pkg/analyzer/test/src/dart/resolution/extension_method_test.dart +++ b/pkg/analyzer/test/src/dart/resolution/extension_method_test.dart @@ -1448,6 +1448,21 @@ class ExtensionMethodsExternalReferenceWithNnbdTest @override bool get typeToStringWithNullability => true; + test_instance_getter_fromInstance_nullable() async { + await assertNoErrorsInCode(''' +extension E on int? { + int get foo => 0; +} + +f(int? a) { + a.foo; +} +'''); + var access = findNode.prefixed('a.foo'); + assertElement(access, findElement.getter('foo', of: 'E')); + assertType(access, 'int'); + } + test_instance_getter_fromInstance_nullAware() async { await assertNoErrorsInCode(''' extension E on int { @@ -1463,6 +1478,21 @@ f(int? a) { assertType(identifier, 'int'); } + test_instance_method_fromInstance_nullable() async { + await assertNoErrorsInCode(''' +extension E on int? { + void foo() {} +} + +f(int? a) { + a.foo(); +} +'''); + var invocation = findNode.methodInvocation('a.foo()'); + assertElement(invocation, findElement.method('foo', of: 'E')); + assertInvokeType(invocation, 'void Function()'); + } + test_instance_method_fromInstance_nullAware() async { await assertNoErrorsInCode(''' extension E on int { @@ -1478,7 +1508,38 @@ f(int? a) { assertInvokeType(invocation, 'void Function()'); } - test_instance_operator_index_formInstance_nullAware() async { + test_instance_operator_binary_fromInstance_nullable() async { + await assertNoErrorsInCode(''' +class A {} + +extension E on A? { + int operator +(int _) => 0; +} + +f(A? a) { + a + 1; +} +'''); + var binary = findNode.binary('a + 1'); + assertElement(binary, findElement.method('+')); + assertType(binary, 'int'); + } + + test_instance_operator_index_fromInstance_nullable() async { + await assertNoErrorsInCode(''' +extension E on int? { + int operator [](int index) => 0; +} + +f(int? a) { + a[0]; +} +'''); + var index = findNode.index('a[0]'); + assertElement(index, findElement.method('[]')); + } + + test_instance_operator_index_fromInstance_nullAware() async { await assertNoErrorsInCode(''' extension E on int { int operator [](int index) => 0; @@ -1489,7 +1550,72 @@ f(int? a) { } '''); var index = findNode.index('a?.[0]'); - assertElement(index, findElement.method('[]', of: 'E')); + assertElement(index, findElement.method('[]')); + } + + test_instance_operator_postfixInc_fromInstance_nullable() async { + await assertNoErrorsInCode(''' +class A {} + +extension E on A? { + A? operator +(int _) => this; +} + +f(A? a) { + a++; +} +'''); + var expression = findNode.postfix('a++'); + assertElement(expression, findElement.method('+')); + assertType(expression, 'A?'); + } + + test_instance_operator_prefixInc_fromInstance_nullable() async { + await assertNoErrorsInCode(''' +class A {} + +extension E on A? { + A? operator +(int _) => this; +} + +f(A? a) { + ++a; +} +'''); + var expression = findNode.prefix('++a'); + assertElement(expression, findElement.method('+')); + assertType(expression, 'A?'); + } + + test_instance_operator_unaryMinus_fromInstance_nullable() async { + await assertNoErrorsInCode(''' +class A {} + +extension E on A? { + A? operator -() => this; +} + +f(A? a) { + -a; +} +'''); + var expression = findNode.prefix('-a'); + assertElement(expression, findElement.method('unary-')); + assertType(expression, 'A?'); + } + + test_instance_setter_fromInstance_nullable() async { + await assertNoErrorsInCode(''' +extension E on int? { + set foo(int _) {} +} + +f(int? a) { + a.foo = 1; +} +'''); + var access = findNode.prefixed('a.foo'); + assertElement(access, findElement.setter('foo')); } test_instance_setter_fromInstance_nullAware() async { diff --git a/pkg/analyzer/test/src/diagnostics/unchecked_use_of_nullable_value_test.dart b/pkg/analyzer/test/src/diagnostics/unchecked_use_of_nullable_value_test.dart index cc17c45c018..68b772b865d 100644 --- a/pkg/analyzer/test/src/diagnostics/unchecked_use_of_nullable_value_test.dart +++ b/pkg/analyzer/test/src/diagnostics/unchecked_use_of_nullable_value_test.dart @@ -305,6 +305,20 @@ m() { ]); } + test_getter_nullable_notNullableExtension() async { + await assertErrorsInCode(r''' +extension E on int { + int get foo => 0; +} + +m(int? x) { + x.foo; +} +''', [ + error(StaticTypeWarningCode.UNDEFINED_GETTER, 60, 3), + ]); + } + test_if_nonNullable() async { await assertNoErrorsInCode(r''' m() { @@ -536,6 +550,20 @@ m() { ]); } + test_method_nullable_notNullableExtension() async { + await assertErrorsInCode(r''' +extension E on int { + void foo() {} +} + +m(int? x) { + x.foo(); +} +''', [ + error(StaticTypeWarningCode.UNDEFINED_METHOD, 56, 3), + ]); + } + test_method_questionDot_nullable() async { await assertNoErrorsInCode(r''' m() { @@ -553,6 +581,24 @@ m(int x) { '''); } + test_methodInvocation_call_notNullable() async { + await assertNoErrorsInCode(r''' +m(Function x) { + x.call(); +} +'''); + } + + test_methodInvocation_call_nullable() async { + await assertErrorsInCode(r''' +m(Function? x) { + x.call(); +} +''', [ + error(StaticWarningCode.UNCHECKED_USE_OF_NULLABLE_VALUE, 19, 1), + ]); + } + test_minusEq_nonNullable() async { await assertNoErrorsInCode(r''' m() { @@ -682,6 +728,22 @@ m(int? x) { ]); } + test_operatorPostfixInc_nullable_notNullableExtension() async { + await assertErrorsInCode(r''' +class A {} + +extension E on A { + A operator +(int _) => this; +} + +m(A? x) { + x++; +} +''', [ + error(StaticTypeWarningCode.UNDEFINED_OPERATOR, 78, 2), + ]); + } + test_operatorPrefixDec_nonNullable() async { await assertNoErrorsInCode(r''' m() { @@ -742,6 +804,22 @@ m() { ]); } + test_operatorUnaryMinus_nullable_notNullableExtension() async { + await assertErrorsInCode(r''' +class A {} + +extension E on A { + A operator -() => this; +} + +m(A? x) { + -x; +} +''', [ + error(StaticTypeWarningCode.UNDEFINED_OPERATOR, 72, 1), + ]); + } + test_or_nonNullable() async { await assertNoErrorsInCode(r''' m() {