flake8_simplify : SIM401

[chammika-become]

Ref #998

• Implements SIM401 with fix
• Added tests

Notes:

• only recognize simple ExprKind::Name variables in expr patterns for now
• bug-fix from reference implementation: check 3-conditions (dict-key, target-variable, dict-name) to be equal, flake8_simplify only test first two (only first in second pattern)

@charliermarsh I would like to handle cases marked nice-to-have (they are handled in flake8_simplify by flattening the expr into strings). Appreciate, any pointers in improving helper match_name_expr for more complex exprs.

[chammika-become]

@charliermarsh please review.
I am not entirely happy the way a new match_comp_expr turn out to be. I could get rid of this function, if I could directly convert &Located<ExprKind> => &ComparableExpr . For example,

// test_key : &Box<Located<ExprKind>>
 let test_key_comp: &ComparableExpr = (&test_key).into(); // <= failed

Hope you can improve on that.

[charliermarsh]

Thank you!

I wasn't able to remove that method entirely, but I was able to simplify it a bit.

[chammika-become]

@charliermarsh some afterthought... we are not going to flag this at all if there are comments in the stmt ? If that's the case we can move has_comments() check all the way up to the top.

[charliermarsh]

@chammika-become - Yeah, it's really hard to preserve comments correctly since there are so many cases to handle (above the statement, below the statement, inline, etc.), so for now, I'm just skipping commented blocks when collapsing multi- to single-line statements. We do the same for the ternary-rewrite rule.

I prefer keeping has_comments lower as its more computationally expensive (we have to re-tokenize the block).