Partial configuration

build.fsx

@@ -274,7 +274,6 @@ Target.create "SelfCheck" (fun _ ->
             "trailingWhitespaceOnLine"
 
             // TODO: we should enable at some point
-            "typePrefixing"
             "unnestedFunctionNames"
             "nestedFunctionNames"
             "nestedStatements"

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.
 

fsharplint.json

@@ -0,0 +1,8 @@
+{
+    "typePrefixing": {
+        "enabled": true,
+        "config": {
+            "mode": "Always"
+        }
+    }
+}

src/FSharpLint.Console/Program.fs

@@ -152,7 +152,6 @@ let private start (arguments:ParseResults<ToolArgs>) (toolsPath:Ionide.ProjInfo.
             | Some configPath -> FromFile configPath
             | None -> Default
 
-
         let lintParams =
             { CancellationToken = None
               ReceivedWarning = Some output.WriteWarning

src/FSharpLint.Core/Application/Configuration.fs

@@ -428,13 +428,18 @@ with
 
 // </Deprecated>
 
-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<Configuration>, 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
                 // </Deprecated>
 
-                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 =

src/FSharpLint.Core/Application/Lint.fs

@@ -424,22 +424,28 @@ 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
             with
             | :? 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.

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.
+    [<Test>]
+    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.
+    [<Test>]
+    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
 

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)
+        Assert.AreEqual(expectedConfig.NoTabCharacters, actualConfig.NoTabCharacters)
+
+    [<Test>]
+    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)
+
+    [<Test>]
+    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)
+
+    [<Test>]
+    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)