diff --git a/build.fsx b/build.fsx index 8c69e65fe..81a7a0121 100644 --- a/build.fsx +++ b/build.fsx @@ -274,7 +274,6 @@ Target.create "SelfCheck" (fun _ -> "trailingWhitespaceOnLine" // TODO: we should enable at some point - "typePrefixing" "unnestedFunctionNames" "nestedFunctionNames" "nestedStatements" diff --git a/docs/content/how-tos/rule-configuration.md b/docs/content/how-tos/rule-configuration.md index b0af74e66..6cab0093d 100644 --- a/docs/content/how-tos/rule-configuration.md +++ b/docs/content/how-tos/rule-configuration.md @@ -10,7 +10,7 @@ menu_order: 3 The linter by default looks for a file named `fsharplint.json` in the current working directory. Typically you would have this file in the root of your project. -At this moment in time the configuration requires every rule to be added to your file, rather than a typical approach where you would override just the rules you want to change from their defaults. This will be addressed in a future version see: [Issue #488](https://github.com/fsprojects/FSharpLint/issues/488). +Entries in configuration file override corresponding entries in default configuration, so you only need to specify rules that you want to change from their defaults. Check out the [default configuration](https://github.com/fsprojects/FSharpLint/blob/master/src/FSharpLint.Core/fsharplint.json) that the tool comes with to see all possible settings and their default values. diff --git a/fsharplint.json b/fsharplint.json new file mode 100644 index 000000000..5d29c6a2e --- /dev/null +++ b/fsharplint.json @@ -0,0 +1,8 @@ +{ + "typePrefixing": { + "enabled": true, + "config": { + "mode": "Always" + } + } +} diff --git a/src/FSharpLint.Console/Program.fs b/src/FSharpLint.Console/Program.fs index b5e59f28a..4c3da483e 100644 --- a/src/FSharpLint.Console/Program.fs +++ b/src/FSharpLint.Console/Program.fs @@ -152,7 +152,6 @@ let private start (arguments:ParseResults) (toolsPath:Ionide.ProjInfo. | Some configPath -> FromFile configPath | None -> Default - let lintParams = { CancellationToken = None ReceivedWarning = Some output.WriteWarning diff --git a/src/FSharpLint.Core/Application/Configuration.fs b/src/FSharpLint.Core/Application/Configuration.fs index 86d784e9e..569948899 100644 --- a/src/FSharpLint.Core/Application/Configuration.fs +++ b/src/FSharpLint.Core/Application/Configuration.fs @@ -428,13 +428,18 @@ with // -let private getOrEmptyList hints = Option.defaultValue Array.empty hints - type HintConfig = { add:string [] option ignore:string [] option } +let private flattenHints (config: HintConfig) = + let ignores = + Option.defaultValue Array.empty config.ignore + |> Set.ofArray + Option.defaultValue Array.empty config.add + |> Array.filter (fun hint -> not <| ignores.Contains hint) + type GlobalConfig = { numIndentationSpaces:int option } @@ -663,6 +668,43 @@ let loadConfig (configPath:string) = File.ReadAllText configPath |> parseConfig +let private combineHints (baseConfig: HintConfig) (overridingConfig: HintConfig) = + let mergeLists baseList overridingList = + match (baseList, overridingList) with + | Some baseArray, None -> Some baseArray + | Some baseArray, Some overridingArray -> Array.append baseArray overridingArray |> Array.distinct |> Some + | None, _ -> overridingList + { + add = mergeLists baseConfig.add overridingConfig.add + ignore = mergeLists baseConfig.ignore overridingConfig.ignore + } + +/// Combine two configs into one: all values that are Some in second config override +/// corresponding values in first config. +let combineConfigs (baseConfig: Configuration) (overridingConfig: Configuration) : Configuration = + let baseFields = FSharp.Reflection.FSharpValue.GetRecordFields baseConfig + let partialConfigFields = FSharp.Reflection.FSharpValue.GetRecordFields overridingConfig + + let resultingRecordFields = + Array.map2 + (fun baseValue overridingValue -> + if isNull overridingValue then + baseValue + else + overridingValue) + baseFields + partialConfigFields + + let mergedConfig = + FSharp.Reflection.FSharpValue.MakeRecord(typeof, resultingRecordFields) + :?> Configuration + + match (baseConfig.Hints, overridingConfig.Hints) with + | Some baseHints, Some overridingHints -> + { mergedConfig with Hints = Some(combineHints baseHints overridingHints) } + | _ -> + mergedConfig + /// A default configuration specifying every analyser and rule is included as a resource file in the framework. /// This function loads and returns this default configuration. let defaultConfiguration = @@ -752,7 +794,7 @@ let flattenConfig (config:Configuration) = config.typography |> Option.map (fun config -> config.Flatten()) |> Option.toArray |> Array.concat // - config.Hints |> Option.map (fun config -> HintMatcher.rule { HintMatcher.Config.HintTrie = parseHints (getOrEmptyList config.add) }) |> Option.toArray + config.Hints |> Option.map (fun config -> HintMatcher.rule { HintMatcher.Config.HintTrie = parseHints (flattenHints config) }) |> Option.toArray |] let allRules = diff --git a/src/FSharpLint.Core/Application/Lint.fs b/src/FSharpLint.Core/Application/Lint.fs index da988cc96..0195a07a2 100644 --- a/src/FSharpLint.Core/Application/Lint.fs +++ b/src/FSharpLint.Core/Application/Lint.fs @@ -424,15 +424,7 @@ module Lint = /// Gets a FSharpLint Configuration based on the provided ConfigurationParam. let private getConfig (configParam:ConfigurationParam) = - match configParam with - | Configuration config -> Ok config - | FromFile filePath -> - try - Configuration.loadConfig filePath - |> Ok - with - | ex -> Error (string ex) - | Default -> + let getDefault () = try Configuration.loadConfig "./fsharplint.json" |> Ok @@ -440,6 +432,20 @@ module Lint = | :? System.IO.FileNotFoundException -> Ok Configuration.defaultConfiguration | ex -> Error (string ex) + + match configParam with + | Configuration config -> Ok config + | FromFile filePath -> + try + match getDefault () with + | Ok defaultConfig -> + let partialConfig = Configuration.loadConfig filePath + Configuration.combineConfigs defaultConfig partialConfig |> Ok + | Error(_) as err -> err + with + | ex -> Error (string ex) + | Default -> + getDefault () /// Lints an entire F# project by retrieving the files from a given /// path to the `.fsproj` file. diff --git a/tests/FSharpLint.Console.Tests/TestApp.fs b/tests/FSharpLint.Console.Tests/TestApp.fs index ec55109c1..60184f0ad 100644 --- a/tests/FSharpLint.Console.Tests/TestApp.fs +++ b/tests/FSharpLint.Console.Tests/TestApp.fs @@ -64,7 +64,7 @@ type TestConsoleApplication() = member _.``Lint source with valid config to disable rule, disabled rule is not triggered for given source.``() = let fileContent = """ { - "InterfaceNames": { + "interfaceNames": { "enabled": false } } @@ -117,6 +117,64 @@ type TestConsoleApplication() = Assert.AreEqual(-1, returnCode) Assert.AreEqual(set ["Use prefix syntax for generic type."], errors) + + /// Regression test for: https://github.com/fsprojects/FSharpLint/issues/466 + /// Hints listed in the ignore section of the config were not being ignored. + [] + member __.``Ignoring a hint in the configuration should stop it from being output as warning.``() = + let input = """ + let x = [1; 2; 3; 4] |> List.map (fun x -> x + 2) |> List.map (fun x -> x + 2) + let y = not (1 = 1) + """ + + let commonErrorMessage = "`not (a = b)` might be able to be refactored into `a <> b`." + let disabledHintErrorMessage = "`List.map f (List.map g x)` might be able to be refactored into `List.map (g >> f) x`." + + let (returnCode, errors) = main [| "lint"; input |] + + // Check default config triggers the hint we're expecting to ignore later. + Assert.AreEqual(-1, returnCode) + CollectionAssert.AreEquivalent(set [ commonErrorMessage; disabledHintErrorMessage ], errors) + + let config = """ + { + "hints": { + "ignore": [ + "List.map f (List.map g x) ===> List.map (g >> f) x" + ] + } + } + """ + use config = new TemporaryFile(config, "json") + + let (returnCode, errors) = main [| "lint"; "--lint-config"; config.FileName; input |] + + Assert.AreEqual(-1, returnCode) + CollectionAssert.AreEquivalent(Set.singleton commonErrorMessage, errors) + + /// Regression for bug discovered during: https://github.com/fsprojects/FSharpLint/issues/466 + /// Adding a rule to the config was disabling other rules unless they're explicitly specified. + [] + member __.``Adding a rule to a custom config should not have side effects on other rules (from the default config).``() = + let config = """ + { + "exceptionNames": { + "enabled": false + } + } + """ + use config = new TemporaryFile(config, "json") + + let input = """ + type Signature = + abstract member Encoded : string + abstract member PathName : string + """ + + let (returnCode, errors) = main [| "lint"; "--lint-config"; config.FileName; input |] + + Assert.AreEqual(-1, returnCode) + Assert.AreEqual(set ["Consider changing `Signature` to be prefixed with `I`."], errors) open FSharpLint.Console.Program diff --git a/tests/FSharpLint.Core.Tests/Framework/TestConfiguration.fs b/tests/FSharpLint.Core.Tests/Framework/TestConfiguration.fs index 2da2f1738..950277c3f 100644 --- a/tests/FSharpLint.Core.Tests/Framework/TestConfiguration.fs +++ b/tests/FSharpLint.Core.Tests/Framework/TestConfiguration.fs @@ -1,6 +1,7 @@ module FSharpLint.Core.Tests.TestConfiguration open NUnit.Framework +open FSharpLint.Framework open FSharpLint.Framework.Configuration type System.String with @@ -110,4 +111,32 @@ type TestConfiguration() = """ let actualConfig = parseConfig config - Assert.AreEqual(expectedConfig.NoTabCharacters, actualConfig.NoTabCharacters) \ No newline at end of file + Assert.AreEqual(expectedConfig.NoTabCharacters, actualConfig.NoTabCharacters) + + [] + member __.``Combining config with empty one results in unchanged original config`` () = + let defaultConfig = Configuration.defaultConfiguration + + let combinedConfig = Configuration.combineConfigs defaultConfig Configuration.Zero + + Assert.AreEqual(defaultConfig, combinedConfig) + + [] + member __.``When combining one config with another non-empty values are overriden`` () = + let baseConfig = Configuration.Zero + + let partialConfig = { Configuration.Zero with NoTabCharacters = Some { Enabled = true; Config = None } } + + let combinedConfig = Configuration.combineConfigs baseConfig partialConfig + + Assert.AreEqual(combinedConfig.NoTabCharacters, partialConfig.NoTabCharacters) + + [] + member __.``When combining one config with another empty values are not overriden`` () = + let baseConfig = { Configuration.Zero with NoTabCharacters = Some { Enabled = true; Config = None } } + + let partialConfig = Configuration.Zero + + let combinedConfig = Configuration.combineConfigs baseConfig partialConfig + + Assert.AreEqual(combinedConfig.NoTabCharacters, baseConfig.NoTabCharacters)