SIM108: Fix false-positive (#120)

Closes #115

flake8_simplify/__init__.py

@@ -49,7 +49,7 @@
     get_sim208,
 )
 from flake8_simplify.rules.ast_with import get_sim117
-from flake8_simplify.utils import Call, For, UnaryOp
+from flake8_simplify.utils import Call, For, If, UnaryOp
 
 logger = logging.getLogger(__name__)
 
@@ -99,7 +99,7 @@ def visit_BoolOp(self, node: ast.BoolOp) -> None:
     def visit_If(self, node: ast.If) -> None:
         self.errors += get_sim102(node)
         self.errors += get_sim103(node)
-        self.errors += get_sim108(node)
+        self.errors += get_sim108(If(node))
         self.errors += get_sim114(node)
         self.errors += get_sim116(node)
         self.errors += get_sim908(node)

flake8_simplify/rules/ast_if.py

@@ -4,7 +4,12 @@
 
 # First party
 from flake8_simplify.constants import AST_CONST_TYPES, BOOL_CONST_TYPES
-from flake8_simplify.utils import get_if_body_pairs, is_body_same, to_source
+from flake8_simplify.utils import (
+    If,
+    get_if_body_pairs,
+    is_body_same,
+    to_source,
+)
 
 
 def get_sim102(node: ast.If) -> List[Tuple[int, int, str]]:
@@ -95,7 +100,7 @@ def get_sim103(node: ast.If) -> List[Tuple[int, int, str]]:
     return errors
 
 
-def get_sim108(node: ast.If) -> List[Tuple[int, int, str]]:
+def get_sim108(node: If) -> List[Tuple[int, int, str]]:
     """
     Get a list of all if-elses which could be a ternary operator assignment.
 
@@ -117,16 +122,16 @@ def get_sim108(node: ast.If) -> List[Tuple[int, int, str]]:
             ],
         ),
     """
-    SIM108 = (
+    RULE = (
         "SIM108 Use ternary operator "
         "'{assign} = {body} if {cond} else {orelse}' "
         "instead of if-else-block"
     )
     errors: List[Tuple[int, int, str]] = []
     if not (
         len(node.body) == 1
-        and len(node.orelse) == 1
         and isinstance(node.body[0], ast.Assign)
+        and len(node.orelse) == 1
         and isinstance(node.orelse[0], ast.Assign)
         and len(node.body[0].targets) == 1
         and len(node.orelse[0].targets) == 1
@@ -135,13 +140,25 @@ def get_sim108(node: ast.If) -> List[Tuple[int, int, str]]:
         and node.body[0].targets[0].id == node.orelse[0].targets[0].id
     ):
         return errors
-    assign = to_source(node.body[0].targets[0])
+
+    target_var = node.body[0].targets[0]
+    assign = to_source(target_var)
+
+    # It's part of a bigger if-elseif block:
+    # https://github.com/MartinThoma/flake8-simplify/issues/115
+    if isinstance(node.parent, ast.If):
+        for n in node.parent.body:
+            if (
+                isinstance(n, ast.Assign)
+                and isinstance(n.targets[0], ast.Name)
+                and n.targets[0].id == target_var.id
+            ):
+                return errors
+
     body = to_source(node.body[0].value)
     cond = to_source(node.test)
     orelse = to_source(node.orelse[0].value)
-    new_code = SIM108.format(
-        assign=assign, body=body, cond=cond, orelse=orelse
-    )
+    new_code = RULE.format(assign=assign, body=body, cond=cond, orelse=orelse)
     if len(new_code) > 79:
         return errors
     errors.append((node.lineno, node.col_offset, new_code))
@@ -308,6 +325,7 @@ def get_sim908(node: ast.If) -> List[Tuple[int, int, str]]:
         and len(node.orelse) == 0
     ):
         return errors
+
     # We might still be left with a check if a value is in a list or in
     # the body the developer might remove the element from the list
     # We need to have a look at the body

flake8_simplify/utils.py

@@ -35,6 +35,22 @@ def __init__(self, orig: ast.Call) -> None:
         self.parent: ast.Expr = orig.parent  # type: ignore
 
 
+class If(ast.If):
+    """For mypy so that it knows that added attributes exist."""
+
+    def __init__(self, orig: ast.If) -> None:
+        self.test = orig.test
+        self.body = orig.body
+        self.orelse = orig.orelse
+
+        # For all ast.*:
+        self.lineno = orig.lineno
+        self.col_offset = orig.col_offset
+
+        # Added attributes
+        self.parent: ast.Expr = orig.parent  # type: ignore
+
+
 class For(ast.For):
     """For mypy so that it knows that added attributes exist."""
 

tests/test_100_rules.py

@@ -147,6 +147,19 @@ def test_sim108():
     assert ret == {exp}
 
 
+def test_sim108_false_positive():
+    ret = _results(
+        """if E == 0:
+    M = 3
+elif E == 1:
+    M = 2
+else:
+    M = 0.5"""
+    )
+    for el in ret:
+        assert "SIM108" not in el
+
+
 def test_sim109():
     ret = _results("a == b or a == c")
     assert ret == {