From 84d8dce38757b476ea165c4c4eddef7479dfa965 Mon Sep 17 00:00:00 2001 From: Sydney Jodon Date: Thu, 24 Oct 2024 12:04:00 -0700 Subject: [PATCH] Update other suggestors --- .../callback_ref_hint_suggestor.dart | 5 + .../state_mixin_suggestor.dart | 5 + .../callback_ref_hint_suggestor_test.dart | 60 +++++++++++ ...component_required_initial_state_test.dart | 2 +- .../state_mixin_suggestor_test.dart | 99 +++++++++++++++++++ 5 files changed, 170 insertions(+), 1 deletion(-) diff --git a/lib/src/dart3_suggestors/null_safety_prep/callback_ref_hint_suggestor.dart b/lib/src/dart3_suggestors/null_safety_prep/callback_ref_hint_suggestor.dart index dbdc16ed..892aa85b 100644 --- a/lib/src/dart3_suggestors/null_safety_prep/callback_ref_hint_suggestor.dart +++ b/lib/src/dart3_suggestors/null_safety_prep/callback_ref_hint_suggestor.dart @@ -141,6 +141,11 @@ class CallbackRefHintSuggestor extends RecursiveAstVisitor 'Could not get resolved result for "${context.relativePath}"'); } result = r; + // Don't make any updates if the file is already null safe. + if (result.libraryElement.isNonNullableByDefault) { + return; + } + result.unit.visitChildren(this); } } diff --git a/lib/src/dart3_suggestors/null_safety_prep/state_mixin_suggestor.dart b/lib/src/dart3_suggestors/null_safety_prep/state_mixin_suggestor.dart index f7e19a3f..284ceee8 100644 --- a/lib/src/dart3_suggestors/null_safety_prep/state_mixin_suggestor.dart +++ b/lib/src/dart3_suggestors/null_safety_prep/state_mixin_suggestor.dart @@ -62,6 +62,11 @@ class StateMixinSuggestor extends RecursiveAstVisitor throw Exception( 'Could not get resolved result for "${context.relativePath}"'); } + + // Don't make any updates if the file is already null safe. + if (r.libraryElement.isNonNullableByDefault) { + return; + } r.unit.accept(this); } } diff --git a/test/dart3_suggestors/null_safety_prep/callback_ref_hint_suggestor_test.dart b/test/dart3_suggestors/null_safety_prep/callback_ref_hint_suggestor_test.dart index a128c6a2..828be85a 100644 --- a/test/dart3_suggestors/null_safety_prep/callback_ref_hint_suggestor_test.dart +++ b/test/dart3_suggestors/null_safety_prep/callback_ref_hint_suggestor_test.dart @@ -18,6 +18,7 @@ import 'package:test/test.dart'; import '../../mui_suggestors/components/shared.dart'; import '../../resolved_file_context.dart'; import '../../util.dart'; +import '../../util/component_usage_migrator_test.dart'; void main() { final resolvedContext = SharedAnalysisContext.wsd; @@ -280,5 +281,64 @@ void main() { '''), ); }); + + group('makes no update if file is already on a null safe Dart version', () { + final resolvedContext = SharedAnalysisContext.overReactNullSafe; + + // Warm up analysis in a setUpAll so that if getting the resolved AST times out + // (which is more common for the WSD context), it fails here instead of failing the first test. + setUpAll(resolvedContext.warmUpAnalysis); + + late SuggestorTester nullSafeTestSuggestor; + + setUp(() { + nullSafeTestSuggestor = getSuggestorTester( + CallbackRefHintSuggestor(), + resolvedContext: resolvedContext, + ); + }); + + test('', () async { + await nullSafeTestSuggestor( + expectedPatchCount: 0, + input: withOverReactImport(/*language=dart*/ ''' + import 'dart:html'; + + content() { + var ref; + (Dom.div()..ref = (ButtonElement r) { ref = r; })(); + ref; + } + '''), + ); + }); + + test('unless there is a lang version comment', () async { + await nullSafeTestSuggestor( + input: withOverReactImport(/*language=dart*/ ''' + import 'dart:html'; + + content() { + var ref; + (Dom.div()..ref = (ButtonElement r) { ref = r; })(); + ref; + } + ''', filePrefix: '// @dart=2.11\n'), + expectedOutput: withOverReactImport(/*language=dart*/ ''' + import 'dart:html'; + + content() { + var ref; + (Dom.div()..ref = (ButtonElement /*?*/ r) { ref = r; })(); + ref; + } + ''', filePrefix: '// @dart=2.11\n'), + // Ignore error on language version comment. + isExpectedError: (error) => + error.errorCode.name.toLowerCase() == + 'illegal_language_version_override', + ); + }); + }); }, tags: 'wsd'); } diff --git a/test/dart3_suggestors/null_safety_prep/class_component_required_initial_state_test.dart b/test/dart3_suggestors/null_safety_prep/class_component_required_initial_state_test.dart index 17d9810a..d1876cd8 100644 --- a/test/dart3_suggestors/null_safety_prep/class_component_required_initial_state_test.dart +++ b/test/dart3_suggestors/null_safety_prep/class_component_required_initial_state_test.dart @@ -383,7 +383,7 @@ void main() { ''', filePrefix: '// @dart=2.11\n'), // Ignore error on language version comment. isExpectedError: (error) => - error.errorCode.name.toLowerCase() == + error.errorCode.name.toLowerCase() == 'illegal_language_version_override', ); }); diff --git a/test/dart3_suggestors/null_safety_prep/state_mixin_suggestor_test.dart b/test/dart3_suggestors/null_safety_prep/state_mixin_suggestor_test.dart index 4466acef..f829f966 100644 --- a/test/dart3_suggestors/null_safety_prep/state_mixin_suggestor_test.dart +++ b/test/dart3_suggestors/null_safety_prep/state_mixin_suggestor_test.dart @@ -147,5 +147,104 @@ void main() { '''), ); }); + + group('makes no update if file is already on a null safe Dart version', () { + final resolvedContext = SharedAnalysisContext.overReactNullSafe; + + // Warm up analysis in a setUpAll so that if getting the resolved AST times out + // (which is more common for the WSD context), it fails here instead of failing the first test. + setUpAll(resolvedContext.warmUpAnalysis); + + late SuggestorTester nullSafeTestSuggestor; + + setUp(() { + nullSafeTestSuggestor = getSuggestorTester( + StateMixinSuggestor(), + resolvedContext: resolvedContext, + ); + }); + + test('', () async { + await nullSafeTestSuggestor( + expectedPatchCount: 0, + input: withOverReactImport(/*language=dart*/ r''' + // ignore: undefined_identifier + UiFactory Foo = castUiFactory(_$Foo); + mixin FooProps on UiProps { + String? prop1; + } + mixin FooStateMixin on UiState { + String? state1; + late num state2; + } + mixin SomeOtherStateMixin on UiState { + String? state3; + } + class FooState = UiState with FooStateMixin, SomeOtherStateMixin; + class FooComponent extends UiStatefulComponent2 { + @override + render() => null; + } + '''), + ); + }); + + test('unless there is a lang version comment', () async { + await nullSafeTestSuggestor( + input: withOverReactImport(/*language=dart*/ r''' + // ignore: undefined_identifier + UiFactory Foo = castUiFactory(_$Foo); + mixin FooProps on UiProps { + String prop1; + } + mixin FooStateMixin on UiState { + String state1; + num state2; + /// This is a doc comment + /*late*/ String/*!*/ alreadyPatched; + /*late*/ String/*?*/ alreadyPatchedButNoDocComment; + String/*?*/ alreadyPatchedOptional; + } + mixin SomeOtherStateMixin on UiState { + String state3; + String/*?*/ alreadyPatchedOptional2; + } + class FooState = UiState with FooStateMixin, SomeOtherStateMixin; + class FooComponent extends UiStatefulComponent2 { + @override + render() => null; + } + ''', filePrefix: '// @dart=2.11\n'), + expectedOutput: withOverReactImport(/*language=dart*/ r''' + // ignore: undefined_identifier + UiFactory Foo = castUiFactory(_$Foo); + mixin FooProps on UiProps { + String prop1; + } + mixin FooStateMixin on UiState { + String/*?*/ state1; + num/*?*/ state2; + /// This is a doc comment + /*late*/ String/*!*/ alreadyPatched; + /*late*/ String/*?*/ alreadyPatchedButNoDocComment; + String/*?*/ alreadyPatchedOptional; + } + mixin SomeOtherStateMixin on UiState { + String/*?*/ state3; + String/*?*/ alreadyPatchedOptional2; + } + class FooState = UiState with FooStateMixin, SomeOtherStateMixin; + class FooComponent extends UiStatefulComponent2 { + @override + render() => null; + } + ''', filePrefix: '// @dart=2.11\n'), + // Ignore error on language version comment. + isExpectedError: (error) => + error.errorCode.name.toLowerCase() == + 'illegal_language_version_override', + ); + }); + }); }); }