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 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
88 changes: 88 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pycodestyle/E502.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
a = 2 + 2

a = (2 + 2)

a = 2 + \
3 \
+ 4

a = (3 -\
2 + \
7)

z = 5 + \
(3 -\
2 + \
7) + \
4

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)
7 changes: 5 additions & 2 deletions crates/ruff_linter/src/checkers/logical_lines.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::line_width::IndentWidth;
use ruff_diagnostics::Diagnostic;
use ruff_python_codegen::Stylist;
use ruff_python_index::Indexer;
use ruff_python_parser::lexer::LexResult;
use ruff_python_parser::TokenKind;
use ruff_source_file::Locator;
Expand All @@ -9,8 +10,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 All @@ -35,6 +36,7 @@ pub(crate) fn expand_indent(line: &str, indent_width: IndentWidth) -> usize {
pub(crate) fn check_logical_lines(
tokens: &[LexResult],
locator: &Locator,
indexer: &Indexer,
stylist: &Stylist,
settings: &LinterSettings,
) -> Vec<Diagnostic> {
Expand Down Expand Up @@ -73,6 +75,7 @@ pub(crate) fn check_logical_lines(

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

// Extract the indentation level.
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
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
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/linter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ pub fn check_path(
.any(|rule_code| rule_code.lint_source().is_logical_lines())
{
diagnostics.extend(crate::checkers::logical_lines::check_logical_lines(
&tokens, locator, stylist, settings,
&tokens, locator, indexer, stylist, settings,
));
}

Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/registry.rs
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_index::Indexer;
use ruff_python_parser::TokenKind;
use ruff_source_file::Locator;
use ruff_text_size::{Ranged, TextRange, TextSize};

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

use super::LogicalLine;

/// ## 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()
}
}

/// E502
pub(crate) fn redundant_backslash(
line: &LogicalLine,
locator: &Locator,
indexer: &Indexer,
context: &mut LogicalLinesContext,
) {
let mut parens = 0;
let continuation_lines = indexer.continuation_line_starts();
let mut start_index = 0;

for token in line.tokens() {
match token.kind() {
TokenKind::Lpar | TokenKind::Lsqb | TokenKind::Lbrace => {
if parens == 0 {
let start = locator.line_start(token.start());
start_index = continuation_lines
.binary_search(&start)
.map_or_else(|err_index| err_index, |ok_index| ok_index);
}
parens += 1;
}
TokenKind::Rpar | TokenKind::Rsqb | TokenKind::Rbrace => {
parens -= 1;
if parens == 0 {
let end = locator.line_start(token.start());
let end_index = continuation_lines
.binary_search(&end)
.map_or_else(|err_index| err_index, |ok_index| ok_index);
for continuation_line in &continuation_lines[start_index..end_index] {
let backslash_end = locator.line_end(*continuation_line);
let backslash_start = backslash_end - TextSize::new(1);
let mut diagnostic = Diagnostic::new(
RedundantBackslash,
TextRange::new(backslash_start, backslash_end),
);
diagnostic.set_fix(Fix::safe_edit(Edit::deletion(
backslash_start,
backslash_end,
)));
context.push_diagnostic(diagnostic);
}
}
}
_ => continue,
}
}
}
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

Loading
Loading