Add SimplifyLambdaFix #176
Conversation
DedSec256
force-pushed
the
ber.a/simplifyLambdaFix
branch
from
927ca40 to
4cd1105
Compare
There was a problem hiding this comment.
Lots of warnings reported in #175 don't have a quick fix implemented here, we should provide it or disable the warnings until it's done.
Please looks at the comments about cases where the quick fix will break code.
|
|
||
| let countRedundantArgs expr = | ||
| let rec countRedundantArgsRec (expr: IFSharpExpression) i = | ||
| let expr = PrefixAppExprNavigator.GetByFunctionExpression expr |
There was a problem hiding this comment.
It'll fail on this code:
fun a b -> (foo ()) b| use writeCookie = WriteLockCookie.Create(lambda.IsPhysical()) | ||
| use disableFormatter = new DisableCodeFormatter() | ||
|
|
||
| replaceWithCopy lambda.Expression (replaceCandidate.IgnoreInnerParens()) |
There was a problem hiding this comment.
Syntax error after applying:
fun a b -> foo [ 1
2 ] b
| @@ -33,6 +33,7 @@ type LambdaAnalyzer() = | |||
| let hasMatches = i > 0 | |||
|
|
|||
| match expr with | |||
| | :? IParenExpr as expr -> compareArgsRec (expr.IgnoreInnerParens()) i | |||
There was a problem hiding this comment.
Why not match expr.IgnoreInnerParens() with?
| deleteChildRange firstNodeToDelete lastNodeToDelete | ||
| addNodesBefore expr [Whitespace(indentDiff)] |> ignore | ||
| shiftWithWhitespaceBefore -indentDiff expr |
There was a problem hiding this comment.
Why shiftWithWhitespaceBefore instead of shiftExpr?
|
|
||
| if equal then compareArgsRec app (i + 1) else (hasMatches, false, app) | ||
| | _ -> hasMatches, i = pats.Count, expr | ||
| if equal then compareArgsRec funExpr (i + 1) else (hasMatches, false, app :> IFSharpExpression) |
There was a problem hiding this comment.
Is explicit type needed here?
There was a problem hiding this comment.
Yes, since type inference tries to infer a narrower type
There was a problem hiding this comment.
What does it infer to when :> _ is used?
| use disableFormatter = new DisableCodeFormatter() | ||
|
|
||
| let indentDiff = lambda.Indent - replaceCandidate.Indent | ||
| let expr = ModificationUtil.ReplaceChild(lambda, replaceCandidate) |
There was a problem hiding this comment.
ReplaceChild says it should not be done this way:
/// <summary>
/// Warning: <paramref name="newChild" /> should not be child of <paramref name="oldChild" />
/// </summary>Also, the quick fix could inherit ReplaceWithInnerExpressionFixBase.
| @@ -148,6 +148,7 @@ | |||
| <Message value="{0}"> | |||
| <Argument>getLambdaCanBeReplacedWarningText replaceCandidate</Argument> | |||
| </Message> | |||
| <QuickFix>ReplaceLambdaFix</QuickFix> | |||
There was a problem hiding this comment.
Let's try using a clearer name? Replace lambda with what?
There was a problem hiding this comment.
Maybe 'with expression'?
P.S.
to replace it with 'id', I plan to write a separate quickfix
DedSec256
force-pushed
the
ber.a/simplifyLambdaFix
branch
from
6c0782c to
f90a838
Compare
|
Another case to consider is fun (struct (a, b)) -> a, b
fun (a, b) -> struct (a, b) |
DedSec256
force-pushed
the
ber.a/simplifyLambdaFix
branch
2 times, most recently
from
3d252f9 to
efecff3
Compare
|
🤔 module Foo =
let [<Literal>] Bar = 123
fun Foo.Bar -> Bar
let [<Literal>] Bar = 123
fun Bar -> Bar
|
auduchinok
force-pushed
the
ber.a/simplifyLambdaFix
branch
from
edade10 to
c83b8ea
Compare
#175