From d8732e982032af10d36d37e728c9cd8ef4a17451 Mon Sep 17 00:00:00 2001 From: Paul Berry Date: Mon, 30 Oct 2023 21:31:51 +0000 Subject: [PATCH] When field promotion fails, report all reasons via context messages. Previously, if field promotion failed both because the language version was less than 3.2, *and* for some other reason(s), the analyzer and CFE only reported the other reason(s). The rationale was that this was better than reporting just that the language version was less than 3.2, because if a user upgraded their language version to 3.2 in an attempt to get field promotion to work, and *then* found out that the property in question was unpromotable for some other reason, that could be quite frustrating. With this change, if field promotion fails both because the language version is less than 3.2 and for some other reason, the analyzer and CFE report *all* the reasons. Change-Id: Ib5d3a4621273c1e80d66b66b456119f9053e18b1 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/332485 Reviewed-by: Chloe Stefantsova Commit-Queue: Paul Berry Reviewed-by: Brian Wilkerson --- .../lib/src/flow_analysis/flow_analysis.dart | 17 +++++---- .../lib/src/messages/codes_generated.dart | 4 +- pkg/analyzer/lib/src/generated/resolver.dart | 37 +++++++++++-------- .../inference_visitor_base.dart | 36 +++++++++++------- pkg/front_end/messages.yaml | 2 +- .../why_not_promoted_disabled_error_test.dart | 19 +++++++++- 6 files changed, 75 insertions(+), 40 deletions(-) diff --git a/pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart b/pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart index 99d70111101..a3a6261f087 100644 --- a/pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart +++ b/pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart @@ -3575,7 +3575,11 @@ abstract base class PropertyNotPromoted /// the client as a convenience for ID testing. final Type staticType; - PropertyNotPromoted(this.propertyName, this.propertyMember, this.staticType); + /// Whether field promotion is enabled for the current library. + final bool fieldPromotionEnabled; + + PropertyNotPromoted(this.propertyName, this.propertyMember, this.staticType, + {required this.fieldPromotionEnabled}); } /// Non-promotion reason describing the situation where an expression was not @@ -3587,7 +3591,8 @@ final class PropertyNotPromotedForInherentReason final PropertyNonPromotabilityReason whyNotPromotable; PropertyNotPromotedForInherentReason(super.propertyName, super.propertyMember, - super.staticType, this.whyNotPromotable); + super.staticType, this.whyNotPromotable, + {required super.fieldPromotionEnabled}); @override NonPromotionDocumentationLink get documentationLink => @@ -3632,12 +3637,9 @@ final class PropertyNotPromotedForInherentReason /// user. final class PropertyNotPromotedForNonInherentReason extends PropertyNotPromoted { - /// Whether field promotion is enabled for the current library. - final bool fieldPromotionEnabled; - PropertyNotPromotedForNonInherentReason( super.propertyName, super.propertyMember, super.staticType, - {required this.fieldPromotionEnabled}); + {required super.fieldPromotionEnabled}); @override Null get documentationLink => null; @@ -5748,7 +5750,8 @@ class _FlowAnalysisImpl( "FieldNotPromotedBecauseNotEnabled", problemMessageTemplate: - r"""'#name' refers to a field. It couldn't be promoted because field promotion is only available in Dart 3.2 and above.""", + r"""'#name' couldn't be promoted because field promotion is only available in Dart 3.2 and above.""", correctionMessageTemplate: r"""See #string""", withArguments: _withArgumentsFieldNotPromotedBecauseNotEnabled); @@ -5544,7 +5544,7 @@ Message _withArgumentsFieldNotPromotedBecauseNotEnabled( if (string.isEmpty) throw 'No string provided'; return new Message(codeFieldNotPromotedBecauseNotEnabled, problemMessage: - """'${name}' refers to a field. It couldn't be promoted because field promotion is only available in Dart 3.2 and above.""", + """'${name}' couldn't be promoted because field promotion is only available in Dart 3.2 and above.""", correctionMessage: """See ${string}""", arguments: {'name': name, 'string': string}); } diff --git a/pkg/analyzer/lib/src/generated/resolver.dart b/pkg/analyzer/lib/src/generated/resolver.dart index 97e2a89c208..3b35996d143 100644 --- a/pkg/analyzer/lib/src/generated/resolver.dart +++ b/pkg/analyzer/lib/src/generated/resolver.dart @@ -5394,7 +5394,9 @@ class _WhyNotPromotedVisitor message: message, offset: property.nonSynthetic.nameOffset, length: property.nameLength, - url: reason.documentationLink.url) + url: reason.documentationLink.url), + if (!reason.fieldPromotionEnabled) + _fieldPromotionUnavailableMessage(property, propertyName) ]; } else { assert(receiverElement == null, @@ -5458,21 +5460,14 @@ class _WhyNotPromotedVisitor .conflictingNoSuchMethodForwarder); } } - if (messages.isEmpty) { + if (reason.fieldPromotionEnabled) { // The only possible non-inherent reasons for field promotion to fail - // are because of conflicts and because field promotion is disabled. The - // loops above failed to find any conflicts, so field promotion must - // have failed because it was disabled. - assert(!reason.fieldPromotionEnabled); - messages.add(DiagnosticMessageImpl( - filePath: property.source.fullName, - message: - "'$propertyName' refers to a field. It couldn't be promoted " - "because field promotion is only available in Dart 3.2 and " - "above.", - offset: property.nonSynthetic.nameOffset, - length: property.nameLength, - url: NonPromotionDocumentationLink.fieldPromotionUnavailable.url)); + // are because of conflicts and because field promotion is disabled. So + // if field promotion is enabled, the loops above should have found a + // conflict. + assert(messages.isNotEmpty); + } else { + messages.add(_fieldPromotionUnavailableMessage(property, propertyName)); } return messages; } else { @@ -5504,4 +5499,16 @@ class _WhyNotPromotedVisitor length: node.length, url: reason.documentationLink.url); } + + DiagnosticMessageImpl _fieldPromotionUnavailableMessage( + PropertyAccessorElement property, String propertyName) { + return DiagnosticMessageImpl( + filePath: property.source.fullName, + message: "'$propertyName' couldn't be promoted " + "because field promotion is only available in Dart 3.2 and " + "above.", + offset: property.nonSynthetic.nameOffset, + length: property.nameLength, + url: NonPromotionDocumentationLink.fieldPromotionUnavailable.url); + } } diff --git a/pkg/front_end/lib/src/fasta/type_inference/inference_visitor_base.dart b/pkg/front_end/lib/src/fasta/type_inference/inference_visitor_base.dart index 58521b1b6b2..ef35c35b6fc 100644 --- a/pkg/front_end/lib/src/fasta/type_inference/inference_visitor_base.dart +++ b/pkg/front_end/lib/src/fasta/type_inference/inference_visitor_base.dart @@ -4549,19 +4549,14 @@ class _WhyNotPromotedVisitor .withLocation(nsmClass.fileUri, nsmClass.fileOffset, noLength)); } } - if (messages.isEmpty) { + if (reason.fieldPromotionEnabled) { // The only possible non-inherent reasons for field promotion to fail are - // because of conflicts and because field promotion is disabled. The loops - // above failed to find any conflicts, so field promotion must have failed - // because it was disabled. - assert(!reason.fieldPromotionEnabled); - Object? member = reason.propertyMember; - if (member is Member) { - messages.add(templateFieldNotPromotedBecauseNotEnabled - .withArguments(reason.propertyName, - NonPromotionDocumentationLink.fieldPromotionUnavailable.url) - .withLocation(member.fileUri, member.fileOffset, noLength)); - } + // because of conflicts and because field promotion is disabled. So if + // field promotion is enabled, the loops above should have found a + // conflict. + assert(messages.isNotEmpty); + } else { + _addFieldPromotionUnavailableMessage(reason, messages); } return messages; } @@ -4589,11 +4584,15 @@ class _WhyNotPromotedVisitor PropertyNonPromotabilityReason.isNotFinal => templateFieldNotPromotedBecauseNotFinal }; - return [ + List messages = [ template .withArguments(reason.propertyName, reason.documentationLink.url) .withLocation(member.fileUri, member.fileOffset, noLength) ]; + if (!reason.fieldPromotionEnabled) { + _addFieldPromotionUnavailableMessage(reason, messages); + } + return messages; } else { assert(member == null, 'Unrecognized property member: ${member.runtimeType}'); @@ -4609,6 +4608,17 @@ class _WhyNotPromotedVisitor .withoutLocation() ]; } + + void _addFieldPromotionUnavailableMessage( + PropertyNotPromoted reason, List messages) { + Object? member = reason.propertyMember; + if (member is Member) { + messages.add(templateFieldNotPromotedBecauseNotEnabled + .withArguments(reason.propertyName, + NonPromotionDocumentationLink.fieldPromotionUnavailable.url) + .withLocation(member.fileUri, member.fileOffset, noLength)); + } + } } /// Sentinel type used as the result in top level inference when the type is diff --git a/pkg/front_end/messages.yaml b/pkg/front_end/messages.yaml index 0ed1a52d195..a0ec14c8ded 100644 --- a/pkg/front_end/messages.yaml +++ b/pkg/front_end/messages.yaml @@ -5481,7 +5481,7 @@ VariableCouldBeNullDueToWrite: correctionMessage: "Try null checking the variable after the assignment. See #string" FieldNotPromotedBecauseNotEnabled: - problemMessage: "'#name' refers to a field. It couldn't be promoted because field promotion is only available in Dart 3.2 and above." + problemMessage: "'#name' couldn't be promoted because field promotion is only available in Dart 3.2 and above." correctionMessage: "See #string" script: > // @dart=3.1 diff --git a/tests/language/inference_update_2/why_not_promoted_disabled_error_test.dart b/tests/language/inference_update_2/why_not_promoted_disabled_error_test.dart index 9968863733b..1af81dbe1d9 100644 --- a/tests/language/inference_update_2/why_not_promoted_disabled_error_test.dart +++ b/tests/language/inference_update_2/why_not_promoted_disabled_error_test.dart @@ -18,23 +18,38 @@ class C1 { final int? _wouldBePromotable = 0; // ^^^^^^^^^^^^^^^^^^ - // [context 1] '_wouldBePromotable' refers to a field. It couldn't be promoted because field promotion is only available in Dart 3.2 and above. See http://dart.dev/go/non-promo-field-promotion-unavailable - // [context 8] '_wouldBePromotable' refers to a field. It couldn't be promoted because field promotion is only available in Dart 3.2 and above. + // [context 1] '_wouldBePromotable' couldn't be promoted because field promotion is only available in Dart 3.2 and above. See http://dart.dev/go/non-promo-field-promotion-unavailable + // [context 8] '_wouldBePromotable' couldn't be promoted because field promotion is only available in Dart 3.2 and above. int? get _notField => 0; // ^^^^^^^^^ + // [context 2] '_notField' couldn't be promoted because field promotion is only available in Dart 3.2 and above. See http://dart.dev/go/non-promo-field-promotion-unavailable // [context 2] '_notField' refers to a getter so it couldn't be promoted. See http://dart.dev/go/non-promo-non-field + // [context 9] '_notField' couldn't be promoted because field promotion is only available in Dart 3.2 and above. // [context 9] '_notField' refers to a getter so it couldn't be promoted. final int? notPrivate = 0; // ^^^^^^^^^^ + // [context 3] 'notPrivate' couldn't be promoted because field promotion is only available in Dart 3.2 and above. See http://dart.dev/go/non-promo-field-promotion-unavailable // [context 3] 'notPrivate' refers to a public field so it couldn't be promoted. See http://dart.dev/go/non-promo-public-field + // [context 10] 'notPrivate' couldn't be promoted because field promotion is only available in Dart 3.2 and above. // [context 10] 'notPrivate' refers to a public field so it couldn't be promoted. int? _notFinal = 0; // ^^^^^^^^^ + // [context 4] '_notFinal' couldn't be promoted because field promotion is only available in Dart 3.2 and above. See http://dart.dev/go/non-promo-field-promotion-unavailable // [context 4] '_notFinal' refers to a non-final field so it couldn't be promoted. See http://dart.dev/go/non-promo-non-final-field + // [context 11] '_notFinal' couldn't be promoted because field promotion is only available in Dart 3.2 and above. // [context 11] '_notFinal' refers to a non-final field so it couldn't be promoted. final int? _conflictingGetter = 0; + // ^^^^^^^^^^^^^^^^^^ + // [context 5] '_conflictingGetter' couldn't be promoted because field promotion is only available in Dart 3.2 and above. See http://dart.dev/go/non-promo-field-promotion-unavailable + // [context 12] '_conflictingGetter' couldn't be promoted because field promotion is only available in Dart 3.2 and above. final int? _conflictingField = 0; + // ^^^^^^^^^^^^^^^^^ + // [context 6] '_conflictingField' couldn't be promoted because field promotion is only available in Dart 3.2 and above. See http://dart.dev/go/non-promo-field-promotion-unavailable + // [context 13] '_conflictingField' couldn't be promoted because field promotion is only available in Dart 3.2 and above. final int? _conflictingNsmForwarder = 0; + // ^^^^^^^^^^^^^^^^^^^^^^^^ + // [context 7] '_conflictingNsmForwarder' couldn't be promoted because field promotion is only available in Dart 3.2 and above. See http://dart.dev/go/non-promo-field-promotion-unavailable + // [context 14] '_conflictingNsmForwarder' couldn't be promoted because field promotion is only available in Dart 3.2 and above. } class C2 {