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 new file mode 100644 index 00000000..924b47b4 --- /dev/null +++ b/lib/src/dart3_suggestors/null_safety_prep/callback_ref_hint_suggestor.dart @@ -0,0 +1,156 @@ +// 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"); +// 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/element/element.dart'; +import 'package:collection/collection.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 '../../util.dart'; +import '../../util/class_suggestor.dart'; +import 'analyzer_plugin_utils.dart'; + +/// Suggestor to add nullability hints to ref types. +/// +/// (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(); + + late ResolvedUnitResult result; + + @override + Future visitCascadeExpression(CascadeExpression node) async { + super.visitCascadeExpression(node); + + final cascadedProps = node.cascadeSections + .whereType() + .where((assignment) => assignment.leftHandSide is PropertyAccess) + .map((assignment) => PropAssignment(assignment)); + + for (final prop in cascadedProps) { + if (prop.name.name == 'ref') { + final rhs = + prop.rightHandSide.unParenthesized.tryCast(); + if (rhs == 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 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) { + final lhs = parent.leftHandSide; + if (lhs is Identifier) { + 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); + } + } + } + } + } + } + + // 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); + } + } + } + } + } + + @override + Future generatePatches() async { + final r = await context.getResolvedUnit(); + if (r == null) { + throw Exception( + 'Could not get resolved result for "${context.relativePath}"'); + } + result = r; + result.unit.visitChildren(this); + } +} + +/// 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 = '/*?*/'; diff --git a/lib/src/executables/null_safety_prep.dart b/lib/src/executables/null_safety_prep.dart index 8879e6ff..f9eb42ee 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/dom_cal import 'package:over_react_codemod/src/dart3_suggestors/null_safety_prep/use_ref_init_migration.dart'; import 'package:over_react_codemod/src/util.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: pub global activate over_react_codemod @@ -37,6 +39,7 @@ void main(List args) async { aggregate([ UseRefInitMigration(), DomCallbackNullArgs(), + CallbackRefHintSuggestor(), ]), defaultYes: true, args: parsedArgs.rest, 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 new file mode 100644 index 00000000..a128c6a2 --- /dev/null +++ b/test/dart3_suggestors/null_safety_prep/callback_ref_hint_suggestor_test.dart @@ -0,0 +1,284 @@ +// 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/callback_ref_hint_suggestor.dart'; +import 'package:test/test.dart'; + +import '../../mui_suggestors/components/shared.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('CallbackRefHintSuggestor', () { + final testSuggestor = getSuggestorTester( + CallbackRefHintSuggestor(), + resolvedContext: resolvedContext, + ); + + group('adds nullability hint to ref prop typed parameters', () { + test('', () async { + await testSuggestor( + input: withOverReactAndWsdImports(/*language=dart*/ ''' + content() { + var ref; + (ButtonToolbar()..ref = (ButtonElement r) => ref = r)(); + (Dom.div()..ref = (ButtonElement r) { ref = r; })(); + ref; + } + '''), + expectedOutput: withOverReactAndWsdImports(/*language=dart*/ ''' + content() { + var ref; + (ButtonToolbar()..ref = (ButtonElement /*?*/ r) => ref = r)(); + (Dom.div()..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); + (Dom.div()..ref = (ButtonElement r) { ref = r; }); + ref; + } + '''), + expectedOutput: withOverReactAndWsdImports(/*language=dart*/ ''' + content() { + var ref; + (ButtonToolbar()..ref = (ButtonElement /*?*/ r) => ref = r); + (Dom.div()..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)(); + (Dom.div()..ref = (r) { ref = r as ButtonElement; })(); + ref; + } + '''), + expectedOutput: withOverReactAndWsdImports(/*language=dart*/ ''' + content() { + var ref; + (ButtonToolbar()..ref = (r) => ref = r as ButtonElement /*?*/)(); + (Dom.div()..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); + (Dom.div()..ref = (r) { ref = r as ButtonElement; }); + ref; + } + '''), + expectedOutput: withOverReactAndWsdImports(/*language=dart*/ ''' + content() { + var ref; + (ButtonToolbar()..ref = (r) => ref = r as ButtonElement /*?*/); + (Dom.div()..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; + })(); + (Dom.div() + ..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 /*?*/; + })(); + (Dom.div() + ..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; + } + '''), + ); + }); + }); + + group('adds nullability hint to class ref variables', () { + test('', () async { + await testSuggestor( + input: withOverReactAndWsdImports(/*language=dart*/ ''' + ButtonElement ref1; + content() { + ButtonElement ref2; + ButtonElement ref3; + (ButtonToolbar()..ref = (r) => ref1 = r)(); + (Dom.div()..ref = (r) { + ButtonElement ref4; + ref2 = r; + final a = ButtonElement(); + ref3 = a; + ref4 = r; + ref4; + }); + ref1; + ref2; + ref3; + } + '''), + expectedOutput: withOverReactAndWsdImports(/*language=dart*/ ''' + ButtonElement /*?*/ ref1; + content() { + ButtonElement /*?*/ ref2; + ButtonElement ref3; + (ButtonToolbar()..ref = (r) => ref1 = r)(); + (Dom.div()..ref = (r) { + ButtonElement /*?*/ ref4; + ref2 = r; + final a = ButtonElement(); + ref3 = a; + ref4 = r; + ref4; + }); + ref1; + ref2; + ref3; + } + '''), + ); + }); + + 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; + } + '''), + ); + }); + }); + + test('does not add hints if they already exist', () async { + await testSuggestor( + input: withOverReactAndWsdImports(/*language=dart*/ ''' + content() { + ButtonElement /*?*/ ref; + (ButtonToolbar()..ref = (ButtonElement /*?*/ r) => ref = r)(); + (Dom.div()..ref = (r) { ref = r as ButtonElement /*?*/; })(); + ref; + } + '''), + expectedOutput: withOverReactAndWsdImports(/*language=dart*/ ''' + content() { + ButtonElement /*?*/ ref; + (ButtonToolbar()..ref = (ButtonElement /*?*/ r) => ref = r)(); + (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; + } + '''), + ); + }); + }, tags: 'wsd'); +} 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;