dart-lang/sdk
1
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases Wiki Activity

Flow analysis: expand analyzer support for "why not promoted" messages.

Browse Source 
This CL adds support for the following scenarios to the analyzer:

- Attempt to use a non-promoted nullable expression as the iterable of
  a for-in loop

- Attempt to use a non-promoted nullable expression as the argument of
  a `yield *` statement

- Attempt to implicitly invoke `.call` on a non-promoted nullable
  expression

- Attempt to use a non-promoted nullable expression as the argument of
  a spread operator (`...`) that is not null-aware

Some of these cases are already handled by the CFE.  Others will be
addressed in a follow-up CL.

Change-Id: I3cf31b1496e1bd92fdd3f8192f04c98dff15077c
Bug: https://github.com/dart-lang/sdk/issues/44898
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/186320
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This commit is contained in:
Paul Berry
2021-02-23 17:51:16 +00:00
committed by commit-bot@chromium.org
parent 2c982b0c1f
commit 588fadcd49
10 changed files with 246 additions and 147 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/_fe_analyzer_shared/test/flow_analysis/why_not_promoted/data/for_in_loop_type_not_iterable_nullability_error.dart
+21
View File
@@ -0,0 +1,21 @@
// 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.
// This test contains a test case for each condition that can lead to the front
// end's `ForInLoopTypeNotIterableNullability` or
// `ForInLoopTypeNotIterablePartNullability` errors, for which we wish to report
// "why not promoted" context information.
// TODO(paulberry): get this to work with the CFE and add additional test cases
// if needed.
class C1 {
List<int>? bad;
}
test(C1 c) {
if (c.bad == null) return;
for (var x
in /*analyzer.notPromoted(propertyNotPromoted(member:C1.bad))*/ c.bad) {}
}
pkg/_fe_analyzer_shared/test/flow_analysis/why_not_promoted/data/invalid_assignment_error_nullability_error.dart
+20
View File
@@ -0,0 +1,20 @@
// 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.
// This test contains a test case for each condition that can lead to the front
// end's `InvalidAssignmentErrorNullability` or
// `InvalidAssignmentErrorPartNullability` errors, for which we wish to report
// "why not promoted" context information.
// TODO(paulberry): get this to work with the CFE and add additional test cases
// if needed.
class C1 {
List<int>? bad;
}
test(C1 c) sync* {
if (c.bad == null) return;
yield* /*analyzer.notPromoted(propertyNotPromoted(member:C1.bad))*/ c.bad;
}
pkg/_fe_analyzer_shared/test/flow_analysis/why_not_promoted/data/nullable_expression_call_error.dart
+5 -5
View File
@@ -16,7 +16,7 @@ class C2 {
instance_method_invocation(C1 c) {
if (c.bad == null) return;
c.bad
/*analyzer.notPromoted(propertyNotPromoted(member:C1.bad))*/ c.bad
/*cfe.invoke: notPromoted(propertyNotPromoted(member:C1.bad))*/
();
}
@@ -42,7 +42,7 @@ extension_invocation_method(C3 c) {
if (c.ok == null) return;
c.ok();
if (c.bad == null) return;
c.bad
/*analyzer.notPromoted(propertyNotPromoted(member:C3.bad))*/ c.bad
/*cfe.invoke: notPromoted(propertyNotPromoted(member:C3.bad))*/
();
}
@@ -57,7 +57,7 @@ class C7 {
instance_getter_invocation(C6 c) {
if (c.bad == null) return;
c.bad
/*analyzer.notPromoted(propertyNotPromoted(member:C6.bad))*/ c.bad
/*cfe.invoke: notPromoted(propertyNotPromoted(member:C6.bad))*/
();
}
@@ -83,7 +83,7 @@ extension_invocation_getter(C8 c) {
if (c.ok == null) return;
c.ok();
if (c.bad == null) return;
c.bad
/*analyzer.notPromoted(propertyNotPromoted(member:C8.bad))*/ c.bad
/*cfe.invoke: notPromoted(propertyNotPromoted(member:C8.bad))*/
();
}
@@ -94,7 +94,7 @@ class C11 {
function_invocation(C11 c) {
if (c.bad == null) return;
c.bad
/*analyzer.notPromoted(propertyNotPromoted(member:C11.bad))*/ c.bad
/*cfe.invoke: notPromoted(propertyNotPromoted(member:C11.bad))*/
();
}
pkg/_fe_analyzer_shared/test/flow_analysis/why_not_promoted/data/nullable_spread_error.dart
+21
View File
@@ -0,0 +1,21 @@
// 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.
// This test contains a test case for each condition that can lead to the front
// end's `NullableSpreadError` error, for which we wish to report "why not
// promoted" context information.
// TODO(paulberry): get this to work with the CFE and add additional test cases
// if needed.
class C1 {
List<int>? bad;
}
test(C1 c) {
if (c.bad == null) return;
return [
... /*analyzer.notPromoted(propertyNotPromoted(member:C1.bad))*/ c.bad
];
}
pkg/analyzer/lib/src/dart/resolver/flow_analysis_visitor.dart
+13
View File
@@ -240,6 +240,19 @@ class FlowAnalysisHelper {
flow!.finish();
}
/// Transfers any test data that was recorded for [oldNode] so that it is now
/// associated with [newNode]. We need to do this when doing AST rewriting,
/// so that test data can be found using the rewritten tree.
void transferTestData(AstNode oldNode, AstNode newNode) {
var dataForTesting = this.dataForTesting;
if (dataForTesting != null) {
var oldNonPromotionReasons = dataForTesting.nonPromotionReasons[oldNode];
if (oldNonPromotionReasons != null) {
dataForTesting.nonPromotionReasons[newNode] = oldNonPromotionReasons;
}
}
}
void variableDeclarationList(VariableDeclarationList node) {
if (flow != null) {
var variables = node.variables;
pkg/analyzer/lib/src/dart/resolver/function_expression_invocation_resolver.dart
+11 -7
View File
@@ -42,17 +42,21 @@ class FunctionExpressionInvocationResolver {
return;
}
_nullableDereferenceVerifier.expression(function,
errorCode: CompileTimeErrorCode.UNCHECKED_INVOCATION_OF_NULLABLE_VALUE);
var receiverType = function.staticType;
if (receiverType is FunctionType) {
_resolve(node, receiverType);
if (receiverType is InterfaceType) {
// Note: in this circumstance it's not necessary to call
// `_nullableDereferenceVerifier.expression` because
// `_resolveReceiverInterfaceType` calls `TypePropertyResolver.resolve`,
// which does the necessary null checking.
_resolveReceiverInterfaceType(node, function, receiverType);
return;
}
if (receiverType is InterfaceType) {
_resolveReceiverInterfaceType(node, function, receiverType);
_nullableDereferenceVerifier.expression(function,
errorCode: CompileTimeErrorCode.UNCHECKED_INVOCATION_OF_NULLABLE_VALUE);
if (receiverType is FunctionType) {
_resolve(node, receiverType);
return;
}
pkg/analyzer/lib/src/dart/resolver/method_invocation_resolver.dart
+3
View File
@@ -752,6 +752,8 @@ class MethodInvocationResolver {
node.methodName,
);
}
_resolver.flowAnalysis?.flow
?.propertyGet(functionExpression, target, node.methodName.name);
functionExpression.staticType = targetType;
}
@@ -763,6 +765,7 @@ class MethodInvocationResolver {
NodeReplacer.replace(node, invocation);
node.setProperty(_rewriteResultKey, invocation);
InferenceContext.setTypeFromNode(invocation, node);
_resolver.flowAnalysis?.transferTestData(node, invocation);
}
void _setDynamicResolution(MethodInvocation node,
pkg/analyzer/lib/src/dart/resolver/type_property_resolver.dart
+2 -133
View File
@@ -2,7 +2,6 @@
// 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:_fe_analyzer_shared/src/flow_analysis/flow_analysis.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/syntactic_entity.dart';
import 'package:analyzer/dart/element/element.dart';
@@ -13,13 +12,9 @@ import 'package:analyzer/src/dart/element/inheritance_manager3.dart';
import 'package:analyzer/src/dart/element/type_provider.dart';
import 'package:analyzer/src/dart/element/type_system.dart';
import 'package:analyzer/src/dart/resolver/extension_member_resolver.dart';
import 'package:analyzer/src/dart/resolver/flow_analysis_visitor.dart';
import 'package:analyzer/src/dart/resolver/resolution_result.dart';
import 'package:analyzer/src/diagnostic/diagnostic.dart';
import 'package:analyzer/src/error/codes.dart';
import 'package:analyzer/src/generated/resolver.dart';
import 'package:analyzer/src/generated/source.dart';
import 'package:analyzer/src/util/ast_data_extractor.dart';
/// Helper for resolving properties (getters, setters, or methods).
class TypePropertyResolver {
@@ -120,35 +115,8 @@ class TypePropertyResolver {
}
}
List<DiagnosticMessage> messages = [];
if (receiver != null) {
var whyNotPromoted =
_resolver.flowAnalysis?.flow?.whyNotPromoted(receiver);
if (whyNotPromoted != null) {
for (var entry in whyNotPromoted.entries) {
var whyNotPromotedVisitor = _WhyNotPromotedVisitor(_resolver.source,
receiver, _resolver.flowAnalysis!.dataForTesting);
if (_typeSystem.isPotentiallyNullable(entry.key)) continue;
var message = entry.value.accept(whyNotPromotedVisitor);
if (message != null) {
if (_resolver.flowAnalysis!.dataForTesting != null) {
var nonPromotionReasonText = entry.value.shortName;
if (whyNotPromotedVisitor.propertyReference != null) {
var id =
computeMemberId(whyNotPromotedVisitor.propertyReference!);
nonPromotionReasonText += '($id)';
}
_resolver.flowAnalysis!.dataForTesting!
.nonPromotionReasons[nameErrorEntity] =
nonPromotionReasonText;
}
messages = [message];
}
break;
}
}
}
List<DiagnosticMessage> messages =
_resolver.computeWhyNotPromotedMessages(receiver, nameErrorEntity);
_resolver.nullableDereferenceVerifier.report(
receiverErrorNode, receiverType,
errorCode: errorCode, arguments: [name], messages: messages);
@@ -299,102 +267,3 @@ class TypePropertyResolver {
);
}
}
class _WhyNotPromotedVisitor
implements
NonPromotionReasonVisitor<DiagnosticMessage?, AstNode, Expression,
PromotableElement> {
final Source source;
final Expression _receiver;
final FlowAnalysisDataForTesting? _dataForTesting;
PropertyAccessorElement? propertyReference;
_WhyNotPromotedVisitor(this.source, this._receiver, this._dataForTesting);
@override
DiagnosticMessage? visitDemoteViaExplicitWrite(
DemoteViaExplicitWrite<PromotableElement, Expression> reason) {
var writeExpression = reason.writeExpression;
if (_dataForTesting != null) {
_dataForTesting!.nonPromotionReasonTargets[writeExpression] =
reason.shortName;
}
var variableName = reason.variable.name;
if (variableName == null) return null;
return _contextMessageForWrite(variableName, writeExpression);
}
@override
DiagnosticMessage? visitDemoteViaForEachVariableWrite(
DemoteViaForEachVariableWrite<PromotableElement, AstNode> reason) {
var node = reason.node;
var variableName = reason.variable.name;
if (variableName == null) return null;
ForLoopParts parts;
if (node is ForStatement) {
parts = node.forLoopParts;
} else if (node is ForElement) {
parts = node.forLoopParts;
} else {
assert(false, 'Unexpected node type');
return null;
}
if (parts is ForEachPartsWithIdentifier) {
var identifier = parts.identifier;
if (_dataForTesting != null) {
_dataForTesting!.nonPromotionReasonTargets[identifier] =
reason.shortName;
}
return _contextMessageForWrite(variableName, identifier);
} else {
assert(false, 'Unexpected parts type');
return null;
}
}
@override
DiagnosticMessage? visitPropertyNotPromoted(PropertyNotPromoted reason) {
var receiver = _receiver;
Element? receiverElement;
if (receiver is SimpleIdentifier) {
receiverElement = receiver.staticElement;
} else if (receiver is PropertyAccess) {
receiverElement = receiver.propertyName.staticElement;
} else if (receiver is PrefixedIdentifier) {
receiverElement = receiver.identifier.staticElement;
} else {
assert(false, 'Unrecognized receiver: ${receiver.runtimeType}');
}
if (receiverElement is PropertyAccessorElement) {
propertyReference = receiverElement;
return _contextMessageForProperty(receiverElement, reason.propertyName);
} else {
assert(receiverElement == null,
'Unrecognized receiver element: ${receiverElement.runtimeType}');
return null;
}
}
DiagnosticMessageImpl _contextMessageForProperty(
PropertyAccessorElement property, String propertyName) {
return DiagnosticMessageImpl(
filePath: property.source.fullName,
message:
"'$propertyName' refers to a property so it could not be promoted.",
offset: property.nameOffset,
length: property.nameLength);
}
DiagnosticMessageImpl _contextMessageForWrite(
String variableName, Expression writeExpression) {
return DiagnosticMessageImpl(
filePath: source.fullName,
message: "Variable '$variableName' could be null due to an intervening "
"write.",
offset: writeExpression.offset,
length: writeExpression.length);
}
}
pkg/analyzer/lib/src/error/nullable_dereference_verifier.dart
+12 -2
View File
@@ -11,17 +11,23 @@ import 'package:analyzer/src/dart/ast/extensions.dart';
import 'package:analyzer/src/dart/element/type.dart';
import 'package:analyzer/src/dart/element/type_system.dart';
import 'package:analyzer/src/error/codes.dart';
import 'package:analyzer/src/generated/resolver.dart';
/// Helper for checking potentially nullable dereferences.
class NullableDereferenceVerifier {
final TypeSystemImpl _typeSystem;
final ErrorReporter _errorReporter;
/// The resolver driving this participant.
final ResolverVisitor _resolver;
NullableDereferenceVerifier({
required TypeSystemImpl typeSystem,
required ErrorReporter errorReporter,
required ResolverVisitor resolver,
}) : _typeSystem = typeSystem,
_errorReporter = errorReporter;
_errorReporter = errorReporter,
_resolver = resolver;
bool expression(Expression expression,
{DartType? type, ErrorCode? errorCode}) {
@@ -59,7 +65,11 @@ class NullableDereferenceVerifier {
return false;
}
report(errorNode, receiverType, errorCode: errorCode);
List<DiagnosticMessage>? messages;
if (errorNode is Expression) {
messages = _resolver.computeWhyNotPromotedMessages(errorNode, errorNode);
}
report(errorNode, receiverType, errorCode: errorCode, messages: messages);
return true;
}
}
pkg/analyzer/lib/src/generated/resolver.dart
+138
View File
@@ -4,14 +4,17 @@
import 'dart:collection';
import 'package:_fe_analyzer_shared/src/flow_analysis/flow_analysis.dart';
import 'package:analyzer/dart/analysis/features.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/syntactic_entity.dart';
import 'package:analyzer/dart/ast/visitor.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/dart/element/nullability_suffix.dart';
import 'package:analyzer/dart/element/scope.dart';
import 'package:analyzer/dart/element/type.dart';
import 'package:analyzer/dart/element/type_provider.dart';
import 'package:analyzer/diagnostic/diagnostic.dart';
import 'package:analyzer/error/error.dart';
import 'package:analyzer/error/listener.dart';
import 'package:analyzer/src/dart/ast/ast.dart';
@@ -47,6 +50,7 @@ import 'package:analyzer/src/dart/resolver/type_property_resolver.dart';
import 'package:analyzer/src/dart/resolver/typed_literal_resolver.dart';
import 'package:analyzer/src/dart/resolver/variable_declaration_resolver.dart';
import 'package:analyzer/src/dart/resolver/yield_statement_resolver.dart';
import 'package:analyzer/src/diagnostic/diagnostic.dart';
import 'package:analyzer/src/error/bool_expression_verifier.dart';
import 'package:analyzer/src/error/codes.dart';
import 'package:analyzer/src/error/dead_code_verifier.dart';
@@ -61,6 +65,7 @@ import 'package:analyzer/src/generated/static_type_analyzer.dart';
import 'package:analyzer/src/generated/this_access_tracker.dart';
import 'package:analyzer/src/generated/type_promotion_manager.dart';
import 'package:analyzer/src/generated/variable_type_provider.dart';
import 'package:analyzer/src/util/ast_data_extractor.dart';
import 'package:meta/meta.dart';
/// Maintains and manages contextual type information used for
@@ -315,6 +320,7 @@ class ResolverVisitor extends ScopedVisitor {
nullableDereferenceVerifier = NullableDereferenceVerifier(
typeSystem: typeSystem,
errorReporter: errorReporter,
resolver: this,
);
boolExpressionVerifier = BoolExpressionVerifier(
typeSystem: typeSystem,
@@ -505,6 +511,39 @@ class ResolverVisitor extends ScopedVisitor {
nullSafetyDeadCodeVerifier.visitNode(node);
}
/// Computes the appropriate set of context messages to report along with an
/// error that may have occurred because [receiver] was not type promoted.
List<DiagnosticMessage> computeWhyNotPromotedMessages(
Expression? receiver, SyntacticEntity errorEntity) {
List<DiagnosticMessage> messages = [];
if (receiver != null) {
var whyNotPromoted = flowAnalysis?.flow?.whyNotPromoted(receiver);
if (whyNotPromoted != null) {
for (var entry in whyNotPromoted.entries) {
var whyNotPromotedVisitor = _WhyNotPromotedVisitor(
source, receiver, flowAnalysis!.dataForTesting);
if (typeSystem.isPotentiallyNullable(entry.key)) continue;
var message = entry.value.accept(whyNotPromotedVisitor);
if (message != null) {
if (flowAnalysis!.dataForTesting != null) {
var nonPromotionReasonText = entry.value.shortName;
if (whyNotPromotedVisitor.propertyReference != null) {
var id =
computeMemberId(whyNotPromotedVisitor.propertyReference!);
nonPromotionReasonText += '($id)';
}
flowAnalysis!.dataForTesting!.nonPromotionReasons[errorEntity] =
nonPromotionReasonText;
}
messages = [message];
}
break;
}
}
}
return messages;
}
/// Return the static element associated with the given expression whose type
/// can be overridden, or `null` if there is no element whose type can be
/// overridden.
@@ -3297,3 +3336,102 @@ class _SwitchExhaustiveness {
return null;
}
}
class _WhyNotPromotedVisitor
implements
NonPromotionReasonVisitor<DiagnosticMessage?, AstNode, Expression,
PromotableElement> {
final Source source;
final Expression _receiver;
final FlowAnalysisDataForTesting? _dataForTesting;
PropertyAccessorElement? propertyReference;
_WhyNotPromotedVisitor(this.source, this._receiver, this._dataForTesting);
@override
DiagnosticMessage? visitDemoteViaExplicitWrite(
DemoteViaExplicitWrite<PromotableElement, Expression> reason) {
var writeExpression = reason.writeExpression;
if (_dataForTesting != null) {
_dataForTesting!.nonPromotionReasonTargets[writeExpression] =
reason.shortName;
}
var variableName = reason.variable.name;
if (variableName == null) return null;
return _contextMessageForWrite(variableName, writeExpression);
}
@override
DiagnosticMessage? visitDemoteViaForEachVariableWrite(
DemoteViaForEachVariableWrite<PromotableElement, AstNode> reason) {
var node = reason.node;
var variableName = reason.variable.name;
if (variableName == null) return null;
ForLoopParts parts;
if (node is ForStatement) {
parts = node.forLoopParts;
} else if (node is ForElement) {
parts = node.forLoopParts;
} else {
assert(false, 'Unexpected node type');
return null;
}
if (parts is ForEachPartsWithIdentifier) {
var identifier = parts.identifier;
if (_dataForTesting != null) {
_dataForTesting!.nonPromotionReasonTargets[identifier] =
reason.shortName;
}
return _contextMessageForWrite(variableName, identifier);
} else {
assert(false, 'Unexpected parts type');
return null;
}
}
@override
DiagnosticMessage? visitPropertyNotPromoted(PropertyNotPromoted reason) {
var receiver = _receiver;
Element? receiverElement;
if (receiver is SimpleIdentifier) {
receiverElement = receiver.staticElement;
} else if (receiver is PropertyAccess) {
receiverElement = receiver.propertyName.staticElement;
} else if (receiver is PrefixedIdentifier) {
receiverElement = receiver.identifier.staticElement;
} else {
assert(false, 'Unrecognized receiver: ${receiver.runtimeType}');
}
if (receiverElement is PropertyAccessorElement) {
propertyReference = receiverElement;
return _contextMessageForProperty(receiverElement, reason.propertyName);
} else {
assert(receiverElement == null,
'Unrecognized receiver element: ${receiverElement.runtimeType}');
return null;
}
}
DiagnosticMessageImpl _contextMessageForProperty(
PropertyAccessorElement property, String propertyName) {
return DiagnosticMessageImpl(
filePath: property.source.fullName,
message:
"'$propertyName' refers to a property so it could not be promoted.",
offset: property.nameOffset,
length: property.nameLength);
}
DiagnosticMessageImpl _contextMessageForWrite(
String variableName, Expression writeExpression) {
return DiagnosticMessageImpl(
filePath: source.fullName,
message: "Variable '$variableName' could be null due to an intervening "
"write.",
offset: writeExpression.offset,
length: writeExpression.length);
}
}