From fe95771b6931c22519fa1180b0af2723c7fa78c2 Mon Sep 17 00:00:00 2001 From: sheridanchris Date: Tue, 14 Nov 2023 20:33:07 -0600 Subject: [PATCH 1/3] Add an analyzer that detects Option.get usage --- docs/suggestion/006.md | 30 ++++++++++++++++ src/Ionide.Analyzers/Ionide.Analyzers.fsproj | 1 + ...ceOptionGetWithGracefulHandlingAnalyzer.fs | 35 +++++++++++++++++++ .../Ionide.Analyzers.Tests.fsproj | 1 + ...ionGetWithGracefulHandlingAnalyzerTests.fs | 32 +++++++++++++++++ 5 files changed, 99 insertions(+) create mode 100644 docs/suggestion/006.md create mode 100644 src/Ionide.Analyzers/Suggestion/ReplaceOptionGetWithGracefulHandlingAnalyzer.fs create mode 100644 tests/Ionide.Analyzers.Tests/Suggestion/ReplaceOptionGetWithGracefulHandlingAnalyzerTests.fs diff --git a/docs/suggestion/006.md b/docs/suggestion/006.md new file mode 100644 index 0000000..1e17087 --- /dev/null +++ b/docs/suggestion/006.md @@ -0,0 +1,30 @@ +--- +title: ReplaceOptionGetWithGracefulHandling +category: suggestion +categoryindex: 2 +index: 5 +--- +# OptionGetAnalyzer + +## Problem +The option type is used to model a potentially missing value. Unwrapping an `'a option` into an `'a` by using `Option.get` circumvents the graceful handling of the `None` case. You should unwrap the value using a function like `Option.defaultValue` or pattern matching instead. + +```fsharp +let option = None +let value = Option.get option // Triggers analyzer +``` + +## Fix + +Gracefully handle the option value by accounting for both cases. + +```fsharp +let option = None + +match option with +| Some value -> () +| None -> () + +// or +Option.defaultValue () option +``` diff --git a/src/Ionide.Analyzers/Ionide.Analyzers.fsproj b/src/Ionide.Analyzers/Ionide.Analyzers.fsproj index 88e54aa..fd794e2 100644 --- a/src/Ionide.Analyzers/Ionide.Analyzers.fsproj +++ b/src/Ionide.Analyzers/Ionide.Analyzers.fsproj @@ -17,6 +17,7 @@ + diff --git a/src/Ionide.Analyzers/Suggestion/ReplaceOptionGetWithGracefulHandlingAnalyzer.fs b/src/Ionide.Analyzers/Suggestion/ReplaceOptionGetWithGracefulHandlingAnalyzer.fs new file mode 100644 index 0000000..7075a87 --- /dev/null +++ b/src/Ionide.Analyzers/Suggestion/ReplaceOptionGetWithGracefulHandlingAnalyzer.fs @@ -0,0 +1,35 @@ +module Ionide.Analyzers.Suggestion.ReplaceOptionGetWithGracefulHandlingAnalyzer + +open FSharp.Analyzers.SDK +open FSharp.Analyzers.SDK.TASTCollecting +open FSharp.Compiler.Symbols +open FSharp.Compiler.Text + +[] +let optionGetAnalyzer (ctx: CliContext) = + async { + let messages = ResizeArray() + + let walker = + { new TypedTreeCollectorBase() with + override x.WalkCall _ (mfv: FSharpMemberOrFunctionOrValue) _ _ (args: FSharpExpr list) (m: range) = + if mfv.FullName = "Microsoft.FSharp.Core.Option.get" && args.Length = 1 then + messages.Add + { + Type = "ReplaceOptionGetWithGracefulHandlingAnalyzer" + Message = "Replace Option.get with graceful handling of each case." + Code = "IONIDE-006" + Severity = Severity.Hint + Range = m + Fixes = [] + } + } + + match ctx.TypedTree with + | None -> return [] + | Some typedTree -> + walkTast walker typedTree + return Seq.toList messages + } diff --git a/tests/Ionide.Analyzers.Tests/Ionide.Analyzers.Tests.fsproj b/tests/Ionide.Analyzers.Tests/Ionide.Analyzers.Tests.fsproj index 9466366..bc58588 100644 --- a/tests/Ionide.Analyzers.Tests/Ionide.Analyzers.Tests.fsproj +++ b/tests/Ionide.Analyzers.Tests/Ionide.Analyzers.Tests.fsproj @@ -13,6 +13,7 @@ + diff --git a/tests/Ionide.Analyzers.Tests/Suggestion/ReplaceOptionGetWithGracefulHandlingAnalyzerTests.fs b/tests/Ionide.Analyzers.Tests/Suggestion/ReplaceOptionGetWithGracefulHandlingAnalyzerTests.fs new file mode 100644 index 0000000..293e46d --- /dev/null +++ b/tests/Ionide.Analyzers.Tests/Suggestion/ReplaceOptionGetWithGracefulHandlingAnalyzerTests.fs @@ -0,0 +1,32 @@ +module Ionide.Analyzers.Tests.Suggestion.ReplaceOptionGetWithGracefulHandlingAnalyzerTests + +open NUnit.Framework +open FSharp.Compiler.CodeAnalysis +open FSharp.Analyzers.SDK.Testing +open Ionide.Analyzers.Suggestion.ReplaceOptionGetWithGracefulHandlingAnalyzer + +let mutable projectOptions: FSharpProjectOptions = FSharpProjectOptions.zero + +[] +let Setup () = + task { + let! opts = mkOptionsFromProject "net7.0" [] + projectOptions <- opts + } + +[] +let ``Option.get is detected`` () = + async { + let source = + """ +module M + +let option = Some 10 +let value = Option.get option + """ + + let ctx = getContext projectOptions source + let! msgs = optionGetAnalyzer ctx + Assert.IsNotEmpty msgs + Assert.IsTrue(Assert.messageContains "Replace Option.get with graceful handling of each case." msgs[0]) + } From a108cf5e4a2260e5e987cc643ea4cdc350782361 Mon Sep 17 00:00:00 2001 From: sheridanchris Date: Wed, 15 Nov 2023 07:24:38 -0600 Subject: [PATCH 2/3] Update CHANGELOG.md --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a411391..5b604de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +## Unreleased + +### Added +* ReplaceOptionGetWithGracefulHandlingAnalyzer [#32](https://github.com/ionide/ionide-analyzers/pull/32) + ## 0.3.0 - 2023-11-13 ### Changed From 2c1bcf2f44d978fcae86a1354682bba03396ec4b Mon Sep 17 00:00:00 2001 From: sheridanchris Date: Wed, 15 Nov 2023 07:27:09 -0600 Subject: [PATCH 3/3] Bump version in CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b604de..164c497 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -## Unreleased +## 0.4.0 - 2023-11-15 ### Added * ReplaceOptionGetWithGracefulHandlingAnalyzer [#32](https://github.com/ionide/ionide-analyzers/pull/32)