AF-1917: Make over_react_codemod script idempotent

[sebastianmalysa-wf]

DO NOT MERGE UNTIL #4 IS MERGED

Changes

• made migrater.py idempotent
• added tests

@corwinsheahan-wf @evanweible-wf @maxwellpeterson-wf

[aviary-wf]

Security Insights

No security relevant content was detected by automated scans.

Action Items

• Review PR for security impact; comment "security review required" if needed or unsure
• Verify aviary.yaml coverage of security relevant code

---

Questions or Comments? Reach out on HipChat: InfoSec Forum.

[rmconsole-wf]

Merge Requirements Met ✅

Request Rosie to automerge this pull request by including @Workiva/release-management-p in a comment.

• Required artifacts are reported in build. See documentation for more details on build artifact requirements.
  • python requires one of: pip.lock, python3_pip_deps.txt, python2_pip_deps.txt

General Information

Ticket(s):

• AF-1917 | remove

Code Review(s): #5

Reviewers: corwinsheahan-wf, evanweible-wf

Additional Information

Watchlist Notifications: None

	When this pull is merged I will add it to the following release:
	Version: over_react_codemod 0.1.0
	Release Ticket(s): None

---

Note: This is a shortened report. Click here to view Rosie's full evaluation.
Last updated on Monday, September 24 03:40 PM CST

[evanweible-wf]

one too many indents here

[evanweible-wf]

This is a lot of extra setup for each of these idempotent tests. Instead, I think we could accomplish this via an optional boolean param on the setupAndMakeComparison function like bool assertIdempotence. If set, then the function can run the script once, capture the result, run it again, capture the second result, and ensure that they're identical.

[evanweible-wf]

Prior to this boilerplate change, factory definitions would always be on one line. But with this change, dartfmt may break the right-hand-side of the factory assignment onto its own line like so:

UiFactory<NotificationManagerPrimitiveProps> NotificationManagerPrimitive =
    $NotificationManagerPrimitive;

We'll need to account for this; currently the script throws when it encounters this.

I think we should add a new method that determines whether or not the current line needs a factory update (which also accounts for lines that have already been transformed). Something like this:

diff --git a/migrater.py b/migrater.py
index c850d6d..4d25e7c 100644
--- a/migrater.py
+++ b/migrater.py
@@ -9,6 +9,7 @@ IGNORE_GENERATED_URI_COMMENT_LINE = '// ignore: uri_does_not_exist\n'
 GENERATED_PART_EXTENSION = '.generated.dart'
 DOLLAR_PROPS_USAGE_REGEX = r'(?:const|new)\s+\$Props\s*\(\s*([$A-Za-z0-9_.]+)\s*\)'
 DOLLAR_PROP_KEYS_USAGE_REGEX = r'(?:const|new)\s+\$PropKeys\s*\(\s*([$A-Za-z0-9_.]+)\s*\)'
+FACTORY_REGEX = r'^UiFactory(<\w+>) (\w+);'
 
 
 def get_class_name(line):
@@ -32,10 +33,10 @@ def get_factory_name(line):
     >>> get_factory_name('UiFactory<DemoProps> Demo;')
     'Demo'
     """
-    match = re.search(r'(\w+);', line)
+    match = re.search(FACTORY_REGEX, line)
     if not match:
         raise Exception('Could not parse factory name from:\n%s' % line)
-    return match.group(1)
+    return match.group(2)
 
 
 def get_part_name(path):
@@ -88,6 +89,10 @@ def has_dollar_prop_keys_usages(lines):
     return re.search(DOLLAR_PROP_KEYS_USAGE_REGEX, s, flags=re.MULTILINE) is not None
 
 
+def needs_factory_update(line):
+    return re.search(FACTORY_REGEX, line) is not None
+
+
 def update_dollar_props_usages(lines):
     s = ''.join(lines)
     match = re.search(DOLLAR_PROPS_USAGE_REGEX, s, flags=re.MULTILINE)
@@ -139,16 +144,12 @@ def factories_suggest(lines, path):
     need_part = False
 
     for line_number, line in enumerate(lines):
-        if line.startswith('UiFactory'):
+        if needs_factory_update(line):
             factory_name = get_factory_name(line)
-
-            if '$' + factory_name in line:
-                continue 
-            
             need_part = True
 
             ignore_line = '// ignore: undefined_identifier\n'
-            new_line = line.replace(';\n', ' = $%s;\n' % (factory_name))
+            new_line = line.replace(';\n', ' = $%s;\n' % factory_name)
 
             patches.append(codemod.Patch(line_number, new_lines=[
                 ignore_line,

[evanweible-wf]

QA +1

• CI passes

@Workiva/release-management-p

[rmconsole-wf]

@evanweible-wf I will not merge this because:

• 'Hold Auto-Merge' label is applied

[rmconsole-wf]

+1 from RM