Skip to content

Commit

Permalink
Avoid if-else simplification for TYPE_CHECKING blocks
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Oct 19, 2023
1 parent ec1be60 commit 05d64d5
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -126,3 +126,12 @@ def f():
x = yield 3
else:
x = yield 5


from typing import TYPE_CHECKING

# OK
if TYPE_CHECKING:
x = 3
else:
x = 5
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1050,7 +1050,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
flake8_simplify::rules::manual_dict_lookup(checker, if_);
}
if checker.enabled(Rule::IfElseBlockInsteadOfIfExp) {
flake8_simplify::rules::use_ternary_operator(checker, stmt);
flake8_simplify::rules::use_ternary_operator(checker, if_);
}
if checker.enabled(Rule::IfElseBlockInsteadOfDictGet) {
flake8_simplify::rules::use_dict_get_with_default(checker, if_);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::any_over_expr;
use ruff_python_ast::{self as ast, ElifElseClause, Expr, Stmt};
use ruff_python_semantic::analyze::typing::is_type_checking_block;
use ruff_python_semantic::SemanticModel;
use ruff_text_size::{Ranged, TextRange};

Expand Down Expand Up @@ -51,16 +52,14 @@ impl Violation for IfElseBlockInsteadOfIfExp {
}

/// SIM108
pub(crate) fn use_ternary_operator(checker: &mut Checker, stmt: &Stmt) {
let Stmt::If(ast::StmtIf {
pub(crate) fn use_ternary_operator(checker: &mut Checker, if_: &ast::StmtIf) {
let ast::StmtIf {
test,
body,
elif_else_clauses,
range: _,
}) = stmt
else {
return;
};
} = if_;

// `test: None` to only match an `else` clause
let [ElifElseClause {
body: else_body,
Expand Down Expand Up @@ -99,13 +98,6 @@ pub(crate) fn use_ternary_operator(checker: &mut Checker, stmt: &Stmt) {
return;
}

// Avoid suggesting ternary for `if sys.version_info >= ...`-style and
// `if sys.platform.startswith("...")`-style checks.
let ignored_call_paths: &[&[&str]] = &[&["sys", "version_info"], &["sys", "platform"]];
if contains_call_path(test, ignored_call_paths, checker.semantic()) {
return;
}

// Avoid suggesting ternary for `if (yield ...)`-style checks.
// TODO(charlie): Fix precedence handling for yields in generator.
if matches!(
Expand All @@ -121,14 +113,26 @@ pub(crate) fn use_ternary_operator(checker: &mut Checker, stmt: &Stmt) {
return;
}

// Avoid suggesting ternary for `if sys.version_info >= ...`-style and
// `if sys.platform.startswith("...")`-style checks.
let ignored_call_paths: &[&[&str]] = &[&["sys", "version_info"], &["sys", "platform"]];
if contains_call_path(test, ignored_call_paths, checker.semantic()) {
return;
}

// Avoid suggesting ternary for `if TYPE_CHECKING:`-style checks.
if is_type_checking_block(if_, checker.semantic()) {
return;
}

let target_var = &body_target;
let ternary = ternary(target_var, body_value, test, else_value);
let contents = checker.generator().stmt(&ternary);

// Don't flag if the resulting expression would exceed the maximum line length.
if !fits(
&contents,
stmt.into(),
if_.into(),
checker.locator(),
checker.settings.line_length,
checker.settings.tab_size,
Expand All @@ -140,12 +144,12 @@ pub(crate) fn use_ternary_operator(checker: &mut Checker, stmt: &Stmt) {
IfElseBlockInsteadOfIfExp {
contents: contents.clone(),
},
stmt.range(),
if_.range(),
);
if !checker.indexer().has_comments(stmt, checker.locator()) {
if !checker.indexer().has_comments(if_, checker.locator()) {
diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement(
contents,
stmt.range(),
if_.range(),
)));
}
checker.diagnostics.push(diagnostic);
Expand Down

0 comments on commit 05d64d5

Please sign in to comment.