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

Adapted fix to work identical to format #10999

Merged
merged 5 commits into from
Jun 8, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pycodestyle/E20.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,21 @@
x, y = y, x
a[b1, :] == a[b1, ...]
b = a[:, b1]

#: E203 linebreak before ]
predictions = predictions[
len(past_covariates) // datamodule.hparams["downsample"] :
]

#: E203 multi whitespace before :
predictions = predictions[
len(past_covariates) // datamodule.hparams["downsample"] :
]

#: E203 tab before :
predictions = predictions[
len(past_covariates) // datamodule.hparams["downsample"] :
]
Comment on lines +89 to +92
Copy link
Member

Choose a reason for hiding this comment

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

Can we add two more examples where the line end:

  • in a comment
  • in a line continuation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, those would both be legal continuations. Didn't think about that, but will try to add those in the coming days

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added the comment case, but can't figure out how to do the line continuation, without basically rewriting the rule since the \ doesn't show up in the token stream. I could try to adapt E502 for it, if you consider the case important.

On the bright side, both ruff format and black remove a \ in the example situation anyway so there would be no continuing conflict.

Copy link
Member

Choose a reason for hiding this comment

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

You could try to use SimpleTokenizer when you found a potential violation to lex the text coming right after the token and see if the next token is a line continuation.

#:

