diff --git a/CHANGELOG.md b/CHANGELOG.md index 1126b7ad..d3f78846 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,6 @@ +## Unreleased +- Add mui_system_props_migration codemod to migrate from system props to sx + ## 2.37.1 - Use gha-dart-oss diff --git a/bin/mui_system_props_migration.dart b/bin/mui_system_props_migration.dart new file mode 100644 index 00000000..7529f624 --- /dev/null +++ b/bin/mui_system_props_migration.dart @@ -0,0 +1,15 @@ +// Copyright 2026 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/mui_system_props_migration.dart'; diff --git a/lib/src/executables/mui_system_props_migration.dart b/lib/src/executables/mui_system_props_migration.dart new file mode 100644 index 00000000..eacfdee8 --- /dev/null +++ b/lib/src/executables/mui_system_props_migration.dart @@ -0,0 +1,35 @@ +// Copyright 2026 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:codemod/codemod.dart'; +import 'package:over_react_codemod/src/ignoreable.dart'; +import 'package:over_react_codemod/src/mui_suggestors/system_props_to_sx_migrator.dart'; +import 'package:over_react_codemod/src/util.dart'; + +void main(List args) async { + const description = 'Migrates deprecated MUI system props to the `sx` prop,' + '\nensuring that existing `sx` prop values are preserved and merged correctly.'; + + exitCode = await runInteractiveCodemod( + allDartPathsExceptHidden(), + aggregate([ + SystemPropsToSxMigrator(), + ].map((s) => ignoreable(s))), + defaultYes: true, + args: args, + additionalHelpOutput: description, + ); +} diff --git a/lib/src/mui_suggestors/system_props.dart b/lib/src/mui_suggestors/system_props.dart new file mode 100644 index 00000000..4d8ff96b --- /dev/null +++ b/lib/src/mui_suggestors/system_props.dart @@ -0,0 +1,139 @@ +// Copyright 2026 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'; + +/// Duck-types a component props class as a MUI-system-props-supporting props +/// class by checking for the presence of an `sx` prop and at least one system prop. +bool hasSxAndSomeSystemProps(InterfaceElement propsElement) { + const propsToCheck = [ + 'sx', + // The following items are arbitrary; we don't need to check for all system props, + // just a few to help prevent false positives where components have a prop or two + // that happens to match a system prop. + 'bgcolor', + 'm', + 'letterSpacing', + ]; + + return propsToCheck.every((propName) => + propsElement.lookUpGetter(propName, propsElement.library) != null); +} + +/// The names of all the MUI system props. +const systemPropNames = { + 'm', + 'mt', + 'mr', + 'mb', + 'ml', + 'mx', + 'my', + 'p', + 'pt', + 'pr', + 'pb', + 'pl', + 'px', + 'py', + 'width', + 'maxWidth', + 'minWidth', + 'height', + 'maxHeight', + 'minHeight', + 'boxSizing', + 'display', + 'displayPrint', + 'overflow', + 'textOverflow', + 'visibility', + 'whiteSpace', + 'flexBasis', + 'flexDirection', + 'flexWrap', + 'justifyContent', + 'alignItems', + 'alignContent', + 'order', + 'flex', + 'flexGrow', + 'flexShrink', + 'alignSelf', + 'justifyItems', + 'justifySelf', + 'gap', + 'columnGap', + 'rowGap', + 'gridColumn', + 'gridRow', + 'gridAutoFlow', + 'gridAutoColumns', + 'gridAutoRows', + 'gridTemplateColumns', + 'gridTemplateRows', + 'gridTemplateAreas', + 'gridArea', + 'bgcolor', + 'color', + 'zIndex', + 'position', + 'top', + 'right', + 'bottom', + 'left', + 'boxShadow', + 'border', + 'borderTop', + 'borderRight', + 'borderBottom', + 'borderLeft', + 'borderColor', + 'borderRadius', + 'fontFamily', + 'fontSize', + 'fontStyle', + 'fontWeight', + 'letterSpacing', + 'lineHeight', + 'textAlign', + 'textTransform', + 'margin', + 'marginTop', + 'marginRight', + 'marginBottom', + 'marginLeft', + 'marginX', + 'marginY', + 'marginInline', + 'marginInlineStart', + 'marginInlineEnd', + 'marginBlock', + 'marginBlockStart', + 'marginBlockEnd', + 'padding', + 'paddingTop', + 'paddingRight', + 'paddingBottom', + 'paddingLeft', + 'paddingX', + 'paddingY', + 'paddingInline', + 'paddingInlineStart', + 'paddingInlineEnd', + 'paddingBlock', + 'paddingBlockStart', + 'paddingBlockEnd', + 'typography', +}; diff --git a/lib/src/mui_suggestors/system_props_to_sx_migrator.dart b/lib/src/mui_suggestors/system_props_to_sx_migrator.dart new file mode 100644 index 00000000..14cfa404 --- /dev/null +++ b/lib/src/mui_suggestors/system_props_to_sx_migrator.dart @@ -0,0 +1,359 @@ +// Copyright 2026 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/ast/ast.dart'; +import 'package:analyzer/dart/ast/token.dart'; +import 'package:analyzer/dart/element/nullability_suffix.dart'; +import 'package:analyzer/dart/element/type.dart'; +import 'package:collection/collection.dart'; +import 'package:over_react_codemod/src/util.dart'; +import 'package:over_react_codemod/src/util/component_usage.dart'; +import 'package:over_react_codemod/src/util/component_usage_migrator.dart'; +import 'package:over_react_codemod/src/util/element_type_helpers.dart'; + +import 'system_props.dart'; + +/// A suggestor that migrates deprecated MUI system props to the `sx` prop, +/// ensuring that existing `sx` prop values are preserved and merged correctly. +/// +/// ### Example migration +/// +/// Before: +/// ```dart +/// (Box()..m = 2)(); +/// (Box() +/// ..m = 2 +/// ..sx = {'color': '#f00'} +/// )() +/// (Box() +/// ..m = 2 +/// ..addProps(props.getPropsToForward()) +/// )() +/// ``` +/// +/// After +/// ```dart +/// (Box()..sx = {'m': 2})() +/// (Box()..sx = { +/// 'm': 2, +/// 'color': '#f00' +/// })() +/// (Box() +/// ..addProps(props.getPropsToForward()) +/// ..sx = { +/// 'm': 2, +/// ...?props.sx, +/// } +/// )() +/// ``` +class SystemPropsToSxMigrator extends ComponentUsageMigrator { + @override + String get fixmePrefix => 'FIXME(mui_system_props_migration)'; + + // We'll handle conditionally migrating when we collect more data in migrateUsage. + @override + bool shouldMigrateUsage(FluentComponentUsage usage) => true; + + @override + get safeMethodCallNames { + return const { + // These are handled in getForwardedPropSources + 'addUnconsumedProps', + 'addAll', + 'addProps', + 'modifyProps', + // These are unrelated + 'addUnconsumedDomProps', + 'addAutomationId', + 'addTestId', + }; + } + + @override + get shouldFlagExtensionMembers => false; + + @override + get shouldFlagUntypedSingleProp => false; + + @override + get shouldFlagPrefixedProps => false; + + @override + get shouldFlagRefProp => false; + + @override + get shouldFlagClassName => false; + + @override + void migrateUsage(FluentComponentUsage usage) { + super.migrateUsage(usage); + + final systemProps = []; + PropAssignment? existingSxProp; + + // Identify system props and existing sx prop + for (final prop in usage.cascadedProps) { + final propName = prop.name.name; + if (propName == 'sx') { + existingSxProp = prop; + } else if (systemPropNames.contains(propName)) { + final isPropDeprecated = + prop.staticElement?.nonSynthetic.hasDeprecated ?? false; + late final propsClassElement = usage.propsClassElement; + if (isPropDeprecated && + propsClassElement != null && + hasSxAndSomeSystemProps(propsClassElement)) { + systemProps.add(prop); + } + } + } + + // No system props to migrate; bail out for this usage. + if (systemProps.isEmpty) return; + + // Migrate system props to sx entries, and remove old system props. + final migratedSystemPropEntries = []; + for (final prop in systemProps) { + // Carry over comments before the node + // (we won't try to handle end-of-line comments). + final beforeComments = allCommentsForNode(prop.node) + // Don't include end-of-line comments that should stay with the previous line. + .skipWhile((comment) => !_isFirstTokenOnLine(comment)) + .toList(); + + final commentSource = beforeComments.isEmpty + ? '' + // Get full comment text, and trim trailing newline/whitespace + : context.sourceFile + .getText(beforeComments.first.offset, beforeComments.last.end) + .trimRight(); + + migratedSystemPropEntries.add([ + if (commentSource.isNotEmpty) '\n $commentSource', + "'${prop.name.name}': ${context.sourceFor(prop.rightHandSide)}" + ].join('\n')); + + yieldPatch('', beforeComments.firstOrNull?.offset ?? prop.node.offset, + prop.node.end); + } + + final propForwardingSources = _detectPropForwardingSources(usage); + + final bool anySystemPropSetBeforeForwarding; + if (propForwardingSources.isNotEmpty) { + final propForwardingOffsets = + propForwardingSources.map((s) => s.cascadedMethod.node.end).toList(); + final systemPropOffsets = systemProps.map((p) => p.node.end).toList(); + anySystemPropSetBeforeForwarding = + systemPropOffsets.min < propForwardingOffsets.max; + } else { + anySystemPropSetBeforeForwarding = false; + } + + final fixmes = [ + if (anySystemPropSetBeforeForwarding) + 'Previously, it was possible for forwarded system props to overwrite these migrated styles, but not anymore since sx takes precedence over any system props.' + '\n Double-check that this new behavior is okay.', + ]; + String getFixmesSource() { + if (fixmes.isEmpty) return ''; + // Add a leading newline to ensure comments don't get stuck to the previous line. + return '\n' + + fixmes + // Indent with a single space so that dartfmt doesn't make it stick to the beginning of the line. + .map((f) => lineComment('$fixmePrefix - $f', indent: ' ')) + .join(''); + } + + // Add or update sx prop with migrated system props. + if (existingSxProp != null) { + // + // Case 1: add styles to existing sx prop value + final value = existingSxProp.rightHandSide; + if (value is SetOrMapLiteral && value.isMap) { + // + // Case 1a: add styles to existing sx map literal + + // Insert before, to preserve existing behavior where any spread sx trumped these styles. + // Add a leading newline to ensure comments don't get stuck to the opening braces. + yieldPatch( + '${getFixmesSource()}\n${migratedSystemPropEntries.join(',\n')},', + value.leftBracket.end, + value.leftBracket.end); + // Force a multiline in all cases by ensuring there's a trailing comma. + final hadTrailingComma = + value.rightBracket.previous?.type == TokenType.COMMA; + if (!hadTrailingComma && value.elements.isNotEmpty) { + yieldPatch(',', value.rightBracket.offset, value.rightBracket.offset); + } + } else { + // + // Case 1b: spread existing sx value into a new map literal + + final type = value.staticType; + final nonNullable = type != null && + type is! DynamicType && + type.nullabilitySuffix == NullabilitySuffix.none; + final spread = '...${nonNullable ? '' : '?'}'; + // Insert before spread, to preserve existing behavior where any forwarded sx trumped these styles. + yieldPatch( + '{${getFixmesSource()}\n${migratedSystemPropEntries.join(', ')}, $spread', + value.offset, + value.offset); + final maybeTrailingComma = _shouldForceMultiline([ + ...migratedSystemPropEntries, + '$spread${context.sourceFor(value)}', + ]) + ? ',' + : ''; + yieldPatch('$maybeTrailingComma}', value.end, value.end); + } + } else { + // + // Case 2: add new sx prop assignment + + final String? forwardedSxSpread; + + final mightForwardSx = propForwardingSources.any((source) { + final type = source.sourceProps?.staticType?.typeOrBound + .tryCast(); + if (type != null && + type.isPropsClass && + type.element.name != 'UiProps') { + // It's a non-generic props class; check if it has `sx` prop. + return type.element.lookUpGetter('sx', type.element.library) != null; + } else { + // The source props weren't available, or the expression is dynamic + // or a generic type (e.g., Map, UiProps), so we can't be sure it doesn't have sx. + return true; + } + }); + + if (mightForwardSx) { + // Try to access the forwarded sx prop to merge it in. + bool canSafelyGetForwardedSx; + final props = propForwardingSources.singleOrNull?.sourceProps; + if (props != null && (props is Identifier || props is PropertyAccess)) { + final propsElement = + props.staticType?.typeOrBound.tryCast()?.element; + canSafelyGetForwardedSx = + propsElement?.lookUpGetter('sx', propsElement.library) != null; + } else { + canSafelyGetForwardedSx = false; + } + + if (canSafelyGetForwardedSx) { + forwardedSxSpread = '...?${context.sourceFor(props!)}.sx'; + } else { + forwardedSxSpread = null; + fixmes.add( + 'spread in any sx prop forwarded to this component above, if needed (spread should go at the end of this map to preserve behavior)'); + } + } else { + forwardedSxSpread = null; + } + + final elements = [ + // Insert before spread, to preserve existing behavior where any forwarded sx trumped these styles. + ...migratedSystemPropEntries, + if (forwardedSxSpread != null) forwardedSxSpread, + ]; + final maybeTrailingComma = _shouldForceMultiline(elements) ? ',' : ''; + + // Insert after any forwarded props to ensure sx isn't overwritten, + // or where the last system prop used to be to preserve the location and reduce diffs, + // whichever is later. + final insertionLocation = [ + ...propForwardingSources.map((s) => s.cascadedMethod.node.end), + ...systemProps.map((p) => p.node.end) + ].max; + + yieldPatch( + '${getFixmesSource()}..sx = {${elements.join(', ')}$maybeTrailingComma}', + insertionLocation, + insertionLocation); + } + } + + bool _isFirstTokenOnLine(Token token) { + final sourceFile = context.sourceFile; + final lineStart = sourceFile.getOffset(sourceFile.getLine(token.offset)); + return sourceFile.getText(lineStart, token.offset).trim().isEmpty; + } + + static bool _shouldForceMultiline(List mapElements) => + // Force multiline if there are a certain number of entries, or... + mapElements.length >= 3 || + // there's more than one entry and the line is getting too long, or... + (mapElements.length > 1 && mapElements.join(', ').length >= 20) || + // any entry is multiline (including comments). + mapElements.any((e) => e.contains('\n')); +} + +/// A source of props being added (spread) to a component usage. +class _PropSpreadSource { + /// The cascaded invocation that adds props. + final BuilderMethodInvocation cascadedMethod; + + /// And expression representing the source of the props being spread, + /// or null if the source is more complex or could not be resolved. + /// + /// Handles common prop forwarding expressions, returning the original + /// expression that the subsetted forwarded props are coming from. + /// + /// For example, this expression would be: + /// + /// - for `..addAll(getSomeProps())` - `getSomeProps()` + /// - for `..addProps(props.getPropsToForward({FooProps}))` - `props` + /// - for `..modifyProps(somePropsModifier)` - props + final Expression? sourceProps; + + _PropSpreadSource(this.cascadedMethod, this.sourceProps); +} + +/// Returns the sources of props being added or spread to [usage]. +List<_PropSpreadSource> _detectPropForwardingSources( + FluentComponentUsage usage) { + return usage.cascadedMethodInvocations + .map((c) { + final methodName = c.methodName.name; + late final arg = c.node.argumentList.arguments.firstOrNull; + + switch (methodName) { + case 'addUnconsumedProps': + return _PropSpreadSource(c, arg); + case 'addAll': + case 'addProps': + if (arg is MethodInvocation && + arg.methodName.name == 'getPropsToForward') { + return _PropSpreadSource(c, arg.realTarget); + } + return _PropSpreadSource(c, arg); + case 'modifyProps': + if ((arg is MethodInvocation && + arg.methodName.name == 'addPropsToForward')) { + return _PropSpreadSource(c, arg.realTarget); + } + if (arg is Identifier && arg.name == 'addUnconsumedProps') { + return _PropSpreadSource(c, null); + } + return _PropSpreadSource(c, null); + default: + // Not a method that forwards props. + return null; + } + }) + .whereNotNull() + .toList(); +} diff --git a/lib/src/util.dart b/lib/src/util.dart index 5fcb41fb..bb410b56 100644 --- a/lib/src/util.dart +++ b/lib/src/util.dart @@ -549,5 +549,5 @@ extension ParentFieldDeclExtension on VariableDeclaration { String blockComment(String contents) => '/*$contents*/'; -String lineComment(String contents) => - contents.split('\n').map((line) => '// $line\n').join(''); +String lineComment(String contents, {String indent = ''}) => + contents.split('\n').map((line) => '$indent// $line\n').join(''); diff --git a/pubspec.yaml b/pubspec.yaml index bd226b16..53c2c5df 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -49,6 +49,7 @@ executables: null_safety_required_props: null_safety_prep: mui_migration: + mui_system_props_migration: required_flux_props: rmui_preparation: rmui_bundle_update: diff --git a/test/mui_suggestors/system_props_to_sx_migrator_test.dart b/test/mui_suggestors/system_props_to_sx_migrator_test.dart new file mode 100644 index 00000000..29f794f8 --- /dev/null +++ b/test/mui_suggestors/system_props_to_sx_migrator_test.dart @@ -0,0 +1,1272 @@ +// Copyright 2026 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:convert'; + +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/mui_suggestors/system_props.dart'; +import 'package:over_react_codemod/src/mui_suggestors/system_props_to_sx_migrator.dart'; +import 'package:test/test.dart'; + +import '../resolved_file_context.dart'; +import '../util.dart'; + +void main() { + group('SystemPropsToSxMigrator', () { + final resolvedContext = SharedAnalysisContext.overReact; + + late String muiUri; + + setUpAll(() async { + await resolvedContext.warmUpAnalysis(); + + // Set up a file with stubbed MUI components with system props, for the + // test inputs to import and the suggestor to detect. + final muiFile = await resolvedContext.resolvedFileContextForTest( + getStubMuiLibrarySource(filenameWithoutExtension: 'mui_components'), + filename: 'mui_components.dart', + ); + muiUri = Uri.file(muiFile.path).toString(); + }); + + String withImports(String source) => ''' + //@dart=2.19 + import 'package:over_react/over_react.dart'; + import ${jsonEncode(muiUri.toString())}; + $source + '''; + + const sxPrecedenceFixme = + '// FIXME(mui_system_props_migration) - Previously, it was possible for forwarded system props to overwrite these migrated styles, but not anymore since sx takes precedence over any system props.' + '\n // Double-check that this new behavior is okay.'; + + const sxMergeFixme = + '// FIXME(mui_system_props_migration) - spread in any sx prop forwarded to this component above, if needed (spread should go at the end of this map to preserve behavior)'; + + final testSuggestor = getSuggestorTester( + SystemPropsToSxMigrator(), + resolvedContext: resolvedContext, + ); + + group('hasSxAndSomeSystemProps returns as expected for test props:', () { + late ResolvedUnitResult unit; + + setUpAll(() async { + final file = + await resolvedContext.resolvedFileContextForTest(withImports('')); + unit = (await file.getResolvedUnit())!; + }); + + InterfaceElement getProps(String propsName) => + getImportedInterfaceElement(unit, propsName); + + test('with system props', () async { + expect( + hasSxAndSomeSystemProps(getProps('BoxProps')), + isTrue, + ); + expect(hasSxAndSomeSystemProps(getProps('GridProps')), isTrue); + expect(hasSxAndSomeSystemProps(getProps('StackProps')), isTrue); + expect(hasSxAndSomeSystemProps(getProps('TypographyProps')), isTrue); + }); + + test('without system props', () async { + // Test props with sx and a prop named like a system prop. + expect(hasSxAndSomeSystemProps(getProps('TextFieldProps')), isFalse); + // Some other props from over_react. + expect(hasSxAndSomeSystemProps(getProps('DomProps')), isFalse); + }); + }); + + test('migrates single system prop to sx', () async { + await testSuggestor( + input: withImports(''' + content() => + (Box()..mt = 2)(); + '''), + expectedOutput: withImports(''' + content() => + (Box()..sx = {'mt': 2})(); + '''), + ); + }); + + test('migrates multiple system props', () async { + await testSuggestor( + input: withImports(''' + content() => + (Box() + ..mt = 2 + ..p = 3 + ..bgcolor = 'primary.main' + )(); + '''), + expectedOutput: withImports(''' + content() => + (Box() + ..sx = { + 'mt': 2, + 'p': 3, + 'bgcolor': 'primary.main', + } + )(); + '''), + ); + }); + + group('merges with existing sx map literal', () { + test('without trailing commas', () async { + await testSuggestor( + input: withImports(''' + content() => + (Box() + ..sx = {'border': '1px solid black'} + ..mt = 2 + )(); + '''), + expectedOutput: withImports(''' + content() => + (Box() + ..sx = { + 'mt': 2, + 'border': '1px solid black', + } + )(); + '''), + ); + }); + + test('with trailing commas', () async { + await testSuggestor( + input: withImports(''' + content() => + (Box() + ..sx = { + 'border': '1px solid black', + } + ..mt = 2 + )(); + '''), + expectedOutput: withImports(''' + content() => + (Box() + ..sx = { + 'mt': 2, + 'border': '1px solid black', + } + )(); + '''), + ); + }); + + test('that is empty', () async { + await testSuggestor( + input: withImports(''' + content() => + (Box() + ..sx = {} + ..mt = 2 + )(); + '''), + expectedOutput: withImports(''' + content() => + (Box() + ..sx = { + 'mt': 2, + } + )(); + '''), + ); + }); + + test('with multiple existing entries', () async { + await testSuggestor( + input: withImports(''' + content() => + (Box() + ..sx = { + 'backgroundColor': 'white', + 'color': 'blue', + } + ..mt = 2 + )(); + '''), + expectedOutput: withImports(''' + content() => + (Box() + ..sx = { + 'mt': 2, + 'backgroundColor': 'white', + 'color': 'blue', + } + )(); + '''), + ); + }); + + test('with an entry that has the same key as a migrated system prop', + () async { + await testSuggestor( + input: withImports(''' + content() => + (Box() + ..sx = { + 'mt': 1, + } + ..mt = 2 + )(); + '''), + // This will result in duplicate keys in the map, but they're in the + // correct order to preserve the original behavior, and will result in + // a hint that there are duplicate entries. + expectedOutput: withImports(''' + content() => + (Box() + ..sx = { + 'mt': 2, + 'mt': 1, + } + )(); + '''), + ); + }); + }); + + group('merges with forwarded sx prop using spread:', () { + test('nullable', () async { + await testSuggestor( + input: withImports(''' + content(BoxProps props) => + (Box() + ..sx = getSx() + ..mt = 2 + )(); + Map? getSx() => {'color': 'red'}; + '''), + expectedOutput: withImports(''' + content(BoxProps props) => + (Box() + ..sx = { + 'mt': 2, + ...?getSx(), + } + )(); + Map? getSx() => {'color': 'red'}; + '''), + ); + }); + + test('non-nullable', () async { + await testSuggestor( + input: withImports(''' + content(BoxProps props) => + (Box() + ..sx = getSx() + ..mt = 2 + )(); + Map getSx() => {'color': 'red'}; + '''), + expectedOutput: withImports(''' + content(BoxProps props) => + (Box() + ..sx = {'mt': 2, ...getSx()} + )(); + Map getSx() => {'color': 'red'}; + '''), + ); + }); + + test('dynamic', () async { + await testSuggestor( + input: withImports(''' + content(BoxProps props) => + (Box() + ..sx = getSx() + ..mt = 2 + )(); + dynamic getSx() => {'color': 'red'}; + '''), + expectedOutput: withImports(''' + content(BoxProps props) => + (Box() + ..sx = { + 'mt': 2, + ...?getSx(), + } + )(); + dynamic getSx() => {'color': 'red'}; + '''), + ); + }); + }); + + group('merges in sx from forwarded props', () { + group('forwarded with', () { + test('addProps', () async { + await testSuggestor( + input: withImports(''' + content(BoxProps props) => + (Box() + ..addProps(props) + ..mt = 2 + )(); + '''), + expectedOutput: withImports(''' + content(BoxProps props) => + (Box() + ..addProps(props) + ..sx = {'mt': 2, ...?props.sx,} + )(); + '''), + ); + }); + + test('addAll', () async { + await testSuggestor( + input: withImports(''' + content(BoxProps props) => + (Box() + ..addAll(props) + ..mt = 2 + )(); + '''), + expectedOutput: withImports(''' + content(BoxProps props) => + (Box() + ..addAll(props) + ..sx = {'mt': 2, ...?props.sx,} + )(); + '''), + ); + }); + + test('getPropsToForward', () async { + await testSuggestor( + input: withImports(''' + content(BoxProps props) => + (Box() + ..addProps(props.getPropsToForward()) + ..mt = 2 + )(); + '''), + expectedOutput: withImports(''' + content(BoxProps props) => + (Box() + ..addProps(props.getPropsToForward()) + ..sx = {'mt': 2, ...?props.sx,} + )(); + '''), + ); + }); + + test('addPropsToForward', () async { + await testSuggestor( + input: withImports(''' + content(BoxProps props) => + (Box() + ..modifyProps(props.addPropsToForward()) + ..mt = 2 + )(); + '''), + expectedOutput: withImports(''' + content(BoxProps props) => + (Box() + ..modifyProps(props.addPropsToForward()) + ..sx = {'mt': 2, ...?props.sx,} + )(); + '''), + ); + }); + }); + + // Regression test + test('even when there are other unrelated calls in the cascade', + () async { + await testSuggestor( + input: withImports(''' + content(BoxProps props) => + (Box() + ..addProps(props) + ..addTestId('test-id') + ..mt = 2 + )(); + '''), + expectedOutput: withImports(''' + content(BoxProps props) => + (Box() + ..addProps(props) + ..addTestId('test-id') + ..sx = {'mt': 2, ...?props.sx,} + )(); + '''), + ); + }); + }); + + group('adds FIXME comment when forwarding is ambiguous:', () { + test('modifyProps with unknown function', () async { + await testSuggestor( + input: withImports(''' + content(BoxProps props) => + (Box() + ..modifyProps((_) {}) + ..mt = 2 + )(); + '''), + expectedOutput: withImports(''' + content(BoxProps props) => + (Box() + ..modifyProps((_) {}) + + $sxMergeFixme + ..sx = { + 'mt': 2 + } + )(); + '''), + ); + }); + + test('copyUnconsumedProps', () async { + await testSuggestor( + input: withImports(''' + abstract class FooComponent extends UiComponent2 { + content(BoxProps props) => + (Box() + ..addProps(copyUnconsumedProps()) + ..mt = 2 + )(); + } + '''), + expectedOutput: withImports(''' + abstract class FooComponent extends UiComponent2 { + content(BoxProps props) => + (Box() + ..addProps(copyUnconsumedProps()) + + $sxMergeFixme + ..sx = { + 'mt': 2 + } + )(); + } + '''), + ); + }); + + group('generic props:', () { + test('Map', () async { + await testSuggestor( + input: withImports(''' + content(Map props) { + (Box() + ..addAll(props) + ..mt = 2 + )(); + } + '''), + expectedOutput: withImports(''' + content(Map props) { + (Box() + ..addAll(props) + + $sxMergeFixme + ..sx = { + 'mt': 2 + } + )(); + } + '''), + ); + }); + + test('UiProps', () async { + await testSuggestor( + input: withImports(''' + content(UiProps props) { + (Box() + ..addAll(props) + ..mt = 2 + )(); + } + '''), + expectedOutput: withImports(''' + content(UiProps props) { + (Box() + ..addAll(props) + + $sxMergeFixme + ..sx = { + 'mt': 2 + } + )(); + } + '''), + ); + }); + + test('dynamic', () async { + await testSuggestor( + input: withImports(''' + content(dynamic props) { + (Box() + ..addAll(props) + ..mt = 2 + )(); + } + '''), + expectedOutput: withImports(''' + content(dynamic props) { + (Box() + ..addAll(props) + + $sxMergeFixme + ..sx = { + 'mt': 2 + } + )(); + } + '''), + ); + }); + }); + + test('multiple prop forwarding calls', () async { + await testSuggestor( + input: withImports(''' + content(BoxProps props, BoxProps props2) { + (Box() + ..addProps(props) + ..addProps(props2) + ..mt = 2 + )(); + } + '''), + expectedOutput: withImports(''' + content(BoxProps props, BoxProps props2) { + (Box() + ..addProps(props) + ..addProps(props2) + + $sxMergeFixme + ..sx = { + 'mt': 2 + } + )(); + } + '''), + ); + }); + }); + + test('handles complex prop values with expressions', () async { + await testSuggestor( + input: withImports(''' + content(bool condition) { + (Box() + ..mt = condition ? 2 : 4 + ..p = getSpacing() + )(); + } + int getSpacing() => 3; + '''), + expectedOutput: withImports(''' + content(bool condition) { + (Box() + ..sx = { + 'mt': condition ? 2 : 4, + 'p': getSpacing(), + } + )(); + } + int getSpacing() => 3; + '''), + ); + }); + + test('handles responsive system prop values', () async { + await testSuggestor( + input: withImports(''' + content() => + (Box() + ..mt = {'xs': 1, 'sm': 2, 'md': 3} + )(); + '''), + expectedOutput: withImports(''' + content() => + (Box() + ..sx = {'mt': {'xs': 1, 'sm': 2, 'md': 3}} + )(); + '''), + ); + }); + + test('preserves non-system props', () async { + await testSuggestor( + input: withImports(''' + content() => + (Box() + ..id = 'test' + ..mt = 2 + ..className = 'custom-class' + ..onClick = (_) {} + )(); + '''), + expectedOutput: withImports(''' + content() => + (Box() + ..id = 'test' + ..sx = {'mt': 2} + ..className = 'custom-class' + ..onClick = (_) {} + )(); + '''), + ); + }); + + test('handles multiple components in the same file', () async { + await testSuggestor( + input: withImports(''' + content() { + (Box()..mt = 2)(); + (Box()..p = 3)(); + } + '''), + expectedOutput: withImports(''' + content() { + (Box()..sx = {'mt': 2})(); + (Box()..sx = {'p': 3})(); + } + '''), + ); + }); + + test('respects orcm_ignore comments', () async { + await testSuggestor( + input: withImports(''' + // orcm_ignore + content() => (Box()..mt = 2)(); + '''), + ); + }); + + test('does not migrate components without deprecated system props', + () async { + await testSuggestor( + input: withImports(''' + content() => + (Box() + ..id = 'test' + ..className = 'test-class' + )(); + '''), + ); + }); + + test('does not migrate deprecated props with the same name as system props', + () async { + await testSuggestor( + input: withImports(''' + content() => (TextField()..color = '')(); + '''), + ); + }); + + test('does not flag unrelated cascades with FIXMEs', () async { + await testSuggestor( + input: withImports(''' + content() => + (Box() + ..addProp('foo', 'bar') + ..['bar'] = 'baz' + ..ref = (_) {} + ..className = 'test-class' + )(); + '''), + ); + }); + + group('adds a fixme when there are forwarded props after system props and', + () { + test('an existing sx map literal', () async { + await testSuggestor( + input: withImports(''' + content(BoxProps props) => + (Box() + ..mt = 2 + ..addProps(props) + ..sx = {'border': '1px solid black'} + )(); + '''), + expectedOutput: withImports(''' + content(BoxProps props) => + (Box() + ..addProps(props) + ..sx = { + $sxPrecedenceFixme + + 'mt': 2, 'border': '1px solid black', + } + )(); + '''), + ); + }); + + test('an existing sx value', () async { + await testSuggestor( + input: withImports(''' + content(BoxProps props) => + (Box() + ..mt = 2 + ..addProps(props) + ..sx = getSx() + )(); + Map getSx() => {'color': 'red'}; + '''), + expectedOutput: withImports(''' + content(BoxProps props) => + (Box() + ..addProps(props) + ..sx = { + $sxPrecedenceFixme + + 'mt': 2, ...getSx() + } + )(); + Map getSx() => {'color': 'red'}; + '''), + ); + }); + + test('no existing sx', () async { + await testSuggestor( + input: withImports(''' + content(BoxProps props) => + (Box() + ..mt = 2 + ..addProps(props) + )(); + '''), + expectedOutput: withImports(''' + content(BoxProps props) => + (Box() + ..addProps(props) + $sxPrecedenceFixme + ..sx = { + 'mt': 2, + ...?props.sx, + } + )(); + '''), + ); + }); + }); + + group('insertion location:', () { + test('inserts after prop forwarding to avoid being overwritten', + () async { + await testSuggestor( + input: withImports(''' + content(BoxProps props) => + (Box() + ..mt = 2 + ..addProps(props) + ..id = 'test' + )(); + '''), + expectedOutput: withImports(''' + content(BoxProps props) => + (Box() + ..addProps(props) + $sxPrecedenceFixme + ..sx = {'mt': 2, ...?props.sx,} + ..id = 'test' + )(); + '''), + ); + }); + + test('inserts at location of last system prop when no prop forwarding', + () async { + await testSuggestor( + input: withImports(''' + content() => + (Box() + ..id = 'first' + ..mt = 2 + ..className = 'middle' + ..p = 3 + ..onClick = (_) {} + )(); + '''), + expectedOutput: withImports(''' + content() => + (Box() + ..id = 'first' + ..className = 'middle' + ..sx = {'mt': 2, 'p': 3} + ..onClick = (_) {} + )(); + '''), + ); + }); + + test('inserts after latest of (forwarding or last system prop)', + () async { + await testSuggestor( + input: withImports(''' + content(BoxProps props) => + (Box() + ..mt = 2 + ..addProps(props) + ..p = 3 + )(); + '''), + expectedOutput: withImports(''' + content(BoxProps props) => + (Box() + ..addProps(props) + + $sxPrecedenceFixme + ..sx = {'mt': 2, 'p': 3, ...?props.sx,} + )(); + '''), + ); + }); + + test('inserts after all forwarding calls when multiple exist', () async { + await testSuggestor( + input: withImports(''' + content(BoxProps props1, BoxProps props2) => + (Box() + ..addProps(props1) + ..mt = 2 + ..p = 3 + ..addProps(props2) + )(); + '''), + expectedOutput: withImports(''' + content(BoxProps props1, BoxProps props2) => + (Box() + ..addProps(props1) + ..addProps(props2) + $sxPrecedenceFixme + $sxMergeFixme + ..sx = {'mt': 2, 'p': 3} + )(); + '''), + ); + }); + }); + + group('multiline formatting:', () { + test('uses single line for short sx maps (< 3 elements)', () async { + await testSuggestor( + input: withImports(''' + content() => + (Box() + ..mt = 2 + ..p = 3 + )(); + '''), + expectedOutput: withImports(''' + content() => + (Box() + ..sx = {'mt': 2, 'p': 3} + )(); + '''), + ); + }); + + test('uses multiline for 3+ elements', () async { + await testSuggestor( + input: withImports(''' + content() => + (Box() + ..mt = 2 + ..p = 3 + ..mb = 4 + )(); + '''), + expectedOutput: withImports(''' + content() => + (Box() + ..sx = { + 'mt': 2, + 'p': 3, + 'mb': 4, + } + )(); + '''), + ); + }); + + test('uses multiline for long content (>= 20 chars with 2+ elements)', + () async { + await testSuggestor( + input: withImports(''' + content() => + (Box() + ..mt = 2 + ..bgcolor = 'verylongcolor.main' + )(); + '''), + expectedOutput: withImports(''' + content() => + (Box() + ..sx = { + 'mt': 2, + 'bgcolor': 'verylongcolor.main', + } + )(); + '''), + ); + }); + }); + + group('preserves comments before system props:', () { + test('single line comment before single system prop', () async { + await testSuggestor( + input: withImports(''' + content() => + (Box() + // Add margin + ..mt = 2 + )(); + '''), + expectedOutput: withImports(''' + content() => + (Box() + ..sx = { + // Add margin + 'mt': 2, + } + )(); + '''), + ); + }); + + test('single line comment before one of multiple system props', () async { + await testSuggestor( + input: withImports(''' + content() => + (Box() + ..mt = 2 + // Add padding + ..p = 3 + )(); + '''), + expectedOutput: withImports(''' + content() => + (Box() + ..sx = { + 'mt': 2, + // Add padding + 'p': 3, + } + )(); + '''), + ); + }); + + test('multiple comments before different system props', () async { + await testSuggestor( + input: withImports(''' + content() => + (Box() + // Top margin + ..mt = 2 + // Horizontal padding + ..px = 3 + // Background color + ..bgcolor = 'blue' + )(); + '''), + expectedOutput: withImports(''' + content() => + (Box() + ..sx = { + // Top margin + 'mt': 2, + // Horizontal padding + 'px': 3, + // Background color + 'bgcolor': 'blue', + } + )(); + '''), + ); + }); + + test('multi-line comment before system prop', () async { + await testSuggestor( + input: withImports(''' + content() => + (Box() + /* This is a longer comment + explaining the margin */ + ..mt = 2 + )(); + '''), + expectedOutput: withImports(''' + content() => + (Box() + ..sx = { + /* This is a longer comment + explaining the margin */ + 'mt': 2, + } + )(); + '''), + ); + }); + + test('comment before system prop with existing sx', () async { + await testSuggestor( + input: withImports(''' + content() => + (Box() + ..sx = {'border': '1px solid black'} + // Add margin + ..mt = 2 + )(); + '''), + // Not sure why dartfmt allows two entries like this with trailing commas + // on the same line, but it is what it is. + expectedOutput: withImports(''' + content() => + (Box() + ..sx = { + // Add margin + 'mt': 2, 'border': '1px solid black', + } + )(); + '''), + ); + }); + + test('comments before system props mixed with non-system props', + () async { + await testSuggestor( + input: withImports(''' + content() => + (Box() + ..id = 'test' + // Spacing + ..mt = 2 + ..className = 'custom' + // More spacing + ..p = 3 + )(); + '''), + expectedOutput: withImports(''' + content() => + (Box() + ..id = 'test' + ..className = 'custom' + ..sx = { + // Spacing + 'mt': 2, + // More spacing + 'p': 3, + } + )(); + '''), + ); + }); + + test('comment before system prop with forwarding', () async { + await testSuggestor( + input: withImports(''' + content(BoxProps props) => + (Box() + ..addProps(props) + // Override margin + ..mt = 2 + )(); + '''), + // Not sure why dartfmt allows two entries like this with trailing commas + // on the same line, but it is what it is. + expectedOutput: withImports(''' + content(BoxProps props) => + (Box() + ..addProps(props) + + ..sx = { + // Override margin + 'mt': 2, ...?props.sx, + } + )(); + '''), + ); + }); + + test('inline comment on same line as system prop', () async { + await testSuggestor( + input: withImports(''' + content() => + (Box() + ..mt = 2 // top margin + ..p = 3 + )(); + '''), + // Inline comment behavior here isn't great, but it's too much effort + // to deal with in this codemod. + // Just verify the codemod doesn't break or do anything too outlandish. + expectedOutput: withImports(''' + content() => + (Box() + // top margin + ..sx = {'mt': 2, 'p': 3} + )(); + '''), + ); + }); + + test('multiple comment types with system props', () async { + await testSuggestor( + input: withImports(''' + content() => + (Box() + // Line comment + ..mt = 2 // inline comment + /* Block comment */ + ..p = 3 + )(); + '''), + // Inline comment behavior here isn't great, but it's too much effort + // to deal with in this codemod. + // Just verify the codemod doesn't break or do anything too outlandish. + expectedOutput: withImports(''' + content() => + (Box() + // inline comment + ..sx = { + // Line comment + 'mt': 2, + /* Block comment */ + 'p': 3, + } + )(); + '''), + ); + }); + }); + + group('handles different component types with system props:', () { + test('Box', () async { + await testSuggestor( + input: withImports(''' + content() => (Box()..mt = 2)(); + '''), + expectedOutput: withImports(''' + content() => (Box()..sx = {'mt': 2})(); + '''), + ); + }); + + test('Grid', () async { + await testSuggestor( + input: withImports(''' + content() => (Grid()..mt = 2)(); + '''), + expectedOutput: withImports(''' + content() => (Grid()..sx = {'mt': 2})(); + '''), + ); + }); + + test('Stack', () async { + await testSuggestor( + input: withImports(''' + content() => (Stack()..mt = 2)(); + '''), + expectedOutput: withImports(''' + content() => (Stack()..sx = {'mt': 2})(); + '''), + ); + }); + + test('Typography', () async { + await testSuggestor( + input: withImports(''' + content() => (Typography()..mt = 2)(); + '''), + expectedOutput: withImports(''' + content() => (Typography()..sx = {'mt': 2})(); + '''), + ); + }); + }); + }); +} + +String getStubMuiLibrarySource({required String filenameWithoutExtension}) { + final systemPropComponentsSource = [ + 'Box', + 'Grid', + 'Stack', + 'Typography', + ].map((componentName) { + return ''' + UiFactory<${componentName}Props> $componentName = uiFunction((_) {}, _\$${componentName}Config); + + @Props(keyNamespace: '') + mixin ${componentName}Props on UiProps { + @convertJsMapProp + Map? sx; + + ${systemPropNames.map((propName) => " @Deprecated('Use sx.') dynamic ${propName};").join('\n')} + } + '''; + }).join('\n\n'); + + return ''' + //@dart=2.19 + import 'package:over_react/over_react.dart'; + import 'package:over_react/js_component.dart'; + + // ignore: uri_has_not_been_generated + part '$filenameWithoutExtension.over_react.g.dart'; + + $systemPropComponentsSource + + UiFactory TextField = uiFunction((_) {}, _\$TextFieldConfig); + + @Props(keyNamespace: '') + mixin TextFieldProps on UiProps { + @convertJsMapProp + Map? sx; + + @Deprecated('Deprecated, but not the same as the system props color') + dynamic color; + } + '''; +} + +// Borrowed from https://github.com/Workiva/over_react/blob/5.6.0/tools/analyzer_plugin/test/unit/util/prop_declaration/util.dart#L73-L78 + +InterfaceElement getInterfaceElement(ResolvedUnitResult result, String name) => + result.libraryElement.topLevelElements + .whereType() + .singleWhere((e) => e.name == name); + +InterfaceElement getImportedInterfaceElement( + ResolvedUnitResult result, String name) => + result.libraryElement.importedLibraries + .map((l) => l.exportNamespace.get(name)) + .whereNotNull() + .single as InterfaceElement; diff --git a/test/test_fixtures/over_react_project/pubspec.yaml b/test/test_fixtures/over_react_project/pubspec.yaml index 5949bbd1..36042ef2 100644 --- a/test/test_fixtures/over_react_project/pubspec.yaml +++ b/test/test_fixtures/over_react_project/pubspec.yaml @@ -2,4 +2,4 @@ name: over_react_project environment: sdk: '>=2.11.0 <3.0.0' dependencies: - over_react: ^5.0.0 + over_react: ^5.6.0