Improve linting of LHS expressions#2070
Uh oh!
There was an error while loading. Please reload this page.
Conversation
This reverts commit 6109ac7.
(cherry picked from commit a4e2608)
| case AssignmentStatement asst: | ||
| _suppressDiagnostics = true; | ||
| foreach (var lhs in asst.Left ?? Enumerable.Empty<Expression>()){ | ||
| foreach (var lhs in asst.Left.MaybeEnumerate().Where(x => !(x is NameExpression))){ |
There was a problem hiding this comment.
What affect does this have for an assignment like:
deffoo(): return1, 2x=1234x, y=foo()Where y has not been defined but is being walked as a name expression?
I'm thinking that the original solution was "more correct" in that it's trying to avoid lvars being linted, but needed an extra case where it stored _suppressDiagnostics and made it false before doing the first member access (to check the first thing), and then restored it.
| _suppressDiagnostics = true; | ||
| foreach (var lhs in asst.Left ?? Enumerable.Empty<Expression>()){ | ||
| lhs?.Walk(new ExpressionWalker(this)); | ||
| foreach (var l in lhs.Skip(1)){ |
There was a problem hiding this comment.
Not sure this is what I described, it's not about it being the first thing in a tuple, it's about it being the first part of a member or array access, like foo.bar or foo[bar] appearing somewhere on the left.
There was a problem hiding this comment.
Where foo is undefined, but bar is defined.
There was a problem hiding this comment.
Unfortunate typo, meant foo not defined but bar defined.
There was a problem hiding this comment.
That parts works OK i.e. x[y] detects undefined x or y. The remaining part of LHS is trickier since we don't know if RHS returns proper number of items.
There was a problem hiding this comment.
I'll make a test for the case I'm thinking of and see if it works.
jakebailey commented Jun 16, 2020
Seems to fail the NoRightSideCheck test. My test was something like: y={} invalid.access, another.access, (y.okay, fake.variable.access) = [1, 2, (3, 4)]But the current code appears to correctly only squiggle |
2 participants
Fixes#2068
Original condition was too harsh. Intent was to avoid reporting of variables that are actually being defined, but that can be done by skipping over name expressions in LHS.