Skip to content

Commit 4392044

Browse files
janusparhamsaremi
andcommitted
Add fix command-line to apply quickfixes and unit test
Co-authored-by: Parham Saremi <[email protected]>
1 parent 76d6e9d commit 4392044

File tree

6 files changed

+232
-49
lines changed

6 files changed

+232
-49
lines changed

src/FSharpLint.Console/Program.fs

Lines changed: 137 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,11 @@
22

33
open Argu
44
open System
5+
open System.IO
6+
open System.Text
57
open FSharpLint.Framework
68
open FSharpLint.Application
9+
open System.Linq
710

811
/// Output format the linter will use.
912
type private OutputFormat =
@@ -17,17 +20,27 @@ type private FileType =
1720
| File = 3
1821
| Source = 4
1922

23+
type ExitCode =
24+
| Error = -1
25+
| Success = 0
26+
| NoSuchRuleName = 1
27+
| NoSuggestedFix = 2
28+
29+
let fileTypeHelp = "Input type the linter will run against. If this is not set, the file type will be inferred from the file extension."
30+
2031
// Allowing underscores in union case names for proper Argu command line option formatting.
2132
// fsharplint:disable UnionCasesNames
2233
type private ToolArgs =
2334
| [<AltCommandLine("-f")>] Format of OutputFormat
2435
| [<CliPrefix(CliPrefix.None)>] Lint of ParseResults<LintArgs>
36+
| [<CliPrefix(CliPrefix.None)>] Fix of ParseResults<FixArgs>
2537
with
2638
interface IArgParserTemplate with
2739
member this.Usage =
2840
match this with
2941
| Format _ -> "Output format of the linter."
3042
| Lint _ -> "Runs FSharpLint against a file or a collection of files."
43+
| Fix _ -> "Apply quickfixes for specified rule name or names (comma separated)."
3144

3245
// TODO: investigate erroneous warning on this type definition
3346
// fsharplint:disable UnionDefinitionIndentation
@@ -41,10 +54,24 @@ with
4154
member this.Usage =
4255
match this with
4356
| Target _ -> "Input to lint."
44-
| File_Type _ -> "Input type the linter will run against. If this is not set, the file type will be inferred from the file extension."
57+
| File_Type _ -> fileTypeHelp
4558
| Lint_Config _ -> "Path to the config for the lint."
4659
// fsharplint:enable UnionCasesNames
4760

61+
// TODO: investigate erroneous warning on this type definition
62+
// fsharplint:disable UnionDefinitionIndentation
63+
and private FixArgs =
64+
| [<MainCommand; Mandatory>] Fix_Target of ruleName:string * target:string
65+
| Fix_File_Type of FileType
66+
// fsharplint:enable UnionDefinitionIndentation
67+
with
68+
interface IArgParserTemplate with
69+
member this.Usage =
70+
match this with
71+
| Fix_Target _ -> "Rule name to be applied with suggestedFix and input to lint."
72+
| Fix_File_Type _ -> fileTypeHelp
73+
// fsharplint:enable UnionCasesNames
74+
4875
let private parserProgress (output:Output.IOutput) = function
4976
| Starting file ->
5077
String.Format(Resources.GetString("ConsoleStartingFile"), file) |> output.WriteInfo
@@ -70,7 +97,7 @@ let private inferFileType (target:string) =
7097
FileType.Source
7198

7299
let private start (arguments:ParseResults<ToolArgs>) (toolsPath:Ionide.ProjInfo.Types.ToolsPath) =
73-
let mutable exitCode = 0
100+
let mutable exitCode = ExitCode.Success
74101

