-
Notifications
You must be signed in to change notification settings - Fork 7
FED-1919 Codemod to add required flux actions / store prop(s) #268
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 14 commits
1e3f876
10de57d
c60beec
4bbc2ae
61ef7a9
c38224a
e5e98b8
af15a8f
5234055
d69dbbe
6ab4088
647457b
927991d
44b1e38
b7703be
e60a936
b2597ca
a57eec6
580b82c
619910d
aea20f0
7ab4757
6a3aafc
b08151b
5f81eb9
942b8e2
d344edc
2745fc9
9ec4ec8
1bf39c4
d36a656
1d1a288
e8300cb
a603afe
0f3a754
7798be7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| // 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. | ||
|
|
||
| export 'package:over_react_codemod/src/executables/required_flux_props.dart'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,230 @@ | ||
| // 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 'package:analyzer/dart/analysis/results.dart'; | ||
| import 'package:analyzer/dart/ast/ast.dart'; | ||
| import 'package:analyzer/dart/ast/visitor.dart'; | ||
| import 'package:analyzer/dart/element/element.dart'; | ||
| import 'package:analyzer/dart/element/type.dart'; | ||
| import 'package:collection/collection.dart'; | ||
| import 'package:over_react_codemod/src/util/class_suggestor.dart'; | ||
| import 'package:over_react_codemod/src/util/offset_util.dart'; | ||
|
|
||
| /// Suggestor that adds required `store` and/or `actions` prop(s) to the | ||
| /// call-site of `FluxUiComponent` instances that omit them since version | ||
| /// 5.0.0 of over_react makes flux `store`/`actions` props required. | ||
| /// | ||
| /// In the case of a component that is rendered in a scope where a store/actions | ||
| /// instance is available, but simply not passed along to the component, those | ||
| /// instance(s) will be used as the value for `props.store`/`props.actions`, | ||
| /// even though the component itself may not make use of them internally. | ||
| /// | ||
| /// In the case of a component that is rendered in a scope where a store/actions | ||
| /// instance is not available, `null` will be used as the value for the prop(s). | ||
| class RequiredFluxProps extends RecursiveAstVisitor with ClassSuggestor { | ||
| ResolvedUnitResult? _result; | ||
|
|
||
| static const fluxPropsMixinName = 'FluxUiPropsMixin'; | ||
|
|
||
| @override | ||
| visitCascadeExpression(CascadeExpression node) { | ||
| var writesToFluxUiProps = false; | ||
| var actionsAssigned = false; | ||
| var storeAssigned = false; | ||
|
|
||
| DartType? fluxActionsType; | ||
| DartType? fluxStoreType; | ||
| final cascadeWriteEl = node.staticType?.element; | ||
| if (cascadeWriteEl is ClassElement) { | ||
| final maybeFluxUiPropsMixin = cascadeWriteEl.mixins | ||
| .singleWhereOrNull((e) => e.element.name == fluxPropsMixinName); | ||
| writesToFluxUiProps = maybeFluxUiPropsMixin != null; | ||
| fluxActionsType = maybeFluxUiPropsMixin?.typeArguments[0]; | ||
| fluxStoreType = maybeFluxUiPropsMixin?.typeArguments[1]; | ||
| } | ||
|
|
||
| final cascadingAssignments = | ||
| node.cascadeSections.whereType<AssignmentExpression>(); | ||
| storeAssigned = cascadingAssignments.any((cascade) { | ||
| final lhs = cascade.leftHandSide; | ||
| return lhs is PropertyAccess && lhs.propertyName.name == 'store'; | ||
| }); | ||
| actionsAssigned = cascadingAssignments.any((cascade) { | ||
| final lhs = cascade.leftHandSide; | ||
| return lhs is PropertyAccess && lhs.propertyName.name == 'actions'; | ||
| }); | ||
|
|
||
| if (writesToFluxUiProps && !storeAssigned) { | ||
| storeAssigned = true; | ||
| final storeValue = | ||
| _getNameOfVarOrFieldInScopeWithType(node, fluxStoreType) ?? 'null'; | ||
greglittlefield-wf marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| yieldNewCascadeSection(node, '..store = $storeValue'); | ||
| } | ||
|
|
||
| if (writesToFluxUiProps && !actionsAssigned) { | ||
| actionsAssigned = true; | ||
| final actionsValue = | ||
| _getNameOfVarOrFieldInScopeWithType(node, fluxActionsType) ?? 'null'; | ||
| yieldNewCascadeSection(node, '..actions = $actionsValue'); | ||
| } | ||
| } | ||
|
|
||
| void yieldNewCascadeSection(CascadeExpression node, String newSection) { | ||
| final offset = context.sourceFile.getOffsetOfLineAfter(node.target.offset); | ||
| yieldPatch(newSection, offset, offset); | ||
| } | ||
|
|
||
| @override | ||
| Future<void> generatePatches() async { | ||
| _result = await context.getResolvedUnit(); | ||
| if (_result == null) { | ||
| throw Exception( | ||
| 'Could not get resolved result for "${context.relativePath}"'); | ||
| } | ||
| _result!.unit.accept(this); | ||
| } | ||
| } | ||
|
|
||
| String? _getNameOfVarOrFieldInScopeWithType(AstNode node, DartType? type) { | ||
| final inScopeVariableDetector = _InScopeVarDetector(); | ||
| // Find top level vars | ||
| node | ||
| .thisOrAncestorOfType<CompilationUnit>() | ||
| ?.accept(inScopeVariableDetector); | ||
| // Find vars declared in top-level fns (like `main()`) | ||
| node | ||
| .thisOrAncestorOfType<BlockFunctionBody>() | ||
| ?.visitChildren(inScopeVariableDetector); | ||
|
|
||
| final inScopeVarName = inScopeVariableDetector.found | ||
| .firstWhereOrNull((v) { | ||
| final maybeMatchingType = v.declaredElement?.type; | ||
| return maybeMatchingType?.element?.name == type?.element?.name; | ||
| }) | ||
| ?.declaredElement | ||
| ?.name; | ||
|
|
||
| final componentScopePropDetector = _ComponentScopeFluxPropsDetector(); | ||
| // Find actions/store in props of class components | ||
| node | ||
| .thisOrAncestorOfType<ClassDeclaration>() | ||
| ?.accept(componentScopePropDetector); | ||
| // Find actions/store in props of fn components | ||
| node | ||
| .thisOrAncestorOfType<MethodInvocation>() | ||
| ?.accept(componentScopePropDetector); | ||
|
|
||
| final inScopePropName = | ||
| componentScopePropDetector.found.firstWhereOrNull((el) { | ||
| final maybeMatchingType = componentScopePropDetector.getAccessorType(el); | ||
| return maybeMatchingType?.element?.name == type?.element?.name; | ||
| })?.name; | ||
|
|
||
| if (inScopeVarName != null && inScopePropName != null) { | ||
| // TODO: Do we need to handle this edge case with something better than returning null? | ||
| // No way to determine which should be used - the scoped variable or the field on props | ||
| // so return null to avoid setting the incorrect value on the consumer's code. | ||
| return null; | ||
| } | ||
|
|
||
| if (inScopePropName != null) { | ||
| return '${componentScopePropDetector.propsName}.${inScopePropName}'; | ||
| } | ||
|
|
||
| return inScopeVarName; | ||
| } | ||
|
|
||
| bool _isFnComponentDeclaration(Expression? varInitializer) => | ||
| varInitializer is MethodInvocation && | ||
| varInitializer.methodName.name.startsWith('uiF'); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should account for both |
||
|
|
||
| /// A visitor to detect in-scope store/actions variables (top-level and block function scopes) | ||
| class _InScopeVarDetector extends RecursiveAstVisitor<void> { | ||
| final List<VariableDeclaration> found; | ||
|
|
||
| _InScopeVarDetector() : found = []; | ||
|
|
||
| @override | ||
| visitVariableDeclaration(VariableDeclaration node) { | ||
| // Don't visit function component declarations here since we visit them using the _ComponentScopeFluxPropsDetector | ||
| if (_isFnComponentDeclaration(node.initializer)) return; | ||
|
|
||
| if (node.declaredElement != null) { | ||
| found.add(node); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// A visitor to detect store/actions values in a props class (supports both class and fn components) | ||
| class _ComponentScopeFluxPropsDetector extends RecursiveAstVisitor<void> { | ||
greglittlefield-wf marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| final Map<PropertyAccessorElement, DartType> _foundWithMappedTypes; | ||
| List<PropertyAccessorElement> get found => | ||
| _foundWithMappedTypes.keys.toList(); | ||
|
|
||
| _ComponentScopeFluxPropsDetector() : _foundWithMappedTypes = {}; | ||
|
|
||
| String _propsName = 'props'; | ||
|
|
||
| /// The name of the function component props arg, or the class component `props` instance field. | ||
| String get propsName => _propsName; | ||
|
|
||
| DartType? getAccessorType(PropertyAccessorElement el) => | ||
| _foundWithMappedTypes[el]; | ||
|
|
||
| void _lookForFluxStoreAndActionsInPropsClass(Element? elWithProps) { | ||
| if (elWithProps is ClassElement) { | ||
| final fluxPropsEl = elWithProps.mixins.singleWhereOrNull( | ||
| (e) => e.element.name == RequiredFluxProps.fluxPropsMixinName); | ||
|
|
||
| if (fluxPropsEl != null) { | ||
| final actionsType = fluxPropsEl.typeArguments[0]; | ||
| final storeType = fluxPropsEl.typeArguments[1]; | ||
| fluxPropsEl.accessors.forEach((a) { | ||
| final accessorTypeName = a.declaration.variable.type.element?.name; | ||
| if (accessorTypeName == 'ActionsT') { | ||
| _foundWithMappedTypes.putIfAbsent(a.declaration, () => actionsType); | ||
| } else if (accessorTypeName == 'StoresT') { | ||
| _foundWithMappedTypes.putIfAbsent(a.declaration, () => storeType); | ||
| } | ||
| }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Visit function components | ||
| @override | ||
| void visitMethodInvocation(MethodInvocation node) { | ||
| if (!_isFnComponentDeclaration(node)) return; | ||
|
|
||
| final nodeType = node.staticType; | ||
| if (nodeType is FunctionType) { | ||
| final propsArg = | ||
| node.argumentList.arguments.firstOrNull as FunctionExpression?; | ||
| final propsArgName = | ||
| propsArg?.parameters?.parameterElements.firstOrNull?.name; | ||
| if (propsArgName != null) { | ||
| _propsName = propsArgName; | ||
| } | ||
| _lookForFluxStoreAndActionsInPropsClass(nodeType.returnType.element); | ||
| } | ||
| } | ||
|
|
||
| /// Visit composite (class) components | ||
| @override | ||
| void visitClassDeclaration(ClassDeclaration node) { | ||
| final elWithProps = | ||
| node.declaredElement?.supertype?.typeArguments.singleOrNull?.element; | ||
| _lookForFluxStoreAndActionsInPropsClass(elWithProps); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| // 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/dart3_suggestors/null_safety_prep/required_flux_props.dart'; | ||
| import 'package:over_react_codemod/src/ignoreable.dart'; | ||
| import 'package:over_react_codemod/src/util.dart'; | ||
|
|
||
| const _changesRequiredOutput = """ | ||
| To update your code, run the following commands in your repository: | ||
| pub global activate over_react_codemod | ||
| pub global run over_react_codemod:required_flux_props | ||
| """; | ||
|
|
||
| void main(List<String> args) async { | ||
| final parser = ArgParser.allowAnything(); | ||
|
|
||
| final parsedArgs = parser.parse(args); | ||
|
|
||
| exitCode = await runInteractiveCodemod( | ||
| allDartPathsExceptHidden(), | ||
| aggregate([ | ||
| RequiredFluxProps(), | ||
| ].map((s) => ignoreable(s))), | ||
| defaultYes: true, | ||
| args: parsedArgs.rest, | ||
| additionalHelpOutput: parser.usage, | ||
| changesRequiredOutput: _changesRequiredOutput, | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -108,7 +108,7 @@ class ConstantStringMigrator extends GeneralizingAstVisitor | |
| if (node.isConst && | ||
| node.initializer != null && | ||
| node.initializer is SimpleStringLiteral) { | ||
| SimpleStringLiteral literal = node.initializer as SimpleStringLiteral; | ||
| SimpleStringLiteral literal = node.initializer! as SimpleStringLiteral; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There were lints causing CI failures in a few misc. files that are unrelated to the scope of this PR. |
||
| var string = literal.stringValue; | ||
| // I don't see how the parent could possibly be null, but if it's true, bail out. | ||
| if (node.parent == null || string == null || string.length <= 1) return; | ||
|
|
@@ -147,7 +147,7 @@ class ConstantStringMigrator extends GeneralizingAstVisitor | |
| } else { | ||
| // Use a content-based name. | ||
| var contentBasedName = | ||
| toVariableName(stringContent(node.initializer as StringLiteral)!); | ||
| toVariableName(stringContent(node.initializer! as StringLiteral)!); | ||
| return contentBasedName; | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| import 'package:source_span/source_span.dart'; | ||
|
|
||
| extension SourceFileOffsetUtils on SourceFile { | ||
| /// Return the offset of the first character on the line following the line | ||
| /// containing the given [offset]. | ||
| int getOffsetOfLineAfter(int offset) => getOffset(getLine(offset) + 1); | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.