From 33f608d013eb7f3776304e569111c1e1c48ace29 Mon Sep 17 00:00:00 2001 From: webwarrior Date: Mon, 18 Dec 2023 14:16:46 +0100 Subject: [PATCH 1/9] Core,Framework,Tests: implemented partial configs Implemented partial configs and added tests for combining configs. --- .../Application/Configuration.fs | 19 ++++++++++++ src/FSharpLint.Core/Application/Lint.fs | 24 +++++++++++--- src/FSharpLint.Core/Application/Lint.fsi | 2 ++ .../Framework/TestConfiguration.fs | 31 ++++++++++++++++++- 4 files changed, 70 insertions(+), 6 deletions(-) diff --git a/src/FSharpLint.Core/Application/Configuration.fs b/src/FSharpLint.Core/Application/Configuration.fs index 86d784e9e..a7eab7f92 100644 --- a/src/FSharpLint.Core/Application/Configuration.fs +++ b/src/FSharpLint.Core/Application/Configuration.fs @@ -663,6 +663,25 @@ let loadConfig (configPath:string) = File.ReadAllText configPath |> parseConfig +/// 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 + + FSharp.Reflection.FSharpValue.MakeRecord(typeof, resultingRecordFields) + :?> Configuration + /// 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 = diff --git a/src/FSharpLint.Core/Application/Lint.fs b/src/FSharpLint.Core/Application/Lint.fs index da988cc96..0ca1c18e7 100644 --- a/src/FSharpLint.Core/Application/Lint.fs +++ b/src/FSharpLint.Core/Application/Lint.fs @@ -384,6 +384,8 @@ module Lint = type ConfigurationParam = | Configuration of Configuration | FromFile of configPath:string + /// Partial config. Contains only differences from default config. + | FromFilePartial of configPath:string /// Tries to load the config from file `fsharplint.json`. /// If this file doesn't exist or is invalid, falls back to the default configuration. | Default @@ -424,6 +426,15 @@ module Lint = /// Gets a FSharpLint Configuration based on the provided ConfigurationParam. let private getConfig (configParam:ConfigurationParam) = + 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 -> @@ -432,14 +443,17 @@ module Lint = |> Ok with | ex -> Error (string ex) - | Default -> + | FromFilePartial filePath -> try - Configuration.loadConfig "./fsharplint.json" - |> Ok + match getDefault () with + | Ok defaultConfig -> + let partialConfig = Configuration.loadConfig filePath + Configuration.combineConfigs defaultConfig partialConfig |> Ok + | Error(_) as err -> err with - | :? System.IO.FileNotFoundException -> - Ok Configuration.defaultConfiguration | 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/src/FSharpLint.Core/Application/Lint.fsi b/src/FSharpLint.Core/Application/Lint.fsi index 29ddd3332..c2e642f7e 100644 --- a/src/FSharpLint.Core/Application/Lint.fsi +++ b/src/FSharpLint.Core/Application/Lint.fsi @@ -39,6 +39,8 @@ module Lint = type ConfigurationParam = | Configuration of Configuration | FromFile of configPath:string + /// Partial config. Contains only differences from default config. + | FromFilePartial of configPath:string /// Tries to load the config from file `fsharplint.json`. /// If this file doesn't exist or is invalid, falls back to the default configuration. | Default 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) From 7205c889f366b81dd2aad0d20b78bd3be99e0aa0 Mon Sep 17 00:00:00 2001 From: webwarrior Date: Mon, 18 Dec 2023 14:58:48 +0100 Subject: [PATCH 2/9] Console: add option to use partial config Added command line option "--partial-config" that when used, treats config file specified using "--lint-config" as partial config (contains only configs that should be overriden; rules not specified in it use default values). --- src/FSharpLint.Console/Program.fs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/FSharpLint.Console/Program.fs b/src/FSharpLint.Console/Program.fs index b5e59f28a..8631e5829 100644 --- a/src/FSharpLint.Console/Program.fs +++ b/src/FSharpLint.Console/Program.fs @@ -39,6 +39,7 @@ with and private LintArgs = | [] Target of target:string | [] Lint_Config of lintConfig:string + | Partial_Config | File_Type of FileType // fsharplint:enable UnionDefinitionIndentation with @@ -48,6 +49,7 @@ with | Target _ -> "Input to lint." | File_Type _ -> "Input type the linter will run against. If this is not set, the file type will be inferred from the file extension." | Lint_Config _ -> "Path to the config for the lint." + | Partial_Config -> "Indicates that specified lintConfig is a partial config and is applied as a diff to default config." // fsharplint:enable UnionCasesNames /// Expands a wildcard pattern to a list of matching files. @@ -147,12 +149,14 @@ let private start (arguments:ParseResults) (toolsPath:Ionide.ProjInfo. let lintConfig = lintArgs.TryGetResult Lint_Config + let isPartialConfig = lintArgs.TryGetResult Partial_Config |> Option.isSome + let configParam = match lintConfig with + | Some configPath when isPartialConfig -> FromFilePartial configPath | Some configPath -> FromFile configPath | None -> Default - let lintParams = { CancellationToken = None ReceivedWarning = Some output.WriteWarning From 6c5135050f05c516c909e4311f777a59ac9b18b6 Mon Sep 17 00:00:00 2001 From: Matt Mcveigh Date: Sun, 25 Oct 2020 20:31:05 +0000 Subject: [PATCH 3/9] Add regression test for deeper issue Co-authored-by: webwarrior-ws --- tests/FSharpLint.Console.Tests/TestApp.fs | 24 +++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/FSharpLint.Console.Tests/TestApp.fs b/tests/FSharpLint.Console.Tests/TestApp.fs index ec55109c1..950296788 100644 --- a/tests/FSharpLint.Console.Tests/TestApp.fs +++ b/tests/FSharpLint.Console.Tests/TestApp.fs @@ -117,6 +117,30 @@ type TestConsoleApplication() = Assert.AreEqual(-1, returnCode) Assert.AreEqual(set ["Use prefix syntax for generic type."], 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 From 8e5ca2c0f8cd5e98b6b8a29d748c0bbfac1c9859 Mon Sep 17 00:00:00 2001 From: webwarrior-ws Date: Mon, 5 Jan 2026 11:18:25 +0100 Subject: [PATCH 4/9] Core,Console: use --lint-config as --partial-config And remove --partial-config flag. Configs will now be always partial. --- src/FSharpLint.Console/Program.fs | 5 ----- src/FSharpLint.Core/Application/Lint.fs | 8 -------- src/FSharpLint.Core/Application/Lint.fsi | 2 -- 3 files changed, 15 deletions(-) diff --git a/src/FSharpLint.Console/Program.fs b/src/FSharpLint.Console/Program.fs index 8631e5829..4c3da483e 100644 --- a/src/FSharpLint.Console/Program.fs +++ b/src/FSharpLint.Console/Program.fs @@ -39,7 +39,6 @@ with and private LintArgs = | [] Target of target:string | [] Lint_Config of lintConfig:string - | Partial_Config | File_Type of FileType // fsharplint:enable UnionDefinitionIndentation with @@ -49,7 +48,6 @@ with | Target _ -> "Input to lint." | File_Type _ -> "Input type the linter will run against. If this is not set, the file type will be inferred from the file extension." | Lint_Config _ -> "Path to the config for the lint." - | Partial_Config -> "Indicates that specified lintConfig is a partial config and is applied as a diff to default config." // fsharplint:enable UnionCasesNames /// Expands a wildcard pattern to a list of matching files. @@ -149,11 +147,8 @@ let private start (arguments:ParseResults) (toolsPath:Ionide.ProjInfo. let lintConfig = lintArgs.TryGetResult Lint_Config - let isPartialConfig = lintArgs.TryGetResult Partial_Config |> Option.isSome - let configParam = match lintConfig with - | Some configPath when isPartialConfig -> FromFilePartial configPath | Some configPath -> FromFile configPath | None -> Default diff --git a/src/FSharpLint.Core/Application/Lint.fs b/src/FSharpLint.Core/Application/Lint.fs index 0ca1c18e7..0195a07a2 100644 --- a/src/FSharpLint.Core/Application/Lint.fs +++ b/src/FSharpLint.Core/Application/Lint.fs @@ -384,8 +384,6 @@ module Lint = type ConfigurationParam = | Configuration of Configuration | FromFile of configPath:string - /// Partial config. Contains only differences from default config. - | FromFilePartial of configPath:string /// Tries to load the config from file `fsharplint.json`. /// If this file doesn't exist or is invalid, falls back to the default configuration. | Default @@ -438,12 +436,6 @@ module Lint = match configParam with | Configuration config -> Ok config | FromFile filePath -> - try - Configuration.loadConfig filePath - |> Ok - with - | ex -> Error (string ex) - | FromFilePartial filePath -> try match getDefault () with | Ok defaultConfig -> diff --git a/src/FSharpLint.Core/Application/Lint.fsi b/src/FSharpLint.Core/Application/Lint.fsi index c2e642f7e..29ddd3332 100644 --- a/src/FSharpLint.Core/Application/Lint.fsi +++ b/src/FSharpLint.Core/Application/Lint.fsi @@ -39,8 +39,6 @@ module Lint = type ConfigurationParam = | Configuration of Configuration | FromFile of configPath:string - /// Partial config. Contains only differences from default config. - | FromFilePartial of configPath:string /// Tries to load the config from file `fsharplint.json`. /// If this file doesn't exist or is invalid, falls back to the default configuration. | Default From 29312db0b27274a4ec2960a653552ec09dd5b6c1 Mon Sep 17 00:00:00 2001 From: webwarrior-ws Date: Mon, 5 Jan 2026 11:42:17 +0100 Subject: [PATCH 5/9] Tests(Console): fix test for disabled rule By using camelCase rule name name in config instead of PascalCase (camelCase is proper rule naming scheme in fsharplint.json). --- tests/FSharpLint.Console.Tests/TestApp.fs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/FSharpLint.Console.Tests/TestApp.fs b/tests/FSharpLint.Console.Tests/TestApp.fs index 950296788..af6f7bae5 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 } } From b8974d1d51b1852772d6b33eff47be58bf1a5bae Mon Sep 17 00:00:00 2001 From: webwarrior-ws Date: Mon, 5 Jan 2026 11:50:06 +0100 Subject: [PATCH 6/9] docs: updated documentation about configs To reflect change in how config is applied. --- docs/content/how-tos/rule-configuration.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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. From a0977fcdf592df2b9b14cdebde0a6be8d3806d94 Mon Sep 17 00:00:00 2001 From: Matt Mcveigh Date: Sun, 25 Oct 2020 18:38:28 +0000 Subject: [PATCH 7/9] Ensure ignoring a hint from config works Co-authored-by: webwarrior-ws --- tests/FSharpLint.Console.Tests/TestApp.fs | 34 +++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/FSharpLint.Console.Tests/TestApp.fs b/tests/FSharpLint.Console.Tests/TestApp.fs index af6f7bae5..60184f0ad 100644 --- a/tests/FSharpLint.Console.Tests/TestApp.fs +++ b/tests/FSharpLint.Console.Tests/TestApp.fs @@ -118,6 +118,40 @@ 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. [] From 5360bb6886a3da8535c520856336c8d7290174d4 Mon Sep 17 00:00:00 2001 From: webwarrior-ws Date: Mon, 5 Jan 2026 12:45:56 +0100 Subject: [PATCH 8/9] Core: combine hint configs When combining configs, if both base and overriding configs have Hints config, combine `add` and `ignore` arrays in HintConfig objects instead of totally overriding base hint config by overriding one. Also actually use `ignore` array to filter out available hints when flattening config. Before, `ignore` was not used at all. Fixes https://github.com/fsprojects/FSharpLint/issues/466 --- .../Application/Configuration.fs | 35 +++++++++++++++---- 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/src/FSharpLint.Core/Application/Configuration.fs b/src/FSharpLint.Core/Application/Configuration.fs index a7eab7f92..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,17 @@ 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 = @@ -678,9 +694,16 @@ let combineConfigs (baseConfig: Configuration) (overridingConfig: Configuration) overridingValue) baseFields partialConfigFields - - FSharp.Reflection.FSharpValue.MakeRecord(typeof, resultingRecordFields) - :?> Configuration + + 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. @@ -771,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 = From c4705251e65317e636d35eb1aba14ad98ec049ad Mon Sep 17 00:00:00 2001 From: webwarrior-ws Date: Mon, 5 Jan 2026 13:34:02 +0100 Subject: [PATCH 9/9] build.fsx,fsharplint.json: enable TypePrefixing rule for SelfCheck With Mode=Always. --- build.fsx | 1 - fsharplint.json | 8 ++++++++ 2 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 fsharplint.json 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/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" + } + } +}