diff --git a/resources/test/fixtures/flake8_simplify/SIM108.py b/resources/test/fixtures/flake8_simplify/SIM108.py index d0cb3fc151..9fec9525f6 100644 --- a/resources/test/fixtures/flake8_simplify/SIM108.py +++ b/resources/test/fixtures/flake8_simplify/SIM108.py @@ -1,13 +1,13 @@ -# Bad +# SIM108 if a: b = c else: b = d -# Good +# OK b = c if a else d -# https://github.com/MartinThoma/flake8-simplify/issues/115 +# OK if a: b = c elif c: @@ -15,6 +15,7 @@ elif c: else: b = d +# OK if True: pass elif a: @@ -22,6 +23,7 @@ elif a: else: b = 2 +# OK (false negative) if True: pass else: @@ -30,19 +32,62 @@ else: else: b = 2 + import sys +# OK if sys.version_info >= (3, 9): randbytes = random.randbytes else: randbytes = _get_random_bytes +# OK if sys.platform == "darwin": randbytes = random.randbytes else: randbytes = _get_random_bytes +# OK if sys.platform.startswith("linux"): randbytes = random.randbytes else: randbytes = _get_random_bytes + + +# OK (includes comments) +if x > 0: + # test test + abc = x +else: + # test test test + abc = -x + + +# OK (too long) +if parser.errno == BAD_FIRST_LINE: + req = wrappers.Request(sock, server=self._server) +else: + req = wrappers.Request( + sock, + parser.get_method(), + parser.get_scheme() or _scheme, + parser.get_path(), + parser.get_version(), + parser.get_query_string(), + server=self._server, + ) + + +# SIM108 +if a: + b = cccccccccccccccccccccccccccccccccccc +else: + b = ddddddddddddddddddddddddddddddddddddd + + +# OK (too long) +if True: + if a: + b = cccccccccccccccccccccccccccccccccccc + else: + b = ddddddddddddddddddddddddddddddddddddd diff --git a/src/ast/helpers.rs b/src/ast/helpers.rs index 5fc0dd72b0..c6c129687d 100644 --- a/src/ast/helpers.rs +++ b/src/ast/helpers.rs @@ -430,6 +430,13 @@ pub fn collect_arg_names<'a>(arguments: &'a Arguments) -> FxHashSet<&'a str> { arg_names } +/// Returns `true` if a statement or expression includes at least one comment. +pub fn has_comments(located: &Located, locator: &SourceCodeLocator) -> bool { + lexer::make_tokenizer(&locator.slice_source_code_range(&Range::from_located(located))) + .flatten() + .any(|(_, tok, _)| matches!(tok, Tok::Comment(..))) +} + /// Returns `true` if a call is an argumented `super` invocation. pub fn is_super_call_with_arguments(func: &Expr, args: &[Expr]) -> bool { if let ExprKind::Name { id, .. } = &func.node { diff --git a/src/flake8_simplify/rules/ast_if.rs b/src/flake8_simplify/rules/ast_if.rs index ebf31729c2..ec28d86e31 100644 --- a/src/flake8_simplify/rules/ast_if.rs +++ b/src/flake8_simplify/rules/ast_if.rs @@ -1,7 +1,7 @@ use rustpython_ast::{Constant, Expr, ExprKind, Stmt, StmtKind}; use crate::ast::helpers::{ - contains_call_path, create_expr, create_stmt, unparse_expr, unparse_stmt, + contains_call_path, create_expr, create_stmt, has_comments, unparse_expr, unparse_stmt, }; use crate::ast::types::Range; use crate::autofix::Fix; @@ -201,14 +201,27 @@ pub fn use_ternary_operator(checker: &mut Checker, stmt: &Stmt, parent: Option<& let target_var = &body_targets[0]; let ternary = ternary(target_var, body_value, test, orelse_value); - let content = unparse_stmt(&ternary, checker.style); + let contents = unparse_stmt(&ternary, checker.style); + + // Don't flag for simplified ternaries if the resulting expression would exceed + // the maximum line length. + if stmt.location.column() + contents.len() > checker.settings.line_length { + return; + } + + // Don't flag for simplified ternaries if the if-expression contains any + // comments. + if has_comments(stmt, checker.locator) { + return; + } + let mut diagnostic = Diagnostic::new( - violations::UseTernaryOperator(content.clone()), + violations::UseTernaryOperator(contents.clone()), Range::from_located(stmt), ); if checker.patch(&RuleCode::SIM108) { diagnostic.amend(Fix::replacement( - content, + contents, stmt.location, stmt.end_location.unwrap(), )); diff --git a/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM108_SIM108.py.snap b/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM108_SIM108.py.snap index 5ec2899888..ffacb73b30 100644 --- a/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM108_SIM108.py.snap +++ b/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM108_SIM108.py.snap @@ -1,6 +1,6 @@ --- source: src/flake8_simplify/mod.rs -expression: checks +expression: diagnostics --- - kind: UseTernaryOperator: b = c if a else d @@ -19,4 +19,21 @@ expression: checks row: 5 column: 9 parent: ~ +- kind: + UseTernaryOperator: b = cccccccccccccccccccccccccccccccccccc if a else ddddddddddddddddddddddddddddddddddddd + location: + row: 82 + column: 0 + end_location: + row: 85 + column: 45 + fix: + content: b = cccccccccccccccccccccccccccccccccccc if a else ddddddddddddddddddddddddddddddddddddd + location: + row: 82 + column: 0 + end_location: + row: 85 + column: 45 + parent: ~