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

[pycodestyle] Implement redundant-backslash (E502) #10292

Merged
merged 5 commits into from Mar 12, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
82 changes: 82 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pycodestyle/E502.py
@@ -0,0 +1,82 @@
a = 2 + 2

a = (2 + 2)

a = 2 + \
3 \
+ 4

a = (3 -\
2 + \
7)

b = [2 +
2]

b = [
2 + 4 + 5 + \
44 \
- 5
]

c = (True and
False \
or False \
and True \
)

c = (True and
False)

d = True and \
False or \
False \
and not True


s = {
'x': 2 + \
2
}


s = {
'x': 2 +
2
}


x = {2 + 4 \
+ 3}

y = (
2 + 2 # \
+ 3 # \
+ 4 \
+ 3
)


x = """
(\\
)
"""


("""hello \
""")

("hello \
")


x = "abc" \
"xyz"

x = ("abc" \
"xyz")


def foo():
x = (a + \
2)
5 changes: 3 additions & 2 deletions crates/ruff_linter/src/checkers/logical_lines.rs
Expand Up @@ -9,8 +9,8 @@ use ruff_text_size::{Ranged, TextRange};
use crate::registry::AsRule;
use crate::rules::pycodestyle::rules::logical_lines::{
extraneous_whitespace, indentation, missing_whitespace, missing_whitespace_after_keyword,
missing_whitespace_around_operator, space_after_comma, space_around_operator,
whitespace_around_keywords, whitespace_around_named_parameter_equals,
missing_whitespace_around_operator, redundant_backslash, space_after_comma,
space_around_operator, whitespace_around_keywords, whitespace_around_named_parameter_equals,
whitespace_before_comment, whitespace_before_parameters, LogicalLines, TokenFlags,
};
use crate::settings::LinterSettings;
Expand Down Expand Up @@ -73,6 +73,7 @@ pub(crate) fn check_logical_lines(

if line.flags().contains(TokenFlags::BRACKET) {
whitespace_before_parameters(&line, &mut context);
redundant_backslash(&line, &mut context);
}

// Extract the indentation level.
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Expand Up @@ -146,6 +146,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Pycodestyle, "E401") => (RuleGroup::Stable, rules::pycodestyle::rules::MultipleImportsOnOneLine),
(Pycodestyle, "E402") => (RuleGroup::Stable, rules::pycodestyle::rules::ModuleImportNotAtTopOfFile),
(Pycodestyle, "E501") => (RuleGroup::Stable, rules::pycodestyle::rules::LineTooLong),
(Pycodestyle, "E502") => (RuleGroup::Preview, rules::pycodestyle::rules::logical_lines::RedundantBackslash),
(Pycodestyle, "E701") => (RuleGroup::Stable, rules::pycodestyle::rules::MultipleStatementsOnOneLineColon),
(Pycodestyle, "E702") => (RuleGroup::Stable, rules::pycodestyle::rules::MultipleStatementsOnOneLineSemicolon),
(Pycodestyle, "E703") => (RuleGroup::Stable, rules::pycodestyle::rules::UselessSemicolon),
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/registry.rs
Expand Up @@ -327,6 +327,7 @@ impl Rule {
| Rule::NoSpaceAfterBlockComment
| Rule::NoSpaceAfterInlineComment
| Rule::OverIndented
| Rule::RedundantBackslash
| Rule::TabAfterComma
| Rule::TabAfterKeyword
| Rule::TabAfterOperator
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/pycodestyle/mod.rs
Expand Up @@ -71,6 +71,7 @@ mod tests {
#[test_case(Rule::IsLiteral, Path::new("constant_literals.py"))]
#[test_case(Rule::TypeComparison, Path::new("E721.py"))]
#[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402_2.py"))]
#[test_case(Rule::RedundantBackslash, Path::new("E502.py"))]
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!(
"preview__{}_{}",
Expand Down
Expand Up @@ -3,6 +3,7 @@ pub(crate) use indentation::*;
pub(crate) use missing_whitespace::*;
pub(crate) use missing_whitespace_after_keyword::*;
pub(crate) use missing_whitespace_around_operator::*;
pub(crate) use redundant_backslash::*;
pub(crate) use space_around_operator::*;
pub(crate) use whitespace_around_keywords::*;
pub(crate) use whitespace_around_named_parameter_equals::*;
Expand All @@ -25,6 +26,7 @@ mod indentation;
mod missing_whitespace;
mod missing_whitespace_after_keyword;
mod missing_whitespace_around_operator;
mod redundant_backslash;
mod space_around_operator;
mod whitespace_around_keywords;
mod whitespace_around_named_parameter_equals;
Expand Down
@@ -0,0 +1,116 @@
use ruff_diagnostics::AlwaysFixableViolation;
use ruff_diagnostics::Diagnostic;
use ruff_diagnostics::Edit;
use ruff_diagnostics::Fix;
use ruff_macros::{derive_message_formats, violation};
use ruff_python_parser::TokenKind;
use ruff_text_size::{Ranged, TextRange, TextSize};

use crate::checkers::logical_lines::LogicalLinesContext;

use super::{LogicalLine, LogicalLineToken};

/// ## What it does
/// Checks for redundant backslashes between brackets.
///
/// ## Why is this bad?
/// Explicit line joins using a backslash are redundant between brackets.
///
/// ## Example
/// ```python
/// x = (2 + \
/// 2)
/// ```
///
/// Use instead:
/// ```python
/// x = (2 +
/// 2)
/// ```
///
/// [PEP 8]: https://peps.python.org/pep-0008/#maximum-line-length
#[violation]
pub struct RedundantBackslash;

impl AlwaysFixableViolation for RedundantBackslash {
#[derive_message_formats]
fn message(&self) -> String {
format!("Redundant backslash")
}

fn fix_title(&self) -> String {
"Remove redundant backslash".to_string()
}
}

fn neuter_strings(text: &str, tokens: &[LogicalLineToken]) -> String {
let offset = tokens.first().unwrap().start().to_usize();
let mut new_text = String::with_capacity(text.len());
let mut last_end = 0;

for token in tokens {
if matches!(token.kind(), TokenKind::String | TokenKind::FStringMiddle) {
new_text.push_str(&text[last_end..token.start().to_usize() - offset]);
let token_text =
&text[token.start().to_usize() - offset..token.end().to_usize() - offset];
new_text.push_str(&token_text.replace('\\', " "));
last_end = token.end().to_usize() - offset;
}
}

new_text.push_str(&text[last_end..]);
new_text
}

/// E502
pub(crate) fn redundant_backslash(line: &LogicalLine, context: &mut LogicalLinesContext) {
let mut text = line.text().to_string();
let mut cursor = line.tokens().first().unwrap().start().to_usize();
let mut start = 0;
let mut parens = 0;
let mut comment = false;
let mut backslash = false;

text = neuter_strings(&text, line.tokens());

for c in text.chars() {
match c {
'#' => {
backslash = false;
comment = true;
}
'\r' | '\n' => {
if !comment && backslash && parens > 0 {
let start_s = TextSize::new(u32::try_from(start).unwrap());
let end_s = TextSize::new(u32::try_from(cursor).unwrap());
let mut diagnostic =
Diagnostic::new(RedundantBackslash, TextRange::new(start_s, end_s));
diagnostic.set_fix(Fix::safe_edit(Edit::deletion(start_s, end_s)));
context.push_diagnostic(diagnostic);
}
backslash = false;
comment = false;
}
'(' | '[' | '{' => {
backslash = false;
if !comment {
parens += 1;
}
}
')' | ']' | '}' => {
backslash = false;
if !comment {
parens -= 1;
}
}
'\\' => {
start = cursor;
backslash = true;
}
_ => {
backslash = false;
}
}
cursor += c.len_utf8();
}
}
Copy link
Member

Choose a reason for hiding this comment

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

We have a struct called Indexer that already stores the locations of all continuations (i.e., the line positions following a \). I'm wondering if we could instead iterate over the token stream, identify parenthesized ranges, and then search for continuation_lines offsets within parenthesized ranges?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done