Skip to content
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

CPLAT-9411 Add standalone PropsMixin / StateMixin migrator #77

2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM google/dart:2.4 as build
FROM drydock-prod.workiva.net/workiva/dart2_base_image:1 as build

# Build Environment Vars
ARG BUILD_ID
Expand Down
86 changes: 52 additions & 34 deletions lib/src/boilerplate_suggestors/boilerplate_utilities.dart
Original file line number Diff line number Diff line change
Expand Up @@ -26,32 +26,30 @@ bool isPublicForTest = false;
// Stub while <https://jira.atl.workiva.net/browse/CPLAT-9308> is in progress
bool isPublic(ClassDeclaration node) => isPublicForTest;

/// Returns the annotation node associated with the provided [classNode]
/// that matches the provided [annotationName], if one exists.
AstNode getAnnotationNode(ClassDeclaration classNode, String annotationName) {
if (classNode.metadata.isEmpty) return null;
aaronlademann-wf marked this conversation as resolved.
Show resolved Hide resolved

return classNode.metadata.singleWhere(
aaronlademann-wf marked this conversation as resolved.
Show resolved Hide resolved
(node) => node.name.name == annotationName,
orElse: () => null);
}

/// A simple evaluation of the annotation(s) of the [classNode]
/// to verify it is either a `@PropsMixin()` or `@StateMixin`.
bool isAPropsOrStateMixin(ClassDeclaration classNode) =>
isAPropsMixin(classNode) || isAStateMixin(classNode);

/// Returns the node of a `@PropsMixin()` annotation for the provided [classNode], if one exists.
AstNode getPropsMixinAnnotationNode(ClassDeclaration classNode) =>
classNode.sortedCommentAndAnnotations.singleWhere(
(node) => node?.toSource()?.startsWith('@PropsMixin') == true,
orElse: () => null);

/// A simple evaluation of the annotation(s) of the [classNode]
/// to verify it is a `@PropsMixin()`.
bool isAPropsMixin(ClassDeclaration classNode) =>
getPropsMixinAnnotationNode(classNode) != null;

/// Returns the node of a `@PropsMixin()` annotation for the provided [classNode], if one exists.
AstNode getStateMixinAnnotationNode(ClassDeclaration classNode) =>
classNode.sortedCommentAndAnnotations.singleWhere(
(node) => node?.toSource()?.startsWith('@StateMixin') == true,
orElse: () => null);
getAnnotationNode(classNode, 'PropsMixin') != null;

/// A simple evaluation of the annotation(s) of the [classNode]
/// to verify it is a `@StateMixin()`.
bool isAStateMixin(ClassDeclaration classNode) =>
getStateMixinAnnotationNode(classNode) != null;
getAnnotationNode(classNode, 'StateMixin') != null;

