T201 autofix can break code that actually should print things

[akx]

I'm applying (more!) Ruff to https://github.com/python-babel/babel/ and figured I'd try --select=ALL --fix and look through the diff, and found a surprising combination of breaking autofixes – maybe the autofix for T201 ("print found") should also be made a suggestion (as there's been discussion for some other rules in other issues here), not a default?

def _help(self):
    longest = max(len(command) for command in self.commands)
    format = "  %%-%ds %%s" % max(8, longest + 1)
    commands = sorted(self.commands.items())
    for name, description in commands:
        print(format % (name, description))

cargo run -- --diff --select=ALL --no-cache:

--- tests/bab.py
+++ tests/bab.py
@@ -1,6 +1,6 @@
 def _help(self):
     longest = max(len(command) for command in self.commands)
-    format = "  %%-%ds %%s" % max(8, longest + 1)
+    "  %%-%ds %%s" % max(8, longest + 1)
     commands = sorted(self.commands.items())
-    for name, description in commands:
-        print(format % (name, description))
+    for _name, _description in commands:
+        pass

(and then the interpolation operation becomes a no-op and when longest is no longer used, that gets removed too...)

[charliermarsh]

Is the generated code actually invalid? Or it just doesn't do what was intended? Like does it run? (Just confirming.)

[akx]

Oh, yeah, sorry, should've been more clear there!

It does run but it no longer does anything of value since all it was meant to do was print things and the autofix got rid of that. This is really of course only an issue if T201 is enabled and not marked unfixable :)

[charliermarsh]

No totally! Just making sure we weren't producing invalid syntax.

I'm more and more feeling like we should disable a bunch of autofixes by default and require users to opt-in to them. This is a useful rule but it's probably surprising that it autofixes itself.

[spaceone]

I also wondered what T201 is about. I don't understand its purpose. This maybe makes sense for a library. I aksed that upstream in https://github.com/JBKahn/flake8-print/issues/59.

[akx]

@spaceone It makes a lot of sense for libraries or e.g. webapps where you don't want your views or models or whatnot to randomly print things out (e.g. where the policy would be to use logging).

[charliermarsh]

This was removed.