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.

Sign up for GitHub

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

Jump to bottom

Use non-empty ranges for logical-lines diagnostics #4133

Merged
merged 3 commits into from
May 10, 2023
Merged

Use non-empty ranges for logical-lines diagnostics #4133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
89f2d0a
Use non-empty ranges for logical-lines diagnostics
MichaReiser Apr 27, 2023
301299d
Clippy, other code cleanups
MichaReiser May 4, 2023
cde1e4a
Fix highlighting if leading/trailing whitespace contains multiple tabs.
MichaReiser May 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 14 additions & 7 deletions crates/ruff/src/rules/pycodestyle/rules/logical_lines/extraneous_whitespace.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,12 @@ pub(crate) fn extraneous_whitespace(line: &LogicalLine, context: &mut LogicalLin
let kind = token.kind();
match kind {
TokenKind::Lbrace | TokenKind::Lpar | TokenKind::Lsqb => {
if !matches!(line.trailing_whitespace(token), Whitespace::None) {
context.push(WhitespaceAfterOpenBracket, TextRange::empty(token.end()));
let (trailing, trailing_len) = line.trailing_whitespace(token);
if !matches!(trailing, Whitespace::None) {
context.push(
WhitespaceAfterOpenBracket,
TextRange::at(token.end(), trailing_len),
);
}
}
TokenKind::Rbrace
Expand All @@ -119,10 +123,10 @@ pub(crate) fn extraneous_whitespace(line: &LogicalLine, context: &mut LogicalLin
| TokenKind::Comma
| TokenKind::Semi
| TokenKind::Colon => {
if let (Whitespace::Single | Whitespace::Many | Whitespace::Tab, offset) =
line.leading_whitespace(token)
{
if !matches!(last_token, TokenKind::Comma | TokenKind::EndOfFile) {
if !matches!(last_token, TokenKind::Comma | TokenKind::EndOfFile) {
if let (Whitespace::Single | Whitespace::Many | Whitespace::Tab, offset) =
line.leading_whitespace(token)
{
let diagnostic_kind = if matches!(
kind,
TokenKind::Comma | TokenKind::Semi | TokenKind::Colon
Expand All @@ -132,7 +136,10 @@ pub(crate) fn extraneous_whitespace(line: &LogicalLine, context: &mut LogicalLin
DiagnosticKind::from(WhitespaceBeforeCloseBracket)
};

context.push(diagnostic_kind, TextRange::empty(token.start() - offset));
context.push(
diagnostic_kind,
TextRange::at(token.start() - offset, offset),
);
}
}
}
Expand Down
5 changes: 2 additions & 3 deletions crates/ruff/src/rules/pycodestyle/rules/logical_lines/missing_whitespace.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use ruff_diagnostics::Edit;
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::token_kind::TokenKind;
use ruff_text_size::{TextRange, TextSize};
use ruff_text_size::TextSize;

#[violation]
pub struct MissingWhitespace {
Expand Down Expand Up @@ -82,8 +82,7 @@ pub(crate) fn missing_whitespace(
}

let kind = MissingWhitespace { token: kind };

let mut diagnostic = Diagnostic::new(kind, TextRange::empty(token.start()));
let mut diagnostic = Diagnostic::new(kind, token.range());

if autofix {
diagnostic.set_fix(Fix::unspecified(Edit::insertion(
Expand Down
3 changes: 1 addition & 2 deletions crates/ruff/src/rules/pycodestyle/rules/logical_lines/missing_whitespace_after_keyword.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use crate::rules::pycodestyle::rules::logical_lines::LogicalLine;
use ruff_diagnostics::Violation;
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::token_kind::TokenKind;
use ruff_text_size::TextRange;

#[violation]
pub struct MissingWhitespaceAfterKeyword;
Expand Down Expand Up @@ -35,7 +34,7 @@ pub(crate) fn missing_whitespace_after_keyword(
|| matches!(tok1_kind, TokenKind::Colon | TokenKind::Newline))
&& tok0.end() == tok1.start()
{
context.push(MissingWhitespaceAfterKeyword, TextRange::empty(tok0.end()));
context.push(MissingWhitespaceAfterKeyword, tok0.range());
}
}
}
23 changes: 6 additions & 17 deletions crates/ruff/src/rules/pycodestyle/rules/logical_lines/missing_whitespace_around_operator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use crate::rules::pycodestyle::rules::logical_lines::{LogicalLine, LogicalLineTo
use ruff_diagnostics::{DiagnosticKind, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::token_kind::TokenKind;
use ruff_text_size::TextRange;

// E225
#[violation]
Expand Down Expand Up @@ -129,30 +128,20 @@ pub(crate) fn missing_whitespace_around_operator(

match (has_leading_trivia, has_trailing_trivia) {
// Operator with trailing but no leading space, enforce consistent spacing
(false, true) => {
context.push(
MissingWhitespaceAroundOperator,
TextRange::empty(token.start()),
);
}
(false, true) |
// Operator with leading but no trailing space, enforce consistent spacing.
(true, false) => {
context.push(
MissingWhitespaceAroundOperator,
TextRange::empty(token.end()),
);
(true, false)
=> {
context.push(MissingWhitespaceAroundOperator, token.range());
}
// Operator with no space, require spaces if it is required by the operator.
(false, false) => {
if needs_space == NeedsSpace::Yes {
context.push(
diagnostic_kind_for_operator(kind),
TextRange::empty(token.start()),
);
context.push(diagnostic_kind_for_operator(kind), token.range());
}
}
(true, true) => {
// Operator has leading and trailing space, all good
// Operator has leading and trailing spaces, all good
}
}
}
Expand Down
50 changes: 32 additions & 18 deletions crates/ruff/src/rules/pycodestyle/rules/logical_lines/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,8 @@ impl<'a> LogicalLine<'a> {
.slice(TextRange::new(first_token.start(), token.start()))
}

/// Returns the whitespace *after* the `token`
pub fn trailing_whitespace(&self, token: &'a LogicalLineToken) -> Whitespace {
/// Returns the whitespace *after* the `token` with the byte length
pub fn trailing_whitespace(&self, token: &'a LogicalLineToken) -> (Whitespace, TextSize) {
Whitespace::leading(self.text_after(token))
}

Expand Down Expand Up @@ -358,35 +358,45 @@ pub(crate) enum Whitespace {
}

impl Whitespace {
fn leading(content: &str) -> Self {
fn leading(content: &str) -> (Self, TextSize) {
let mut count = 0u32;
let mut len = TextSize::default();
let mut has_tabs = false;

for c in content.chars() {
if c == '\t' {
return Self::Tab;
has_tabs = true;
len += c.text_len();
} else if matches!(c, '\n' | '\r') {
break;
} else if c.is_whitespace() {
count += 1;
len += c.text_len();
} else {
break;
}
}

match count {
0 => Whitespace::None,
1 => Whitespace::Single,
_ => Whitespace::Many,
if has_tabs {
(Whitespace::Tab, len)
} else {
match count {
0 => (Whitespace::None, len),
1 => (Whitespace::Single, len),
_ => (Whitespace::Many, len),
}
}
}

fn trailing(content: &str) -> (Self, TextSize) {
let mut len = TextSize::default();
let mut count = 0usize;
let mut has_tabs = false;

for c in content.chars().rev() {
if c == '\t' {
return (Self::Tab, len + c.text_len());
has_tabs = true;
len += c.text_len();
} else if matches!(c, '\n' | '\r') {
// Indent
return (Self::None, TextSize::default());
Expand All @@ -398,15 +408,19 @@ impl Whitespace {
}
}

match count {
0 => (Self::None, TextSize::default()),
1 => (Self::Single, len),
_ => {
if len == content.text_len() {
// All whitespace up to the start of the line -> Indent
(Self::None, TextSize::default())
} else {
(Self::Many, len)
if has_tabs {
(Self::Tab, len)
} else {
match count {
0 => (Self::None, TextSize::default()),
1 => (Self::Single, len),
_ => {
if len == content.text_len() {
// All whitespace up to the start of the line -> Indent
(Self::None, TextSize::default())
} else {
(Self::Many, len)
}
}
}
}
Expand Down
19 changes: 9 additions & 10 deletions crates/ruff/src/rules/pycodestyle/rules/logical_lines/space_around_operator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,28 +133,27 @@ pub(crate) fn space_around_operator(line: &LogicalLine, context: &mut LogicalLin
if !after_operator {
match line.leading_whitespace(token) {
(Whitespace::Tab, offset) => {
let start = token.start();
context.push(TabBeforeOperator, TextRange::empty(start - offset));
context.push(
TabBeforeOperator,
TextRange::at(token.start() - offset, offset),
);
}
(Whitespace::Many, offset) => {
let start = token.start();
context.push(
MultipleSpacesBeforeOperator,
TextRange::empty(start - offset),
TextRange::at(token.start() - offset, offset),
);
}
_ => {}
}
}

match line.trailing_whitespace(token) {
Whitespace::Tab => {
let end = token.end();
context.push(TabAfterOperator, TextRange::empty(end));
(Whitespace::Tab, len) => {
context.push(TabAfterOperator, TextRange::at(token.end(), len));
}
Whitespace::Many => {
let end = token.end();
context.push(MultipleSpacesAfterOperator, TextRange::empty(end));
(Whitespace::Many, len) => {
context.push(MultipleSpacesAfterOperator, TextRange::at(token.end(), len));
}
_ => {}
}
Expand Down
14 changes: 6 additions & 8 deletions crates/ruff/src/rules/pycodestyle/rules/logical_lines/whitespace_around_keywords.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,27 +118,25 @@ pub(crate) fn whitespace_around_keywords(line: &LogicalLine, context: &mut Logic
match line.leading_whitespace(token) {
(Whitespace::Tab, offset) => {
let start = token.start();
context.push(TabBeforeKeyword, TextRange::empty(start - offset));
context.push(TabBeforeKeyword, TextRange::at(start - offset, offset));
}
(Whitespace::Many, offset) => {
let start = token.start();
context.push(
MultipleSpacesBeforeKeyword,
TextRange::empty(start - offset),
TextRange::at(start - offset, offset),
);
}
_ => {}
}
}

match line.trailing_whitespace(token) {
Whitespace::Tab => {
let end = token.end();
context.push(TabAfterKeyword, TextRange::empty(end));
(Whitespace::Tab, len) => {
context.push(TabAfterKeyword, TextRange::at(token.end(), len));
}
Whitespace::Many => {
let end = token.end();
context.push(MultipleSpacesAfterKeyword, TextRange::empty(end));
(Whitespace::Many, len) => {
context.push(MultipleSpacesAfterKeyword, TextRange::at(token.end(), len));
}
_ => {}
}
Expand Down
14 changes: 4 additions & 10 deletions ...uff/src/rules/pycodestyle/rules/logical_lines/whitespace_around_named_parameter_equals.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,7 @@ pub(crate) fn whitespace_around_named_parameter_equals(
if annotated_func_arg && parens == 1 {
let start = token.start();
if start == prev_end && prev_end != TextSize::new(0) {
context.push(
MissingWhitespaceAroundParameterEquals,
TextRange::empty(start),
);
context.push(MissingWhitespaceAroundParameterEquals, token.range());
}

while let Some(next) = iter.peek() {
Expand All @@ -91,10 +88,7 @@ pub(crate) fn whitespace_around_named_parameter_equals(
let next_start = next.start();

if next_start == token.end() {
context.push(
MissingWhitespaceAroundParameterEquals,
TextRange::empty(next_start),
);
context.push(MissingWhitespaceAroundParameterEquals, token.range());
}
break;
}
Expand All @@ -103,7 +97,7 @@ pub(crate) fn whitespace_around_named_parameter_equals(
if token.start() != prev_end {
context.push(
UnexpectedSpacesAroundKeywordParameterEquals,
TextRange::empty(prev_end),
TextRange::new(prev_end, token.start()),
);
}

Expand All @@ -114,7 +108,7 @@ pub(crate) fn whitespace_around_named_parameter_equals(
if next.start() != token.end() {
context.push(
UnexpectedSpacesAroundKeywordParameterEquals,
TextRange::empty(token.end()),
TextRange::new(token.end(), next.start()),
);
}
break;
Expand Down
12 changes: 6 additions & 6 deletions ...es/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E201_E20.py.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ E20.py:2:6: E201 Whitespace after '('
|
2 | #: E201:1:6
3 | spam( ham[1], {eggs: 2})
| E201
| ^ E201
4 | #: E201:1:10
5 | spam(ham[ 1], {eggs: 2})
|
Expand All @@ -15,7 +15,7 @@ E20.py:4:10: E201 Whitespace after '('
4 | spam( ham[1], {eggs: 2})
5 | #: E201:1:10
6 | spam(ham[ 1], {eggs: 2})
| E201
| ^ E201
7 | #: E201:1:15
8 | spam(ham[1], { eggs: 2})
|
Expand All @@ -25,7 +25,7 @@ E20.py:6:15: E201 Whitespace after '('
6 | spam(ham[ 1], {eggs: 2})
7 | #: E201:1:15
8 | spam(ham[1], { eggs: 2})
| E201
| ^ E201
9 | #: E201:1:6
10 | spam( ham[1], {eggs: 2})
|
Expand All @@ -35,7 +35,7 @@ E20.py:8:6: E201 Whitespace after '('
8 | spam(ham[1], { eggs: 2})
9 | #: E201:1:6
10 | spam( ham[1], {eggs: 2})
| E201
| ^^^ E201
11 | #: E201:1:10
12 | spam(ham[ 1], {eggs: 2})
|
Expand All @@ -45,7 +45,7 @@ E20.py:10:10: E201 Whitespace after '('
10 | spam( ham[1], {eggs: 2})
11 | #: E201:1:10
12 | spam(ham[ 1], {eggs: 2})
| E201
| ^^^ E201
13 | #: E201:1:15
14 | spam(ham[1], { eggs: 2})
|
Expand All @@ -55,7 +55,7 @@ E20.py:12:15: E201 Whitespace after '('
12 | spam(ham[ 1], {eggs: 2})
13 | #: E201:1:15
14 | spam(ham[1], { eggs: 2})
| E201
| ^^ E201
15 | #: Okay
16 | spam(ham[1], {eggs: 2})
|
Expand Down