Skip to content

Commit

Permalink
Use non-empty ranges for logical-lines diagnostics
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaReiser committed Apr 27, 2023
1 parent 554869d commit 483f479
Show file tree
Hide file tree
Showing 27 changed files with 219 additions and 176 deletions.
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
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};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::token_kind::TokenKind;
use ruff_text_size::{TextRange, TextSize};
use ruff_text_size::{TextLen, TextRange, TextSize};

#[violation]
pub struct MissingWhitespace {
Expand Down Expand Up @@ -82,8 +82,8 @@ 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, TextRange::at(token.start(), TextSize::new(1)));

if autofix {
diagnostic.set_fix(Edit::insertion(" ".to_string(), token.end()));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use itertools::Itertools;
use ruff_text_size::TextRange;
use ruff_text_size::{TextLen, TextRange};

use crate::checkers::logical_lines::LogicalLinesContext;
use crate::rules::pycodestyle::rules::logical_lines::LogicalLine;
Expand Down Expand Up @@ -34,7 +34,16 @@ 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()));
let len = line
.token_text(&tok1)
.chars()
.next()
.map(TextLen::text_len)
.unwrap_or_default();
context.push(
MissingWhitespaceAfterKeyword,
TextRange::at(tok0.end(), len),
);
}
}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use crate::checkers::logical_lines::LogicalLinesContext;
use ruff_diagnostics::Violation;
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::source_code::Locator;
use ruff_python_ast::token_kind::TokenKind;
use ruff_text_size::{TextRange, TextSize};
use ruff_text_size::{TextLen, TextRange, TextSize};

use crate::rules::pycodestyle::rules::logical_lines::LogicalLine;

Expand Down Expand Up @@ -91,7 +92,7 @@ pub(crate) fn missing_whitespace_around_operator(
if needs_space_main != NeedsSpace::Yes && needs_space_aux != NeedsSpace::Yes {
context.push(
MissingWhitespaceAroundOperator,
TextRange::empty(prev_end_aux),
TextRange::at(prev_end_aux, TextSize::new(1)),
);
}
needs_space_main = NeedsSpace::No;
Expand All @@ -110,22 +111,25 @@ pub(crate) fn missing_whitespace_around_operator(
// For more info see PEP570
} else {
if needs_space_main == NeedsSpace::Yes || needs_space_aux == NeedsSpace::Yes {
context.push(MissingWhitespaceAroundOperator, TextRange::empty(prev_end));
context.push(
MissingWhitespaceAroundOperator,
TextRange::at(prev_end, TextSize::new(1)),
);
} else if prev_type != TokenKind::DoubleStar {
if prev_type == TokenKind::Percent {
context.push(
MissingWhitespaceAroundModuloOperator,
TextRange::empty(prev_end_aux),
TextRange::at(prev_end_aux, TextSize::new(1)),
);
} else if !prev_type.is_arithmetic() {
context.push(
MissingWhitespaceAroundBitwiseOrShiftOperator,
TextRange::empty(prev_end_aux),
TextRange::at(prev_end_aux, TextSize::new(1)),
);
} else {
context.push(
MissingWhitespaceAroundArithmeticOperator,
TextRange::empty(prev_end_aux),
TextRange::at(prev_end_aux, TextSize::new(1)),
);
}
}
Expand Down Expand Up @@ -174,7 +178,10 @@ pub(crate) fn missing_whitespace_around_operator(
};
} else if needs_space_main == NeedsSpace::Yes && token.start() == prev_end_aux {
// A needed opening space was not found
context.push(MissingWhitespaceAroundOperator, TextRange::empty(prev_end));
context.push(
MissingWhitespaceAroundOperator,
TextRange::at(prev_end, TextSize::new(1)),
);
needs_space_main = NeedsSpace::No;
needs_space_aux = NeedsSpace::Unset;
prev_end_aux = TextSize::new(0);
Expand Down
20 changes: 13 additions & 7 deletions crates/ruff/src/rules/pycodestyle/rules/logical_lines/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,10 @@ impl<'a> LogicalLine<'a> {
.slice(TextRange::new(token.end(), last_token.end()))
}

pub fn token_text(&self, token: &'a LogicalLineToken) -> &str {
self.lines.locator.slice(token.range)
}

/// Returns the text before `token`
#[inline]
pub fn text_before(&self, token: &'a LogicalLineToken) -> &str {
Expand All @@ -240,8 +244,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,25 +362,27 @@ 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();

for c in content.chars() {
if c == '\t' {
return Self::Tab;
return (Self::Tab, 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,
0 => (Whitespace::None, len),
1 => (Whitespace::Single, len),
_ => (Whitespace::Many, len),
}
}

Expand Down
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::rules::pycodestyle::rules::logical_lines::{LogicalLine, LogicalLineTo
use ruff_diagnostics::Violation;
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::token_kind::TokenKind;
use ruff_text_size::{TextRange, TextSize};
use ruff_text_size::{TextLen, TextRange, TextSize};

#[violation]
pub struct UnexpectedSpacesAroundKeywordParameterEquals;
Expand Down Expand Up @@ -78,9 +78,15 @@ 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) {
let len = line
.token_text(token)
.chars()
.next()
.map(TextLen::text_len)
.unwrap_or_default();
context.push(
MissingWhitespaceAroundParameterEquals,
TextRange::empty(start),
TextRange::at(start, len),
);
}

Expand All @@ -91,9 +97,16 @@ pub(crate) fn whitespace_around_named_parameter_equals(
let next_start = next.start();

if next_start == token.end() {
let len = line
.text_after(token)
.chars()
.next()
.map(TextLen::text_len)
.unwrap_or_default();

context.push(
MissingWhitespaceAroundParameterEquals,
TextRange::empty(next_start),
TextRange::at(next_start, len),
);
}
break;
Expand All @@ -103,7 +116,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 +127,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
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
Loading

0 comments on commit 483f479

Please sign in to comment.