Skip SIM108 violations for complex if-statements (#1802)

We now skip SIM108 violations if: the resulting statement would exceed
the user-specified line length, or the `if` statement contains comments.

Closes #1719.

Closes #1766.

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

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<T>(located: &Located<T>, 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 {

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(),
         ));

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: ~