Skip to content

Comments

Disallow single-line implicit concatenated strings#13928

Merged
MichaReiser merged 1 commit intomainfrom
micha/enforce-multiline-implicit-concatenated-strings
Nov 3, 2024
Merged

Disallow single-line implicit concatenated strings#13928
MichaReiser merged 1 commit intomainfrom
micha/enforce-multiline-implicit-concatenated-strings

Conversation

@MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Oct 25, 2024 •
edited
Loading

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
@github-actions
Copy link
Contributor

github-actions bot commented Oct 25, 2024 •
edited
Loading

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
charliermarsh
charliermarsh approved these changes Nov 2, 2024
View reviewed changes
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

MichaReiser commented Nov 2, 2024

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
@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
Merged
8 tasks
@BrewTestBot BrewTestBot mentioned this pull request Nov 8, 2024
Merged
@MichaReiser MichaReiser mentioned this pull request Nov 27, 2024
Open
MichaReiser added a commit that referenced this pull request Dec 23, 2024
@MichaReiser
Preserve multiline implicit concatenated strings in docstring positions
d9f0ce8 
## Summary
This is a bug fix for #13928

We failed to preserve multiline implicit concatenated
strings that can't be automatically joined in docstring positions:

```py
def test():
  (
     r"This is a docstring"
     "and Ruff can't automatically join it"
  )
```

## Test plan
Added formatter tests
@MichaReiser MichaReiser mentioned this pull request Dec 23, 2024
Merged
MichaReiser added a commit that referenced this pull request Dec 23, 2024
@MichaReiser
Preserve multiline implicit concatenated strings in docstring positions
edc3bb4 
## Summary
This is a bug fix for #13928

We failed to preserve multiline implicit concatenated
strings that can't be automatically joined in docstring positions:

```py
def test():
  (
     r"This is a docstring"
     "and Ruff can't automatically join it"
  )
```

## Test plan
Added formatter tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@charliermarsh charliermarsh charliermarsh approved these changes

@AlexWaygood AlexWaygood Awaiting requested review from AlexWaygood

Assignees

No one assigned

Labels

formatter Related to the formatter preview Related to preview mode features style How should formatted code look

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Consider removing ISC001 from the conflict list with ruff format

2 participants

@MichaReiser @charliermarsh