sourcery-ai / sourcery

Instant AI code reviews
https://sourcery.ai
MIT License
1.53k stars 66 forks source link

Expression length/complexity limit on `use-named-expression` #278

Open adamchainz opened 1 year ago

adamchainz commented 1 year ago

Checklist

Description

Sourcery currently seems to do this with even long expressions:

-f = some_very_long_expression(
-    "spanning",
-    "multiple",
-    "lines",
-    ...
- )
- if f:
-     ...
+ if f := some_very_long_expression("spanning", "multiple", "lines", ...):

For example, with Django ORM calls that may chain several methods in order to create the right query. Combining all that onto one line is less readable IMO.

I suggest some kind of complexity limit on expressions that get walrus-ed by this rule.

Debug Information

IDE Version: n/a

Sourcery Version: 0.12.7

Operating system and Version: macOS

Hellebore commented 1 year ago

Hi @adamchainz - that's a great point - I guess a ~ 80 char limit might well make sense here?

adamchainz commented 1 year ago

If the 80 chars includes the if var :=, then yeah something like that would make sense.

pokulo commented 1 year ago

The same 80 chars condition should apply to refactorings/assign-if-exp, since most of the time in my code the proposed refactoring would exceeds our/blacks line length limit.

adamchainz commented 1 year ago

Oh yeah, I disabled assign-if-exp altogether, I don't like return x if y else z generally, but the long lines did push me.

Expression complexity is worth considering. For example:

return data.get('monthly', {}).get(date, None) if data else None

This is <80 chars, even when indented, but to my eye the number of pieces in the first expression pushes the if data else None to be ignored too easily. A limit of, say, 5 pieces (names, operators) in expressions would stop this happening.