Add 'fix' command to apply fixes (and unit test)#754
Add 'fix' command to apply fixes (and unit test)#754webwarrior-ws wants to merge 2 commits intofsprojects:masterfrom
Conversation
a583792 to
3924085
Compare
3c64772 to
7880339
Compare
14a265e to
27342d3
Compare
df6e26c to
bc9e328
Compare
src/FSharpLint.Console/Program.fs
Outdated
|
|
||
| let handleFixResult (ruleName: string) = function | ||
| | LintResult.Success warnings -> | ||
| String.Format(Resources.GetString "ConsoleApplyingSuggestedFixFile", ruleName) |> output.WriteInfo |
There was a problem hiding this comment.
What should the parameter be then?
There was a problem hiding this comment.
Same as what happens when printing "Linting {0}", obviously
There was a problem hiding this comment.
Changed to print notification for every warning, with file name as parameter. There may be several warnings per file though.
There was a problem hiding this comment.
There may be several warnings per file though.
Then this was wrong, it should only print once per file. (And it was also wrong even if not adding the $0 element.)
There was a problem hiding this comment.
Because I guess that "Linting {0}" only gets printed once per file/project/solution?
There was a problem hiding this comment.
The function is not called once per file, but once per rule.
There was a problem hiding this comment.
I'm not talking about what the code does now, but what the code should do. And it should do the same as Linting{0}
There was a problem hiding this comment.
Changed the logic. Now it is what you meant @knocte ?
a82fe7c to
8beb004
Compare
|
@webwarrior-ws let's rebase this PR when you have time (but beware of recent push from me, to include the last commit; so |
2e3a715 to
918d878
Compare
|
@webwarrior-ws sorry this needs another rebase |
6efbad1 to
7a893d9
Compare
Rebased |
|
I'm going to remove commit c0a3ab1 from this PR because I moved that change to PR793. |
c0a3ab1 to
7a893d9
Compare
src/FSharpLint.Console/Program.fs
Outdated
| else | ||
| FileType.Source | ||
|
|
||
| // can't extract inner functions because they modify exitCode variable |
There was a problem hiding this comment.
@webwarrior-ws this reason shouldn't apply anymore after I merged PR813 in master, so let's rebase and double check
There was a problem hiding this comment.
(this PR should now introduce a new function that doesn't use a mutable exitCode var, AFAIU)
There was a problem hiding this comment.
Refactored code to not use mutable exitCode and extracted inner functions.
There was a problem hiding this comment.
I didn't ask you to remove mutable var from existing lint() function; what I said is that the new function that should be implemented (in this case, applySuggestedFix), should not use a mutable exitCode. Also:
- why did you rename lint() to applyLint()? doesn't seem necessary at all
- a function named "linting()"? function names should use infinitive, not gerund
There was a problem hiding this comment.
These names are probably not from me. But I changed applyLint back to lint and linting to performlinting.
There was a problem hiding this comment.
performlinting? man, that's neither PascalCase nor camelCase, what notation is that?
There was a problem hiding this comment.
Actual name in the source is performLinting.
7a893d9 to
396295a
Compare
85b512e to
1dc6bf2
Compare
src/FSharpLint.Console/Program.fs
Outdated
| output.WriteError $"Lint failed while analysing %s{target}.{Environment.NewLine}Failed with: %s{exn.Message}{Environment.NewLine}Stack trace: {exn.StackTrace}" | ||
| ExitCode.Failure | ||
|
|
||
| let private lint (lintArgs: ParseResults<LintArgs>) (output: Output.IOutput) (toolsPath:Ionide.ProjInfo.Types.ToolsPath) : ExitCode = |
There was a problem hiding this comment.
@webwarrior-ws the previous version of this func had each param in its own line, why break that formatting?
There was a problem hiding this comment.
Made each pram be on its own line.
d05f6e8 to
aa35ca9
Compare
Removed FromText property of SuggestedFix as it duplicates FromRange functionality and is not actually used anywhere.
Co-authored-by: Parham Saremi <[email protected]> Co-authored-by: webwarrior-ws <[email protected]>
aa35ca9 to
c3b0b9a
Compare
Also removed FromText property of SuggestedFix as it duplicates FromRange functionality and is not actually used anywhere.