Skip to content

Commit

Permalink
Merge pull request #81 from Workiva/CPLAT-9565_testing/dev
Browse files Browse the repository at this point in the history 
CPLAT-9565 Address edge cases for boilerplate codemods
  • Loading branch information
@aaronlademann-wf
aaronlademann-wf authored Feb 14, 2020
2 parents c119dcf + 580327a commit bbd95f3
Show file tree
Hide file tree
Showing 8 changed files with 1,514 additions and 83 deletions.
148 changes: 132 additions & 16 deletions lib/src/boilerplate_suggestors/advanced_props_and_state_class_migrator.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,58 @@ class AdvancedPropsAndStateClassMigrator extends GeneralizingAstVisitor

final extendsFromCustomClass = !extendsFromUiPropsOrUiState(node);
final hasMixins = node.withClause != null;
final hasInterfaces = node.implementsClause != null;
final parentClassName = node.extendsClause.superclass.name.name;
final parentClassTypeArgs =
node.extendsClause.superclass.typeArguments ?? '';

final className = stripPrivateGeneratedPrefix(node.name.name);
final classTypeArgs = node.typeParameters ?? '';
final dupeMixinExists =
getNameOfDupeClass(className, node.root, converter) != null;
final mixinWillBeCreatedFromClass =
!dupeMixinExists && node.members.isNotEmpty;
final dupeClassInSameRoot = getDupeClassInSameRoot(className, node.root);
final classNeedsBody = node.members.isNotEmpty &&
dupeMixinExists &&
dupeClassInSameRoot == null;

StringBuffer getMixinsForNewDeclaration({bool includeParentClass = true}) {
final mixinsForNewDeclaration = StringBuffer();
if (extendsFromCustomClass) {
final baseAndParentClassMixins = <String>[];

if (includeParentClass) {
baseAndParentClassMixins.add(
'${getConvertedClassMixinName(parentClassName, converter)}$parentClassTypeArgs');
}

if (mixinWillBeCreatedFromClass) {
baseAndParentClassMixins.add('${className}Mixin$classTypeArgs');
}

if (baseAndParentClassMixins.isNotEmpty) {
mixinsForNewDeclaration.write(baseAndParentClassMixins.join(','));

if (hasMixins) {
mixinsForNewDeclaration.write(', ');
}
}
}

if (hasMixins) {
if (!extendsFromCustomClass && mixinWillBeCreatedFromClass) {
mixinsForNewDeclaration.write('${className}Mixin$classTypeArgs, ');
}

mixinsForNewDeclaration.write(node.withClause.mixinTypes
.joinConvertedClassesByName(
converter: converter, sourceFile: sourceFile));
}

return mixinsForNewDeclaration;
}