#: E201:1:6
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,29 @@ pub(crate) fn extraneous_whitespace(line: &LogicalLine, context: &mut LogicalLin
)));
context.push_diagnostic(diagnostic);
}
} else if iter.peek().is_some_and(|token| {
matches!(token.kind(), TokenKind::NonLogicalNewline)
}) {
// Allow [
// long_expression_calculating_the_index() :
// ]
MichaReiser marked this conversation as resolved.
Show resolved Hide resolved
// But not [
// long_expression_calculating_the_index() :
// ]
// distinct from the above case, because ruff format produces a
// whitespace before the colon and so should the fix
if let (Whitespace::Many | Whitespace::Tab, offset) = whitespace
{
let mut diagnostic = Diagnostic::new(
WhitespaceBeforePunctuation { symbol },
TextRange::at(token.start() - offset, offset),
);
diagnostic.set_fix(Fix::safe_edits(
Edit::range_deletion(diagnostic.range()),
[Edit::insertion(" ".into(), token.start() - offset)],
));
context.push_diagnostic(diagnostic);
}
} else {
// Allow, e.g., `foo[1:2]` or `foo[1 : 2]` or `foo[1 :: 2]`.
let token = iter
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,64 +124,62 @@ E20.py:12:15: E201 [*] Whitespace after '{'
14 14 | spam(ham[1], {eggs: 2})
15 15 | #:

E20.py:81:6: E201 [*] Whitespace after '['
E20.py:96:6: E201 [*] Whitespace after '['
|
80 | #: E201:1:6
81 | spam[ ~ham]
95 | #: E201:1:6
96 | spam[ ~ham]
| ^ E201
82 |
83 | #: Okay
97 |
98 | #: Okay
|
= help: Remove whitespace before '['

ℹ Safe fix
78 78 | #:
79 79 |
80 80 | #: E201:1:6
81 |-spam[ ~ham]
81 |+spam[~ham]
82 82 |
83 83 | #: Okay
84 84 | x = [ #

E20.py:90:5: E201 [*] Whitespace after '['
|
88 | # F-strings
89 | f"{ {'a': 1} }"
90 | f"{[ { {'a': 1} } ]}"
| ^ E201
91 | f"normal { {f"{ { [1, 2] } }" } } normal"
|
= help: Remove whitespace before '['
93 93 | #:
94 94 |
95 95 | #: E201:1:6
96 |-spam[ ~ham]
96 |+spam[~ham]
97 97 |
98 98 | #: Okay
99 99 | x = [ #

E20.py:105:5: E201 [*] Whitespace after '['
|
103 | # F-strings
104 | f"{ {'a': 1} }"
105 | f"{[ { {'a': 1} } ]}"
| ^ E201
106 | f"normal { {f"{ { [1, 2] } }" } } normal"
|
= help: Remove whitespace before '['

ℹ Safe fix
87 87 |
88 88 | # F-strings
89 89 | f"{ {'a': 1} }"
90 |-f"{[ { {'a': 1} } ]}"
90 |+f"{[{ {'a': 1} } ]}"
91 91 | f"normal { {f"{ { [1, 2] } }" } } normal"
92 92 |
93 93 | #: Okay

E20.py:119:5: E201 [*] Whitespace after '['
102 102 |
103 103 | # F-strings
104 104 | f"{ {'a': 1} }"
105 |-f"{[ { {'a': 1} } ]}"
105 |+f"{[{ {'a': 1} } ]}"
106 106 | f"normal { {f"{ { [1, 2] } }" } } normal"
107 107 |
108 108 | #: Okay

E20.py:134:5: E201 [*] Whitespace after '['
|
118 | #: E201:1:5
119 | ham[ : upper]
133 | #: E201:1:5
134 | ham[ : upper]
| ^ E201
120 |
121 | #: Okay
135 |
136 | #: Okay
|
= help: Remove whitespace before '['

ℹ Safe fix
116 116 | ham[lower + offset : upper + offset]
117 117 |
118 118 | #: E201:1:5
119 |-ham[ : upper]
119 |+ham[: upper]
120 120 |
121 121 | #: Okay
122 122 | ham[lower + offset :: upper + offset]


131 131 | ham[lower + offset : upper + offset]
132 132 |
133 133 | #: E201:1:5
134 |-ham[ : upper]
134 |+ham[: upper]
135 135 |
136 136 | #: Okay
137 137 | ham[lower + offset :: upper + offset]
Original file line number Diff line number Diff line change
Expand Up @@ -126,44 +126,42 @@ E20.py:29:11: E202 [*] Whitespace before ']'
31 31 | spam(ham[1], {eggs: 2})
32 32 |

E20.py:90:18: E202 [*] Whitespace before ']'
|
88 | # F-strings
89 | f"{ {'a': 1} }"
90 | f"{[ { {'a': 1} } ]}"
| ^ E202
91 | f"normal { {f"{ { [1, 2] } }" } } normal"
|
= help: Remove whitespace before ']'
E20.py:105:18: E202 [*] Whitespace before ']'
|
103 | # F-strings
104 | f"{ {'a': 1} }"
105 | f"{[ { {'a': 1} } ]}"
| ^ E202
106 | f"normal { {f"{ { [1, 2] } }" } } normal"
|
= help: Remove whitespace before ']'

ℹ Safe fix
87 87 |
88 88 | # F-strings
89 89 | f"{ {'a': 1} }"
90 |-f"{[ { {'a': 1} } ]}"
90 |+f"{[ { {'a': 1} }]}"
91 91 | f"normal { {f"{ { [1, 2] } }" } } normal"
92 92 |
93 93 | #: Okay

E20.py:146:12: E202 [*] Whitespace before ']'
102 102 |
103 103 | # F-strings
104 104 | f"{ {'a': 1} }"
105 |-f"{[ { {'a': 1} } ]}"
105 |+f"{[ { {'a': 1} }]}"
106 106 | f"normal { {f"{ { [1, 2] } }" } } normal"
107 107 |
108 108 | #: Okay

E20.py:161:12: E202 [*] Whitespace before ']'
|
145 | #: E202:1:12
146 | ham[upper : ]
160 | #: E202:1:12
161 | ham[upper : ]
| ^ E202
147 |
148 | #: E203:1:10
162 |
163 | #: E203:1:10
|
= help: Remove whitespace before ']'

ℹ Safe fix
143 143 | ham[upper :]
144 144 |
145 145 | #: E202:1:12
146 |-ham[upper : ]
146 |+ham[upper :]
147 147 |
148 148 | #: E203:1:10
149 149 | ham[upper :]


158 158 | ham[upper :]
159 159 |
160 160 | #: E202:1:12
161 |-ham[upper : ]
161 |+ham[upper :]
162 162 |
163 163 | #: E203:1:10
164 164 | ham[upper :]
Loading
Loading