75102
let output =
76103
match arguments.TryGetResult Format with
@@ -79,38 +106,59 @@ let private start (arguments:ParseResults<ToolArgs>) (toolsPath:Ionide.ProjInfo.
79106
| Some _
80107
| None -> Output.StandardOutput() :> Output.IOutput
81108

82-
let handleError (str:string) =
109+
let handleError (status:ExitCode) (str:string) =
83110
output.WriteError str
84-
exitCode <- -1
85-
86-
match arguments.GetSubCommand() with
87-
| Lint lintArgs ->
88-
89-
let handleLintResult = function
90-
| LintResult.Success(warnings) ->
91-
String.Format(Resources.GetString("ConsoleFinished"), List.length warnings)
92-
|> output.WriteInfo
93-
if not (List.isEmpty warnings) then exitCode <- -1
94-
| LintResult.Failure(failure) ->
95-
handleError failure.Description
96-
97-
let lintConfig = lintArgs.TryGetResult Lint_Config
98-
99-
let configParam =
100-
match lintConfig with
101-
| Some configPath -> FromFile configPath
102-
| None -> Default
103-
104-
105-
let lintParams =
106-
{ CancellationToken = None
107-
ReceivedWarning = Some output.WriteWarning
108-
Configuration = configParam
109-
ReportLinterProgress = Some (parserProgress output) }
110-
111-
let target = lintArgs.GetResult Target
112-
let fileType = lintArgs.TryGetResult File_Type |> Option.defaultValue (inferFileType target)
111+
exitCode <- status
113112

113+
let outputWarnings (warnings: List<Suggestion.LintWarning>) =
114+
String.Format(Resources.GetString "ConsoleFinished", List.length warnings)
115+
|> output.WriteInfo
116+
117+
let handleLintResult = function
118+
| LintResult.Success warnings ->
119+
outputWarnings warnings
120+
if List.isEmpty warnings |> not then
121+
exitCode <- ExitCode.Error
122+
| LintResult.Failure failure -> handleError ExitCode.Error failure.Description
123+
124+
let handleFixResult (ruleName: string) = function
125+
| LintResult.Success warnings ->
126+
Resources.GetString "ConsoleApplyingSuggestedFixFile" |> output.WriteInfo
127+
let increment = 1
128+
let noFixIncrement = 0
129+
let countSuggestedFix =
130+
List.fold (fun acc elem -> acc + elem) 0 (
131+
List.map (fun (element: Suggestion.LintWarning) ->
132+
let sourceCode = File.ReadAllText element.FilePath
133+
if String.Equals(ruleName, element.RuleName, StringComparison.InvariantCultureIgnoreCase) then
134+
match element.Details.SuggestedFix with
135+
| Some suggestedFix ->
136+
suggestedFix.Force()
137+
|> Option.map (fun suggestedFix ->
138+
let updatedSourceCode =
139+
sourceCode.Replace(
140+
suggestedFix.FromText,
141+
suggestedFix.ToText
142+
)
143+
File.WriteAllText(
144+
element.FilePath,
145+
updatedSourceCode,
146+
Encoding.UTF8)
147+
)
148+
|> ignore |> fun () -> increment
149+
| None -> noFixIncrement
150+
else
151+
noFixIncrement) warnings)
152+
outputWarnings warnings
153+
154+
if countSuggestedFix > 0 then
155+
exitCode <- ExitCode.Success
156+
else
157+
exitCode <- ExitCode.NoSuggestedFix
158+
159+
| LintResult.Failure failure -> handleError ExitCode.Error failure.Description
160+
161+
let linting fileType lintParams target toolsPath shouldFix maybeRuleName =
114162
try
115163
let lintResult =
116164
match fileType with
@@ -119,15 +167,69 @@ let private start (arguments:ParseResults<ToolArgs>) (toolsPath:Ionide.ProjInfo.
119167
| FileType.Solution -> Lint.lintSolution lintParams target toolsPath
120168
| FileType.Project
121169
| _ -> Lint.lintProject lintParams target toolsPath
122-
handleLintResult lintResult
170+
if shouldFix then
171+
match maybeRuleName with
172+
| Some ruleName -> handleFixResult ruleName lintResult
173+
| None -> exitCode <- ExitCode.NoSuchRuleName
174+
else
175+
handleLintResult lintResult
123176
with
124177
| e ->
125178
let target = if fileType = FileType.Source then "source" else target
126179
sprintf "Lint failed while analysing %s.\nFailed with: %s\nStack trace: %s" target e.Message e.StackTrace
127-
|> handleError
180+
|> (handleError ExitCode.Error)
181+
182+
let getParams config =
183+
let paramConfig =
184+
match config with
185+
| Some configPath -> FromFile configPath
186+
| None -> Default
187+
188+
{ CancellationToken = None
189+
ReceivedWarning = Some output.WriteWarning
190+
Configuration = paramConfig
191+
ReportLinterProgress = parserProgress output |> Some }
192+
193+
let applyLint (lintArgs: ParseResults<LintArgs>) =
194+
let lintConfig = lintArgs.TryGetResult Lint_Config
195+
196+
let lintParams = getParams lintConfig
197+
let target = lintArgs.GetResult Target
198+
let fileType = lintArgs.TryGetResult File_Type |> Option.defaultValue (inferFileType target)
199+
200+
linting fileType lintParams target toolsPath false None
201+
202+
let applySuggestedFix (fixArgs: ParseResults<FixArgs>) =
203+
let fixParams = getParams None
204+
let ruleName, target = fixArgs.GetResult Fix_Target
205+
let fileType = fixArgs.TryGetResult Fix_File_Type |> Option.defaultValue (inferFileType target)
206+
207+
let allRules =
208+
match getConfig fixParams.Configuration with
209+
| Ok config -> Some (Configuration.flattenConfig config false)
210+
| _ -> None
211+
212+
let allRuleNames =
213+
match allRules with
214+
| Some rules -> (fun (loadedRules:Configuration.LoadedRules) -> ([|
215+
loadedRules.LineRules.IndentationRule |> Option.map (fun rule -> rule.Name) |> Option.toArray
216+
loadedRules.LineRules.NoTabCharactersRule |> Option.map (fun rule -> rule.Name) |> Option.toArray
217+
loadedRules.LineRules.GenericLineRules |> Array.map (fun rule -> rule.Name)
218+
loadedRules.AstNodeRules |> Array.map (fun rule -> rule.Name)
219+
|] |> Array.concat |> Set.ofArray)) rules
220+
| _ -> Set.empty
221+
222+
if allRuleNames.Any(fun aRuleName -> String.Equals(aRuleName, ruleName, StringComparison.InvariantCultureIgnoreCase)) then
223+
linting fileType fixParams target toolsPath true (Some ruleName)
224+
else
225+
sprintf "Rule '%s' does not exist." ruleName |> (handleError ExitCode.NoSuchRuleName)
226+
227+
match arguments.GetSubCommand() with
228+
| Lint lintArgs -> applyLint lintArgs
229+
| Fix fixArgs -> applySuggestedFix fixArgs
128230
| _ -> ()
129231

130-
exitCode
232+
int exitCode
131233

132234
/// Must be called only once per process.
133235
/// We're calling it globally so we can call main multiple times from our tests.

src/FSharpLint.Core/Application/Configuration.fs

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -129,13 +129,20 @@ type RuleConfig<'Config> = {
129129

130130
type EnabledConfig = RuleConfig<unit>
131131

132-
let constructRuleIfEnabled rule ruleConfig = if ruleConfig.Enabled then Some rule else None
132+
let constructRuleIfEnabledBase (onlyEnabled: bool) rule ruleConfig =
133+
if not onlyEnabled || ruleConfig.Enabled then Some rule else None
133134

134-
let constructRuleWithConfig rule ruleConfig =
135-
if ruleConfig.Enabled then
136-
ruleConfig.Config |> Option.map rule
137-
else
138-
None
135+
let constructRuleIfEnabled rule ruleConfig =
136+
constructRuleIfEnabledBase true rule ruleConfig
137+
138+
let constructRuleWithConfigBase (onlyEnabled: bool) (rule: 'TRuleConfig -> 'TRule) (ruleConfig: RuleConfig<'TRuleConfig>): Option<'TRule> =
139+
if not onlyEnabled || ruleConfig.Enabled then
140+
ruleConfig.Config |> Option.map rule
141+
else
142+
None
143+
144+
let constructRuleWithConfig (rule: 'TRuleConfig -> 'TRule) (ruleConfig: RuleConfig<'TRuleConfig>): Option<'TRule> =
145+
constructRuleWithConfigBase true rule ruleConfig
139146

140147
type TupleFormattingConfig =
141148
{ tupleCommaSpacing:EnabledConfig option
@@ -600,7 +607,7 @@ let private parseHints (hints:string []) =
600607
|> Array.toList
601608
|> MergeSyntaxTrees.mergeHints
602609

603-
let flattenConfig (config:Configuration) =
610+
let flattenConfig (config:Configuration) (onlyEnabled:bool) =
604611
let deprecatedAllRules =
605612
[|
606613
config.formatting |> Option.map (fun config -> config.Flatten()) |> Option.toArray |> Array.concat
@@ -609,6 +616,12 @@ let flattenConfig (config:Configuration) =
609616
config.Hints |> Option.map (fun config -> HintMatcher.rule { HintMatcher.Config.HintTrie = parseHints (getOrEmptyList config.add) }) |> Option.toArray
610617
|] |> Array.concat
611618

619+
let constructRuleIfEnabled rule ruleConfig =
620+
constructRuleIfEnabledBase onlyEnabled rule ruleConfig
621+
622+
let constructRuleWithConfig (rule: 'TRuleConfig -> 'TRule) (ruleConfig: RuleConfig<'TRuleConfig>): Option<'TRule> =
623+
constructRuleWithConfigBase onlyEnabled rule ruleConfig
624+
612625
let allRules =
613626
[|
614627
config.TypedItemSpacing |> Option.bind (constructRuleWithConfig TypedItemSpacing.rule)

src/FSharpLint.Core/Application/Lint.fs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ module Lint =
206206
| Some(x) -> not x.IsCancellationRequested
207207
| None -> true
208208

209-
let enabledRules = Configuration.flattenConfig lintInfo.Configuration
209+
let enabledRules = Configuration.flattenConfig lintInfo.Configuration true
210210

211211
let lines = String.toLines fileInfo.Text |> Array.map (fun (line, _, _) -> line)
212212
let allRuleNames =
@@ -371,7 +371,7 @@ module Lint =
371371
}
372372

373373
/// Gets a FSharpLint Configuration based on the provided ConfigurationParam.
374-
let private getConfig (configParam:ConfigurationParam) =
374+
let getConfig (configParam:ConfigurationParam) =
375375
match configParam with
376376
| Configuration config -> Ok config
377377
| FromFile filePath ->

src/FSharpLint.Core/Application/Lint.fsi

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,4 +144,6 @@ module Lint =
144144

145145
/// Lints an F# file that has already been parsed using
146146
/// `FSharp.Compiler.Services` in the calling application.
147-
val lintParsedFile : optionalParams:OptionalLintParameters -> parsedFileInfo:ParsedFileInformation -> filePath:string -> LintResult
147+
val lintParsedFile : optionalParams:OptionalLintParameters -> parsedFileInfo:ParsedFileInformation -> filePath:string -> LintResult
148+
149+
val getConfig : ConfigurationParam -> Result<Configuration,string>

src/FSharpLint.Core/Text.resx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,9 @@
138138
<data name="ConsoleStartingFile" xml:space="preserve">
139139
<value>========== Linting {0} ==========</value>
140140
</data>
141+
<data name="ConsoleApplyingSuggestedFixFile" xml:space="preserve">
142+
<value>========== Applying fixes ==========</value>
143+
</data>
141144
<data name="ConsoleMSBuildFailedToLoadProjectFile" xml:space="preserve">
142145
<value>MSBuild could not load the project file {0} because: {1}</value>
143146
</data>

0 commit comments

Comments
 (0)