diff --git a/build.fsx b/build.fsx index e5b433124..3810b0b82 100644 --- a/build.fsx +++ b/build.fsx @@ -246,8 +246,9 @@ Target.create "SelfCheck" (fun _ -> let consoleProj = Path.Combine(srcDir.FullName, "FSharpLint.Console", "FSharpLint.Console.fsproj") |> FileInfo let sol = Path.Combine(rootDir.FullName, solutionFileName) |> FileInfo + let configFilePath = Path.Combine(sol.DirectoryName, "fsharplint.json") - exec "dotnet" $"run --framework net9.0 lint %s{sol.FullName}" consoleProj.Directory.FullName + exec "dotnet" $"run --framework net9.0 lint --lint-config %s{configFilePath} %s{sol.FullName}" consoleProj.Directory.FullName printfn "Running self-check with default rules..." runLinter () diff --git a/docs/content/how-tos/rule-configuration.md b/docs/content/how-tos/rule-configuration.md index 50e819956..cd20adb0e 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..a35ac0ce5 --- /dev/null +++ b/fsharplint.json @@ -0,0 +1,10 @@ +{ + "genericTypesNames": { + "enabled": true, + "config": { + "naming": "PascalCase", + "underscores": "None", + "prefix": "T" + } + } +} diff --git a/src/FSharpLint.Core/Application/Configuration.fs b/src/FSharpLint.Core/Application/Configuration.fs index c5f3ee8f8..cdf4ea2c5 100644 --- a/src/FSharpLint.Core/Application/Configuration.fs +++ b/src/FSharpLint.Core/Application/Configuration.fs @@ -697,6 +697,43 @@ 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 combineHints (baseHints: HintConfig) (overridingHints: 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 baseHints.add overridingHints.add + ignore = mergeLists baseHints.ignore overridingHints.ignore + } + + 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 = @@ -792,9 +829,14 @@ let flattenConfig (config:Configuration) = |> Array.toList |> MergeSyntaxTrees.mergeHints - let getOrEmptyList hints = Option.defaultValue Array.empty hints - let deprecatedAllRules = + let flattenHints (hintsConfig: HintConfig) = + let ignores = + Option.defaultValue Array.empty hintsConfig.ignore + |> Set.ofArray + Option.defaultValue Array.empty hintsConfig.add + |> Array.filter (fun hint -> not <| ignores.Contains hint) + Array.concat [| // Deprecated grouped configs. TODO: remove in next major release @@ -803,7 +845,7 @@ let flattenConfig (config:Configuration) = config.typography |> Option.map (fun typographyConfig -> typographyConfig.Flatten()) |> Option.toArray |> Array.concat // - config.Hints |> Option.map (fun hintsConfig -> HintMatcher.rule { HintMatcher.Config.HintTrie = parseHints (getOrEmptyList hintsConfig.add) }) |> Option.toArray + config.Hints |> Option.map (fun hintsConfig -> HintMatcher.rule { HintMatcher.Config.HintTrie = parseHints (flattenHints hintsConfig) }) |> Option.toArray |] let allPossibleRules = diff --git a/src/FSharpLint.Core/Application/Lint.fs b/src/FSharpLint.Core/Application/Lint.fs index 4e0c7db22..4088e832e 100644 --- a/src/FSharpLint.Core/Application/Lint.fs +++ b/src/FSharpLint.Core/Application/Lint.fs @@ -397,15 +397,7 @@ module Lint = /// Gets a FSharpLint Configuration based on the provided ConfigurationParam. let 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 $"./{Configuration.SettingsFileName}" |> Ok @@ -413,6 +405,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 22d4e8bd0..61bf8ac0e 100644 --- a/tests/FSharpLint.Console.Tests/TestApp.fs +++ b/tests/FSharpLint.Console.Tests/TestApp.fs @@ -65,7 +65,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 } } @@ -118,6 +118,64 @@ type TestConsoleApplication() = Assert.AreEqual(int ExitCode.Failure, 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 configFile = new TemporaryFile(config, "json") + + let (returnCode, errors) = main [| "lint"; "--lint-config"; configFile.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 configFile = new TemporaryFile(config, "json") + + let input = """ + type Signature = + abstract member Encoded : string + abstract member PathName : string + """ + + let (returnCode, errors) = main [| "lint"; "--lint-config"; configFile.FileName; input |] + + Assert.AreEqual(-1, returnCode) + Assert.AreEqual(set ["Consider changing `Signature` to be prefixed with `I`."], errors) [] type TestFileTypeInference() = diff --git a/tests/FSharpLint.Core.Tests/Framework/TestConfiguration.fs b/tests/FSharpLint.Core.Tests/Framework/TestConfiguration.fs index aa7d97ec7..164924782 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)