Skip to content

Commit

Permalink
Add links to "why not promoted" error messages.
Browse files Browse the repository at this point in the history
These links are not live yet, but they will be filled in with
documentation explaining subtleties of type promotion in more detail.

Bug: #44900
Change-Id: I3e1865597fc5c56495a8108c8a4de98cab0c3e4b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/193749
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
  • Loading branch information
stereotype441 authored and commit-bot@chromium.org committed Apr 1, 2021
1 parent 76e41bc commit c25e748
Show file tree
Hide file tree
Showing 7 changed files with 84 additions and 32 deletions.
17 changes: 17 additions & 0 deletions pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,9 @@ class DemoteViaExplicitWrite<Variable extends Object, Expression extends Object>

DemoteViaExplicitWrite(this.variable, this.writeExpression);

@override
String get documentationLink => 'http://dart.dev/go/non-promo-write';

@override
String get shortName => 'explicitWrite';

Expand Down Expand Up @@ -339,6 +342,9 @@ class DemoteViaForEachVariableWrite<Variable extends Object,

DemoteViaForEachVariableWrite(this.variable, this.node);

@override
String get documentationLink => 'http://dart.dev/go/non-promo-write';

@override
String get shortName => 'explicitWrite';

Expand Down Expand Up @@ -2332,6 +2338,11 @@ class NonPromotionHistory<Type> {

/// Abstract class representing a reason why something was not promoted.
abstract class NonPromotionReason {
/// Link to documentation describing this non-promotion reason; this should be
/// presented to the user as a source of additional information about the
/// error.
String get documentationLink;

/// Short text description of this non-promotion reason; intended for ID
/// testing.
String get shortName;
Expand Down Expand Up @@ -2371,6 +2382,9 @@ class PropertyNotPromoted<Type extends Object> extends NonPromotionReason {

PropertyNotPromoted(this.propertyName, this.staticType);

@override
String get documentationLink => 'http://dart.dev/go/non-promo-property';

@override
String get shortName => 'propertyNotPromoted';

Expand Down Expand Up @@ -2606,6 +2620,9 @@ class SsaNode<Variable extends Object, Type extends Object> {
/// Non-promotion reason describing the situation where an expression was not
/// promoted due to the fact that it's a reference to `this`.
class ThisNotPromoted extends NonPromotionReason {
@override
String get documentationLink => 'http://dart.dev/go/non-promo-this';

@override
String get shortName => 'thisNotPromoted';

Expand Down
57 changes: 40 additions & 17 deletions pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4110,25 +4110,29 @@ const MessageCode messageFieldInitializerOutsideConstructor = const MessageCode(
tip: r"""Try removing 'this.'.""");

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
const Template<Message Function(String name)> templateFieldNotPromoted =
const Template<Message Function(String name)>(
const Template<Message Function(String name, String string)>
templateFieldNotPromoted =
const Template<Message Function(String name, String string)>(
messageTemplate:
r"""'#name' refers to a property so it couldn't be promoted.""",
tipTemplate: r"""See #string""",
withArguments: _withArgumentsFieldNotPromoted);

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
const Code<Message Function(String name)> codeFieldNotPromoted =
const Code<Message Function(String name)>(
const Code<Message Function(String name, String string)> codeFieldNotPromoted =
const Code<Message Function(String name, String string)>(
"FieldNotPromoted",
);

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
Message _withArgumentsFieldNotPromoted(String name) {
Message _withArgumentsFieldNotPromoted(String name, String string) {
if (name.isEmpty) throw 'No name provided';
name = demangleMixinApplicationName(name);
if (string.isEmpty) throw 'No string provided';
return new Message(codeFieldNotPromoted,
message: """'${name}' refers to a property so it couldn't be promoted.""",
arguments: {'name': name});
tip: """See ${string}""",
arguments: {'name': name, 'string': string});
}

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
Expand Down Expand Up @@ -9073,11 +9077,26 @@ const MessageCode messageThisInNullAwareReceiver = const MessageCode(
tip: r"""Try replacing '?.' with '.'""");

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
const Code<Null> codeThisNotPromoted = messageThisNotPromoted;
const Template<Message Function(String string)> templateThisNotPromoted =
const Template<Message Function(String string)>(
messageTemplate: r"""'this' can't be promoted.""",
tipTemplate: r"""See #string""",
withArguments: _withArgumentsThisNotPromoted);

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
const MessageCode messageThisNotPromoted = const MessageCode("ThisNotPromoted",
message: r"""'this' can't be promoted.""");
const Code<Message Function(String string)> codeThisNotPromoted =
const Code<Message Function(String string)>(
"ThisNotPromoted",
);

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
Message _withArgumentsThisNotPromoted(String string) {
if (string.isEmpty) throw 'No string provided';
return new Message(codeThisNotPromoted,
message: """'this' can't be promoted.""",
tip: """See ${string}""",
arguments: {'string': string});
}

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
const Template<Message Function(String string)>
Expand Down Expand Up @@ -9688,29 +9707,33 @@ const MessageCode messageVarReturnType = const MessageCode("VarReturnType",
r"""Try removing the keyword 'var', or replacing it with the name of the return type.""");

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
const Template<Message Function(String name)>
const Template<Message Function(String name, String string)>
templateVariableCouldBeNullDueToWrite =
const Template<Message Function(String name)>(
const Template<Message Function(String name, String string)>(
messageTemplate:
r"""Variable '#name' could be null due to an intervening write.""",
tipTemplate: r"""Try null checking the variable after the write.""",
tipTemplate:
r"""Try null checking the variable after the write. See #string""",
withArguments: _withArgumentsVariableCouldBeNullDueToWrite);

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
const Code<Message Function(String name)> codeVariableCouldBeNullDueToWrite =
const Code<Message Function(String name)>(
const Code<Message Function(String name, String string)>
codeVariableCouldBeNullDueToWrite =
const Code<Message Function(String name, String string)>(
"VariableCouldBeNullDueToWrite",
);

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
Message _withArgumentsVariableCouldBeNullDueToWrite(String name) {
Message _withArgumentsVariableCouldBeNullDueToWrite(
String name, String string) {
if (name.isEmpty) throw 'No name provided';
name = demangleMixinApplicationName(name);
if (string.isEmpty) throw 'No string provided';
return new Message(codeVariableCouldBeNullDueToWrite,
message:
"""Variable '${name}' could be null due to an intervening write.""",
tip: """Try null checking the variable after the write.""",
arguments: {'name': name});
tip: """Try null checking the variable after the write. See ${string}""",
arguments: {'name': name, 'string': string});
}

// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5754,6 +5754,9 @@ Matcher _matchVariableModel(
}

class _MockNonPromotionReason extends NonPromotionReason {
@override
String get documentationLink => fail('Unexpected call to documentationLink');

String get shortName => fail('Unexpected call to shortName');

R accept<R, Node extends Object, Expression extends Object,
Expand Down
23 changes: 13 additions & 10 deletions pkg/analyzer/lib/src/generated/resolver.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3472,7 +3472,7 @@ class _WhyNotPromotedVisitor
}
var variableName = reason.variable.name;
if (variableName == null) return null;
return _contextMessageForWrite(variableName, writeExpression);
return _contextMessageForWrite(variableName, writeExpression, reason);
}

@override
Expand All @@ -3496,7 +3496,7 @@ class _WhyNotPromotedVisitor
_dataForTesting!.nonPromotionReasonTargets[identifier] =
reason.shortName;
}
return _contextMessageForWrite(variableName, identifier);
return _contextMessageForWrite(variableName, identifier, reason);
} else {
assert(false, 'Unexpected parts type');
return null;
Expand All @@ -3521,7 +3521,8 @@ class _WhyNotPromotedVisitor
if (receiverElement is PropertyAccessorElement) {
propertyReference = receiverElement;
propertyType = reason.staticType;
return _contextMessageForProperty(receiverElement, reason.propertyName);
return _contextMessageForProperty(
receiverElement, reason.propertyName, reason);
} else {
assert(receiverElement == null,
'Unrecognized property element: ${receiverElement.runtimeType}');
Expand All @@ -3533,27 +3534,29 @@ class _WhyNotPromotedVisitor
DiagnosticMessage? visitThisNotPromoted(ThisNotPromoted reason) {
return DiagnosticMessageImpl(
filePath: source.fullName,
message: "'this' can't be promoted.",
message: "'this' can't be promoted. See ${reason.documentationLink}",
offset: _errorEntity.offset,
length: _errorEntity.length);
}

DiagnosticMessageImpl _contextMessageForProperty(
PropertyAccessorElement property, String propertyName) {
PropertyAccessorElement property,
String propertyName,
NonPromotionReason reason) {
return DiagnosticMessageImpl(
filePath: property.source.fullName,
message:
"'$propertyName' refers to a property so it couldn't be promoted.",
message: "'$propertyName' refers to a property so it couldn't be "
"promoted. See ${reason.documentationLink}",
offset: property.nameOffset,
length: property.nameLength);
}

DiagnosticMessageImpl _contextMessageForWrite(
String variableName, Expression writeExpression) {
DiagnosticMessageImpl _contextMessageForWrite(String variableName,
Expression writeExpression, NonPromotionReason reason) {
return DiagnosticMessageImpl(
filePath: source.fullName,
message: "Variable '$variableName' could be null due to an intervening "
"write.",
"write. See ${reason.documentationLink}",
offset: writeExpression.offset,
length: writeExpression.length);
}
Expand Down
10 changes: 6 additions & 4 deletions pkg/front_end/lib/src/fasta/type_inference/type_inferrer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5151,7 +5151,7 @@ class _WhyNotPromotedVisitor
}
int offset = reason.writeExpression.fileOffset;
return templateVariableCouldBeNullDueToWrite
.withArguments(reason.variable.name)
.withArguments(reason.variable.name, reason.documentationLink)
.withLocation(inferrer.helper.uri, offset, noLength);
}

Expand All @@ -5160,7 +5160,7 @@ class _WhyNotPromotedVisitor
DemoteViaForEachVariableWrite<VariableDeclaration, Node> reason) {
int offset = (reason.node as TreeNode).fileOffset;
return templateVariableCouldBeNullDueToWrite
.withArguments(reason.variable.name)
.withArguments(reason.variable.name, reason.documentationLink)
.withLocation(inferrer.helper.uri, offset, noLength);
}

Expand All @@ -5183,7 +5183,7 @@ class _WhyNotPromotedVisitor
propertyReference = member;
propertyType = reason.staticType;
return templateFieldNotPromoted
.withArguments(reason.propertyName)
.withArguments(reason.propertyName, reason.documentationLink)
.withLocation(member.fileUri, member.fileOffset, noLength);
} else {
return null;
Expand All @@ -5192,6 +5192,8 @@ class _WhyNotPromotedVisitor

@override
LocatedMessage visitThisNotPromoted(ThisNotPromoted reason) {
return messageThisNotPromoted.withoutLocation();
return templateThisNotPromoted
.withArguments(reason.documentationLink)
.withoutLocation();
}
}
4 changes: 3 additions & 1 deletion pkg/front_end/messages.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4656,13 +4656,15 @@ MultipleVarianceModifiers:

VariableCouldBeNullDueToWrite:
template: "Variable '#name' could be null due to an intervening write."
tip: "Try null checking the variable after the write."
tip: "Try null checking the variable after the write. See #string"

FieldNotPromoted:
template: "'#name' refers to a property so it couldn't be promoted."
tip: "See #string"

ThisNotPromoted:
template: "'this' can't be promoted."
tip: "See #string"

NullablePropertyAccessError:
template: "Property '#name' cannot be accessed on '#type' because it is potentially null."
Expand Down
2 changes: 2 additions & 0 deletions pkg/front_end/test/spell_checking_list_code.txt
Original file line number Diff line number Diff line change
Expand Up @@ -882,6 +882,7 @@ prebuilt
preexisted
preexisting
preorder
presented
presubmit
prev
prime
Expand All @@ -892,6 +893,7 @@ proc
producers
product
progresses
promo
proof
prop
propose
Expand Down

0 comments on commit c25e748

Please sign in to comment.