diff --git a/lib/src/dart3_suggestors/null_safety_prep/fn_prop_null_aware_call_suggestor.dart b/lib/src/dart3_suggestors/null_safety_prep/fn_prop_null_aware_call_suggestor.dart new file mode 100644 index 00000000..9bbc011e --- /dev/null +++ b/lib/src/dart3_suggestors/null_safety_prep/fn_prop_null_aware_call_suggestor.dart @@ -0,0 +1,177 @@ +// 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/analysis/results.dart'; +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/token.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:analyzer/dart/element/type.dart'; +import 'package:over_react_codemod/src/util.dart'; +import 'package:over_react_codemod/src/util/class_suggestor.dart'; + +/// Suggestor that replaces conditional calls to functions declared in props +/// with inline null-aware property access. +/// +/// This is helpful for null-safety migrations because the conditional +/// function calls will otherwise get migrated with `!` modifiers. +/// +/// **Before:** +/// +/// ```dart +/// if (props.someCallback != null) { +/// props.someCallback(someValue); +/// } +/// +/// // Will be migrated to: +/// if (props.someCallback != null) { +/// props.someCallback!(someValue); +/// } +/// ``` +/// +/// **After:** +/// +/// ```dart +/// // This will require no changes during a null-safety migration. +/// props.someCallback?.call(someValue); +/// ``` +class FnPropNullAwareCallSuggestor extends RecursiveAstVisitor + with ClassSuggestor { + ResolvedUnitResult? _result; + + @override + visitExpressionStatement(ExpressionStatement node) { + super.visitExpressionStatement(node); + + if (node.expression is! BinaryExpression) return; + + final relevantExprStatement = + _getPropFunctionExpressionBeingCalledConditionally( + node.expression as BinaryExpression); + final inlineBinaryExpr = + // This cast is safe due to the type checks within `_getPropFunctionExpressionBeingCalledConditionally`. + relevantExprStatement?.expression as BinaryExpression?; + if (inlineBinaryExpr == null) return; + final relevantFnExpr = + // This cast is safe due to the type checks within `_getPropFunctionExpressionBeingCalledConditionally`. + inlineBinaryExpr.rightOperand as FunctionExpressionInvocation; + // This cast is safe due to the type checks within `_getPropFunctionExpressionBeingCalledConditionally`. + final fn = relevantFnExpr.function as PropertyAccess; + + yieldPatch( + '${fn.target}.${fn.propertyName}?.call${relevantFnExpr.argumentList};', + node.offset, + node.end); + } + + @override + visitIfStatement(IfStatement node) { + super.visitIfStatement(node); + + if (node.condition is! BinaryExpression) return; + + final relevantFnExprStatement = + _getPropFunctionExpressionBeingCalledConditionally( + node.condition as BinaryExpression); + final relevantFnExpr = + // This cast is safe due to the type checks within `_getPropFunctionExpressionBeingCalledConditionally`. + relevantFnExprStatement?.expression as FunctionExpressionInvocation?; + if (relevantFnExpr == null) return; + // This cast is safe due to the type checks within `_getPropFunctionExpressionBeingCalledConditionally`. + final fn = relevantFnExpr.function as PropertyAccess?; + if (fn == null) return; + + yieldPatch( + '${fn.target}.${fn.propertyName}?.call${relevantFnExpr.argumentList};', + node.offset, + node.end); + } + + /// Returns the function expression (e.g. `props.onClick(event)`) being called + /// after the null condition is checked. + ExpressionStatement? _getPropFunctionExpressionBeingCalledConditionally( + BinaryExpression condition) { + final parent = condition.parent; + if (parent is! IfStatement) return null; + + final propFunctionBeingNullChecked = + _getPropFunctionBeingNullChecked(condition); + final ifStatement = parent; + if (ifStatement.elseStatement != null) return null; + if (ifStatement.parent?.tryCast()?.elseStatement == + ifStatement) { + // ifStatement is an else-if + return null; + } + final thenStatement = ifStatement.thenStatement; + if (thenStatement is Block && thenStatement.statements.length == 1) { + if (_isMatchingConditionalPropFunctionCallStatement( + thenStatement.statements.single, propFunctionBeingNullChecked)) { + return thenStatement.statements.single as ExpressionStatement?; + } + } else if (thenStatement is ExpressionStatement) { + if (_isMatchingConditionalPropFunctionCallStatement( + thenStatement, propFunctionBeingNullChecked)) { + return thenStatement; + } + } + return null; + } + + bool _isMatchingConditionalPropFunctionCallStatement( + Statement statementWithinThenStatement, + SimpleIdentifier? propFunctionBeingNullChecked) { + if (statementWithinThenStatement is! ExpressionStatement) return false; + final expression = statementWithinThenStatement.expression; + if (expression is! FunctionExpressionInvocation) return false; + final fn = expression.function; + if (fn is! PropertyAccess) return false; + final target = fn.target; + if (target is! SimpleIdentifier) return false; + if (target.name != 'props') return false; + return fn.propertyName.staticElement?.declaration == + propFunctionBeingNullChecked?.staticElement?.declaration; + } + + /// Returns the identifier for the function that is being + /// null checked before being called. + SimpleIdentifier? _getPropFunctionBeingNullChecked( + BinaryExpression condition) { + if (condition.leftOperand is! PrefixedIdentifier) { + return null; + } + final leftOperand = condition.leftOperand as PrefixedIdentifier; + final prefix = leftOperand.prefix; + if (prefix.name != 'props') { + return null; + } + if (leftOperand.identifier.staticType is! FunctionType) { + return null; + } + if (condition.operator.stringValue != '!=' && + condition.operator.next?.keyword != Keyword.NULL) { + return null; + } + return leftOperand.identifier; + } + + @override + Future generatePatches() async { + _result = await context.getResolvedUnit(); + if (_result == null) { + throw Exception( + 'Could not get resolved result for "${context.relativePath}"'); + } + _result!.unit.accept(this); + } +} diff --git a/lib/src/executables/null_safety_prep.dart b/lib/src/executables/null_safety_prep.dart index f9eb42ee..185d7628 100644 --- a/lib/src/executables/null_safety_prep.dart +++ b/lib/src/executables/null_safety_prep.dart @@ -17,6 +17,7 @@ import 'dart:io'; import 'package:args/args.dart'; import 'package:codemod/codemod.dart'; import 'package:over_react_codemod/src/dart3_suggestors/null_safety_prep/dom_callback_null_args.dart'; +import 'package:over_react_codemod/src/dart3_suggestors/null_safety_prep/fn_prop_null_aware_call_suggestor.dart'; import 'package:over_react_codemod/src/dart3_suggestors/null_safety_prep/use_ref_init_migration.dart'; import 'package:over_react_codemod/src/util.dart'; @@ -38,6 +39,7 @@ void main(List args) async { dartPaths, aggregate([ UseRefInitMigration(), + FnPropNullAwareCallSuggestor(), DomCallbackNullArgs(), CallbackRefHintSuggestor(), ]), diff --git a/test/dart3_suggestors/null_safety_prep/fn_prop_null_aware_call_suggestor_test.dart b/test/dart3_suggestors/null_safety_prep/fn_prop_null_aware_call_suggestor_test.dart new file mode 100644 index 00000000..d7f54777 --- /dev/null +++ b/test/dart3_suggestors/null_safety_prep/fn_prop_null_aware_call_suggestor_test.dart @@ -0,0 +1,359 @@ +// 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/dart3_suggestors/null_safety_prep/fn_prop_null_aware_call_suggestor.dart'; +import 'package:test/test.dart'; + +import '../../resolved_file_context.dart'; +import '../../util.dart'; +import '../../util/component_usage_migrator_test.dart'; + +void main() { + final resolvedContext = SharedAnalysisContext.overReact; + + // 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('FnPropNullAwareCallSuggestor', () { + late SuggestorTester testSuggestor; + + setUp(() { + testSuggestor = getSuggestorTester( + FnPropNullAwareCallSuggestor(), + resolvedContext: resolvedContext, + ); + }); + + group('handles block if conditions', () { + test('with a single condition', () async { + await testSuggestor( + expectedPatchCount: 1, + input: withOverReactImport(''' + final Foo = uiFunction( + (props) { + final handleClick = useCallback((e) { + if (props.onClick != null) { + props.onClick(e); + } + }, [props.onClick]); + + return (Dom.button()..onClick = handleClick)(); + }, + UiFactoryConfig(displayName: 'Foo'), + ); + '''), + expectedOutput: withOverReactImport(''' + final Foo = uiFunction( + (props) { + final handleClick = useCallback((e) { + props.onClick?.call(e); + }, [props.onClick]); + + return (Dom.button()..onClick = handleClick)(); + }, + UiFactoryConfig(displayName: 'Foo'), + ); + ''')); + }); + + test( + 'unless the single condition is not a null check of the function being called', + () async { + await testSuggestor( + expectedPatchCount: 0, + input: withOverReactImport(''' + final Foo = uiFunction( + (props) { + final handleClick = useCallback((e) { + if (1 > 0) { + props.onClick(e); + } + }, [props.onClick]); + + return (Dom.button()..onClick = handleClick)(); + }, + UiFactoryConfig(displayName: 'Foo'), + ); + '''), + ); + }); + + test('unless there is an else condition', () async { + await testSuggestor( + expectedPatchCount: 0, + input: withOverReactImport(''' + final Foo = uiFunction( + (props) { + final bar = useState(0); + final handleClick = useCallback((e) { + if (props.onClick != null) { + props.onClick(e); + } else { + bar.set(1); + } + }, [props.onClick]); + + return (Dom.button()..onClick = handleClick)(bar.value); + }, + UiFactoryConfig(displayName: 'Foo'), + ); + '''), + ); + }); + + test('unless there is an else if condition', () async { + await testSuggestor( + expectedPatchCount: 0, + input: withOverReactImport(''' + final Foo = uiFunction( + (props) { + final bar = useState(0); + final handleClick = useCallback((e) { + if (props.onMouseEnter != null) { + bar.set(1); + } else if (props.onClick != null) { + props.onClick(e); + } + }, [props.onClick]); + + return (Dom.button()..onClick = handleClick)(bar.value); + }, + UiFactoryConfig(displayName: 'Foo'), + ); + '''), + ); + }); + + test( + 'unless the single condition involves the function being called, but is not a null check', + () async { + await testSuggestor( + expectedPatchCount: 0, + input: withOverReactImport(''' + final Foo = uiFunction( + (props) { + final handleClick = useCallback((e) { + if (props.onClick is Function) { + props.onClick(e); + } + }, [props.onClick]); + + return (Dom.button()..onClick = handleClick)(); + }, + UiFactoryConfig(displayName: 'Foo'), + ); + '''), + ); + }); + + test('unless the single condition does not involve props at all', + () async { + await testSuggestor( + expectedPatchCount: 0, + input: withOverReactImport(''' + final Foo = uiFunction( + (props) { + final bar = false; + final handleClick = useCallback((e) { + if (bar) { + props.onClick(e); + } + }, [props.onClick]); + + return (Dom.button()..onClick = handleClick)(); + }, + UiFactoryConfig(displayName: 'Foo'), + ); + '''), + ); + }); + + test('unless there are multiple conditions', () async { + await testSuggestor( + expectedPatchCount: 0, + input: withOverReactImport(''' + final Foo = uiFunction( + (props) { + final handleClick = useCallback((e) { + if (props.onClick != null && props.onMouseEnter != null) { + props.onClick(e); + } + }, [props.onClick]); + + return (Dom.button()..onClick = handleClick)(); + }, + UiFactoryConfig(displayName: 'Foo'), + ); + '''), + ); + }); + + test('unless the relevant prop fn is returned within the then statement', + () async { + await testSuggestor( + expectedPatchCount: 0, + input: withOverReactImport(''' + final Foo = uiFunction( + (props) { + final handleClick = useCallback((e) { + if (props.onClick != null) { + return props.onClick(e); + } + }, [props.onClick]); + + return (Dom.button()..onClick = handleClick)(); + }, + UiFactoryConfig(displayName: 'Foo'), + ); + '''), + ); + }); + + test( + 'unless the relevant prop fn is not called within the then statement', + () async { + await testSuggestor( + expectedPatchCount: 0, + input: withOverReactImport(''' + final Foo = uiFunction( + (props) { + final bar = useState(0); + final handleClick = useCallback((e) { + if (props.onClick != null) { + bar.set(1); + } + }, [props.onClick]); + + return (Dom.button()..onClick = handleClick)(); + }, + UiFactoryConfig(displayName: 'Foo'), + ); + '''), + ); + }); + + test('unless there are multiple statements within the then statement', + () async { + await testSuggestor( + expectedPatchCount: 0, + input: withOverReactImport(''' + final Foo = uiFunction( + (props) { + final handleClick = useCallback((e) { + if (props.onClick != null) { + props.onMouseEnter?.call(e); + props.onClick(e); + } + }, [props.onClick, props.onMouseEnter]); + + return (Dom.button()..onClick = handleClick)(); + }, + UiFactoryConfig(displayName: 'Foo'), + ); + '''), + ); + }); + }); + + group('handles inline if conditions', () { + test('with a single condition', () async { + await testSuggestor( + expectedPatchCount: 1, + input: withOverReactImport(''' + final Foo = uiFunction( + (props) { + final handleClick = useCallback((e) { + if (props.onClick != null) props.onClick(e); + }, [props.onClick]); + + return (Dom.button()..onClick = handleClick)(); + }, + UiFactoryConfig(displayName: 'Foo'), + ); + '''), + expectedOutput: withOverReactImport(''' + final Foo = uiFunction( + (props) { + final handleClick = useCallback((e) { + props.onClick?.call(e); + }, [props.onClick]); + + return (Dom.button()..onClick = handleClick)(); + }, + UiFactoryConfig(displayName: 'Foo'), + ); + ''')); + }); + + test( + 'unless the single condition is not a null check of the function being called', + () async { + await testSuggestor( + expectedPatchCount: 0, + input: withOverReactImport(''' + final Foo = uiFunction( + (props) { + final handleClick = useCallback((e) { + if (1 > 0) props.onClick(e); + }, [props.onClick]); + + return (Dom.button()..onClick = handleClick)(); + }, + UiFactoryConfig(displayName: 'Foo'), + ); + '''), + ); + }); + + test( + 'unless the single condition involves the function being called, but is not a null check', + () async { + await testSuggestor( + expectedPatchCount: 0, + input: withOverReactImport(''' + final Foo = uiFunction( + (props) { + final handleClick = useCallback((e) { + if (props.onClick is Function) props.onClick(e); + }, [props.onClick]); + + return (Dom.button()..onClick = handleClick)(); + }, + UiFactoryConfig(displayName: 'Foo'), + ); + '''), + ); + }); + + test('unless there are multiple conditions', () async { + await testSuggestor( + expectedPatchCount: 0, + input: withOverReactImport(''' + final Foo = uiFunction( + (props) { + final handleClick = useCallback((e) { + if (props.onClick != null && props.onMouseEnter != null) props.onClick(e); + }, [props.onClick]); + + return (Dom.button()..onClick = handleClick)(); + }, + UiFactoryConfig(displayName: 'Foo'), + ); + '''), + ); + }); + }); + }); +}