/// Whether a props or state mixin class [classNode] should be migrated as part of the boilerplate codemod.
bool shouldMigratePropsAndStateMixin(ClassDeclaration classNode) =>
Expand All @@ -68,23 +66,33 @@ bool shouldMigratePropsAndStateClass(ClassDeclaration node) {
/// A simple RegExp against the parent of the class to verify it is `UiProps`
/// or `UiState`.
bool extendsFromUiPropsOrUiState(ClassDeclaration classNode) =>
classNode.extendsClause.superclass.name
.toSource()
classNode.extendsClause.superclass.name.name
.contains(RegExp('(UiProps)|(UiState)'));
aaronlademann-wf marked this conversation as resolved.
Show resolved Hide resolved

/// A simple RegExp against the parent of the class to verify it is `UiProps`
/// or `UiState`.
bool implementsUiPropsOrUiState(ClassDeclaration classNode) {
return classNode.implementsClause.interfaces
.map((typeName) => typeName.toSource())
.any((typeStr) => typeStr.contains(RegExp('(UiProps)|(UiState)')));
.map((typeName) => typeName.name.name)
.any({'UiProps', 'UiState'}.contains);
}

/// A simple RegExp against the name of the class to verify it contains `props`
/// or `state`.
bool isAPropsOrStateClass(ClassDeclaration classNode) => classNode.name
.toSource()
.contains(RegExp('([A-Za-z]+Props)|([A-Za-z]+State)'));
/// A simple evaluation of the annotation(s) of the [classNode]
/// to verify it is either `@Props()` or `@State()` annotated class.
bool isAPropsOrStateClass(ClassDeclaration classNode) =>
isAPropsClass(classNode) || isAStateClass(classNode);

/// A simple evaluation of the annotation(s) of the [classNode]
/// to verify it is a `@Props()` annotated class.
bool isAPropsClass(ClassDeclaration classNode) =>
getAnnotationNode(classNode, 'Props') != null ||
getAnnotationNode(classNode, 'AbstractProps') != null;

/// A simple evaluation of the annotation(s) of the [classNode]
/// to verify it is a `@State()` annotated class.
bool isAStateClass(ClassDeclaration classNode) =>
getAnnotationNode(classNode, 'State') != null ||
getAnnotationNode(classNode, 'AbstractState') != null;

/// Detects if the Props or State class is considered simple.
///
Expand All @@ -95,7 +103,7 @@ bool isSimplePropsOrStateClass(ClassDeclaration classNode) {
// Only validate props or state classes
assert(isAPropsOrStateClass(classNode));

final superClass = classNode.extendsClause.superclass.name.toSource();
final superClass = classNode.extendsClause.superclass.name.name;

if (superClass != 'UiProps' && superClass != 'UiState') return false;
if (classNode.withClause != null) return false;
Expand Down Expand Up @@ -168,8 +176,7 @@ void migrateClassToMixin(ClassDeclaration node, YieldPatch yieldPatch,

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

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

if (node.extendsClause?.extendsKeyword != null) {
Expand Down Expand Up @@ -199,25 +206,21 @@ void migrateClassToMixin(ClassDeclaration node, YieldPatch yieldPatch,
} else {
// Implements UiProps / UiState along with other interfaces
final uiInterface = nodeInterfaces.singleWhere((interface) =>
interface.toSource() == 'UiProps' ||
interface.toSource() == 'UiState');
interface.name.name == 'UiProps' ||
interface.name.name == 'UiState');
final otherInterfaces = List.of(nodeInterfaces)..remove(uiInterface);
final otherInterfacesStr =
commaSeparatedAstNodeNames(otherInterfaces);

yieldPatch(node.implementsClause.offset, node.implementsClause.end,
'on ${uiInterface.toSource()} implements $otherInterfacesStr');
'on ${uiInterface.name.name} implements ${otherInterfaces.joinByName()}');
}
} else {
// Does not implement UiProps / UiState
final uiInterfaceStr = isAPropsMixin(node) ? 'UiProps' : 'UiState';

if (nodeInterfaces.isNotEmpty) {
// But does implement other stuff
final otherInterfacesStr = commaSeparatedAstNodeNames(nodeInterfaces);

yieldPatch(node.implementsClause.offset, node.implementsClause.end,
'on $uiInterfaceStr implements $otherInterfacesStr');
'on $uiInterfaceStr implements ${nodeInterfaces.joinByName()}');
} else {
// Does not implement anything
aaronlademann-wf marked this conversation as resolved.
Show resolved Hide resolved
yieldPatch(node.leftBracket.offset - 1, node.leftBracket.offset - 1,
Expand All @@ -236,3 +239,18 @@ void migrateClassToMixin(ClassDeclaration node, YieldPatch yieldPatch,
propsAndStateClassNamesConvertedToNewBoilerplate[originalPublicClassName] =
newMixinName;
}

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 startingString, String endingString, String seperator}) {
final itemString = map((t) => t.name.name).join('${seperator ?? ','} ');
final returnString = StringBuffer()
..write(startingString != null ? '${startingString.trimRight()} ' : '')
..write(itemString)
..write(endingString != null ? '${endingString.trimLeft()}' : '');

return returnString.toString();
}
}
34 changes: 20 additions & 14 deletions lib/src/boilerplate_suggestors/props_mixins_migrator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -44,21 +44,31 @@ class PropsMixinMigrator extends GeneralizingAstVisitor
(getter) => getter.name.name == 'props' || getter.name.name == 'state',
orElse: () => null);
if (propsOrStateGetter != null) {
yieldPatch(propsOrStateGetter.offset, propsOrStateGetter.end + 1, '');
yieldPatch(propsOrStateGetter.offset, propsOrStateGetter.end, '');
}
}

