Skip to content

Commit

Permalink
RUF022, RUF023: Ensure closing parentheses for multiline sequences ar…
Browse files Browse the repository at this point in the history 
…e always on their own line (#9793)

## Summary

Currently these rules apply the heuristic that if the original sequence
doesn't have a newline in between the final sequence item and the
closing parenthesis, the autofix won't add one for you. The feedback
from @ThiefMaster, however, was that this was producing slightly unusual
formatting -- things like this:

```py
__all__ = [
    "b", "c",
    "a", "d"]
```

were being autofixed to this:

```py
__all__ = [
    "a",
    "b",
    "c",
    "d"]
```

When, if it was _going_ to be exploded anyway, they'd prefer something
like this (with the closing parenthesis on its own line, and a trailing comma added):

```py
__all__ = [
    "a",
    "b",
    "c",
    "d",
]
```

I'm still pretty skeptical that we'll be able to please everybody here
with the formatting choices we make; _but_, on the other hand, this
_specific_ change is pretty easy to make.

## Test Plan

`cargo test`. I also ran the autofixes for RUF022 and RUF023 on CPython
to check how they looked; they looked fine to me.
  • Loading branch information
@AlexWaygood
AlexWaygood authored Feb 9, 2024
1 parent 6f0e4ad commit d387d0b
Show file tree
Hide file tree
Showing 5 changed files with 150 additions and 24 deletions.
17 changes: 17 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF022.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,23 @@
,
)

__all__ = ( # comment about the opening paren
# multiline strange comment 0a
# multiline strange comment 0b
"foo" # inline comment about foo
# multiline strange comment 1a
# multiline strange comment 1b
, # comment about the comma??
# comment about bar part a
# comment about bar part b
"bar" # inline comment about bar
# strange multiline comment comment 2a
# strange multiline comment 2b
,
# strange multiline comment 3a
# strange multiline comment 3b
) # comment about the closing paren

###################################
# These should all not get flagged:
###################################
Expand Down
4 changes: 4 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF023.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,10 @@ class BezierBuilder4:
,
)

__slots__ = {"foo", "bar",
"baz", "bingo"
}

###################################
# These should all not get flagged:
###################################
Expand Down
27 changes: 22 additions & 5 deletions crates/ruff_linter/src/rules/ruff/rules/sequence_sorting.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -895,6 +895,27 @@ fn multiline_string_sequence_postlude<'a>(
};
let postlude = locator.slice(TextRange::new(postlude_start, dunder_all_range_end));

// If the postlude consists solely of a closing parenthesis
// (not preceded by any whitespace/newlines),
// plus possibly a single trailing comma prior to the parenthesis,
// fixup the postlude so that the parenthesis appears on its own line,
// and so that the final item has a trailing comma.
// This produces formatting more similar
// to that which the formatter would produce.
if postlude.len() <= 2 {
let mut reversed_postlude_chars = postlude.chars().rev();
if let Some(closing_paren @ (')' | '}' | ']')) = reversed_postlude_chars.next() {
if reversed_postlude_chars.next().map_or(true, |c| c == ',') {
return Cow::Owned(format!(",{newline}{leading_indent}{closing_paren}"));
}
}
}

let newline_chars = ['\r', '\n'];
if !postlude.starts_with(newline_chars) {
return Cow::Borrowed(postlude);
}

// The rest of this function uses heuristics to
// avoid very long indents for the closing paren
// that don't match the style for the rest of the
Expand All @@ -920,18 +941,14 @@ fn multiline_string_sequence_postlude<'a>(
// "y",
// ]
// ```
let newline_chars = ['\r', '\n'];
if !postlude.starts_with(newline_chars) {
return Cow::Borrowed(postlude);
}
if TextSize::of(leading_indentation(
postlude.trim_start_matches(newline_chars),
)) <= TextSize::of(item_indent)
{
return Cow::Borrowed(postlude);
}
let trimmed_postlude = postlude.trim_start();
if trimmed_postlude.starts_with([']', ')']) {
if trimmed_postlude.starts_with([']', ')', '}']) {
return Cow::Owned(format!("{newline}{leading_indent}{trimmed_postlude}"));
}
Cow::Borrowed(postlude)
Expand Down
84 changes: 70 additions & 14 deletions ...ff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF022_RUF022.py.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -386,18 +386,24 @@ RUF022.py:54:11: RUF022 [*] `__all__` is not sorted
76 70 | "SUNDAY",
77 71 | "THURSDAY",
78 72 | "TUESDAY",
73 |+ "WEDNESDAY",
79 |- "TextCalendar",
80 73 | "WEDNESDAY",
74 |+ "Calendar",
75 |+ "Day",
76 |+ "HTMLCalendar",
77 |+ "IllegalMonthError",
78 |+ "LocaleHTMLCalendar",
79 |+ "Month",
79 80 | "TextCalendar",
80 |- "WEDNESDAY",
80 |+ "TextCalendar",
81 81 | "calendar",
82 82 | "timegm",
83 83 | "weekday",
84 |- "weekheader"]
84 |+ "weekheader",
85 |+]
85 86 |
86 87 | ##########################################
87 88 | # Messier multiline __all__ definitions...

