Skip to content

Commit

Permalink
Implement compound-statements (E701, E702, E703, E704) (#2680)
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh authored Feb 9, 2023
1 parent 5a07c9f commit 739a92e
Show file tree
Hide file tree
Showing 14 changed files with 406 additions and 30 deletions.
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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` | 🛠 |
Expand Down
25 changes: 25 additions & 0 deletions crates/ruff/resources/test/fixtures/pycodestyle/E70.py
Original file line number Diff line number Diff line change
@@ -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;
19 changes: 19 additions & 0 deletions crates/ruff/src/checkers/tokens.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down
4 changes: 4 additions & 0 deletions crates/ruff/src/flake8_to_ruff/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
17 changes: 3 additions & 14 deletions crates/ruff/src/lex/docstring_detection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand All @@ -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,
Expand Down
8 changes: 8 additions & 0 deletions crates/ruff/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
36 changes: 20 additions & 16 deletions crates/ruff/src/rules/pycodestyle/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
143 changes: 143 additions & 0 deletions crates/ruff/src/rules/pycodestyle/rules/compound_statements.rs
Original file line number Diff line number Diff line change
@@ -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<Diagnostic> {
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
}
5 changes: 5 additions & 0 deletions crates/ruff/src/rules/pycodestyle/rules/mod.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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: ~

Loading

0 comments on commit 739a92e

Please sign in to comment.