void _removePropsOrStateMixinAnnotation(ClassDeclaration node) {
final propsMixinAnnotationNode = getPropsMixinAnnotationNode(node);
final propsMixinAnnotationNode = getAnnotationNode(node, 'PropsMixin');
if (propsMixinAnnotationNode != null) {
yieldPatch(propsMixinAnnotationNode.offset,
propsMixinAnnotationNode.end + 1, '');
yieldPatch(
propsMixinAnnotationNode.offset,
// +1 to ensure that any comments that were on the line immediately before the annotation
// we are removing - end up on the line immediately before the mixin declaration instead
// of having a newline separating them.
propsMixinAnnotationNode.end + 1,
aaronlademann-wf marked this conversation as resolved.
Show resolved Hide resolved
'');
}

final stateMixinAnnotationNode = getStateMixinAnnotationNode(node);
final stateMixinAnnotationNode = getAnnotationNode(node, 'StateMixin');
if (stateMixinAnnotationNode != null) {
yieldPatch(stateMixinAnnotationNode.offset,
stateMixinAnnotationNode.end + 1, '');
yieldPatch(
stateMixinAnnotationNode.offset,
// +1 to ensure that any comments that were on the line immediately before the annotation
// we are removing - end up on the line immediately before the mixin declaration instead
// of having a newline separating them.
stateMixinAnnotationNode.end + 1,
'');
}
}

Expand All @@ -80,14 +90,10 @@ class PropsMixinMigrator extends GeneralizingAstVisitor
} else {
// Remove the meta field, along with any comment lines that preceded it.
aaronlademann-wf marked this conversation as resolved.
Show resolved Hide resolved
final metaFieldDecl = metaField.parent;
final previousMember = metaFieldDecl == classMembers.first
? null
: classMembers[classMembers.indexOf(metaFieldDecl) - 1];
final begin = previousMember != null
? previousMember.end + 1
: node.leftBracket.offset + 1;
final begin = metaFieldDecl.beginToken.precedingComments?.offset ??
metaField.offset;

yieldPatch(begin, metaField.end + 1, '');
yieldPatch(begin, metaFieldDecl.end, '');
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/visitor.dart';
import 'package:codemod/codemod.dart';

import '../util.dart';
import 'boilerplate_utilities.dart';

/// Suggestor that updates props and state classes to new boilerplate.
Expand Down
11 changes: 0 additions & 11 deletions lib/src/util.dart
Original file line number Diff line number Diff line change
Expand Up @@ -446,14 +446,3 @@ String stripPrivateGeneratedPrefix(String value) {
? value.substring(privateGeneratedPrefix.length)
: value;
}

/// Takes an iterable of AstNodes and returns their name in a comma separated string.
///
/// Optionally, you can pass a function to [getName] if you want something other
/// than [AstNode.toSource] to be used to generate the name for the list.
String commaSeparatedAstNodeNames<T extends AstNode>(Iterable<T> nodeList,
{String Function(T node) getName}) {
getName ??= (node) => node.toSource();

return nodeList.map(getName).toString().replaceAll(RegExp(r'[\(\)]'), '');
}
4 changes: 2 additions & 2 deletions pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ authors:
- Sydney Jodon <[email protected]>

environment:
sdk: ">=2.4.0 <3.0.0"
sdk: ">=2.7.0 <3.0.0"

dependencies:
analyzer: '>=0.37.0 <0.39.0'
Expand All @@ -33,7 +33,7 @@ dependencies:

dev_dependencies:
dart_dev: ^2.0.1
dart_style: ^1.2.0
dart_style: ^1.3.3
dependency_validator: ^1.2.3
mockito: ^4.0.0
pedantic: ^1.4.0
Expand Down