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 all 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
26 changes: 26 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,32 @@
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.


#: E203 single whitespace before : with line a comment
predictions = predictions[
len(past_covariates) // datamodule.hparams["downsample"] : # Just some comment
]

#: E203 multi whitespace before : with line a comment
predictions = predictions[
len(past_covariates) // datamodule.hparams["downsample"] : # Just some comment
]

#:

#: E201:1:6
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,32 @@ 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 | TokenKind::Comment
)
}) {
// 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 '['
|
80 | #: E201:1:6
81 | spam[ ~ham]
| ^ E201
82 |
83 | #: Okay
|
= help: Remove whitespace before '['
E20.py:107:6: E201 [*] Whitespace after '['
|
106 | #: E201:1:6
107 | spam[ ~ham]
| ^ E201
108 |
109 | #: 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 '['
104 104 | #:
105 105 |
106 106 | #: E201:1:6
107 |-spam[ ~ham]
107 |+spam[~ham]
108 108 |
109 109 | #: Okay
110 110 | x = [ #

E20.py:116:5: E201 [*] Whitespace after '['
|
114 | # F-strings
115 | f"{ {'a': 1} }"
116 | f"{[ { {'a': 1} } ]}"
| ^ E201
117 | 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 '['
113 113 |
114 114 | # F-strings
115 115 | f"{ {'a': 1} }"
116 |-f"{[ { {'a': 1} } ]}"
116 |+f"{[{ {'a': 1} } ]}"
117 117 | f"normal { {f"{ { [1, 2] } }" } } normal"
118 118 |
119 119 | #: Okay

E20.py:145:5: E201 [*] Whitespace after '['
|
118 | #: E201:1:5
119 | ham[ : upper]
144 | #: E201:1:5
145 | ham[ : upper]
| ^ E201
120 |
121 | #: Okay
146 |
147 | #: 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]


142 142 | ham[lower + offset : upper + offset]
143 143 |
144 144 | #: E201:1:5
145 |-ham[ : upper]
145 |+ham[: upper]
146 146 |
147 147 | #: Okay
148 148 | 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:116:18: E202 [*] Whitespace before ']'
|
114 | # F-strings
115 | f"{ {'a': 1} }"
116 | f"{[ { {'a': 1} } ]}"
| ^ E202
117 | 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 ']'
113 113 |
114 114 | # F-strings
115 115 | f"{ {'a': 1} }"
116 |-f"{[ { {'a': 1} } ]}"
116 |+f"{[ { {'a': 1} }]}"
117 117 | f"normal { {f"{ { [1, 2] } }" } } normal"
118 118 |
119 119 | #: Okay

E20.py:172:12: E202 [*] Whitespace before ']'
|
145 | #: E202:1:12
146 | ham[upper : ]
171 | #: E202:1:12
172 | ham[upper : ]
| ^ E202
147 |
148 | #: E203:1:10
173 |
174 | #: 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 :]


169 169 | ham[upper :]
170 170 |
171 171 | #: E202:1:12
172 |-ham[upper : ]
172 |+ham[upper :]
173 173 |
174 174 | #: E203:1:10
175 175 | ham[upper :]
Loading
Loading