final newDeclarationBuffer = StringBuffer()
..write('\n\n')
// Write a fix me comment if this class extends a custom class
Expand All @@ -55,30 +104,97 @@ class AdvancedPropsAndStateClassMigrator extends GeneralizingAstVisitor
// 2. Fix any analyzer warnings on this class about missing mixins
''')
// Create the class name
..write('class $className = ')
// Decide if the class is a Props or a State class
..write('Ui${isAPropsClass(node) ? 'Props' : 'State'} ')
// Add the width clause
..write('with ');

if (extendsFromCustomClass) {
newDeclarationBuffer.write(
'${parentClassName}Mixin, ${className}Mixin${hasMixins ? ',' : ''}');
}
..write(node.isAbstract ? 'abstract class ' : 'class ')
..write('$className$classTypeArgs');

if (hasMixins) {
if (!extendsFromCustomClass) {
newDeclarationBuffer.write('${className}Mixin,');
StringBuffer mixins;

if (node.isAbstract) {
mixins = getMixinsForNewDeclaration();
// Since its abstract, we'll create an interface-only class which can then be implemented by
// concrete subclasses that have component classes that extend from the analogous abstract component class.
newDeclarationBuffer.write(' implements $mixins');

if (hasInterfaces) {
newDeclarationBuffer.write(
', ${node.implementsClause.interfaces.joinConvertedClassesByName()}');
}
} else {
// Its a concrete class. Have it extend from UiProps/State with mixins

final willNeedToImplementAbstractInterface =
isAssociatedWithAbstractComponent2(node) && extendsFromCustomClass;
final abstractInterfaceHasAnalogousMixin =
getConvertedClassMixinName(parentClassName, converter) !=
parentClassName;

mixins = willNeedToImplementAbstractInterface &&
!abstractInterfaceHasAnalogousMixin
// Since the parentClassName will be what is implemented in that scenario -
// we don't want to have that class in both the withClause and the implementsClause.
? getMixinsForNewDeclaration(includeParentClass: false)
: getMixinsForNewDeclaration();

newDeclarationBuffer
..write(classNeedsBody || mixins.isEmpty ? ' extends ' : ' = ')
// Decide if the class is a Props or a State class
..write('Ui${isAPropsClass(node) ? 'Props' : 'State'} ')
// Add the with clause
..write(mixins.isEmpty ? '' : 'with $mixins');

if (hasInterfaces || willNeedToImplementAbstractInterface) {
newDeclarationBuffer.write(' implements ');

newDeclarationBuffer.write(node.withClause.mixinTypes.joinByName());
if (willNeedToImplementAbstractInterface) {
// The analogous component instance for this props/state class extends from an abstract component.
// This means that in order for the generic params of the component to continue to work, the
// concrete props/state class will need to implement the abstract props/state class.
newDeclarationBuffer.write('$parentClassName$parentClassTypeArgs');

if (hasInterfaces) {
newDeclarationBuffer.write(', ');
}
}

if (hasInterfaces) {
newDeclarationBuffer.write(
node.implementsClause.interfaces.joinConvertedClassesByName());
}
}
}

newDeclarationBuffer.write(';');
if (dupeMixinExists && node.members.isNotEmpty) {
// If no mixin will be created from the class in the `converter.migrate` step below
// as a result of a mixin with a name that matches the current node's name appended with "Mixin",
// and it has members of its own, we need to preserve those members (fields) by moving them to the
// existing mixin (if possible - within the same root), or create a FIX ME comment indicating what
// needs to be done.
if (dupeClassInSameRoot != null) {
yieldPatch(
dupeClassInSameRoot.rightBracket.offset,
dupeClassInSameRoot.rightBracket.offset,
node.members.map((member) => member.toSource()).join('\n'));

newDeclarationBuffer.write(node.isAbstract ? '{}' : ';');
} else {
newDeclarationBuffer
..write('{\n')
..write('''
// FIXME: Everything in this body needs to be moved to the body of ${getNameOfDupeClass(className, node.root, converter)}.
// Once that is done, the body can be removed, and `extends` can be replaced with `=`.
''')
..writeAll(node.members.map((member) => member.toSource()))
..write('\n}');
}
} else {
newDeclarationBuffer
.write(node.isAbstract || mixins.isEmpty ? '{}' : ';');
}

converter.migrate(node, yieldPatch,
shouldAddMixinToName: true,
shouldSwapParentClass: extendsFromCustomClass);
shouldSwapParentClass: extendsFromCustomClass,
sourceFile: sourceFile);
yieldPatch(node.end, node.end, newDeclarationBuffer.toString());
}
}
Expand Down
121 changes: 112 additions & 9 deletions lib/src/boilerplate_suggestors/boilerplate_utilities.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
import 'package:analyzer/dart/ast/ast.dart';
import 'package:meta/meta.dart';
import 'package:over_react_codemod/src/constants.dart';
import 'package:over_react_codemod/src/react16_suggestors/react16_utilities.dart';
import 'package:over_react_codemod/src/util.dart';
import 'package:source_span/source_span.dart';

typedef YieldPatch = void Function(
int startingOffset, int endingOffset, String replacement);
Expand Down Expand Up @@ -125,6 +127,37 @@ bool isAdvancedPropsOrStateClass(ClassDeclaration classNode) {
return !isSimplePropsOrStateClass(classNode);
}

/// Returns the class/mixin that was converted in a previous migration
/// that has the same name as [className] when appended with "Mixin".
ClassOrMixinDeclaration getDupeClassInSameRoot(
String className, CompilationUnit root) {
final possibleDupeClassesInSameRoot =
root.declarations.whereType<ClassOrMixinDeclaration>();
for (var decl in possibleDupeClassesInSameRoot) {
if (decl.name.name == '${className}Mixin') {
return decl;
}
}

return null;
}

/// Returns the name of a mixin that was converted in a previous migration
/// that has the same name as [className] when appended with "Mixin".
String getNameOfDupeClass(
String className, CompilationUnit root, ClassToMixinConverter converter) {
final possibleDupeClasses = converter.convertedClassNames.keys;
String nameOfDupeClass;
for (var possibleDupeClassName in possibleDupeClasses) {
if (possibleDupeClassName == '${className}Mixin') {
nameOfDupeClass = possibleDupeClassName;
break;
}
}

return nameOfDupeClass ?? getDupeClassInSameRoot(className, root)?.name?.name;
}

/// A class used to handle the conversion of props / state classes to mixins.
///
/// Should only be constructed once to initialize the value of [convertedClassNames].
Expand Down Expand Up @@ -184,15 +217,31 @@ class ClassToMixinConverter {
/// so that suggestors that come after the suggestor that called this function - can know
/// whether to yield a patch based on that information.
void migrate(ClassDeclaration node, YieldPatch yieldPatch,
{bool shouldAddMixinToName = false, bool shouldSwapParentClass = false}) {
{bool shouldAddMixinToName = false,
bool shouldSwapParentClass = false,
SourceFile sourceFile}) {
final originalPublicClassName = stripPrivateGeneratedPrefix(node.name.name);

if (shouldAddMixinToName) {
// Check to make sure we're not creating a duplicate mixin
final dupeClassName =
getNameOfDupeClass(originalPublicClassName, node.root, this);
if (dupeClassName != null || node.members.isEmpty) {
// Delete the class since a mixin with the same name already exists,
// or the class has no members of its own.
yieldPatch(node.offset, node.end, '');
convertedClassNames[originalPublicClassName] = originalPublicClassName;
return;
}
}

if (node.abstractKeyword != null) {
yieldPatch(node.abstractKeyword.offset, node.abstractKeyword.charEnd, '');
}

yieldPatch(node.classKeyword.offset, node.classKeyword.charEnd, 'mixin');

final originalPublicClassName = stripPrivateGeneratedPrefix(node.name.name);
String newMixinName = originalPublicClassName;
var newMixinName = originalPublicClassName;

if (node.extendsClause?.extendsKeyword != null) {
// --- Convert concrete props/state class to a mixin --- //
Expand All @@ -211,7 +260,7 @@ class ClassToMixinConverter {
if (shouldSwapParentClass) {
yieldPatch(
node.extendsClause.superclass.name.offset,
node.extendsClause.superclass.name.end,
node.extendsClause.superclass.end,
isAPropsClass(node) ? 'UiProps' : 'UiState');
}

Expand All @@ -238,7 +287,7 @@ class ClassToMixinConverter {
..remove(uiInterface);

yieldPatch(node.implementsClause.offset, node.implementsClause.end,
'on ${uiInterface.name.name} implements ${otherInterfaces.joinByName()}');
'on ${uiInterface.name.name} implements ${otherInterfaces.joinConvertedClassesByName()}');
}
} else {
// Does not implement UiProps / UiState
Expand All @@ -247,7 +296,7 @@ class ClassToMixinConverter {
if (nodeInterfaces.isNotEmpty) {
// But does implement other stuff
yieldPatch(node.implementsClause.offset, node.implementsClause.end,
'on $uiInterfaceStr implements ${nodeInterfaces.joinByName()}');
'on $uiInterfaceStr implements ${nodeInterfaces.joinConvertedClassesByName()}');
}
}
} else {
Expand Down Expand Up @@ -278,10 +327,64 @@ String getPropsClassNameFromFactoryDeclaration(
?.toSource();
}

/// Returns the name of the mixin that the [originalClassName] was converted to, or the [originalClassName]
/// if no matching key is found within [ClassToMixinConverter.convertedClassNames] on the provided [converter] instance.
String getConvertedClassMixinName(
String originalClassName, ClassToMixinConverter converter) {
const reservedBaseClassNames = {
'FluxUiProps': 'FluxUiPropsMixin',
'BuiltReduxUiProps': 'BuiltReduxUiPropsMixin',
};

// If it is a "reserved" base class that the consumer doesn't control / won't be converted as
// part of running the codemod in their repo, return the new "known" mixin name.
if (reservedBaseClassNames.containsKey(originalClassName)) {
return reservedBaseClassNames[originalClassName];
}

return converter.convertedClassNames[originalClassName] ?? originalClassName;
}

extension IterableAstUtils on Iterable<NamedType> {
/// Utility to join an `Iterable` based on the `name` of the `name` field
/// rather than the `toString()` of the object.
String joinByName({String separator}) {
return map((t) => t.name.name).join('${separator ?? ','} ');
/// rather than the `toString()` of the object when the named type is:
///
/// 1. A non-generated _(no `$` prefix)_ mixin / class
/// 2. A generated mixin name that has not been converted by a migrator
/// * The `// ignore` comments will be preserved in this case
String joinConvertedClassesByName(
{ClassToMixinConverter converter,
SourceFile sourceFile,
String separator}) {
bool _mixinHasBeenConverted(NamedType t) => converter.convertedClassNames
.containsKey(t.name.name.replaceFirst('\$', ''));

bool _mixinNameIsOldGeneratedBoilerplate(NamedType t) =>
t.name.name.startsWith('\$');

String _typeArgs(NamedType t) => '${t.typeArguments ?? ''}';

return where((t) {
if (converter == null) return true;
return !_mixinNameIsOldGeneratedBoilerplate(t) ||
!_mixinHasBeenConverted(t);
}).map((t) {
if (converter != null &&
sourceFile != null &&
_mixinNameIsOldGeneratedBoilerplate(t) &&
!_mixinHasBeenConverted(t)) {
// Preserve ignore comments for generated, unconverted props mixins
if (hasComment(
t, sourceFile, 'ignore: mixin_of_non_class, undefined_class')) {
return '// ignore: mixin_of_non_class, undefined_class\n${getConvertedClassMixinName(t.name.name, converter)}${_typeArgs(t)}';
}
}

if (converter != null) {
return '${getConvertedClassMixinName(t.name.name, converter)}${_typeArgs(t)}';
}

return '${t.name.name}${_typeArgs(t)}';
}).join('${separator ?? ','} ');
}
}
Loading

0 comments on commit bbd95f3

Please sign in to comment.