From b0e3c4318a8f642a7403d31d5d8448ff2d71cc4e Mon Sep 17 00:00:00 2001 From: Sydney Jodon Date: Mon, 1 Jan 2024 19:32:05 -0600 Subject: [PATCH 01/11] Add initial codemod to update ref usages with nullability hint --- bin/null_safety_migration.dart | 15 ++++ .../executables/null_safety_migration.dart | 48 ++++++++++ .../callback_ref_hint_suggestor.dart | 90 +++++++++++++++++++ pubspec.yaml | 1 + .../callback_ref_hint_suggestor_test.dart | 75 ++++++++++++++++ 5 files changed, 229 insertions(+) create mode 100644 bin/null_safety_migration.dart create mode 100644 lib/src/executables/null_safety_migration.dart create mode 100644 lib/src/null_safety_migration_suggestors/callback_ref_hint_suggestor.dart create mode 100644 test/null_safety_migration_suggestors/callback_ref_hint_suggestor_test.dart diff --git a/bin/null_safety_migration.dart b/bin/null_safety_migration.dart new file mode 100644 index 00000000..aae9978b --- /dev/null +++ b/bin/null_safety_migration.dart @@ -0,0 +1,15 @@ +// Copyright 2024 Workiva Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +export 'package:over_react_codemod/src/executables/null_safety_migration.dart'; diff --git a/lib/src/executables/null_safety_migration.dart b/lib/src/executables/null_safety_migration.dart new file mode 100644 index 00000000..b16d7789 --- /dev/null +++ b/lib/src/executables/null_safety_migration.dart @@ -0,0 +1,48 @@ +// Copyright 2023 Workiva Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import 'dart:io'; + +import 'package:args/args.dart'; +import 'package:codemod/codemod.dart'; +import 'package:over_react_codemod/src/ignoreable.dart'; +import 'package:over_react_codemod/src/null_safety_migration_suggestors/callback_ref_hint_suggestor.dart'; +import 'package:over_react_codemod/src/rmui_bundle_update_suggestors/constants.dart'; +import 'package:over_react_codemod/src/rmui_bundle_update_suggestors/dart_script_updater.dart'; +import 'package:over_react_codemod/src/rmui_bundle_update_suggestors/html_script_updater.dart'; +import 'package:over_react_codemod/src/util.dart'; +import 'package:over_react_codemod/src/util/pubspec_upgrader.dart'; + +const _changesRequiredOutput = """ + To update your code, run the following commands in your repository: + dart pub global activate over_react_codemod + dart pub global run over_react_codemod:null_safety_migration +"""; + +void main(List args) async { + final parser = ArgParser.allowAnything(); + + final parsedArgs = parser.parse(args); + + exitCode = await runInteractiveCodemodSequence( + allDartPathsExceptHidden(), + [ + CallbackRefHintSuggestor(), + ], + defaultYes: true, + args: parsedArgs.rest, + additionalHelpOutput: parser.usage, + changesRequiredOutput: _changesRequiredOutput, + ); +} diff --git a/lib/src/null_safety_migration_suggestors/callback_ref_hint_suggestor.dart b/lib/src/null_safety_migration_suggestors/callback_ref_hint_suggestor.dart new file mode 100644 index 00000000..2a19c016 --- /dev/null +++ b/lib/src/null_safety_migration_suggestors/callback_ref_hint_suggestor.dart @@ -0,0 +1,90 @@ +// Copyright 2024 Workiva Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import 'package:over_react_codemod/src/util/component_usage.dart'; +import 'package:over_react_codemod/src/util/component_usage_migrator.dart'; +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:codemod/codemod.dart'; +import 'package:logging/logging.dart'; + +final _log = Logger('CallbackrefHintSuggestor'); + +// todo update comment +/// Suggestor that performs all the updates needed to migrate from the react_material_ui package +/// to the unify_ui package: +/// +/// - Rename specific components and objects +/// - Update WSD ButtonColor usages +/// - Rename import namespaces 'mui' => 'unify' +/// - Add fix me comments for manual checks +/// +/// Also see migration guide: https://github.com/Workiva/react_material_ui/tree/master/react_material_ui#how-to-migrate-from-reactmaterialui-to-unifyui +class CallbackRefHintSuggestor extends ComponentUsageMigrator { + @override + bool shouldMigrateUsage(_) => true; + + @override + void migrateUsage(FluentComponentUsage usage) { + super.migrateUsage(usage); + + for(final prop in usage.cascadedProps) { + if(prop.name.name == 'ref') { + final rhs = prop.rightHandSide; + if(rhs is FunctionExpression) { + // todo add fixme if there are more than one params + + // Add nullability hint to parameter if typed. + final param = rhs.parameters?.parameters.first; + if(param is SimpleFormalParameter) { + final type = param.type; + if(type != null) { + yieldPatch(nullabilityHint, type.end, type.end); + } + } + + // Add nullability hint to any casts in the body of the callback ref. + final castVisitor = RefCastingVisitor(); + rhs.body.visitChildren(castVisitor); + for(final location in castVisitor.locationsNeedingHints) { + yieldPatch(nullabilityHint, location, location); + } + } + } + } + } + + @override + String get fixmePrefix => 'FIXME(null_safety_migration)'; + + @override + // Override so that no flags are added. + void flagCommon(_) {} +} + +// todo add doc comment +class RefCastingVisitor extends RecursiveAstVisitor with AstVisitingSuggestor{ + + final locationsNeedingHints = []; + RefCastingVisitor(); + + @override + void visitAsExpression(AsExpression node) { + super.visitAsExpression(node); + // todo check here to match ref type to be sure + locationsNeedingHints.add(node.type.end); + } +} + +const nullabilityHint = '/*?*/'; diff --git a/pubspec.yaml b/pubspec.yaml index b8bd5a41..73a517ba 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -47,6 +47,7 @@ executables: intl_message_migration: sort_intl: unify_package_rename: + null_safety_migration: dependency_validator: ignore: diff --git a/test/null_safety_migration_suggestors/callback_ref_hint_suggestor_test.dart b/test/null_safety_migration_suggestors/callback_ref_hint_suggestor_test.dart new file mode 100644 index 00000000..940588dc --- /dev/null +++ b/test/null_safety_migration_suggestors/callback_ref_hint_suggestor_test.dart @@ -0,0 +1,75 @@ +// Copyright 2024 Workiva Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import 'package:over_react_codemod/src/null_safety_migration_suggestors/callback_ref_hint_suggestor.dart'; +import 'package:test/test.dart'; + +import '../resolved_file_context.dart'; +import '../util.dart'; + +void main() { + final resolvedContext = SharedAnalysisContext.wsd; + + // 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); + + group('MuiButtonToolbarMigrator', () { + final testSuggestor = getSuggestorTester( + CallbackRefHintSuggestor(), + resolvedContext: resolvedContext, + ); + + group('migrates WSD ButtonToolbars', () { + test('that are either unnamespaced or namespaced, and either v1 or v2', + () async { + await testSuggestor( + input:/*language=dart*/ ''' + import 'dart:html'; + + import 'package:over_react/over_react.dart'; + import 'package:web_skin_dart/component2/all.dart'; + + content() { + var ref; + (ButtonToolbar()..ref = (ButtonElement r) => ref = r)(); + (ButtonToolbar()..ref = (r) => ref = r as ButtonElement)(); + (ButtonToolbar()..ref = (ButtonElement r) { ref = r; })(); + (ButtonToolbar()..ref = (r) { ref = r as ButtonElement; })(); + ref; + } + ''', + // todo add block function, and other test cases, non-ref prop names + expectedOutput: /*language=dart*/ ''' + import 'dart:html'; + + import 'package:over_react/over_react.dart'; + import 'package:web_skin_dart/component2/all.dart'; + + content() { + var ref; + (ButtonToolbar()..ref = (ButtonElement /*?*/ r) => ref = r)(); + (ButtonToolbar()..ref = (r) => ref = r as ButtonElement /*?*/)(); + (ButtonToolbar()..ref = (ButtonElement /*?*/ r) { ref = r; })(); + (ButtonToolbar()..ref = (r) { ref = r as ButtonElement /*?*/; })(); + ref; + } + ''', + ); + }); + + + }); + }); +} From c87cee1b1c6664e0bcb6f2a38164001497d45a97 Mon Sep 17 00:00:00 2001 From: Sydney Jodon Date: Tue, 2 Jan 2024 10:24:12 -0600 Subject: [PATCH 02/11] Update to check param name before updating casts --- .../callback_ref_hint_suggestor.dart | 72 ++++---- test/mui_suggestors/components/shared.dart | 2 + .../callback_ref_hint_suggestor_test.dart | 162 +++++++++++++++--- 3 files changed, 175 insertions(+), 61 deletions(-) diff --git a/lib/src/null_safety_migration_suggestors/callback_ref_hint_suggestor.dart b/lib/src/null_safety_migration_suggestors/callback_ref_hint_suggestor.dart index 2a19c016..59e60cd7 100644 --- a/lib/src/null_safety_migration_suggestors/callback_ref_hint_suggestor.dart +++ b/lib/src/null_safety_migration_suggestors/callback_ref_hint_suggestor.dart @@ -13,77 +13,71 @@ // limitations under the License. import 'package:over_react_codemod/src/util/component_usage.dart'; -import 'package:over_react_codemod/src/util/component_usage_migrator.dart'; import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/visitor.dart'; import 'package:codemod/codemod.dart'; -import 'package:logging/logging.dart'; -final _log = Logger('CallbackrefHintSuggestor'); +/// Suggestor to add nullability hints to ref types. +class CallbackRefHintSuggestor extends RecursiveAstVisitor + with AstVisitingSuggestor { + CallbackRefHintSuggestor(); -// todo update comment -/// Suggestor that performs all the updates needed to migrate from the react_material_ui package -/// to the unify_ui package: -/// -/// - Rename specific components and objects -/// - Update WSD ButtonColor usages -/// - Rename import namespaces 'mui' => 'unify' -/// - Add fix me comments for manual checks -/// -/// Also see migration guide: https://github.com/Workiva/react_material_ui/tree/master/react_material_ui#how-to-migrate-from-reactmaterialui-to-unifyui -class CallbackRefHintSuggestor extends ComponentUsageMigrator { @override - bool shouldMigrateUsage(_) => true; + void visitCascadeExpression(CascadeExpression node) { + super.visitCascadeExpression(node); - @override - void migrateUsage(FluentComponentUsage usage) { - super.migrateUsage(usage); + final cascadedProps = node.cascadeSections + .whereType() + .where((assignment) => assignment.leftHandSide is PropertyAccess) + .map((assignment) => PropAssignment(assignment)); - for(final prop in usage.cascadedProps) { - if(prop.name.name == 'ref') { + for (final prop in cascadedProps) { + if (prop.name.name == 'ref') { final rhs = prop.rightHandSide; - if(rhs is FunctionExpression) { + if (rhs is FunctionExpression) { // todo add fixme if there are more than one params // Add nullability hint to parameter if typed. final param = rhs.parameters?.parameters.first; - if(param is SimpleFormalParameter) { + if (param is SimpleFormalParameter) { final type = param.type; - if(type != null) { + if (type != null) { yieldPatch(nullabilityHint, type.end, type.end); } } // Add nullability hint to any casts in the body of the callback ref. - final castVisitor = RefCastingVisitor(); - rhs.body.visitChildren(castVisitor); - for(final location in castVisitor.locationsNeedingHints) { - yieldPatch(nullabilityHint, location, location); + final refParamName = param?.name?.toString(); + if(refParamName != null) { + final castVisitor = RefCastVisitor(refParamName); + rhs.body.visitChildren(castVisitor); + for (final location in castVisitor.locationsNeedingHints) { + yieldPatch(nullabilityHint, location, location); + } } } } } } - - @override - String get fixmePrefix => 'FIXME(null_safety_migration)'; - - @override - // Override so that no flags are added. - void flagCommon(_) {} } -// todo add doc comment -class RefCastingVisitor extends RecursiveAstVisitor with AstVisitingSuggestor{ +/// Visitor to find [locationsNeedingHints] where ref types are casted within the body of a collback ref. +class RefCastVisitor extends RecursiveAstVisitor { + RefCastVisitor(this.refParamName); - final locationsNeedingHints = []; - RefCastingVisitor(); + late String refParamName; + + /// A list of offsets where a [nullabilityHint] patch should be added. + final locationsNeedingHints = []; @override void visitAsExpression(AsExpression node) { super.visitAsExpression(node); // todo check here to match ref type to be sure - locationsNeedingHints.add(node.type.end); + final varName = node.expression.toSource(); + if(varName == refParamName) { + locationsNeedingHints.add(node.type.end); + } } } diff --git a/test/mui_suggestors/components/shared.dart b/test/mui_suggestors/components/shared.dart index 5890ae51..a0b22d43 100644 --- a/test/mui_suggestors/components/shared.dart +++ b/test/mui_suggestors/components/shared.dart @@ -13,6 +13,8 @@ // limitations under the License. String withOverReactAndWsdImports(String source) => /*language=dart*/ ''' + import 'dart:html'; + import 'package:over_react/over_react.dart'; import 'package:web_skin_dart/component2/all.dart'; import 'package:web_skin_dart/component2/all.dart' as wsd_v2; diff --git a/test/null_safety_migration_suggestors/callback_ref_hint_suggestor_test.dart b/test/null_safety_migration_suggestors/callback_ref_hint_suggestor_test.dart index 940588dc..601e3cd2 100644 --- a/test/null_safety_migration_suggestors/callback_ref_hint_suggestor_test.dart +++ b/test/null_safety_migration_suggestors/callback_ref_hint_suggestor_test.dart @@ -15,9 +15,12 @@ import 'package:over_react_codemod/src/null_safety_migration_suggestors/callback_ref_hint_suggestor.dart'; import 'package:test/test.dart'; +import '../mui_suggestors/components/shared.dart'; import '../resolved_file_context.dart'; import '../util.dart'; +// todo add block function, and other test cases, non-ref prop names, mui/dom usages + void main() { final resolvedContext = SharedAnalysisContext.wsd; @@ -25,51 +28,166 @@ void main() { // (which is more common for the WSD context), it fails here instead of failing the first test. setUpAll(resolvedContext.warmUpAnalysis); - group('MuiButtonToolbarMigrator', () { + group('CallbackRefHintSuggestor', () { final testSuggestor = getSuggestorTester( CallbackRefHintSuggestor(), resolvedContext: resolvedContext, ); - group('migrates WSD ButtonToolbars', () { - test('that are either unnamespaced or namespaced, and either v1 or v2', - () async { + group('adds nullability hint to ref prop typed parameters', () { + test('', () async { await testSuggestor( - input:/*language=dart*/ ''' - import 'dart:html'; - - import 'package:over_react/over_react.dart'; - import 'package:web_skin_dart/component2/all.dart'; - + input: withOverReactAndWsdImports(/*language=dart*/ ''' content() { var ref; (ButtonToolbar()..ref = (ButtonElement r) => ref = r)(); - (ButtonToolbar()..ref = (r) => ref = r as ButtonElement)(); (ButtonToolbar()..ref = (ButtonElement r) { ref = r; })(); - (ButtonToolbar()..ref = (r) { ref = r as ButtonElement; })(); ref; } - ''', - // todo add block function, and other test cases, non-ref prop names - expectedOutput: /*language=dart*/ ''' - import 'dart:html'; - - import 'package:over_react/over_react.dart'; - import 'package:web_skin_dart/component2/all.dart'; - + '''), + expectedOutput: withOverReactAndWsdImports(/*language=dart*/ ''' content() { var ref; (ButtonToolbar()..ref = (ButtonElement /*?*/ r) => ref = r)(); - (ButtonToolbar()..ref = (r) => ref = r as ButtonElement /*?*/)(); (ButtonToolbar()..ref = (ButtonElement /*?*/ r) { ref = r; })(); + ref; + } + '''), + ); + }); + + test('for builders', () async { + await testSuggestor( + input: withOverReactAndWsdImports(/*language=dart*/ ''' + content() { + var ref; + (ButtonToolbar()..ref = (ButtonElement r) => ref = r); + (ButtonToolbar()..ref = (ButtonElement r) { ref = r; }); + ref; + } + '''), + expectedOutput: withOverReactAndWsdImports(/*language=dart*/ ''' + content() { + var ref; + (ButtonToolbar()..ref = (ButtonElement /*?*/ r) => ref = r); + (ButtonToolbar()..ref = (ButtonElement /*?*/ r) { ref = r; }); + ref; + } + '''), + ); + }); + }); + + group('adds nullability hint to casts in a callback ref body', () { + test('', () async { + await testSuggestor( + input: withOverReactAndWsdImports(/*language=dart*/ ''' + content() { + var ref; + (ButtonToolbar()..ref = (r) => ref = r as ButtonElement)(); + (ButtonToolbar()..ref = (r) { ref = r as ButtonElement; })(); + ref; + } + '''), + expectedOutput: withOverReactAndWsdImports(/*language=dart*/ ''' + content() { + var ref; + (ButtonToolbar()..ref = (r) => ref = r as ButtonElement /*?*/)(); (ButtonToolbar()..ref = (r) { ref = r as ButtonElement /*?*/; })(); ref; } - ''', + '''), + ); + }); + + test('for builders', () async { + await testSuggestor( + input: withOverReactAndWsdImports(/*language=dart*/ ''' + content() { + var ref; + (ButtonToolbar()..ref = (r) => ref = r as ButtonElement); + (ButtonToolbar()..ref = (r) { ref = r as ButtonElement; }); + ref; + } + '''), + expectedOutput: withOverReactAndWsdImports(/*language=dart*/ ''' + content() { + var ref; + (ButtonToolbar()..ref = (r) => ref = r as ButtonElement /*?*/); + (ButtonToolbar()..ref = (r) { ref = r as ButtonElement /*?*/; }); + ref; + } + '''), ); }); + test('only for casts of the ref param', () async { + await testSuggestor( + input: withOverReactAndWsdImports(/*language=dart*/ ''' + content() { + var ref; + final a = 1; + (ButtonToolbar() + ..ref = (r) { + ref = r as int; + ref = a as ButtonElement; + ref as int; + ref = r as ButtonElement; + })(); + (ButtonToolbar() + ..ref = (ButtonElement r) { + ref = r as int; + ref = a as ButtonElement; + })(); + (ButtonToolbar()..ref = (ButtonElement r) => ref = a as ButtonElement)(); + (ButtonToolbar()..ref = (_) => ref = a as ButtonElement)(); + ref; + } + '''), + expectedOutput: withOverReactAndWsdImports(/*language=dart*/ ''' + content() { + var ref; + final a = 1; + (ButtonToolbar() + ..ref = (r) { + ref = r as int /*?*/; + ref = a as ButtonElement; + ref as int; + ref = r as ButtonElement /*?*/; + })(); + (ButtonToolbar() + ..ref = (ButtonElement /*?*/ r) { + ref = r as int /*?*/; + ref = a as ButtonElement; + })(); + (ButtonToolbar()..ref = (ButtonElement /*?*/ r) => ref = a as ButtonElement)(); + (ButtonToolbar()..ref = (_) => ref = a as ButtonElement)(); + ref; + } + '''), + ); + }); + }); + test('does not add hints if they already exist', () async { + await testSuggestor( + input: withOverReactAndWsdImports(/*language=dart*/ ''' + content() { + var ref; + (ButtonToolbar()..ref = (ButtonElement /*?*/ r) => ref = r)(); + (ButtonToolbar()..ref = (r) { ref = r as ButtonElement /*?*/; })(); + ref; + } + '''), + expectedOutput: withOverReactAndWsdImports(/*language=dart*/ ''' + content() { + var ref; + (ButtonToolbar()..ref = (ButtonElement /*?*/ r) => ref = r)(); + (ButtonToolbar()..ref = (r) { ref = r as ButtonElement /*?*/; })(); + ref; + } + '''), + ); }); }); } From 0c80fb5b6b1272a6c850e2283fe4841119023ec5 Mon Sep 17 00:00:00 2001 From: Sydney Jodon Date: Wed, 6 Mar 2024 13:30:30 -0700 Subject: [PATCH 03/11] Resolve conflicts from master --- bin/null_safety_migration.dart | 15 ------ .../executables/null_safety_migration.dart | 48 ------------------- lib/src/executables/null_safety_prep.dart | 5 +- pubspec.yaml | 1 - 4 files changed, 4 insertions(+), 65 deletions(-) delete mode 100644 bin/null_safety_migration.dart delete mode 100644 lib/src/executables/null_safety_migration.dart diff --git a/bin/null_safety_migration.dart b/bin/null_safety_migration.dart deleted file mode 100644 index aae9978b..00000000 --- a/bin/null_safety_migration.dart +++ /dev/null @@ -1,15 +0,0 @@ -// Copyright 2024 Workiva Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -export 'package:over_react_codemod/src/executables/null_safety_migration.dart'; diff --git a/lib/src/executables/null_safety_migration.dart b/lib/src/executables/null_safety_migration.dart deleted file mode 100644 index b16d7789..00000000 --- a/lib/src/executables/null_safety_migration.dart +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright 2023 Workiva Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -import 'dart:io'; - -import 'package:args/args.dart'; -import 'package:codemod/codemod.dart'; -import 'package:over_react_codemod/src/ignoreable.dart'; -import 'package:over_react_codemod/src/null_safety_migration_suggestors/callback_ref_hint_suggestor.dart'; -import 'package:over_react_codemod/src/rmui_bundle_update_suggestors/constants.dart'; -import 'package:over_react_codemod/src/rmui_bundle_update_suggestors/dart_script_updater.dart'; -import 'package:over_react_codemod/src/rmui_bundle_update_suggestors/html_script_updater.dart'; -import 'package:over_react_codemod/src/util.dart'; -import 'package:over_react_codemod/src/util/pubspec_upgrader.dart'; - -const _changesRequiredOutput = """ - To update your code, run the following commands in your repository: - dart pub global activate over_react_codemod - dart pub global run over_react_codemod:null_safety_migration -"""; - -void main(List args) async { - final parser = ArgParser.allowAnything(); - - final parsedArgs = parser.parse(args); - - exitCode = await runInteractiveCodemodSequence( - allDartPathsExceptHidden(), - [ - CallbackRefHintSuggestor(), - ], - defaultYes: true, - args: parsedArgs.rest, - additionalHelpOutput: parser.usage, - changesRequiredOutput: _changesRequiredOutput, - ); -} diff --git a/lib/src/executables/null_safety_prep.dart b/lib/src/executables/null_safety_prep.dart index 6321665c..cf1b75c6 100644 --- a/lib/src/executables/null_safety_prep.dart +++ b/lib/src/executables/null_safety_prep.dart @@ -20,6 +20,8 @@ import 'package:over_react_codemod/src/dart3_suggestors/null_safety_prep/use_ref import 'package:over_react_codemod/src/ignoreable.dart'; import 'package:over_react_codemod/src/util.dart'; +import '../null_safety_migration_suggestors/callback_ref_hint_suggestor.dart'; + const _changesRequiredOutput = """ To update your code, run the following commands in your repository: pub global activate over_react_codemod @@ -36,7 +38,8 @@ void main(List args) async { dartPaths, aggregate([ UseRefInitMigration(), - ].map((s) => ignoreable(s))), + CallbackRefHintSuggestor(), + ]), defaultYes: true, args: parsedArgs.rest, additionalHelpOutput: parser.usage, diff --git a/pubspec.yaml b/pubspec.yaml index c1398980..696c061f 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -49,7 +49,6 @@ executables: intl_message_migration: sort_intl: unify_package_rename: - null_safety_migration: dependency_validator: ignore: From 1224cf4a10b71e392ca56738503c96661026166f Mon Sep 17 00:00:00 2001 From: Sydney Jodon Date: Wed, 6 Mar 2024 14:13:25 -0700 Subject: [PATCH 04/11] Move files around --- .../null_safety_prep}/callback_ref_hint_suggestor.dart | 0 lib/src/executables/null_safety_prep.dart | 2 +- .../callback_ref_hint_suggestor_test.dart | 8 ++++---- 3 files changed, 5 insertions(+), 5 deletions(-) rename lib/src/{null_safety_migration_suggestors => dart3_suggestors/null_safety_prep}/callback_ref_hint_suggestor.dart (100%) rename test/{null_safety_migration_suggestors => dart3_suggestors/null_safety_prep}/callback_ref_hint_suggestor_test.dart (96%) diff --git a/lib/src/null_safety_migration_suggestors/callback_ref_hint_suggestor.dart b/lib/src/dart3_suggestors/null_safety_prep/callback_ref_hint_suggestor.dart similarity index 100% rename from lib/src/null_safety_migration_suggestors/callback_ref_hint_suggestor.dart rename to lib/src/dart3_suggestors/null_safety_prep/callback_ref_hint_suggestor.dart diff --git a/lib/src/executables/null_safety_prep.dart b/lib/src/executables/null_safety_prep.dart index cf1b75c6..59c60589 100644 --- a/lib/src/executables/null_safety_prep.dart +++ b/lib/src/executables/null_safety_prep.dart @@ -20,7 +20,7 @@ import 'package:over_react_codemod/src/dart3_suggestors/null_safety_prep/use_ref import 'package:over_react_codemod/src/ignoreable.dart'; import 'package:over_react_codemod/src/util.dart'; -import '../null_safety_migration_suggestors/callback_ref_hint_suggestor.dart'; +import '../dart3_suggestors/null_safety_prep/callback_ref_hint_suggestor.dart'; const _changesRequiredOutput = """ To update your code, run the following commands in your repository: diff --git a/test/null_safety_migration_suggestors/callback_ref_hint_suggestor_test.dart b/test/dart3_suggestors/null_safety_prep/callback_ref_hint_suggestor_test.dart similarity index 96% rename from test/null_safety_migration_suggestors/callback_ref_hint_suggestor_test.dart rename to test/dart3_suggestors/null_safety_prep/callback_ref_hint_suggestor_test.dart index 601e3cd2..e3656203 100644 --- a/test/null_safety_migration_suggestors/callback_ref_hint_suggestor_test.dart +++ b/test/dart3_suggestors/null_safety_prep/callback_ref_hint_suggestor_test.dart @@ -12,12 +12,12 @@ // See the License for the specific language governing permissions and // limitations under the License. -import 'package:over_react_codemod/src/null_safety_migration_suggestors/callback_ref_hint_suggestor.dart'; +import 'package:over_react_codemod/src/dart3_suggestors/null_safety_prep/callback_ref_hint_suggestor.dart'; import 'package:test/test.dart'; -import '../mui_suggestors/components/shared.dart'; -import '../resolved_file_context.dart'; -import '../util.dart'; +import '../../mui_suggestors/components/shared.dart'; +import '../../resolved_file_context.dart'; +import '../../util.dart'; // todo add block function, and other test cases, non-ref prop names, mui/dom usages From 8b354b8043a1db628fd2c23a7470b9a64bf5250d Mon Sep 17 00:00:00 2001 From: Sydney Jodon Date: Wed, 6 Mar 2024 16:05:57 -0700 Subject: [PATCH 05/11] Fix hint already exists test --- .../callback_ref_hint_suggestor.dart | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) 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 59e60cd7..25e1b928 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 @@ -35,13 +35,11 @@ class CallbackRefHintSuggestor extends RecursiveAstVisitor if (prop.name.name == 'ref') { final rhs = prop.rightHandSide; if (rhs is FunctionExpression) { - // todo add fixme if there are more than one params - // Add nullability hint to parameter if typed. final param = rhs.parameters?.parameters.first; if (param is SimpleFormalParameter) { final type = param.type; - if (type != null) { + if (type != null && !_hintAlreadyExists(type)) { yieldPatch(nullabilityHint, type.end, type.end); } } @@ -73,12 +71,17 @@ class RefCastVisitor extends RecursiveAstVisitor { @override void visitAsExpression(AsExpression node) { super.visitAsExpression(node); - // todo check here to match ref type to be sure final varName = node.expression.toSource(); - if(varName == refParamName) { + if(varName == refParamName && !_hintAlreadyExists(node.type)) { locationsNeedingHints.add(node.type.end); } } } +/// Whether the nullability hint already exists after [type]. +bool _hintAlreadyExists(TypeAnnotation type) { + // The nullability hint will follow the type so we need to check the next token to find the comment if it exists. + return type.endToken.next?.precedingComments?.value().contains(nullabilityHint) ?? false; +} + const nullabilityHint = '/*?*/'; From fdba71b62eb340af359ec4e9f5025a5c4cc4d958 Mon Sep 17 00:00:00 2001 From: Sydney Jodon Date: Thu, 7 Mar 2024 16:43:05 -0700 Subject: [PATCH 06/11] Get partial implementation for variable declaration hint --- .../callback_ref_hint_suggestor.dart | 103 +++++++++++++++++- .../callback_ref_hint_suggestor_test.dart | 45 ++++++++ 2 files changed, 142 insertions(+), 6 deletions(-) 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 25e1b928..1b1a1f19 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 @@ -12,14 +12,25 @@ // See the License for the specific language governing permissions and // limitations under the License. +import 'package:analyzer/dart/element/element.dart'; +import 'package:analyzer/dart/element/type.dart'; +import 'package:collection/collection.dart'; +import 'package:logging/logging.dart'; import 'package:over_react_codemod/src/util/component_usage.dart'; import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/visitor.dart'; import 'package:codemod/codemod.dart'; +import '../../util.dart'; +import '../../util/class_suggestor.dart'; + +final _log = Logger('CallbackRefHintSuggestor'); + /// Suggestor to add nullability hints to ref types. +/// +/// todo doc comment examples class CallbackRefHintSuggestor extends RecursiveAstVisitor - with AstVisitingSuggestor { + with ClassSuggestor { CallbackRefHintSuggestor(); @override @@ -47,9 +58,45 @@ class CallbackRefHintSuggestor extends RecursiveAstVisitor // Add nullability hint to any casts in the body of the callback ref. final refParamName = param?.name?.toString(); if(refParamName != null) { - final castVisitor = RefCastVisitor(refParamName); - rhs.body.visitChildren(castVisitor); - for (final location in castVisitor.locationsNeedingHints) { + final bodyVisitor = RefBodyVisitor(refParamName); + + final function = rhs.unParenthesized.tryCast(); + + if (function == null) return null; + + final refCallbackArg = function.parameters?.parameters.firstOrNull; + if (refCallbackArg == null) return null; + + final referencesToArg = allDescendantsOfType(function.body) + .where((identifier) => identifier.staticElement == refCallbackArg.declaredElement); + + for (final reference in referencesToArg) { + final parent = reference.parent; + if (parent is AssignmentExpression && parent.rightHandSide == reference) { + final lhs = parent.leftHandSide; + if (lhs is Identifier) { + final varInFnComponent = lhs.staticElement?.tryCast(); + final varInClassComponent = + lhs.parent?.tryCast()?.writeElement?.tryCast()?.variable; + final type = varInClassComponent?.type; + if (varInClassComponent != null + // && !_hintAlreadyExists(type) + ) { + final a = 1; + yieldPatch(nullabilityHint, varInClassComponent.nameOffset, varInClassComponent.nameOffset); + }if (varInFnComponent != null + // && !_hintAlreadyExists(type) + ) { + final a = 1; + yieldPatch(nullabilityHint, varInFnComponent.nameOffset, varInFnComponent.nameOffset); + } + // return varInFnComponent ?? varInClassComponent; + } + } + } + + rhs.body.visitChildren(bodyVisitor); + for (final location in bodyVisitor.locationsNeedingHints) { yieldPatch(nullabilityHint, location, location); } } @@ -57,11 +104,23 @@ class CallbackRefHintSuggestor extends RecursiveAstVisitor } } } + + @override + Future generatePatches() async { + _log.info('Resolving ${context.relativePath}...'); + + final result = await context.getResolvedUnit(); + if (result == null) { + throw Exception( + 'Could not get resolved result for "${context.relativePath}"'); + } + result.unit.visitChildren(this); + } } /// Visitor to find [locationsNeedingHints] where ref types are casted within the body of a collback ref. -class RefCastVisitor extends RecursiveAstVisitor { - RefCastVisitor(this.refParamName); +class RefBodyVisitor extends RecursiveAstVisitor with ClassSuggestor { + RefBodyVisitor(this.refParamName); late String refParamName; @@ -76,6 +135,38 @@ class RefCastVisitor extends RecursiveAstVisitor { locationsNeedingHints.add(node.type.end); } } + + @override + void visitAssignmentExpression(AssignmentExpression node) { + super.visitAssignmentExpression(node); + // todo - see if there are edge cases for this like casting + final rhs = node.rightHandSide; + String? varName; + if(rhs is SimpleIdentifier) { + varName = rhs.name; + } if(rhs is AsExpression) { + varName = rhs.expression.toSource(); + } + if(varName == refParamName) { + final lhs = node.leftHandSide; + if(lhs is SimpleIdentifier) { + + } + } + // TODO: implement visitAssignmentExpression + } + + @override + Future generatePatches() async { + _log.info('Resolving ${context.relativePath}...'); + + final result = await context.getResolvedUnit(); + if (result == null) { + throw Exception( + 'Could not get resolved result for "${context.relativePath}"'); + } + result.unit.visitChildren(this); + } } /// Whether the nullability hint already exists after [type]. 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 e3656203..23a26d7e 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 @@ -169,6 +169,51 @@ void main() { }); }); + group('adds nullability hint to class ref variables', () { + test('', () async { + await testSuggestor( + input: withOverReactAndWsdImports(/*language=dart*/ ''' + ButtonElement ref5; + content() { + ButtonElement ref2; + ButtonElement ref4; + (ButtonToolbar()..ref = (r) => ref5 = r)(); + (ButtonToolbar()..ref = (r) { + ButtonElement ref3; + ref2 = r; + ref3 = r as ButtonElement; + final a = ButtonElement(); + ref4 = a; + ref3; + }); + ref5; + ref2; + ref4; + } + '''), + expectedOutput: withOverReactAndWsdImports(/*language=dart*/ ''' + content() { + ButtonElement /*?*/ ref5; + ButtonElement /*?*/ ref2; + ButtonElement ref4; + (ButtonToolbar()..ref = (r) => ref5 = r)(); + (ButtonToolbar()..ref = (r) { + ButtonElement /*?*/ ref3; + ref2 = r; + ref3 = r as ButtonElement /*?*/; + final a = 1; + ref4 = a; + }); + ref5; + ref2; + ref3; + ref4; + } + '''), + ); + }); + }); + test('does not add hints if they already exist', () async { await testSuggestor( input: withOverReactAndWsdImports(/*language=dart*/ ''' From b8cac5200b2dadbed0d0548bfbdda43834b5db12 Mon Sep 17 00:00:00 2001 From: Sydney Jodon Date: Tue, 19 Mar 2024 16:03:55 -0700 Subject: [PATCH 07/11] Fix resolved ast to get the right offset --- .../callback_ref_hint_suggestor.dart | 262 ++++++++++++------ .../callback_ref_hint_suggestor_test.dart | 7 +- 2 files changed, 185 insertions(+), 84 deletions(-) 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 1b1a1f19..f99ab245 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 @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +import 'package:analyzer/dart/analysis/results.dart'; import 'package:analyzer/dart/element/element.dart'; import 'package:analyzer/dart/element/type.dart'; import 'package:collection/collection.dart'; @@ -33,8 +34,10 @@ class CallbackRefHintSuggestor extends RecursiveAstVisitor with ClassSuggestor { CallbackRefHintSuggestor(); + late ResolvedUnitResult result; + @override - void visitCascadeExpression(CascadeExpression node) { + Future visitCascadeExpression(CascadeExpression node) async { super.visitCascadeExpression(node); final cascadedProps = node.cascadeSections @@ -44,61 +47,73 @@ class CallbackRefHintSuggestor extends RecursiveAstVisitor for (final prop in cascadedProps) { if (prop.name.name == 'ref') { - final rhs = prop.rightHandSide; - if (rhs is FunctionExpression) { - // Add nullability hint to parameter if typed. - final param = rhs.parameters?.parameters.first; - if (param is SimpleFormalParameter) { - final type = param.type; - if (type != null && !_hintAlreadyExists(type)) { - yieldPatch(nullabilityHint, type.end, type.end); - } - } - - // Add nullability hint to any casts in the body of the callback ref. - final refParamName = param?.name?.toString(); - if(refParamName != null) { - final bodyVisitor = RefBodyVisitor(refParamName); - - final function = rhs.unParenthesized.tryCast(); - - if (function == null) return null; + final rhs = + prop.rightHandSide.unParenthesized.tryCast(); + if (rhs == null) return null; - final refCallbackArg = function.parameters?.parameters.firstOrNull; - if (refCallbackArg == null) return null; + // Add nullability hint to parameter if typed. + final param = rhs.parameters?.parameters.first; + if (param is SimpleFormalParameter) { + final type = param.type; + if (type != null && !_hintAlreadyExists(type)) { + yieldPatch(nullabilityHint, type.end, type.end); + } + } - final referencesToArg = allDescendantsOfType(function.body) - .where((identifier) => identifier.staticElement == refCallbackArg.declaredElement); + final refParamName = param?.name?.toString(); + if (refParamName != null) { + // Add nullability hint to ref variable declarations. + final refCallbackArg = rhs.parameters?.parameters.firstOrNull; + if (refCallbackArg != null) { + final referencesToArg = allDescendantsOfType(rhs.body) + .where((identifier) => + identifier.staticElement == refCallbackArg.declaredElement); for (final reference in referencesToArg) { final parent = reference.parent; - if (parent is AssignmentExpression && parent.rightHandSide == reference) { + if (parent is AssignmentExpression && + parent.rightHandSide == reference) { final lhs = parent.leftHandSide; if (lhs is Identifier) { - final varInFnComponent = lhs.staticElement?.tryCast(); - final varInClassComponent = - lhs.parent?.tryCast()?.writeElement?.tryCast()?.variable; - final type = varInClassComponent?.type; + final varInFnComponent = + lhs.staticElement?.tryCast(); + final varInClassComponent = lhs.parent + ?.tryCast() + ?.writeElement + ?.tryCast() + ?.variable; if (varInClassComponent != null - // && !_hintAlreadyExists(type) - ) { - final a = 1; - yieldPatch(nullabilityHint, varInClassComponent.nameOffset, varInClassComponent.nameOffset); - }if (varInFnComponent != null - // && !_hintAlreadyExists(type) + ) { + final decl = lookUpVariable(varInClassComponent, result.unit)?.parent.tryCast()?.type; + if(decl != null && !_hintAlreadyExists(decl)) { + yieldPatch( + nullabilityHint, + decl.end, + decl.end); + } + } else if (varInFnComponent != null ) { - final a = 1; - yieldPatch(nullabilityHint, varInFnComponent.nameOffset, varInFnComponent.nameOffset); + final decl = lookUpVariable(varInFnComponent, result.unit)?.parent.tryCast()?.type; + if(decl != null && !_hintAlreadyExists(decl)) { + yieldPatch( + nullabilityHint, + decl.end, + decl.end); + } } // return varInFnComponent ?? varInClassComponent; } } } + } - rhs.body.visitChildren(bodyVisitor); - for (final location in bodyVisitor.locationsNeedingHints) { - yieldPatch(nullabilityHint, location, location); - } + // Add nullability hint to any casts in the body of the callback ref. + final refCasts = allDescendantsOfType(rhs.body).where( + (expression) => + expression.expression.toSource() == refParamName && + !_hintAlreadyExists(expression.type)); + for (final cast in refCasts) { + yieldPatch(nullabilityHint, cast.type.end, cast.type.end); } } } @@ -109,70 +124,153 @@ class CallbackRefHintSuggestor extends RecursiveAstVisitor Future generatePatches() async { _log.info('Resolving ${context.relativePath}...'); - final result = await context.getResolvedUnit(); - if (result == null) { + final r = await context.getResolvedUnit(); + if (r == null) { throw Exception( 'Could not get resolved result for "${context.relativePath}"'); } + result = r; result.unit.visitChildren(this); } } -/// Visitor to find [locationsNeedingHints] where ref types are casted within the body of a collback ref. -class RefBodyVisitor extends RecursiveAstVisitor with ClassSuggestor { - RefBodyVisitor(this.refParamName); +/// Whether the nullability hint already exists after [type]. +bool _hintAlreadyExists(TypeAnnotation type) { + // The nullability hint will follow the type so we need to check the next token to find the comment if it exists. + return type.endToken.next?.precedingComments + ?.value() + .contains(nullabilityHint) ?? + false; +} - late String refParamName; +/// Returns the AST node of the variable declaration associated with the [element] within [root], +/// or null if the [element] doesn't correspond to a variable declaration, or if it can't be found in [root]. +VariableDeclaration? lookUpVariable(Element element, AstNode root) { + final node = NodeLocator2(element.nameOffset).searchWithin(root); + if (node is VariableDeclaration && node.declaredElement == element) { + return node; + } - /// A list of offsets where a [nullabilityHint] patch should be added. - final locationsNeedingHints = []; + return null; +} - @override - void visitAsExpression(AsExpression node) { - super.visitAsExpression(node); - final varName = node.expression.toSource(); - if(varName == refParamName && !_hintAlreadyExists(node.type)) { - locationsNeedingHints.add(node.type.end); +/// An object used to locate the [AstNode] associated with a source range. +/// More specifically, they will return the deepest [AstNode] which completely +/// encompasses the specified range with some exceptions: +/// +/// - Offsets that fall between the name and type/formal parameter list of a +/// declaration will return the declaration node and not the parameter list +/// node. +class NodeLocator2 extends UnifyingAstVisitor { + /// The inclusive start offset of the range used to identify the node. + final int _startOffset; + + /// The inclusive end offset of the range used to identify the node. + final int _endOffset; + + /// The found node or `null` if there is no such node. + AstNode? _foundNode; + + /// Initialize a newly created locator to locate the deepest [AstNode] for + /// which `node.offset <= [startOffset]` and `[endOffset] < node.end`. + /// + /// If [endOffset] is not provided, then it is considered the same as the + /// given [startOffset]. + NodeLocator2(int startOffset, [int? endOffset]) + : _startOffset = startOffset, + _endOffset = endOffset ?? startOffset; + + /// Search within the given AST [node] and return the node that was found, + /// or `null` if no node was found. + AstNode? searchWithin(AstNode? node) { + if (node == null) { + return null; + } + try { + node.accept(this); + } catch (_) { + return null; } + return _foundNode; } @override - void visitAssignmentExpression(AssignmentExpression node) { - super.visitAssignmentExpression(node); - // todo - see if there are edge cases for this like casting - final rhs = node.rightHandSide; - String? varName; - if(rhs is SimpleIdentifier) { - varName = rhs.name; - } if(rhs is AsExpression) { - varName = rhs.expression.toSource(); + void visitConstructorDeclaration(ConstructorDeclaration node) { + // Names do not have AstNodes but offsets at the end should be treated as + // part of the declaration (not parameter list). + if (_startOffset == _endOffset && + _startOffset == (node.name ?? node.returnType).end) { + _foundNode = node; + return; } - if(varName == refParamName) { - final lhs = node.leftHandSide; - if(lhs is SimpleIdentifier) { - } - } - // TODO: implement visitAssignmentExpression + super.visitConstructorDeclaration(node); } @override - Future generatePatches() async { - _log.info('Resolving ${context.relativePath}...'); + void visitFunctionDeclaration(FunctionDeclaration node) { + // Names do not have AstNodes but offsets at the end should be treated as + // part of the declaration (not parameter list). + if (_startOffset == _endOffset && _startOffset == node.name.end) { + _foundNode = node; + return; + } - final result = await context.getResolvedUnit(); - if (result == null) { - throw Exception( - 'Could not get resolved result for "${context.relativePath}"'); + super.visitFunctionDeclaration(node); + } + + @override + void visitMethodDeclaration(MethodDeclaration node) { + // Names do not have AstNodes but offsets at the end should be treated as + // part of the declaration (not parameter list). + if (_startOffset == _endOffset && _startOffset == node.name.end) { + _foundNode = node; + return; } - result.unit.visitChildren(this); + + super.visitMethodDeclaration(node); } -} -/// Whether the nullability hint already exists after [type]. -bool _hintAlreadyExists(TypeAnnotation type) { - // The nullability hint will follow the type so we need to check the next token to find the comment if it exists. - return type.endToken.next?.precedingComments?.value().contains(nullabilityHint) ?? false; + @override + void visitNode(AstNode node) { + // Don't visit a new tree if the result has been already found. + if (_foundNode != null) { + return; + } + // Check whether the current node covers the selection. + var beginToken = node.beginToken; + var endToken = node.endToken; + // Don't include synthetic tokens. + while (endToken != beginToken) { + // Fasta scanner reports unterminated string literal errors + // and generates a synthetic string token with non-zero length. + // Because of this, check for length > 0 rather than !isSynthetic. + if (endToken.isEof || endToken.length > 0) { + break; + } + endToken = endToken.previous!; + } + var end = endToken.end; + var start = node.offset; + if (end <= _startOffset || start > _endOffset) { + return; + } + // Check children. + try { + node.visitChildren(this); + } catch (_) { + // Ignore the exception and proceed in order to visit the rest of the + // structure. + } + // Found a child. + if (_foundNode != null) { + return; + } + // Check this node. + if (start <= _startOffset && _endOffset < end) { + _foundNode = node; + } + } } const nullabilityHint = '/*?*/'; 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 23a26d7e..f5a92e10 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 @@ -175,7 +175,7 @@ void main() { input: withOverReactAndWsdImports(/*language=dart*/ ''' ButtonElement ref5; content() { - ButtonElement ref2; + ButtonElement /*?*/ ref2; ButtonElement ref4; (ButtonToolbar()..ref = (r) => ref5 = r)(); (ButtonToolbar()..ref = (r) { @@ -184,6 +184,8 @@ void main() { ref3 = r as ButtonElement; final a = ButtonElement(); ref4 = a; + ref2 = r; + ref5 = r; ref3; }); ref5; @@ -192,8 +194,8 @@ void main() { } '''), expectedOutput: withOverReactAndWsdImports(/*language=dart*/ ''' + ButtonElement /*?*/ ref5; content() { - ButtonElement /*?*/ ref5; ButtonElement /*?*/ ref2; ButtonElement ref4; (ButtonToolbar()..ref = (r) => ref5 = r)(); @@ -203,6 +205,7 @@ void main() { ref3 = r as ButtonElement /*?*/; final a = 1; ref4 = a; + ref2 = r; }); ref5; ref2; From c75eab6f43e25a9a671f31e5b3373c43c5d3f663 Mon Sep 17 00:00:00 2001 From: Sydney Jodon Date: Tue, 19 Mar 2024 17:38:01 -0700 Subject: [PATCH 08/11] Update tests --- .../analyzer_plugin_utils.dart | 151 +++++++++++++++ .../callback_ref_hint_suggestor.dart | 179 +++--------------- .../callback_ref_hint_suggestor_test.dart | 74 +++++--- 3 files changed, 223 insertions(+), 181 deletions(-) create mode 100644 lib/src/dart3_suggestors/null_safety_prep/analyzer_plugin_utils.dart diff --git a/lib/src/dart3_suggestors/null_safety_prep/analyzer_plugin_utils.dart b/lib/src/dart3_suggestors/null_safety_prep/analyzer_plugin_utils.dart new file mode 100644 index 00000000..5afcc755 --- /dev/null +++ b/lib/src/dart3_suggestors/null_safety_prep/analyzer_plugin_utils.dart @@ -0,0 +1,151 @@ +// These utilities were copied from analyzer utils in over_react/analyzer_plugin +// Permalink: https://github.com/Workiva/over_react/blob/a8129f38ea8dfa0023d06250349fc8e86025df3a/tools/analyzer_plugin/lib/src/util/analyzer_util.dart#L4 +// Permalink: https://github.com/Workiva/over_react/blob/a8129f38ea8dfa0023d06250349fc8e86025df3a/tools/analyzer_plugin/lib/src/util/ast_util.dart +// +// Copyright 2024 Workiva Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import 'package:analyzer/dart/element/element.dart'; +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; + +/// Returns the AST node of the variable declaration associated with the [element] within [root], +/// or null if the [element] doesn't correspond to a variable declaration, or if it can't be found in [root]. +VariableDeclaration? lookUpVariable(Element element, AstNode root) { + final node = NodeLocator2(element.nameOffset).searchWithin(root); + if (node is VariableDeclaration && node.declaredElement == element) { + return node; + } + + return null; +} + +/// An object used to locate the [AstNode] associated with a source range. +/// More specifically, they will return the deepest [AstNode] which completely +/// encompasses the specified range with some exceptions: +/// +/// - Offsets that fall between the name and type/formal parameter list of a +/// declaration will return the declaration node and not the parameter list +/// node. +class NodeLocator2 extends UnifyingAstVisitor { + /// The inclusive start offset of the range used to identify the node. + final int _startOffset; + + /// The inclusive end offset of the range used to identify the node. + final int _endOffset; + + /// The found node or `null` if there is no such node. + AstNode? _foundNode; + + /// Initialize a newly created locator to locate the deepest [AstNode] for + /// which `node.offset <= [startOffset]` and `[endOffset] < node.end`. + /// + /// If [endOffset] is not provided, then it is considered the same as the + /// given [startOffset]. + NodeLocator2(int startOffset, [int? endOffset]) + : _startOffset = startOffset, + _endOffset = endOffset ?? startOffset; + + /// Search within the given AST [node] and return the node that was found, + /// or `null` if no node was found. + AstNode? searchWithin(AstNode? node) { + if (node == null) { + return null; + } + try { + node.accept(this); + } catch (_) { + return null; + } + return _foundNode; + } + + @override + void visitConstructorDeclaration(ConstructorDeclaration node) { + // Names do not have AstNodes but offsets at the end should be treated as + // part of the declaration (not parameter list). + if (_startOffset == _endOffset && + _startOffset == (node.name ?? node.returnType).end) { + _foundNode = node; + return; + } + + super.visitConstructorDeclaration(node); + } + + @override + void visitFunctionDeclaration(FunctionDeclaration node) { + // Names do not have AstNodes but offsets at the end should be treated as + // part of the declaration (not parameter list). + if (_startOffset == _endOffset && _startOffset == node.name.end) { + _foundNode = node; + return; + } + + super.visitFunctionDeclaration(node); + } + + @override + void visitMethodDeclaration(MethodDeclaration node) { + // Names do not have AstNodes but offsets at the end should be treated as + // part of the declaration (not parameter list). + if (_startOffset == _endOffset && _startOffset == node.name.end) { + _foundNode = node; + return; + } + + super.visitMethodDeclaration(node); + } + + @override + void visitNode(AstNode node) { + // Don't visit a new tree if the result has been already found. + if (_foundNode != null) { + return; + } + // Check whether the current node covers the selection. + var beginToken = node.beginToken; + var endToken = node.endToken; + // Don't include synthetic tokens. + while (endToken != beginToken) { + // Fasta scanner reports unterminated string literal errors + // and generates a synthetic string token with non-zero length. + // Because of this, check for length > 0 rather than !isSynthetic. + if (endToken.isEof || endToken.length > 0) { + break; + } + endToken = endToken.previous!; + } + var end = endToken.end; + var start = node.offset; + if (end <= _startOffset || start > _endOffset) { + return; + } + // Check children. + try { + node.visitChildren(this); + } catch (_) { + // Ignore the exception and proceed in order to visit the rest of the + // structure. + } + // Found a child. + if (_foundNode != null) { + return; + } + // Check this node. + if (start <= _startOffset && _endOffset < end) { + _foundNode = node; + } + } +} 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 f99ab245..4694bfe8 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 @@ -1,3 +1,6 @@ +// Adapted from the add_create_ref assist in over_react/analyzer_plugin +// Permalink: https://github.com/Workiva/over_react/blob/a8129f38ea8dfa0023d06250349fc8e86025df3a/tools/analyzer_plugin/lib/src/assist/refs/add_create_ref.dart#L4 +// // Copyright 2024 Workiva Inc. // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -14,16 +17,15 @@ import 'package:analyzer/dart/analysis/results.dart'; import 'package:analyzer/dart/element/element.dart'; -import 'package:analyzer/dart/element/type.dart'; import 'package:collection/collection.dart'; import 'package:logging/logging.dart'; import 'package:over_react_codemod/src/util/component_usage.dart'; import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/visitor.dart'; -import 'package:codemod/codemod.dart'; import '../../util.dart'; import '../../util/class_suggestor.dart'; +import 'analyzer_plugin_utils.dart'; final _log = Logger('CallbackRefHintSuggestor'); @@ -75,33 +77,26 @@ class CallbackRefHintSuggestor extends RecursiveAstVisitor parent.rightHandSide == reference) { final lhs = parent.leftHandSide; if (lhs is Identifier) { - final varInFnComponent = - lhs.staticElement?.tryCast(); - final varInClassComponent = lhs.parent - ?.tryCast() - ?.writeElement - ?.tryCast() - ?.variable; - if (varInClassComponent != null - ) { - final decl = lookUpVariable(varInClassComponent, result.unit)?.parent.tryCast()?.type; - if(decl != null && !_hintAlreadyExists(decl)) { - yieldPatch( - nullabilityHint, - decl.end, - decl.end); - } - } else if (varInFnComponent != null - ) { - final decl = lookUpVariable(varInFnComponent, result.unit)?.parent.tryCast()?.type; - if(decl != null && !_hintAlreadyExists(decl)) { - yieldPatch( - nullabilityHint, - decl.end, - decl.end); + final varElement = + // Variable in function component. + lhs.staticElement?.tryCast() ?? + // Variable in class component. + lhs.parent + ?.tryCast() + ?.writeElement + ?.tryCast() + ?.variable; + if (varElement != null) { + final varType = lookUpVariable(varElement, result.unit) + ?.parent + .tryCast() + ?.type; + if (varType != null && + !_hintAlreadyExists(varType) && + varType.toSource() != 'dynamic') { + yieldPatch(nullabilityHint, varType.end, varType.end); } } - // return varInFnComponent ?? varInClassComponent; } } } @@ -143,134 +138,4 @@ bool _hintAlreadyExists(TypeAnnotation type) { false; } -/// Returns the AST node of the variable declaration associated with the [element] within [root], -/// or null if the [element] doesn't correspond to a variable declaration, or if it can't be found in [root]. -VariableDeclaration? lookUpVariable(Element element, AstNode root) { - final node = NodeLocator2(element.nameOffset).searchWithin(root); - if (node is VariableDeclaration && node.declaredElement == element) { - return node; - } - - return null; -} - -/// An object used to locate the [AstNode] associated with a source range. -/// More specifically, they will return the deepest [AstNode] which completely -/// encompasses the specified range with some exceptions: -/// -/// - Offsets that fall between the name and type/formal parameter list of a -/// declaration will return the declaration node and not the parameter list -/// node. -class NodeLocator2 extends UnifyingAstVisitor { - /// The inclusive start offset of the range used to identify the node. - final int _startOffset; - - /// The inclusive end offset of the range used to identify the node. - final int _endOffset; - - /// The found node or `null` if there is no such node. - AstNode? _foundNode; - - /// Initialize a newly created locator to locate the deepest [AstNode] for - /// which `node.offset <= [startOffset]` and `[endOffset] < node.end`. - /// - /// If [endOffset] is not provided, then it is considered the same as the - /// given [startOffset]. - NodeLocator2(int startOffset, [int? endOffset]) - : _startOffset = startOffset, - _endOffset = endOffset ?? startOffset; - - /// Search within the given AST [node] and return the node that was found, - /// or `null` if no node was found. - AstNode? searchWithin(AstNode? node) { - if (node == null) { - return null; - } - try { - node.accept(this); - } catch (_) { - return null; - } - return _foundNode; - } - - @override - void visitConstructorDeclaration(ConstructorDeclaration node) { - // Names do not have AstNodes but offsets at the end should be treated as - // part of the declaration (not parameter list). - if (_startOffset == _endOffset && - _startOffset == (node.name ?? node.returnType).end) { - _foundNode = node; - return; - } - - super.visitConstructorDeclaration(node); - } - - @override - void visitFunctionDeclaration(FunctionDeclaration node) { - // Names do not have AstNodes but offsets at the end should be treated as - // part of the declaration (not parameter list). - if (_startOffset == _endOffset && _startOffset == node.name.end) { - _foundNode = node; - return; - } - - super.visitFunctionDeclaration(node); - } - - @override - void visitMethodDeclaration(MethodDeclaration node) { - // Names do not have AstNodes but offsets at the end should be treated as - // part of the declaration (not parameter list). - if (_startOffset == _endOffset && _startOffset == node.name.end) { - _foundNode = node; - return; - } - - super.visitMethodDeclaration(node); - } - - @override - void visitNode(AstNode node) { - // Don't visit a new tree if the result has been already found. - if (_foundNode != null) { - return; - } - // Check whether the current node covers the selection. - var beginToken = node.beginToken; - var endToken = node.endToken; - // Don't include synthetic tokens. - while (endToken != beginToken) { - // Fasta scanner reports unterminated string literal errors - // and generates a synthetic string token with non-zero length. - // Because of this, check for length > 0 rather than !isSynthetic. - if (endToken.isEof || endToken.length > 0) { - break; - } - endToken = endToken.previous!; - } - var end = endToken.end; - var start = node.offset; - if (end <= _startOffset || start > _endOffset) { - return; - } - // Check children. - try { - node.visitChildren(this); - } catch (_) { - // Ignore the exception and proceed in order to visit the rest of the - // structure. - } - // Found a child. - if (_foundNode != null) { - return; - } - // Check this node. - if (start <= _startOffset && _endOffset < end) { - _foundNode = node; - } - } -} - const nullabilityHint = '/*?*/'; 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 f5a92e10..39219c69 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 @@ -173,44 +173,70 @@ void main() { test('', () async { await testSuggestor( input: withOverReactAndWsdImports(/*language=dart*/ ''' - ButtonElement ref5; + ButtonElement ref1; content() { - ButtonElement /*?*/ ref2; - ButtonElement ref4; - (ButtonToolbar()..ref = (r) => ref5 = r)(); + ButtonElement ref2; + ButtonElement ref3; + (ButtonToolbar()..ref = (r) => ref1 = r)(); (ButtonToolbar()..ref = (r) { - ButtonElement ref3; + ButtonElement ref4; ref2 = r; - ref3 = r as ButtonElement; final a = ButtonElement(); - ref4 = a; - ref2 = r; - ref5 = r; - ref3; + ref3 = a; + ref4 = r; + ref4; }); - ref5; + ref1; ref2; - ref4; + ref3; } '''), expectedOutput: withOverReactAndWsdImports(/*language=dart*/ ''' - ButtonElement /*?*/ ref5; + ButtonElement /*?*/ ref1; content() { ButtonElement /*?*/ ref2; - ButtonElement ref4; - (ButtonToolbar()..ref = (r) => ref5 = r)(); + ButtonElement ref3; + (ButtonToolbar()..ref = (r) => ref1 = r)(); (ButtonToolbar()..ref = (r) { - ButtonElement /*?*/ ref3; - ref2 = r; - ref3 = r as ButtonElement /*?*/; - final a = 1; - ref4 = a; + ButtonElement /*?*/ ref4; ref2 = r; + final a = ButtonElement(); + ref3 = a; + ref4 = r; + ref4; }); - ref5; + ref1; ref2; ref3; - ref4; + } + '''), + ); + }); + + test('unless there is no type on the declaration', () async { + await testSuggestor( + input: withOverReactAndWsdImports(/*language=dart*/ ''' + content() { + dynamic ref1; + var ref2; + (ButtonToolbar()..ref = (r) { + ref1 = r; + ref2 = r; + }); + ref1; + ref2; + } + '''), + expectedOutput: withOverReactAndWsdImports(/*language=dart*/ ''' + content() { + dynamic ref1; + var ref2; + (ButtonToolbar()..ref = (r) { + ref1 = r; + ref2 = r; + }); + ref1; + ref2; } '''), ); @@ -221,7 +247,7 @@ void main() { await testSuggestor( input: withOverReactAndWsdImports(/*language=dart*/ ''' content() { - var ref; + ButtonElement /*?*/ ref; (ButtonToolbar()..ref = (ButtonElement /*?*/ r) => ref = r)(); (ButtonToolbar()..ref = (r) { ref = r as ButtonElement /*?*/; })(); ref; @@ -229,7 +255,7 @@ void main() { '''), expectedOutput: withOverReactAndWsdImports(/*language=dart*/ ''' content() { - var ref; + ButtonElement /*?*/ ref; (ButtonToolbar()..ref = (ButtonElement /*?*/ r) => ref = r)(); (ButtonToolbar()..ref = (r) { ref = r as ButtonElement /*?*/; })(); ref; From 1660c9279c07b2609bc879570d78379ee14e4b0f Mon Sep 17 00:00:00 2001 From: Sydney Jodon Date: Wed, 20 Mar 2024 12:25:10 -0700 Subject: [PATCH 09/11] Clean up todos --- .../callback_ref_hint_suggestor.dart | 22 ++++++++- .../callback_ref_hint_suggestor_test.dart | 49 +++++++++++++------ 2 files changed, 54 insertions(+), 17 deletions(-) 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 4694bfe8..fc7a093a 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 @@ -31,7 +31,27 @@ final _log = Logger('CallbackRefHintSuggestor'); /// Suggestor to add nullability hints to ref types. /// -/// todo doc comment examples +/// (1) For ref prop param types: +/// ``` +/// - (ButtonToolbar()..ref = (ButtonElement r) => ref = r)(); +/// + (ButtonToolbar()..ref = (ButtonElement /*?*/ r) => ref = r)(); +/// ``` +/// +/// (2) For ref variable declarations: +/// ``` +/// - ButtonElement ref; +/// + ButtonElement /*?*/ ref; +/// (ButtonToolbar()..ref = (r) => ref = r)(); +/// ``` +/// +/// (3) For ref prop type casts: +/// ``` +/// - (ButtonToolbar()..ref = (r) => ref = r as ButtonElement)(); +/// + (ButtonToolbar()..ref = (r) => ref = r as ButtonElement /*?*/)(); +/// ``` +/// +/// These hints are needed because the null-safety migration tool does not do +/// well at inferring that ref types should be nullable. class CallbackRefHintSuggestor extends RecursiveAstVisitor with ClassSuggestor { CallbackRefHintSuggestor(); 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 39219c69..3b4bb1f8 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 @@ -19,8 +19,6 @@ import '../../mui_suggestors/components/shared.dart'; import '../../resolved_file_context.dart'; import '../../util.dart'; -// todo add block function, and other test cases, non-ref prop names, mui/dom usages - void main() { final resolvedContext = SharedAnalysisContext.wsd; @@ -41,7 +39,7 @@ void main() { content() { var ref; (ButtonToolbar()..ref = (ButtonElement r) => ref = r)(); - (ButtonToolbar()..ref = (ButtonElement r) { ref = r; })(); + (Dom.div()..ref = (ButtonElement r) { ref = r; })(); ref; } '''), @@ -49,7 +47,7 @@ void main() { content() { var ref; (ButtonToolbar()..ref = (ButtonElement /*?*/ r) => ref = r)(); - (ButtonToolbar()..ref = (ButtonElement /*?*/ r) { ref = r; })(); + (Dom.div()..ref = (ButtonElement /*?*/ r) { ref = r; })(); ref; } '''), @@ -62,7 +60,7 @@ void main() { content() { var ref; (ButtonToolbar()..ref = (ButtonElement r) => ref = r); - (ButtonToolbar()..ref = (ButtonElement r) { ref = r; }); + (Dom.div()..ref = (ButtonElement r) { ref = r; }); ref; } '''), @@ -70,7 +68,7 @@ void main() { content() { var ref; (ButtonToolbar()..ref = (ButtonElement /*?*/ r) => ref = r); - (ButtonToolbar()..ref = (ButtonElement /*?*/ r) { ref = r; }); + (Dom.div()..ref = (ButtonElement /*?*/ r) { ref = r; }); ref; } '''), @@ -85,7 +83,7 @@ void main() { content() { var ref; (ButtonToolbar()..ref = (r) => ref = r as ButtonElement)(); - (ButtonToolbar()..ref = (r) { ref = r as ButtonElement; })(); + (Dom.div()..ref = (r) { ref = r as ButtonElement; })(); ref; } '''), @@ -93,7 +91,7 @@ void main() { content() { var ref; (ButtonToolbar()..ref = (r) => ref = r as ButtonElement /*?*/)(); - (ButtonToolbar()..ref = (r) { ref = r as ButtonElement /*?*/; })(); + (Dom.div()..ref = (r) { ref = r as ButtonElement /*?*/; })(); ref; } '''), @@ -106,7 +104,7 @@ void main() { content() { var ref; (ButtonToolbar()..ref = (r) => ref = r as ButtonElement); - (ButtonToolbar()..ref = (r) { ref = r as ButtonElement; }); + (Dom.div()..ref = (r) { ref = r as ButtonElement; }); ref; } '''), @@ -114,7 +112,7 @@ void main() { content() { var ref; (ButtonToolbar()..ref = (r) => ref = r as ButtonElement /*?*/); - (ButtonToolbar()..ref = (r) { ref = r as ButtonElement /*?*/; }); + (Dom.div()..ref = (r) { ref = r as ButtonElement /*?*/; }); ref; } '''), @@ -134,7 +132,7 @@ void main() { ref as int; ref = r as ButtonElement; })(); - (ButtonToolbar() + (Dom.div() ..ref = (ButtonElement r) { ref = r as int; ref = a as ButtonElement; @@ -155,7 +153,7 @@ void main() { ref as int; ref = r as ButtonElement /*?*/; })(); - (ButtonToolbar() + (Dom.div() ..ref = (ButtonElement /*?*/ r) { ref = r as int /*?*/; ref = a as ButtonElement; @@ -178,7 +176,7 @@ void main() { ButtonElement ref2; ButtonElement ref3; (ButtonToolbar()..ref = (r) => ref1 = r)(); - (ButtonToolbar()..ref = (r) { + (Dom.div()..ref = (r) { ButtonElement ref4; ref2 = r; final a = ButtonElement(); @@ -197,7 +195,7 @@ void main() { ButtonElement /*?*/ ref2; ButtonElement ref3; (ButtonToolbar()..ref = (r) => ref1 = r)(); - (ButtonToolbar()..ref = (r) { + (Dom.div()..ref = (r) { ButtonElement /*?*/ ref4; ref2 = r; final a = ButtonElement(); @@ -249,7 +247,7 @@ void main() { content() { ButtonElement /*?*/ ref; (ButtonToolbar()..ref = (ButtonElement /*?*/ r) => ref = r)(); - (ButtonToolbar()..ref = (r) { ref = r as ButtonElement /*?*/; })(); + (Dom.div()..ref = (r) { ref = r as ButtonElement /*?*/; })(); ref; } '''), @@ -257,7 +255,26 @@ void main() { content() { ButtonElement /*?*/ ref; (ButtonToolbar()..ref = (ButtonElement /*?*/ r) => ref = r)(); - (ButtonToolbar()..ref = (r) { ref = r as ButtonElement /*?*/; })(); + (Dom.div()..ref = (r) { ref = r as ButtonElement /*?*/; })(); + ref; + } + '''), + ); + }); + + test('does not add hints for non-ref props', () async { + await testSuggestor( + input: withOverReactAndWsdImports(/*language=dart*/ ''' + content() { + ButtonElement ref; + (ButtonToolbar()..onClick = (r) { ref = r as ButtonElement; })(); + ref; + } + '''), + expectedOutput: withOverReactAndWsdImports(/*language=dart*/ ''' + content() { + ButtonElement ref; + (ButtonToolbar()..onClick = (r) { ref = r as ButtonElement; })(); ref; } '''), From 9679962b4a6862e6f6401926e35622a9dcc6ce02 Mon Sep 17 00:00:00 2001 From: Sydney Jodon Date: Wed, 20 Mar 2024 16:25:04 -0700 Subject: [PATCH 10/11] Add wsd tag --- .../null_safety_prep/callback_ref_hint_suggestor_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 3b4bb1f8..a128c6a2 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 @@ -280,5 +280,5 @@ void main() { '''), ); }); - }); + }, tags: 'wsd'); } From 89e64cc8743e0a290693268a27724df67d158670 Mon Sep 17 00:00:00 2001 From: Sydney Jodon Date: Thu, 21 Mar 2024 09:22:15 -0700 Subject: [PATCH 11/11] Remove log --- .../null_safety_prep/callback_ref_hint_suggestor.dart | 5 ----- 1 file changed, 5 deletions(-) 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 fc7a093a..924b47b4 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 @@ -18,7 +18,6 @@ import 'package:analyzer/dart/analysis/results.dart'; import 'package:analyzer/dart/element/element.dart'; import 'package:collection/collection.dart'; -import 'package:logging/logging.dart'; import 'package:over_react_codemod/src/util/component_usage.dart'; import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/visitor.dart'; @@ -27,8 +26,6 @@ import '../../util.dart'; import '../../util/class_suggestor.dart'; import 'analyzer_plugin_utils.dart'; -final _log = Logger('CallbackRefHintSuggestor'); - /// Suggestor to add nullability hints to ref types. /// /// (1) For ref prop param types: @@ -137,8 +134,6 @@ class CallbackRefHintSuggestor extends RecursiveAstVisitor @override Future generatePatches() async { - _log.info('Resolving ${context.relativePath}...'); - final r = await context.getResolvedUnit(); if (r == null) { throw Exception(