Skip to content

Commit 4886f2e

Browse files
committed
Fix exitCodes for Fix command
return exitCode 1 if rulename is disabled or doesn't exist and add exitCode 2 if the rulename has no suggested fix.
1 parent 01140f7 commit 4886f2e

File tree

5 files changed

+123
-32
lines changed

5 files changed

+123
-32
lines changed

src/FSharpLint.Console/Program.fs

Lines changed: 67 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ open System.IO
66
open System.Text
77
open FSharpLint.Framework
88
open FSharpLint.Application
9+
open System.Linq
910

1011
/// Output format the linter will use.
1112
type private OutputFormat =
@@ -19,6 +20,12 @@ type private FileType =
1920
| File = 3
2021
| Source = 4
2122

23+
type private ExitCode =
24+
| Error = -1
25+
| Success = 0
26+
| NoSuchRuleName = 1
27+
| NoSuggestedFix = 2
28+
2229
let fileTypeHelp = "Input type the linter will run against. If this is not set, the file type will be inferred from the file extension."
2330

2431
// Allowing underscores in union case names for proper Argu command line option formatting.
@@ -90,7 +97,7 @@ let private inferFileType (target:string) =
9097
FileType.Source
9198

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

95102
let output =
96103
match arguments.TryGetResult Format with
@@ -99,9 +106,9 @@ let private start (arguments:ParseResults<ToolArgs>) (toolsPath:Ionide.ProjInfo.
99106
| Some _
100107
| None -> Output.StandardOutput() :> Output.IOutput
101108

102-
let handleError (str:string) =
109+
let handleError (status:ExitCode) (str:string) =
103110
output.WriteError str
104-
exitCode <- -1
111+
exitCode <- status
105112

106113
let outputWarnings (warnings: List<Suggestion.LintWarning>) =
107114
String.Format(Resources.GetString "ConsoleFinished", List.length warnings)
@@ -111,25 +118,45 @@ let private start (arguments:ParseResults<ToolArgs>) (toolsPath:Ionide.ProjInfo.
111118
| LintResult.Success warnings ->
112119
outputWarnings warnings
113120
if List.isEmpty warnings |> not then
114-
exitCode <- -1
115-
| LintResult.Failure failure -> handleError failure.Description
121+
exitCode <- ExitCode.Error
122+
| LintResult.Failure failure -> handleError ExitCode.Error failure.Description
116123

117124
let handleFixResult (ruleName: string) = function
118125
| LintResult.Success warnings ->
119126
Resources.GetString "ConsoleApplyingSuggestedFixFile" |> output.WriteInfo
120-
List.iter (fun (element: Suggestion.LintWarning) ->
121-
let sourceCode = File.ReadAllText element.FilePath
122-
match element.Details.SuggestedFix with
123-
| Some suggestedFix when String.Equals(ruleName, element.RuleName, StringComparison.InvariantCultureIgnoreCase) ->
124-
suggestedFix.Force()
125-
|> Option.map (fun suggestedFix ->
126-
let updatedSourceCode = sourceCode.Replace(suggestedFix.FromText, suggestedFix.ToText)
127-
File.WriteAllText(element.FilePath, updatedSourceCode, Encoding.UTF8)) |> ignore
128-
| _ -> ()) warnings
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)
129152
outputWarnings warnings
130-
if List.isEmpty warnings |> not then
131-
exitCode <- 0
132-
| LintResult.Failure failure -> handleError failure.Description
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
133160

134161
let linting fileType lintParams target toolsPath shouldFix maybeRuleName =
135162
try
@@ -143,14 +170,14 @@ let private start (arguments:ParseResults<ToolArgs>) (toolsPath:Ionide.ProjInfo.
143170
if shouldFix then
144171
match maybeRuleName with
145172
| Some ruleName -> handleFixResult ruleName lintResult
146-
| None -> exitCode <- 1
173+
| None -> exitCode <- ExitCode.NoSuchRuleName
147174
else
148175
handleLintResult lintResult
149176
with
150177
| e ->
151178
let target = if fileType = FileType.Source then "source" else target
152179
sprintf "Lint failed while analysing %s.\nFailed with: %s\nStack trace: %s" target e.Message e.StackTrace
153-
|> handleError
180+
|> (handleError ExitCode.Error)
154181

155182
let getParams config =
156183
let paramConfig =
@@ -176,15 +203,33 @@ let private start (arguments:ParseResults<ToolArgs>) (toolsPath:Ionide.ProjInfo.
176203
let fixParams = getParams None
177204
let ruleName, target = fixArgs.GetResult Fix_Target
178205
let fileType = fixArgs.TryGetResult Fix_File_Type |> Option.defaultValue (inferFileType target)
179-
180-
linting fileType fixParams target toolsPath true (Some ruleName)
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)
181226

182227
match arguments.GetSubCommand() with
183228
| Lint lintArgs -> applyLint lintArgs
184229
| Fix fixArgs -> applySuggestedFix fixArgs
185230
| _ -> ()
186231

187-
exitCode
232+
int exitCode
188233

189234
/// Must be called only once per process.
190235
/// 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>

tests/FSharpLint.Console.Tests/TestApp.fs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,3 +126,34 @@ module Fall =
126126
Assert.AreEqual(0, exitCode)
127127
Assert.AreEqual(set ["Usage of `new` keyword here is redundant."], errors)
128128
Assert.AreEqual(expected, File.ReadAllText input.FileName)
129+
130+
[<Test>]
131+
member __.``Lint source with fix option with wrong rulename``() =
132+
let sourceCode = """
133+
printfn "Hello"
134+
"""
135+
136+
let ruleName = "ssrffss"
137+
use input = new TemporaryFile(sourceCode, "fs")
138+
let (exitCode, errors) = main [| "fix"; ruleName; input.FileName |]
139+
140+
Assert.AreEqual(1, exitCode)
141+
142+
[<Test>]
143+
member __.``Lint source with fix option no need for fix``() =
144+
let sourceCode = """
145+
module Fass =
146+
let foo = System.Collections.Generic.Dictionary<string, string>() |> ignore
147+
let goo = Guid() |> ignore
148+
let ntoo = Int32() |> ignore
149+
module Fall =
150+
let uoo = Uid() |> ignore
151+
let version = System.Version()
152+
let xoo = Uint32() |> ignore
153+
"""
154+
let ruleName = "RedundantNewKeyword"
155+
use input = new TemporaryFile(sourceCode, "fs")
156+
let (exitCode, errors) = main [| "fix"; ruleName; input.FileName |]
157+
158+
Assert.AreEqual(2, exitCode)
159+
Assert.AreEqual(sourceCode, File.ReadAllText input.FileName)

0 commit comments

Comments
 (0)