From 739a92e99d5a1a38f7917562f3ab401103ae579c Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 8 Feb 2023 22:57:39 -0500 Subject: [PATCH] Implement compound-statements (E701, E702, E703, E704) (#2680) --- README.md | 4 + .../test/fixtures/pycodestyle/E70.py | 25 +++ crates/ruff/src/checkers/tokens.rs | 19 +++ crates/ruff/src/flake8_to_ruff/parser.rs | 4 + crates/ruff/src/lex/docstring_detection.rs | 17 +-- crates/ruff/src/registry.rs | 8 + crates/ruff/src/rules/pycodestyle/mod.rs | 36 +++-- .../pycodestyle/rules/compound_statements.rs | 143 ++++++++++++++++++ .../ruff/src/rules/pycodestyle/rules/mod.rs | 5 + ...ules__pycodestyle__tests__E701_E70.py.snap | 35 +++++ ...ules__pycodestyle__tests__E702_E70.py.snap | 45 ++++++ ...ules__pycodestyle__tests__E703_E70.py.snap | 35 +++++ ...ules__pycodestyle__tests__E704_E70.py.snap | 55 +++++++ ruff.schema.json | 5 + 14 files changed, 406 insertions(+), 30 deletions(-) create mode 100644 crates/ruff/resources/test/fixtures/pycodestyle/E70.py create mode 100644 crates/ruff/src/rules/pycodestyle/rules/compound_statements.rs create mode 100644 crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E701_E70.py.snap create mode 100644 crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E702_E70.py.snap create mode 100644 crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E703_E70.py.snap create mode 100644 crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E704_E70.py.snap diff --git a/README.md b/README.md index e3aeaaadd25a0..9a13645180039 100644 --- a/README.md +++ b/README.md @@ -725,6 +725,10 @@ For more, see [pycodestyle](https://pypi.org/project/pycodestyle/) on PyPI. | E401 | multiple-imports-on-one-line | Multiple imports on one line | | | E402 | module-import-not-at-top-of-file | Module level import not at top of file | | | E501 | line-too-long | Line too long ({length} > {limit} characters) | | +| E701 | multiple-statements-on-one-line-colon | Multiple statements on one line (colon) | | +| E702 | multiple-statements-on-one-line-semicolon | Multiple statements on one line (semicolon) | | +| E703 | useless-semicolon | Statement ends with an unnecessary semicolon | | +| E704 | multiple-statements-on-one-line-def | Multiple statements on one line (def) | | | E711 | none-comparison | Comparison to `None` should be `cond is None` | 🛠 | | E712 | true-false-comparison | Comparison to `True` should be `cond is True` | 🛠 | | E713 | not-in-test | Test for membership should be `not in` | 🛠 | diff --git a/crates/ruff/resources/test/fixtures/pycodestyle/E70.py b/crates/ruff/resources/test/fixtures/pycodestyle/E70.py new file mode 100644 index 0000000000000..b03f2c8ecd89b --- /dev/null +++ b/crates/ruff/resources/test/fixtures/pycodestyle/E70.py @@ -0,0 +1,25 @@ +#: E701:1:5 +if a: a = False +#: E701:1:40 +if not header or header[:6] != 'bytes=': return +#: E702:1:10 +a = False; b = True +#: E702:1:17 +import bdist_egg; bdist_egg.write_safety_flag(cmd.egg_info, safe) +#: E703:1:13 +import shlex; +#: E702:1:9 E703:1:23 +del a[:]; a.append(42); +#: E704:1:1 +def f(x): return 2 +#: E704:1:1 +async def f(x): return 2 +#: E704:1:1 E271:1:6 +async def f(x): return 2 +#: E704:1:1 E226:1:19 +def f(x): return 2*x +#: E704:2:5 E226:2:23 +while all is round: + def f(x): return 2*x +#: +if True: x; y; diff --git a/crates/ruff/src/checkers/tokens.rs b/crates/ruff/src/checkers/tokens.rs index 5f9c25280eecb..51a191d79dcdf 100644 --- a/crates/ruff/src/checkers/tokens.rs +++ b/crates/ruff/src/checkers/tokens.rs @@ -34,6 +34,16 @@ pub fn check_tokens( || settings.rules.enabled(&Rule::BadQuotesDocstring) || settings.rules.enabled(&Rule::AvoidQuoteEscape); let enforce_commented_out_code = settings.rules.enabled(&Rule::CommentedOutCode); + let enforce_compound_statements = settings + .rules + .enabled(&Rule::MultipleStatementsOnOneLineColon) + || settings + .rules + .enabled(&Rule::MultipleStatementsOnOneLineSemicolon) + || settings.rules.enabled(&Rule::UselessSemicolon) + || settings + .rules + .enabled(&Rule::MultipleStatementsOnOneLineDef); let enforce_invalid_escape_sequence = settings.rules.enabled(&Rule::InvalidEscapeSequence); let enforce_implicit_string_concatenation = settings .rules @@ -108,6 +118,15 @@ pub fn check_tokens( } } + // E701, E702, E703, E704 + if enforce_compound_statements { + diagnostics.extend( + pycodestyle::rules::compound_statements(tokens) + .into_iter() + .filter(|diagnostic| settings.rules.enabled(diagnostic.kind.rule())), + ); + } + // Q001, Q002, Q003 if enforce_quotes { diagnostics.extend( diff --git a/crates/ruff/src/flake8_to_ruff/parser.rs b/crates/ruff/src/flake8_to_ruff/parser.rs index 7086a28b33c0c..ea5df04e7c826 100644 --- a/crates/ruff/src/flake8_to_ruff/parser.rs +++ b/crates/ruff/src/flake8_to_ruff/parser.rs @@ -282,6 +282,10 @@ mod tests { pattern: "examples/*".to_string(), prefix: RuleCodePrefix::F841.into(), }, + PatternPrefixPair { + pattern: "*.pyi".to_string(), + prefix: RuleCodePrefix::E704.into(), + }, ]; assert_eq!(actual, expected); diff --git a/crates/ruff/src/lex/docstring_detection.rs b/crates/ruff/src/lex/docstring_detection.rs index 7c27375289661..01509c20c49f8 100644 --- a/crates/ruff/src/lex/docstring_detection.rs +++ b/crates/ruff/src/lex/docstring_detection.rs @@ -6,9 +6,10 @@ use rustpython_parser::lexer::Tok; -#[derive(Debug)] +#[derive(Default)] enum State { // Start of the module: first string gets marked as a docstring. + #[default] ExpectModuleDocstring, // After seeing a class definition, we're waiting for the block colon (and do bracket // counting). @@ -23,25 +24,13 @@ enum State { Other, } +#[derive(Default)] pub struct StateMachine { state: State, bracket_count: usize, } -impl Default for StateMachine { - fn default() -> Self { - Self::new() - } -} - impl StateMachine { - pub const fn new() -> Self { - Self { - state: State::ExpectModuleDocstring, - bracket_count: 0, - } - } - pub fn consume(&mut self, tok: &Tok) -> bool { if matches!( tok, diff --git a/crates/ruff/src/registry.rs b/crates/ruff/src/registry.rs index e8ce5c9159788..3dac81d97cf2e 100644 --- a/crates/ruff/src/registry.rs +++ b/crates/ruff/src/registry.rs @@ -60,6 +60,10 @@ ruff_macros::define_rule_mapping!( E401 => rules::pycodestyle::rules::MultipleImportsOnOneLine, E402 => rules::pycodestyle::rules::ModuleImportNotAtTopOfFile, E501 => rules::pycodestyle::rules::LineTooLong, + E701 => rules::pycodestyle::rules::MultipleStatementsOnOneLineColon, + E702 => rules::pycodestyle::rules::MultipleStatementsOnOneLineSemicolon, + E703 => rules::pycodestyle::rules::UselessSemicolon, + E704 => rules::pycodestyle::rules::MultipleStatementsOnOneLineDef, E711 => rules::pycodestyle::rules::NoneComparison, E712 => rules::pycodestyle::rules::TrueFalseComparison, E713 => rules::pycodestyle::rules::NotInTest, @@ -759,6 +763,10 @@ impl Rule { | Rule::SingleLineImplicitStringConcatenation | Rule::TrailingCommaMissing | Rule::TrailingCommaOnBareTupleProhibited + | Rule::MultipleStatementsOnOneLineColon + | Rule::UselessSemicolon + | Rule::MultipleStatementsOnOneLineDef + | Rule::MultipleStatementsOnOneLineSemicolon | Rule::TrailingCommaProhibited => &LintSource::Tokens, Rule::IOError => &LintSource::Io, Rule::UnsortedImports | Rule::MissingRequiredImport => &LintSource::Imports, diff --git a/crates/ruff/src/rules/pycodestyle/mod.rs b/crates/ruff/src/rules/pycodestyle/mod.rs index 1775fc1640e8b..e8c4448281c9a 100644 --- a/crates/ruff/src/rules/pycodestyle/mod.rs +++ b/crates/ruff/src/rules/pycodestyle/mod.rs @@ -17,29 +17,33 @@ mod tests { use crate::test::test_path; use crate::{assert_yaml_snapshot, settings}; - #[test_case(Rule::MultipleImportsOnOneLine, Path::new("E40.py"))] - #[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E40.py"))] - #[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402.py"))] - #[test_case(Rule::LineTooLong, Path::new("E501.py"))] - #[test_case(Rule::NoneComparison, Path::new("E711.py"))] - #[test_case(Rule::TrueFalseComparison, Path::new("E712.py"))] - #[test_case(Rule::NotInTest, Path::new("E713.py"))] - #[test_case(Rule::NotIsTest, Path::new("E714.py"))] - #[test_case(Rule::TypeComparison, Path::new("E721.py"))] - #[test_case(Rule::DoNotUseBareExcept, Path::new("E722.py"))] - #[test_case(Rule::DoNotAssignLambda, Path::new("E731.py"))] - #[test_case(Rule::AmbiguousVariableName, Path::new("E741.py"))] #[test_case(Rule::AmbiguousClassName, Path::new("E742.py"))] #[test_case(Rule::AmbiguousFunctionName, Path::new("E743.py"))] - #[test_case(Rule::SyntaxError, Path::new("E999.py"))] + #[test_case(Rule::AmbiguousVariableName, Path::new("E741.py"))] + #[test_case(Rule::DoNotAssignLambda, Path::new("E731.py"))] + #[test_case(Rule::DoNotUseBareExcept, Path::new("E722.py"))] + #[test_case(Rule::InvalidEscapeSequence, Path::new("W605_0.py"))] + #[test_case(Rule::InvalidEscapeSequence, Path::new("W605_1.py"))] + #[test_case(Rule::LineTooLong, Path::new("E501.py"))] + #[test_case(Rule::MixedSpacesAndTabs, Path::new("E101.py"))] + #[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E40.py"))] + #[test_case(Rule::ModuleImportNotAtTopOfFile, Path::new("E402.py"))] + #[test_case(Rule::MultipleImportsOnOneLine, Path::new("E40.py"))] + #[test_case(Rule::MultipleStatementsOnOneLineColon, Path::new("E70.py"))] + #[test_case(Rule::MultipleStatementsOnOneLineDef, Path::new("E70.py"))] + #[test_case(Rule::MultipleStatementsOnOneLineSemicolon, Path::new("E70.py"))] #[test_case(Rule::NoNewLineAtEndOfFile, Path::new("W292_0.py"))] #[test_case(Rule::NoNewLineAtEndOfFile, Path::new("W292_1.py"))] #[test_case(Rule::NoNewLineAtEndOfFile, Path::new("W292_2.py"))] #[test_case(Rule::NoNewLineAtEndOfFile, Path::new("W292_3.py"))] #[test_case(Rule::NoNewLineAtEndOfFile, Path::new("W292_4.py"))] - #[test_case(Rule::InvalidEscapeSequence, Path::new("W605_0.py"))] - #[test_case(Rule::InvalidEscapeSequence, Path::new("W605_1.py"))] - #[test_case(Rule::MixedSpacesAndTabs, Path::new("E101.py"))] + #[test_case(Rule::NoneComparison, Path::new("E711.py"))] + #[test_case(Rule::NotInTest, Path::new("E713.py"))] + #[test_case(Rule::NotIsTest, Path::new("E714.py"))] + #[test_case(Rule::SyntaxError, Path::new("E999.py"))] + #[test_case(Rule::TrueFalseComparison, Path::new("E712.py"))] + #[test_case(Rule::TypeComparison, Path::new("E721.py"))] + #[test_case(Rule::UselessSemicolon, Path::new("E70.py"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.code(), path.to_string_lossy()); let diagnostics = test_path( diff --git a/crates/ruff/src/rules/pycodestyle/rules/compound_statements.rs b/crates/ruff/src/rules/pycodestyle/rules/compound_statements.rs new file mode 100644 index 0000000000000..102c7bc0d3f95 --- /dev/null +++ b/crates/ruff/src/rules/pycodestyle/rules/compound_statements.rs @@ -0,0 +1,143 @@ +use rustpython_parser::lexer::{LexResult, Tok}; + +use ruff_macros::{define_violation, derive_message_formats}; + +use crate::ast::types::Range; +use crate::registry::Diagnostic; +use crate::violation::Violation; + +define_violation!( + pub struct MultipleStatementsOnOneLineColon; +); +impl Violation for MultipleStatementsOnOneLineColon { + #[derive_message_formats] + fn message(&self) -> String { + format!("Multiple statements on one line (colon)") + } +} + +define_violation!( + pub struct MultipleStatementsOnOneLineSemicolon; +); +impl Violation for MultipleStatementsOnOneLineSemicolon { + #[derive_message_formats] + fn message(&self) -> String { + format!("Multiple statements on one line (semicolon)") + } +} + +define_violation!( + pub struct UselessSemicolon; +); +impl Violation for UselessSemicolon { + #[derive_message_formats] + fn message(&self) -> String { + format!("Statement ends with an unnecessary semicolon") + } +} + +define_violation!( + pub struct MultipleStatementsOnOneLineDef; +); +impl Violation for MultipleStatementsOnOneLineDef { + #[derive_message_formats] + fn message(&self) -> String { + format!("Multiple statements on one line (def)") + } +} + +pub fn compound_statements(lxr: &[LexResult]) -> Vec { + let mut diagnostics = vec![]; + + // Track the last seen instance of a variety of tokens. + let mut def = None; + let mut colon = None; + let mut semi = None; + + // Track the bracket depth. + let mut par_count = 0; + let mut sqb_count = 0; + let mut brace_count = 0; + + for &(start, ref tok, end) in lxr.iter().flatten() { + match tok { + Tok::Lpar => { + par_count += 1; + } + Tok::Rpar => { + par_count -= 1; + } + Tok::Lsqb => { + sqb_count += 1; + } + Tok::Rsqb => { + sqb_count -= 1; + } + Tok::Lbrace => { + brace_count += 1; + } + Tok::Rbrace => { + brace_count -= 1; + } + _ => {} + } + + if par_count > 0 || sqb_count > 0 || brace_count > 0 { + continue; + } + + match tok { + Tok::Newline => { + if let Some((start, end)) = semi { + diagnostics.push(Diagnostic::new(UselessSemicolon, Range::new(start, end))); + } + + // Reset. + def = None; + colon = None; + semi = None; + } + Tok::Def => { + def = Some((start, end)); + } + Tok::Colon => { + colon = Some((start, end)); + } + Tok::Semi => { + semi = Some((start, end)); + } + Tok::Comment(..) | Tok::Indent | Tok::Dedent | Tok::NonLogicalNewline => {} + _ => { + if let Some((start, end)) = semi { + diagnostics.push(Diagnostic::new( + MultipleStatementsOnOneLineSemicolon, + Range::new(start, end), + )); + + // Reset. + semi = None; + } + + if let Some((start, end)) = colon { + if let Some((start, end)) = def { + diagnostics.push(Diagnostic::new( + MultipleStatementsOnOneLineDef, + Range::new(start, end), + )); + } else { + diagnostics.push(Diagnostic::new( + MultipleStatementsOnOneLineColon, + Range::new(start, end), + )); + } + + // Reset. + def = None; + colon = None; + } + } + } + } + + diagnostics +} diff --git a/crates/ruff/src/rules/pycodestyle/rules/mod.rs b/crates/ruff/src/rules/pycodestyle/rules/mod.rs index c555d17955246..16ee4d9a11941 100644 --- a/crates/ruff/src/rules/pycodestyle/rules/mod.rs +++ b/crates/ruff/src/rules/pycodestyle/rules/mod.rs @@ -1,6 +1,10 @@ pub use ambiguous_class_name::{ambiguous_class_name, AmbiguousClassName}; pub use ambiguous_function_name::{ambiguous_function_name, AmbiguousFunctionName}; pub use ambiguous_variable_name::{ambiguous_variable_name, AmbiguousVariableName}; +pub use compound_statements::{ + compound_statements, MultipleStatementsOnOneLineColon, MultipleStatementsOnOneLineDef, + MultipleStatementsOnOneLineSemicolon, UselessSemicolon, +}; pub use do_not_assign_lambda::{do_not_assign_lambda, DoNotAssignLambda}; pub use do_not_use_bare_except::{do_not_use_bare_except, DoNotUseBareExcept}; pub use doc_line_too_long::{doc_line_too_long, DocLineTooLong}; @@ -41,6 +45,7 @@ pub use whitespace_before_comment::{ mod ambiguous_class_name; mod ambiguous_function_name; mod ambiguous_variable_name; +mod compound_statements; mod do_not_assign_lambda; mod do_not_use_bare_except; mod doc_line_too_long; diff --git a/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E701_E70.py.snap b/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E701_E70.py.snap new file mode 100644 index 0000000000000..31f61c5df030d --- /dev/null +++ b/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E701_E70.py.snap @@ -0,0 +1,35 @@ +--- +source: crates/ruff/src/rules/pycodestyle/mod.rs +expression: diagnostics +--- +- kind: + MultipleStatementsOnOneLineColon: ~ + location: + row: 2 + column: 4 + end_location: + row: 2 + column: 5 + fix: ~ + parent: ~ +- kind: + MultipleStatementsOnOneLineColon: ~ + location: + row: 4 + column: 39 + end_location: + row: 4 + column: 40 + fix: ~ + parent: ~ +- kind: + MultipleStatementsOnOneLineColon: ~ + location: + row: 25 + column: 7 + end_location: + row: 25 + column: 8 + fix: ~ + parent: ~ + diff --git a/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E702_E70.py.snap b/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E702_E70.py.snap new file mode 100644 index 0000000000000..d5a59f44e5ec9 --- /dev/null +++ b/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E702_E70.py.snap @@ -0,0 +1,45 @@ +--- +source: crates/ruff/src/rules/pycodestyle/mod.rs +expression: diagnostics +--- +- kind: + MultipleStatementsOnOneLineSemicolon: ~ + location: + row: 6 + column: 9 + end_location: + row: 6 + column: 10 + fix: ~ + parent: ~ +- kind: + MultipleStatementsOnOneLineSemicolon: ~ + location: + row: 8 + column: 16 + end_location: + row: 8 + column: 17 + fix: ~ + parent: ~ +- kind: + MultipleStatementsOnOneLineSemicolon: ~ + location: + row: 12 + column: 8 + end_location: + row: 12 + column: 9 + fix: ~ + parent: ~ +- kind: + MultipleStatementsOnOneLineSemicolon: ~ + location: + row: 25 + column: 10 + end_location: + row: 25 + column: 11 + fix: ~ + parent: ~ + diff --git a/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E703_E70.py.snap b/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E703_E70.py.snap new file mode 100644 index 0000000000000..50f0b6212518d --- /dev/null +++ b/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E703_E70.py.snap @@ -0,0 +1,35 @@ +--- +source: crates/ruff/src/rules/pycodestyle/mod.rs +expression: diagnostics +--- +- kind: + UselessSemicolon: ~ + location: + row: 10 + column: 12 + end_location: + row: 10 + column: 13 + fix: ~ + parent: ~ +- kind: + UselessSemicolon: ~ + location: + row: 12 + column: 22 + end_location: + row: 12 + column: 23 + fix: ~ + parent: ~ +- kind: + UselessSemicolon: ~ + location: + row: 25 + column: 13 + end_location: + row: 25 + column: 14 + fix: ~ + parent: ~ + diff --git a/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E704_E70.py.snap b/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E704_E70.py.snap new file mode 100644 index 0000000000000..af2e1ee93402f --- /dev/null +++ b/crates/ruff/src/rules/pycodestyle/snapshots/ruff__rules__pycodestyle__tests__E704_E70.py.snap @@ -0,0 +1,55 @@ +--- +source: crates/ruff/src/rules/pycodestyle/mod.rs +expression: diagnostics +--- +- kind: + MultipleStatementsOnOneLineDef: ~ + location: + row: 14 + column: 0 + end_location: + row: 14 + column: 3 + fix: ~ + parent: ~ +- kind: + MultipleStatementsOnOneLineDef: ~ + location: + row: 16 + column: 6 + end_location: + row: 16 + column: 9 + fix: ~ + parent: ~ +- kind: + MultipleStatementsOnOneLineDef: ~ + location: + row: 18 + column: 7 + end_location: + row: 18 + column: 10 + fix: ~ + parent: ~ +- kind: + MultipleStatementsOnOneLineDef: ~ + location: + row: 20 + column: 0 + end_location: + row: 20 + column: 3 + fix: ~ + parent: ~ +- kind: + MultipleStatementsOnOneLineDef: ~ + location: + row: 23 + column: 4 + end_location: + row: 23 + column: 7 + fix: ~ + parent: ~ + diff --git a/ruff.schema.json b/ruff.schema.json index b4f1a6e210866..b3122779d947e 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -1461,6 +1461,11 @@ "E50", "E501", "E7", + "E70", + "E701", + "E702", + "E703", + "E704", "E71", "E711", "E712",