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

Implement flynt's "join and concat to f-string" transforms #2102

Open
1 of 2 tasks
akx opened this issue Jan 23, 2023 · 2 comments
Open
1 of 2 tasks

Implement flynt's "join and concat to f-string" transforms #2102

akx opened this issue Jan 23, 2023 · 2 comments
Labels
rule Implementing or modifying a lint rule

Comments

@akx
Copy link
Contributor

akx commented Jan 23, 2023

flynt is a specialized linter for f-string usage.

UP031 and UP032 implement flynt's core features, but the two extra transforms

  -tc, --transform-concats
                        Replace string concatenations (defined as +
                        operations involving string literals) with
                        f-strings. Available only if flynt is
                        installed with 3.8+ interpreter.
  -tj, --transform-joins
                        Replace static joins (where the joiner is a
                        string literal and the joinee is a static-
                        length list) with f-strings. Available only
                        if flynt is installed with 3.8+ interpreter.

i.e.

 a = "Hello"
-msg = a + " World"
+msg = f"{a} World"
-msg2 = "Finally, " + a + " World"
+msg2 = "Finally, {a} World"

and

 a = "Hello"
-msg1 = " ".join([a, " World"])
-msg2 = "".join(["Finally, ", a, " World"])
-msg3 = "x".join(("1", "2", "3"))
-msg4 = "x".join({"4", "5", "yee"})
-msg5 = "y".join([1, 2, 3])  # Should be transformed
+msg1 = f"{a}  World"
+msg2 = f"Finally, {a} World"
+msg3 = "1x2x3"
+msg4 = "4x5xyee"
+msg5 = f"{1}y{2}y{3}"  # Should be transformed
 msg6 = a.join(["1", "2", "3"])  # Should not be transformed (not a static joiner)
 msg7 = "a".join(a)  # Should not be transformed (not a static joinee)
 msg8 = "a".join([a, a, *a])  # Should not be transformed (not a static length)

respectively could be implemented in Ruff too. (I'd like to work on them! 😄) Should these be FLY series, or should they be RUF?

Refs #2097 (relating to f-strings)

@charliermarsh
Copy link
Member

Let's put these in FLY (but we should do a quick check to verify that FLY isn't used by any other popular plugins). Then, we can mark UP031 and UP032 as aliases of FLY rules once we support rule aliasing, so all the string upgrade stuff is in one place (or vice versa -- add FLY rules that alias the UP rules).

Separately, have you been using UP031 and UP032 much? Any issues in practice? Any notable false negatives? There are a few cases we don't yet fix (e.g., if you put the % in a percent-formatted string on its own line, we abort to avoid wonky formatting).

Separately, do you have an opinion on whether UP031 and UP032 should always flag % and .format calls? Or only flag the ones it can fix? Right now, it does the latter. But doing the former would effectively implement #2097.

@charliermarsh charliermarsh added the rule Implementing or modifying a lint rule label Jan 23, 2023
@akx
Copy link
Contributor Author

akx commented Jan 25, 2023

I have UP031 and UP032 enabled by way of select = "UP" and haven't seen any issues yet! 👍

Maybe (and this is tangential to the #2142 RUF005 discussion) UP03[12] should have a "suggestion" flag for % and .format they can't auto-fix, like "Consider using an f-string".
That said, I could see it being a nuisance if we had to noqa places like "foo {bar} {baz}".format(**something_dynamic) (though that's better served by format_map anyway), or "foo %(bar)s %(baz)s" % something_dynamic)...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

No branches or pull requests

2 participants