Merge pull request #33 from sheridanchris/option-unwrapping-analyzer

Unsafe option unwrapping analyzer

CHANGELOG.md

@@ -1,5 +1,10 @@
 # Changelog
 
+## Unreleased
+
+### Changed
+* Reworks `ReplaceOptionGetWithGracefulHandlingAnalyzer` to handle ValueOption's and `.Value` member access. 
+
 ## 0.4.0 - 2023-11-15
 
 ### Added

docs/suggestion/006.md

@@ -1,17 +1,18 @@
 ---
-title: ReplaceOptionGetWithGracefulHandling
+title: ReplaceOptionUnwrapWithGracefulHandling
 category: suggestion
 categoryindex: 2
 index: 5
 ---
-# OptionGetAnalyzer
+# HandleOptionGracefullyAnalyzer
 
 ## 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.
+The option type is used to model a potentially missing value. Unwrapping an `'a option` into an `'a` by using `Option.get` or `option.Value` 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 
+let value' = option.Value // Triggers analyzer
 ```
 
 ## Fix

src/Ionide.Analyzers/Ionide.Analyzers.fsproj

@@ -17,7 +17,7 @@
         <Compile Include="Suggestion\IgnoreFunctionAnalyzer.fs"/>
         <Compile Include="Suggestion\UnnamedDiscriminatedUnionFieldAnalyzer.fs"/>
         <Compile Include="Suggestion\EmptyStringAnalyzer.fs"/>
-        <Compile Include="Suggestion\ReplaceOptionGetWithGracefulHandlingAnalyzer.fs"/>
+        <Compile Include="Suggestion\HandleOptionGracefullyAnalyzer.fs"/>
         <Compile Include="Style\SquareBracketArrayAnalyzer.fs"/>
     </ItemGroup>
 

src/Ionide.Analyzers/Suggestion/HandleOptionGracefullyAnalyzer.fs

@@ -0,0 +1,45 @@
+module Ionide.Analyzers.Suggestion.HandleOptionGracefullyAnalyzer
+
+open System
+open FSharp.Analyzers.SDK
+open FSharp.Analyzers.SDK.TASTCollecting
+open FSharp.Compiler.Symbols
+open FSharp.Compiler.Text
+
+[<CliAnalyzer("HandleOptionGracefullyAnalyzer",
+              "Replace unsafe option unwrapping with graceful handling of each case.",
+              "https://ionide.io/ionide-analyzers/suggestion/006.html")>]
+let optionGetAnalyzer (ctx: CliContext) =
+    async {
+        let messages = ResizeArray<Message>()
+
+        let walker =
+            { new TypedTreeCollectorBase() with
+                override x.WalkCall _ (mfv: FSharpMemberOrFunctionOrValue) _ _ (args: FSharpExpr list) (m: range) =
+                    let fullyQualifiedCall =
+                        String.Join(".", mfv.DeclaringEntity.Value.FullName, mfv.DisplayName)
+
+                    if
+                        (mfv.FullName = "Microsoft.FSharp.Core.Option.get"
+                         || mfv.FullName = "Microsoft.FSharp.Core.ValueOption.get"
+                         || fullyQualifiedCall = "Microsoft.FSharp.Core.FSharpOption`1.Value"
+                         || fullyQualifiedCall = "Microsoft.FSharp.Core.FSharpValueOption`1.Value")
+                        && args.Length = 1
+                    then
+                        messages.Add
+                            {
+                                Type = "HandleOptionGracefully"
+                                Message = "Replace unsafe option unwrapping with graceful handling of each case."
+                                Code = "IONIDE-006"
+                                Severity = Severity.Warning
+                                Range = m
+                                Fixes = []
+                            }
+            }
+
+        match ctx.TypedTree with
+        | None -> return []
+        | Some typedTree ->
+            walkTast walker typedTree
+            return Seq.toList messages
+    }

tests/Ionide.Analyzers.Tests/Ionide.Analyzers.Tests.fsproj

@@ -13,7 +13,7 @@
     <Compile Include="Suggestion\IgnoreFunctionAnalyzerTests.fs" />
     <Compile Include="Suggestion\UnnamedDiscriminatedUnionFieldAnalyzerTests.fs" />
     <Compile Include="Suggestion\EmptyStringAnalyzerTests.fs" />
-    <Compile Include="Suggestion\ReplaceOptionGetWithGracefulHandlingAnalyzerTests.fs" />
+    <Compile Include="Suggestion\HandleOptionGracefullyAnalyzerTests.fs" />
     <Compile Include="Style\SquareBracketArrayAnalyzerTests.fs" />
     <Compile Include="Program.fs" />
   </ItemGroup>

tests/Ionide.Analyzers.Tests/Suggestion/HandleOptionGracefullyAnalyzerTests.fs

@@ -0,0 +1,86 @@
+module Ionide.Analyzers.Tests.Suggestion.HandleOptionGracefullyAnalyzerTests
+
+open NUnit.Framework
+open FSharp.Compiler.CodeAnalysis
+open FSharp.Analyzers.SDK.Testing
+open Ionide.Analyzers.Suggestion.HandleOptionGracefullyAnalyzer
+
+let mutable projectOptions: FSharpProjectOptions = FSharpProjectOptions.zero
+
+let messageString =
+    "Replace unsafe option unwrapping with graceful handling of each case."
+
+[<SetUp>]
+let Setup () =
+    task {
+        let! opts = mkOptionsFromProject "net7.0" []
+        projectOptions <- opts
+    }
+
+[<Test>]
+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 messageString msgs[0])
+    }
+
+[<Test>]
+let ``ValueOption.get is detected`` () =
+    async {
+        let source =
+            """
+module M
+
+let voption = ValueSome 10
+let value = ValueOption.get voption
+    """
+
+        let ctx = getContext projectOptions source
+        let! msgs = optionGetAnalyzer ctx
+        Assert.IsNotEmpty msgs
+        Assert.IsTrue(Assert.messageContains messageString msgs[0])
+    }
+
+[<Test>]
+let ``Option.Value member is detected`` () =
+    async {
+        let source =
+            """
+module M
+
+let option = Some 10
+let value = option.Value
+    """
+
+        let ctx = getContext projectOptions source
+        let! msgs = optionGetAnalyzer ctx
+        Assert.IsNotEmpty msgs
+        Assert.IsTrue(Assert.messageContains messageString msgs[0])
+    }
+
+[<Test>]
+let ``ValueOption.Value member is detected`` () =
+    async {
+        let source =
+            """
+module M
+
+let voption = ValueSome 10
+let value = voption.Value
+    """
+
+        let ctx = getContext projectOptions source
+        let! msgs = optionGetAnalyzer ctx
+        Assert.IsNotEmpty msgs
+        Assert.IsTrue(Assert.messageContains messageString msgs[0])
+    }