RUF022.py:91:11: RUF022 [*] `__all__` is not sorted
|
Expand Down Expand Up @@ -559,10 +565,11 @@ RUF022.py:110:11: RUF022 [*] `__all__` is not sorted
151 |+ "register_error",
152 |+ "replace_errors",
153 |+ "strict_errors",
154 |+ "xmlcharrefreplace_errors"]
124 155 |
125 156 | __all__: tuple[str, ...] = ( # a comment about the opening paren
126 157 | # multiline comment about "bbb" part 1
154 |+ "xmlcharrefreplace_errors",
155 |+]
124 156 |
125 157 | __all__: tuple[str, ...] = ( # a comment about the opening paren
126 158 | # multiline comment about "bbb" part 1

RUF022.py:125:28: RUF022 [*] `__all__` is not sorted
|
Expand Down Expand Up @@ -918,13 +925,13 @@ RUF022.py:225:11: RUF022 [*] `__all__` is not sorted
223 223 | ############################################################
224 224 |
225 225 | __all__ = (
226 |- "loads",
227 |- "dumps",)
226 |+ "dumps",
227 |+ "loads",)
228 228 |
229 229 | __all__ = [
230 230 | "loads",
226 227 | "loads",
227 |- "dumps",)
228 |+)
228 229 |
229 230 | __all__ = [
230 231 | "loads",

RUF022.py:229:11: RUF022 [*] `__all__` is not sorted
|
Expand Down Expand Up @@ -1002,7 +1009,7 @@ RUF022.py:243:11: RUF022 [*] `__all__` is not sorted
251 | | )
| |_^ RUF022
252 |
253 | ###################################
253 | __all__ = ( # comment about the opening paren
|
= help: Apply an isort-style sorting to `__all__`

Expand All @@ -1021,4 +1028,53 @@ RUF022.py:243:11: RUF022 [*] `__all__` is not sorted
250 249 | ,
251 250 | )

RUF022.py:253:11: RUF022 [*] `__all__` is not sorted
|
251 | )
252 |
253 | __all__ = ( # comment about the opening paren
| ___________^
254 | | # multiline strange comment 0a
255 | | # multiline strange comment 0b
256 | | "foo" # inline comment about foo
257 | | # multiline strange comment 1a
258 | | # multiline strange comment 1b
259 | | , # comment about the comma??
260 | | # comment about bar part a
261 | | # comment about bar part b
262 | | "bar" # inline comment about bar
263 | | # strange multiline comment comment 2a
264 | | # strange multiline comment 2b
265 | | ,
266 | | # strange multiline comment 3a
267 | | # strange multiline comment 3b
268 | | ) # comment about the closing paren
| |_^ RUF022
269 |
270 | ###################################
|
= help: Apply an isort-style sorting to `__all__`

ℹ Safe fix
251 251 | )
252 252 |
253 253 | __all__ = ( # comment about the opening paren
254 |- # multiline strange comment 0a
255 |- # multiline strange comment 0b
256 |- "foo" # inline comment about foo
257 254 | # multiline strange comment 1a
258 255 | # multiline strange comment 1b
259 |- , # comment about the comma??
256 |+ # comment about the comma??
260 257 | # comment about bar part a
261 258 | # comment about bar part b
262 |- "bar" # inline comment about bar
259 |+ "bar", # inline comment about bar
260 |+ # multiline strange comment 0a
261 |+ # multiline strange comment 0b
262 |+ "foo" # inline comment about foo
263 263 | # strange multiline comment comment 2a
264 264 | # strange multiline comment 2b
265 265 | ,


42 changes: 37 additions & 5 deletions ...ff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF023_RUF023.py.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -564,10 +564,11 @@ RUF023.py:162:17: RUF023 [*] `BezierBuilder.__slots__` is not sorted
162 |+ __slots__ = (
163 |+ 'canvas',
164 |+ 'xp',
165 |+ 'yp',)
164 166 |
165 167 | class BezierBuilder2:
166 168 | __slots__ = {'xp', 'yp',
165 |+ 'yp',
166 |+ )
164 167 |
165 168 | class BezierBuilder2:
166 169 | __slots__ = {'xp', 'yp',

RUF023.py:166:17: RUF023 [*] `BezierBuilder2.__slots__` is not sorted
|
Expand Down Expand Up @@ -643,7 +644,7 @@ RUF023.py:181:17: RUF023 [*] `BezierBuilder4.__slots__` is not sorted
189 | | )
| |_____^ RUF023
190 |
191 | ###################################
191 | __slots__ = {"foo", "bar",
|
= help: Apply a natural sort to `BezierBuilder4.__slots__`

Expand All @@ -662,4 +663,35 @@ RUF023.py:181:17: RUF023 [*] `BezierBuilder4.__slots__` is not sorted
188 187 | ,
189 188 | )

RUF023.py:191:17: RUF023 [*] `BezierBuilder4.__slots__` is not sorted
|
189 | )
190 |
191 | __slots__ = {"foo", "bar",
| _________________^
192 | | "baz", "bingo"
193 | | }
| |__________________^ RUF023
194 |
195 | ###################################
|
= help: Apply a natural sort to `BezierBuilder4.__slots__`

ℹ Safe fix
188 188 | ,
189 189 | )
190 190 |
191 |- __slots__ = {"foo", "bar",
192 |- "baz", "bingo"
193 |- }
191 |+ __slots__ = {
192 |+ "bar",
193 |+ "baz",
194 |+ "bingo",
195 |+ "foo"
196 |+ }
194 197 |
195 198 | ###################################
196 199 | # These should all not get flagged:


0 comments on commit d387d0b

Please sign in to comment.