Skip to content

Commit

Permalink
Merge pull request #32 from sheridanchris/option-get-analyzer
Browse files Browse the repository at this point in the history
Add an analyzer that detects Option.get usage
  • Loading branch information
nojaf authored Nov 15, 2023
2 parents a8a548f + 2c1bcf2 commit ab29042
Show file tree
Hide file tree
Showing 6 changed files with 104 additions and 0 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# Changelog

## 0.4.0 - 2023-11-15

### Added
* ReplaceOptionGetWithGracefulHandlingAnalyzer [#32](https://github.com/ionide/ionide-analyzers/pull/32)

## 0.3.0 - 2023-11-13

### Changed
Expand Down
30 changes: 30 additions & 0 deletions docs/suggestion/006.md
Original file line number Diff line number Diff line change
@@ -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
```
1 change: 1 addition & 0 deletions src/Ionide.Analyzers/Ionide.Analyzers.fsproj
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
<Compile Include="Suggestion\IgnoreFunctionAnalyzer.fs"/>
<Compile Include="Suggestion\UnnamedDiscriminatedUnionFieldAnalyzer.fs"/>
<Compile Include="Suggestion\EmptyStringAnalyzer.fs"/>
<Compile Include="Suggestion\ReplaceOptionGetWithGracefulHandlingAnalyzer.fs"/>
<Compile Include="Style\SquareBracketArrayAnalyzer.fs"/>
</ItemGroup>

Expand Down
Original file line number Diff line number Diff line change
@@ -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

[<CliAnalyzer("ReplaceOptionGetWithGracefulHandlingAnalyzer",
"Replace Option.get 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) =
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
}
1 change: 1 addition & 0 deletions tests/Ionide.Analyzers.Tests/Ionide.Analyzers.Tests.fsproj
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
<Compile Include="Suggestion\IgnoreFunctionAnalyzerTests.fs" />
<Compile Include="Suggestion\UnnamedDiscriminatedUnionFieldAnalyzerTests.fs" />
<Compile Include="Suggestion\EmptyStringAnalyzerTests.fs" />
<Compile Include="Suggestion\ReplaceOptionGetWithGracefulHandlingAnalyzerTests.fs" />
<Compile Include="Style\SquareBracketArrayAnalyzerTests.fs" />
<Compile Include="Program.fs" />
</ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
@@ -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

[<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 "Replace Option.get with graceful handling of each case." msgs[0])
}

0 comments on commit ab29042

Please sign in to comment.