From c3fb57010512acdc680593186e86a32f345ec694 Mon Sep 17 00:00:00 2001 From: Sam Rawlins Date: Mon, 17 Oct 2022 15:24:57 +0000 Subject: [PATCH] Add "Add 'late' hint" feature Tested with: * top-level variables * local variables * instance and static fields declared in classes and mixins (enhanced enums are not supported pre-null safety) Fixes https://github.com/dart-lang/sdk/issues/41389 Change-Id: I8a89668fffe64fff508d0aeb36aaebef5c84f223 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/263460 Reviewed-by: Paul Berry Commit-Queue: Samuel Rawlins --- .../lib/src/front_end/info_builder.dart | 48 +++++ .../test/front_end/info_builder_test.dart | 188 +++++++++++++++++- 2 files changed, 235 insertions(+), 1 deletion(-) diff --git a/pkg/nnbd_migration/lib/src/front_end/info_builder.dart b/pkg/nnbd_migration/lib/src/front_end/info_builder.dart index beb0e9610fa..9e437290b9c 100644 --- a/pkg/nnbd_migration/lib/src/front_end/info_builder.dart +++ b/pkg/nnbd_migration/lib/src/front_end/info_builder.dart @@ -8,6 +8,7 @@ import 'package:analyzer/dart/analysis/features.dart'; import 'package:analyzer/dart/analysis/results.dart'; import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/file_system/file_system.dart'; +import 'package:analyzer/src/dart/ast/utilities.dart'; import 'package:analyzer_plugin/protocol/protocol_common.dart' show SourceFileEdit; import 'package:analyzer_plugin/protocol/protocol_common.dart' as protocol; @@ -225,6 +226,13 @@ class InfoBuilder { case NullabilityFixKind.typeNotMadeNullable: edits.add(EditDetail('Add /*!*/ hint', offset, 0, '/*!*/')); edits.add(EditDetail('Add /*?*/ hint', offset, 0, '/*?*/')); + var declarationList = _findVariableDeclaration(result.unit, offset); + if (declarationList != null) { + var lateOffset = _offsetForPossibleLateModifier(declarationList); + if (lateOffset != null) { + edits.add(EditDetail('Add late hint', lateOffset, 0, '/*late*/')); + } + } break; case NullabilityFixKind.makeTypeNullableDueToHint: edits.add(changeHint('Change to /*!*/ hint', '/*!*/')); @@ -504,6 +512,18 @@ class InfoBuilder { return null; } + /// Returns the variable declaration which covers [offset], or `null` if none + /// does. + VariableDeclarationList? _findVariableDeclaration( + CompilationUnit unit, int offset) { + var nodeLocator = NodeLocator2(offset); + var node = nodeLocator.searchWithin(unit); + if (node == null) { + return null; + } + return node.thisOrAncestorOfType(); + } + TraceEntryInfo _makeTraceEntry( String description, CodeReference? codeReference, {List hintActions = const []}) { @@ -527,6 +547,34 @@ class InfoBuilder { .toList()); } + /// Returns the offset for a possible `late` modifier which could be inserted + /// into [declarationList], or `null` if none is possible. + int? _offsetForPossibleLateModifier(VariableDeclarationList declarationList) { + if (declarationList.isLate || declarationList.isConst) { + // Don't offer an ofset. + return null; + } + var keyword = declarationList.keyword; + if (keyword != null) { + // Offset for possible `late` is before `var`, `const`, or `final`. + return keyword.offset; + } + + var typeAnnotation = declarationList.type; + if (typeAnnotation != null) { + // Without a `keyword`, offset for possible `late` is before the type + // annotation. + return typeAnnotation.offset; + } + + assert( + false, + 'In this VariableDeclarationList, there is no `var`, ' + '`const`, or `final` keyword, nor any type annotation. This variable ' + 'declaration list is not valid: $declarationList'); + return null; + } + TraceEntryInfo _stepToTraceEntry(PropagationStepInfo step) { var description = step.edge?.description; description ??= step.toString(); // TODO(paulberry): improve this message. diff --git a/pkg/nnbd_migration/test/front_end/info_builder_test.dart b/pkg/nnbd_migration/test/front_end/info_builder_test.dart index 9bfd701e407..7e92ff97546 100644 --- a/pkg/nnbd_migration/test/front_end/info_builder_test.dart +++ b/pkg/nnbd_migration/test/front_end/info_builder_test.dart @@ -384,6 +384,192 @@ void main() { replacement: '')); } + Future test_addLateHint_classMultipleTypedInstanceVariable() async { + addMetaPackage(); + var unit = await buildInfoForSingleTestFile(''' +class C { + int f, g; + C() { + f = 1; + } +} +''', migratedContent: ''' +class C { + int? f, g; + C() { + f = 1; + } +} +'''); + var regions = unit.fixRegions; + expect(regions, hasLength(1)); + var region = regions[0]; + var edits = region.edits; + assertRegion( + region: region, + offset: 15, + length: 1, + explanation: "Changed type 'int' to be nullable", + kind: NullabilityFixKind.makeTypeNullable); + assertEdit(edit: edits[2], offset: 12, length: 0, replacement: '/*late*/'); + } + + Future test_addLateHint_classTypedFinalInstanceVariable() async { + addMetaPackage(); + var unit = await buildInfoForSingleTestFile(''' +class C { + final int f; + C({this.f}); +} +''', migratedContent: ''' +class C { + final int? f; + C({this.f}); +} +'''); + var regions = unit.fixRegions; + expect(regions, hasLength(1)); + var region = regions[0]; + var edits = region.edits; + assertRegion( + region: region, + offset: 21, + length: 1, + explanation: "Changed type 'int' to be nullable", + kind: NullabilityFixKind.makeTypeNullable); + assertEdit(edit: edits[2], offset: 12, length: 0, replacement: '/*late*/'); + } + + Future test_addLateHint_classTypedInstanceVariable() async { + addMetaPackage(); + var unit = await buildInfoForSingleTestFile(''' +class C { + int f; + C() { + f = 1; + } +} +''', migratedContent: ''' +class C { + int? f; + C() { + f = 1; + } +} +'''); + var regions = unit.fixRegions; + expect(regions, hasLength(1)); + var region = regions[0]; + var edits = region.edits; + assertRegion( + region: region, + offset: 15, + length: 1, + explanation: "Changed type 'int' to be nullable", + kind: NullabilityFixKind.makeTypeNullable); + assertEdit(edit: edits[2], offset: 12, length: 0, replacement: '/*late*/'); + } + + Future test_addLateHint_classTypedStaticVariable() async { + addMetaPackage(); + var unit = await buildInfoForSingleTestFile(''' +class C { + static int x; + void f() => x = null; +} +''', migratedContent: ''' +class C { + static int? x; + void f() => x = null; +} +'''); + var regions = unit.fixRegions; + expect(regions, hasLength(1)); + var region = regions[0]; + var edits = region.edits; + assertRegion( + region: region, + offset: 22, + length: 1, + explanation: "Changed type 'int' to be nullable", + kind: NullabilityFixKind.makeTypeNullable); + assertEdit(edit: edits[2], offset: 19, length: 0, replacement: '/*late*/'); + } + + Future test_addLateHint_mixinVarInstanceVariable() async { + addMetaPackage(); + var unit = await buildInfoForSingleTestFile(''' +mixin M { + int f; + void m() => f = null; +} +''', migratedContent: ''' +mixin M { + int? f; + void m() => f = null; +} +'''); + var regions = unit.fixRegions; + expect(regions, hasLength(1)); + var region = regions[0]; + var edits = region.edits; + assertRegion( + region: region, + offset: 15, + length: 1, + explanation: "Changed type 'int' to be nullable", + kind: NullabilityFixKind.makeTypeNullable); + assertEdit(edit: edits[2], offset: 12, length: 0, replacement: '/*late*/'); + } + + Future test_addLateHint_typedLocalVariable() async { + addMetaPackage(); + var unit = await buildInfoForSingleTestFile(''' +void f() { + int x; + void g() => x = null; +} +''', migratedContent: ''' +void f() { + int? x; + void g() => x = null; +} +'''); + var regions = unit.fixRegions; + expect(regions, hasLength(1)); + var region = regions[0]; + var edits = region.edits; + assertRegion( + region: region, + offset: 16, + length: 1, + explanation: "Changed type 'int' to be nullable", + kind: NullabilityFixKind.makeTypeNullable); + assertEdit(edit: edits[2], offset: 13, length: 0, replacement: '/*late*/'); + } + + Future test_addLateHint_typedTopLevelVariable() async { + addMetaPackage(); + var unit = await buildInfoForSingleTestFile(''' +int x; +void f() => x = null; +''', migratedContent: ''' +int? x; +void f() => x = null; +'''); + var regions = unit.fixRegions; + expect(regions, hasLength(1)); + var region = regions[0]; + var edits = region.edits; + assertRegion( + region: region, + offset: 3, + length: 1, + explanation: "Changed type 'int' to be nullable", + kind: NullabilityFixKind.makeTypeNullable); + assertEdit(edit: edits[2], offset: 0, length: 0, replacement: '/*late*/'); + } + Future test_compound_assignment_nullable_result() async { var unit = await buildInfoForSingleTestFile(''' abstract class C { @@ -1623,7 +1809,7 @@ String? g() => 1 == 2 ? "Hello" : null; expect(region.lineNumber, 1); expect(region.explanation, "Type 'int' was not made nullable"); expect(region.edits.map((edit) => edit.description).toSet(), - {'Add /*?*/ hint', 'Add /*!*/ hint'}); + {'Add /*?*/ hint', 'Add /*!*/ hint', 'Add late hint'}); var trace = region.traces.first; expect(trace.description, 'Non-nullability reason'); var entries = trace.entries;