Run pyupgrade until convergance (pantsbuild#18128)

Fixes pantsbuild#18090

Test fails before change, passes after

src/python/pants/backend/python/lint/pyupgrade/rules.py

@@ -2,6 +2,7 @@
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 from __future__ import annotations
 
+import logging
 from dataclasses import dataclass
 
 from pants.backend.python.lint.pyupgrade.skip_field import SkipPyUpgradeField
@@ -15,7 +16,9 @@
 from pants.engine.rules import Get, collect_rules, rule
 from pants.engine.target import FieldSet, Target
 from pants.util.logging import LogLevel
-from pants.util.strutil import pluralize
+from pants.util.strutil import pluralize, softwrap
+
+logger = logging.getLogger(__name__)
 
 
 @dataclass(frozen=True)
@@ -39,17 +42,39 @@ class PyUpgradeRequest(FixTargetsRequest):
 async def pyupgrade_fix(request: PyUpgradeRequest.Batch, pyupgrade: PyUpgrade) -> FixResult:
     pyupgrade_pex = await Get(VenvPex, PexRequest, pyupgrade.to_pex_request())
 
-    result = await Get(
-        FallibleProcessResult,
-        VenvPexProcess(
-            pyupgrade_pex,
-            argv=(*pyupgrade.args, *request.files),
-            input_digest=request.snapshot.digest,
-            output_files=request.files,
-            description=f"Run pyupgrade on {pluralize(len(request.files), 'file')}.",
-            level=LogLevel.DEBUG,
-        ),
-    )
+    # NB: Pyupgrade isn't idempotent, but eventually converges. So keep running until it stops
+    # changing code. See https://github.com/asottile/pyupgrade/issues/703
+    # (Technically we could not do this. It doesn't break Pants since the next run on the CLI would
+    # use the new file with the new digest. However that isn't the UX we want for our users.)
+    input_digest = request.snapshot.digest
+    for _ in range(10):  # Give the loop an upper bound to guard against infinite runs
+        result = await Get(
+            FallibleProcessResult,
+            VenvPexProcess(
+                pyupgrade_pex,
+                argv=(*pyupgrade.args, *request.files),
+                input_digest=input_digest,
+                output_files=request.files,
+                description=f"Run pyupgrade on {pluralize(len(request.files), 'file')}.",
+                level=LogLevel.DEBUG,
+            ),
+        )
+        if input_digest == result.output_digest:
+            # Nothing changed, either due to failure or because it is fixed
+            break
+        input_digest = result.output_digest
+    else:
+        logger.error(
+            softwrap(
+                """
+                Pants ran Pyupgrade continuously on the code 10 times and it changed all 10.
+
+                Pyupgrade is not idempotent, but should eventually converge. This is either a bug in
+                Pyupgrade, or Pyupgrade is still trying to converge on fixed code.
+                """
+            )
+        )
+
     return await FixResult.create(request, result)
 
 

src/python/pants/backend/python/lint/pyupgrade/rules_integration_test.py

@@ -107,6 +107,18 @@ def test_passing(rule_runner: RuleRunner, major_minor_interpreter: str) -> None:
     assert fix_result.did_change is False
 
 
+def test_convergance(rule_runner: RuleRunner) -> None:
+    # NB: Testing the fact that we re-run pyupgrade until it converges
+    percent_s_string_formatting = '"%s %s" % (foo, bar)\n'
+    rule_runner.write_files(
+        {"f.py": percent_s_string_formatting, "BUILD": "python_sources(name='t')"}
+    )
+    tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py"))
+    fix_result = run_pyupgrade(rule_runner, [tgt], extra_args=["--pyupgrade-args=--py36-plus"])
+    assert fix_result.output == get_snapshot(rule_runner, {"f.py": 'f"{foo} {bar}"\n'})
+    assert fix_result.did_change is True
+
+
 def test_failing(rule_runner: RuleRunner) -> None:
     rule_runner.write_files({"f.py": PY_36_BAD_FILE, "BUILD": "python_sources(name='t')"})
     tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py"))