SIM108 turns readable code into unreadable code

[karolinepauls]

The rule should only apply to trivial conditions with the simplest expressions (e.g. even having 2 sub-expressions joined with and or or is too much).

a.py:

from typing import Optional

# an ugly but realistic type
something: Optional[list] = None

if something is not None and len(something) > 1:
    runner_image = something[0].url
else:
    runner_image = None

cmd: ruff --isolated --select SIM --line-length=120 a.py

Output:

a.py:5:1: SIM108 [*] Use ternary operator `runner_image = something[0].url if something is not None and len(something) > 1 else None` instead of `if`-`else`-block

My eyes refuse to read runner_image = something[0].url if something is not None and len(something) > 1 else None.

[tjkuson]

Perhaps I am in the minority on this, but I prefer the ternary syntax in the example here, so I would prefer any change to this to be optional.

[charliermarsh]

I'm open to an argument that we should only apply this rule when the test is a single condition (since the and in the ternary makes it harder to parse), but I don't want to make it configurable -- we should either change it or not. I'll leave this open to give folks a chance to chime in.

[ssweber]

Can’t you just use noqa as needed?

The simplify rules are opinionated. Usually those long expressions are ‘variable = value condition or None’ anyway - my mental model has adjusted.

[dorschw]

Devs in our organization also complained about it. While great for simple conditions and expressions, the longer they become, the messier and harder to read it makes the code.

I see @charliermarsh is not a fan of configurability for this one, but imo any way to control a maximal complexity level (of either the condition or the return values) would make it a great addition.

[jankatins]

In a former company we had the (code style) rule that ternary expression could only be a single line and only "simple" conditions or assignments (e.g. something like x.x if single_comparison else y.y). I like the idea of measuring the complexity and only apply if it less than a (low) default.

[karolinepauls]

@ssweber

Can’t you just use noqa as needed?

This style of response isn't helpful. It fits the pattern: "Can't you just use <an inferior solution the requester certainly knows about and has rejected it> instead? <here goes an explanation why the troublesome feature should stay as it is and we shouldn't try to improve it>"

It should be clear that the person reporting the issue knows about noqa, and finds the use of noqa to be deficient, hence they're reporting the issue.

Why is noqa a non-solution:

1. You have to place it in advance, before autofix rewrites your code. What about disabling autofix to this rule? Someone may still want this rule to autofix trivial conditions and leave non-trival conditions intact.
2. There will always be some devs in a team who adhere to lint rules uncritically. You cannot control that. Tools to help devs write better code interact with all devs in the project, not only the observant ones.

[ssweber]

@ssweber

Can’t you just use noqa as needed?

This style of response isn't helpful.

You’re right, that wasn’t a very considerate suggestion. I’ve enjoyed SIM108, but I’d understand if someone else wanted a more robust solution than “use noqa”.

[Avasam]

I find the issue to be formatting related with long ternary conditionals. For example in JS/TS with ESLint I force-wrap all ternaries, but with Python having to add extra parenthesis (which are formatted on their own lines), or \ (which Ruff and Black disallow), even such an option wouldn't work that great.
Here's some more thoughts: #11753

[NeilGirdhar]

My eyes refuse to read runner_image = something[0].url if something is not None and len(something) > 1 else None.

I agree, but I suggest writing the ternary expression as:

runner_image = (something[0].url 
                if something is not None and len(something) > 1 
                else None)

I propose that this is more legible and briefer than the original code.

(I hope we keep SIM108 in all cases.)