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

Disallow single-line implicit concatenated strings #13928

Merged

Conversation

MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Oct 25, 2024

Summary

Fixes #8272

The formatter is incompatible with ISC001 because it can introduce new single-line implicit concatenated strings if all parts fit on the line. The new implicit concatenated string formatting style already addresses this incompatibility for regular strings and most f-string but not for triple quoted strings and raw strings.

This PR proposes to change the formatter style to never collapse the parts of an implicit concatenated string onto a single line unless it has been formatted on a single line by the author.

(
	r"aaaaaaa" "bbbbbbbbbbbb"
)

Is preserved as is because the author wrote the implicit concatenated string on a single line and it fits. The source i already incompatible with ISC001.

(
	r"aaaaaaa"
	"bbbbbbbbbbbb"
)

The current style collapses this string to match the example above. The style proposed in this PR preserves the multiline formatting in this case because there's a line break between the two parts.

Preview gating

It's technically not required to gate this change behind preview because it doesn't change the formatting of any existing code. But it fits nicely into the other implicit concatenated work that we've been doing and the new implicit concatenated string formatting also largely mitigates that this leads to slightly less consistent formatting (because it's up to the author whether the string remains multiline or not)

Test Plan

Added tests

@MichaReiser MichaReiser added formatter Related to the formatter style How should formatted code look labels Oct 25, 2024
Copy link
Contributor

github-actions bot commented Oct 25, 2024

ruff-ecosystem results

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

ℹ️ ecosystem check detected format changes. (+18 -6 lines in 4 files in 3 projects; 51 projects unchanged)

bokeh/bokeh (+11 -3 lines across 2 files)

ruff format --preview

examples/styling/mathtext/latex_bessel.py~L21

     width=700,
     height=500,
     title=(
-        r"Bessel functions of the first kind: $$J_\alpha(x) = \sum_{m=0}^{\infty}" r"\frac{(-1)^m}{m!\:\Gamma(m+\alpha+1)} \left(\frac{x}{2}\right)^{2m+\alpha}$$"
+        r"Bessel functions of the first kind: $$J_\alpha(x) = \sum_{m=0}^{\infty}"
+        r"\frac{(-1)^m}{m!\:\Gamma(m+\alpha+1)} \left(\frac{x}{2}\right)^{2m+\alpha}$$"
     ),
 )
 p.x_range.range_padding = 0

src/bokeh/core/property/color.py~L126

             Regex(r"^#[0-9a-fA-F]{4}$"),
             Regex(r"^#[0-9a-fA-F]{6}$"),
             Regex(r"^#[0-9a-fA-F]{8}$"),
-            Regex(r"^rgba\(((25[0-5]|2[0-4]\d|1\d{1,2}|\d\d?)\s*," r"\s*?){2}(25[0-5]|2[0-4]\d|1\d{1,2}|\d\d?)\s*," r"\s*([01]\.?\d*?)\)"),
-            Regex(r"^rgb\(((25[0-5]|2[0-4]\d|1\d{1,2}|\d\d?)\s*," r"\s*?){2}(25[0-5]|2[0-4]\d|1\d{1,2}|\d\d?)\s*?\)"),
+            Regex(
+                r"^rgba\(((25[0-5]|2[0-4]\d|1\d{1,2}|\d\d?)\s*,"
+                r"\s*?){2}(25[0-5]|2[0-4]\d|1\d{1,2}|\d\d?)\s*,"
+                r"\s*([01]\.?\d*?)\)"
+            ),
+            Regex(
+                r"^rgb\(((25[0-5]|2[0-4]\d|1\d{1,2}|\d\d?)\s*,"
+                r"\s*?){2}(25[0-5]|2[0-4]\d|1\d{1,2}|\d\d?)\s*?\)"
+            ),
             Tuple(Byte, Byte, Byte),
             Tuple(Byte, Byte, Byte, Percent),
             RGB,

pandas-dev/pandas (+4 -2 lines across 1 file)

ruff format --preview

pandas/tests/tools/test_to_datetime.py~L1346

         assert res is NaT
 
         msg = "|".join([
-            r'^time data "a" doesn\'t match format "%H:%M:%S". ' f"{PARSING_ERR_MSG}$",
+            r'^time data "a" doesn\'t match format "%H:%M:%S". '
+            f"{PARSING_ERR_MSG}$",
             r'^Given date string "a" not likely a datetime$',
             r'^unconverted data remains when parsing with format "%H:%M:%S": "9". '
             f"{PARSING_ERR_MSG}$",

pandas/tests/tools/test_to_datetime.py~L1396

 
         msg = "|".join([
             r'^Given date string "a" not likely a datetime$',
-            r'^time data "a" doesn\'t match format "%H:%M:%S". ' f"{PARSING_ERR_MSG}$",
+            r'^time data "a" doesn\'t match format "%H:%M:%S". '
+            f"{PARSING_ERR_MSG}$",
             r'^unconverted data remains when parsing with format "%H:%M:%S": "9". '
             f"{PARSING_ERR_MSG}$",
             r"^second must be in 0..59: 00:01:99$",

rotki/rotki (+3 -1 lines across 1 file)

ruff format --preview

rotkehlchen/exchanges/coinbase.py~L68

 LEGACY_RE: re.Pattern = re.compile(r"^[\w]+$")
 NEW_RE: re.Pattern = re.compile(r"^organizations/[\w-]+/apiKeys/[\w-]+$")
 PRIVATE_KEY_RE: re.Pattern = re.compile(
-    r"^-----BEGIN EC PRIVATE KEY-----\n" r"[\w+/=\n]+" r"-----END EC PRIVATE KEY-----\n?$",
+    r"^-----BEGIN EC PRIVATE KEY-----\n"
+    r"[\w+/=\n]+"
+    r"-----END EC PRIVATE KEY-----\n?$",
     re.MULTILINE,
 )
 

@MichaReiser MichaReiser force-pushed the micha/enforce-multiline-implicit-concatenated-strings branch 2 times, most recently from d9bec17 to ba5d612 Compare November 1, 2024 16:43
@MichaReiser MichaReiser added the preview Related to preview mode features label Nov 1, 2024
@MichaReiser MichaReiser force-pushed the micha/enforce-multiline-implicit-concatenated-strings branch from ba5d612 to 08de539 Compare November 1, 2024 16:52
@MichaReiser MichaReiser marked this pull request as ready for review November 1, 2024 17:05
Copy link
Member

@charliermarsh charliermarsh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a smart change. We should document it in the Black deviations though.

@MichaReiser
Copy link
Member Author

I think this is a smart change. We should document it in the Black deviations though.

Thanks for the feedback. I plan to document the deviations as part of promoting the new style guide.

@MichaReiser MichaReiser merged commit 443fd3b into main Nov 3, 2024
20 checks passed
@MichaReiser MichaReiser deleted the micha/enforce-multiline-implicit-concatenated-strings branch November 3, 2024 11:49
@MichaReiser MichaReiser mentioned this pull request Nov 3, 2024
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
formatter Related to the formatter preview Related to preview mode features style How should formatted code look
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider removing ISC001 from the conflict list with ruff format
2 participants