JsonSerializerOptions analyzer (#4)

* - add a JsonSerializerOptions analyzer
- refactor testing code
- add tests for new analyzer

* use net6.0

* update package.lock.json files

* Revert "use net6.0"

This reverts commit b4cab32.

* Add proj reference after split in SDK

* Check the Assembly of the called mfv and also of the called ctor

* Move TAST handling to it's own module and start the ground work to have different handlers for the various parts of the TAST an analyzer might want to handle.

* use the set datatype for method names

* use code with analyzer-specific namespace

src/FSharp.Analyzers/FSharp.Analyzers.fsproj

@@ -12,6 +12,7 @@
         <Compile Include="ASTCollecting.fs"/>
         <Compile Include="TASTCollecting.fs" />
         <Compile Include="StringAnalyzers.fs"/>
+        <Compile Include="JsonSerializerOptionsAnalyzer.fs" />
     </ItemGroup>
 
     <ItemGroup>

src/FSharp.Analyzers/JsonSerializerOptionsAnalyzer.fs

@@ -0,0 +1,78 @@
+namespace ``G-Research``.FSharp.Analyzers
+
+open System
+open FSharp.Analyzers.SDK
+open FSharp.Compiler.Symbols
+open FSharp.Compiler.Text
+open FSharp.Compiler.Symbols.FSharpExprPatterns
+open TASTCollection
+
+module JsonSerializerOptionsAnalyzer =
+
+    [<Literal>]
+    let Code = "GRA-JSONOPTS-001"
+
+    [<CliAnalyzer "JsonSerializerOptionsAnalyzer">]
+    let jsonSerializerOptionsAnalyzer : Analyzer<CliContext> =
+        fun ctx ->
+            async {
+                let state = ResizeArray<range> ()
+
+                let namesToWarnAbount =
+                    set
+                        [
+                            "System.Text.Json.JsonSerializer.Deserialize"
+                            "System.Text.Json.JsonSerializer.DeserializeAsync"
+                            "System.Text.Json.JsonSerializer.DeserializeAsyncEnumerable"
+                            "System.Text.Json.JsonSerializer.Serialize"
+                            "System.Text.Json.JsonSerializer.SerializeAsync"
+                            "System.Text.Json.JsonSerializer.SerializeToDocument"
+                            "System.Text.Json.JsonSerializer.SerializeToElement"
+                            "System.Text.Json.JsonSerializer.SerializeToNode"
+                            "System.Text.Json.JsonSerializer.SerializeToUtf8Bytes"
+                        ]
+
+                let handler : Handler =
+                    let callHandler (range : range) (m : FSharpMemberOrFunctionOrValue) (args : FSharpExpr list) =
+                        let name = String.Join (".", m.DeclaringEntity.Value.FullName, m.DisplayName)
+                        let assemblyName = "System.Text.Json"
+
+                        let containsSerOptsCtorCall =
+                            args
+                            |> List.exists (
+                                function
+                                | NewObject (objType, _, _) when
+                                    objType.FullName = "System.Text.Json.JsonSerializerOptions"
+                                    && objType.Assembly.SimpleName = assemblyName
+                                    ->
+                                    true
+                                | _ -> false
+                            )
+
+                        if
+                            m.Assembly.SimpleName = assemblyName
+                            && Set.contains name namesToWarnAbount
+                            && containsSerOptsCtorCall
+                        then
+                            state.Add range
+
+                    Handler.CallHandler callHandler
+
+                match ctx.TypedTree with
+                | None -> ()
+                | Some typedTree -> typedTree.Declarations |> List.iter (visitDeclaration handler)
+
+                return
+                    state
+                    |> Seq.map (fun r ->
+                        {
+                            Type = "JsonSerializerOptions analyzer"
+                            Message = "JsonSerializerOptions instances should be cached."
+                            Code = Code
+                            Severity = Warning
+                            Range = r
+                            Fixes = []
+                        }
+                    )
+                    |> Seq.toList
+            }

src/FSharp.Analyzers/TASTCollection.fs

@@ -0,0 +1,110 @@
+namespace ``G-Research``.FSharp.Analyzers
+
+module TASTCollection =
+
+    open FSharp.Compiler.Symbols
+    open FSharp.Compiler.Text
+    open FSharp.Compiler.Symbols.FSharpExprPatterns
+
+    type Handler = | CallHandler of (range -> FSharpMemberOrFunctionOrValue -> FSharpExpr list -> unit)
+
+    let rec visitExpr (handler : Handler) (e : FSharpExpr) =
+        match e with
+        | AddressOf lvalueExpr -> visitExpr handler lvalueExpr
+        | AddressSet (lvalueExpr, rvalueExpr) ->
+            visitExpr handler lvalueExpr
+            visitExpr handler rvalueExpr
+        | Application (funcExpr, _typeArgs, argExprs) ->
+            visitExpr handler funcExpr
+            visitExprs handler argExprs
+        | Call (objExprOpt, memberOrFunc, _typeArgs1, _typeArgs2, argExprs) ->
+            match handler with
+            | CallHandler f -> f e.Range memberOrFunc argExprs
+
+            visitObjArg handler objExprOpt
+            visitExprs handler argExprs
+        | Coerce (_targetType, inpExpr) -> visitExpr handler inpExpr
+        | FastIntegerForLoop (startExpr, limitExpr, consumeExpr, _isUp, _debugPointAtFor, _debugPointAtInOrTo) ->
+            visitExpr handler startExpr
+            visitExpr handler limitExpr
+            visitExpr handler consumeExpr
+        | ILAsm (_asmCode, _typeArgs, argExprs) -> visitExprs handler argExprs
+        | ILFieldGet (objExprOpt, _fieldType, _fieldName) -> visitObjArg handler objExprOpt
+        | ILFieldSet (objExprOpt, _fieldType, _fieldName, _valueExpr) -> visitObjArg handler objExprOpt
+        | IfThenElse (guardExpr, thenExpr, elseExpr) ->
+            visitExpr handler guardExpr
+            visitExpr handler thenExpr
+            visitExpr handler elseExpr
+        | Lambda (_lambdaVar, bodyExpr) -> visitExpr handler bodyExpr
+        | Let ((_bindingVar, bindingExpr, _debugPointAtBinding), bodyExpr) ->
+            visitExpr handler bindingExpr
+            visitExpr handler bodyExpr
+        | LetRec (recursiveBindings, bodyExpr) ->
+            let recursiveBindings' =
+                recursiveBindings |> List.map (fun (mfv, expr, _dp) -> (mfv, expr))
+
+            List.iter (snd >> visitExpr handler) recursiveBindings'
+            visitExpr handler bodyExpr
+        | NewArray (_arrayType, argExprs) -> visitExprs handler argExprs
+        | NewDelegate (_delegateType, delegateBodyExpr) -> visitExpr handler delegateBodyExpr
+        | NewObject (_objType, _typeArgs, argExprs) -> visitExprs handler argExprs
+        | NewRecord (_recordType, argExprs) -> visitExprs handler argExprs
+        | NewTuple (_tupleType, argExprs) -> visitExprs handler argExprs
+        | NewUnionCase (_unionType, _unionCase, argExprs) -> visitExprs handler argExprs
+        | Quote quotedExpr -> visitExpr handler quotedExpr
+        | FSharpFieldGet (objExprOpt, _recordOrClassType, _fieldInfo) -> visitObjArg handler objExprOpt
+        | FSharpFieldSet (objExprOpt, _recordOrClassType, _fieldInfo, argExpr) ->
+            visitObjArg handler objExprOpt
+            visitExpr handler argExpr
+        | Sequential (firstExpr, secondExpr) ->
+            visitExpr handler firstExpr
+            visitExpr handler secondExpr
+        | TryFinally (bodyExpr, finalizeExpr, _debugPointAtTry, _debugPointAtFinally) ->
+            visitExpr handler bodyExpr
+            visitExpr handler finalizeExpr
+        | TryWith (bodyExpr, _, _, _catchVar, catchExpr, _debugPointAtTry, _debugPointAtWith) ->
+            visitExpr handler bodyExpr
+            visitExpr handler catchExpr
+        | TupleGet (_tupleType, _tupleElemIndex, tupleExpr) -> visitExpr handler tupleExpr
+        | DecisionTree (decisionExpr, decisionTargets) ->
+            visitExpr handler decisionExpr
+            List.iter (snd >> visitExpr handler) decisionTargets
+        | DecisionTreeSuccess (_decisionTargetIdx, decisionTargetExprs) -> visitExprs handler decisionTargetExprs
+        | TypeLambda (_genericParam, bodyExpr) -> visitExpr handler bodyExpr
+        | TypeTest (_ty, inpExpr) -> visitExpr handler inpExpr
+        | UnionCaseSet (unionExpr, _unionType, _unionCase, _unionCaseField, valueExpr) ->
+            visitExpr handler unionExpr
+            visitExpr handler valueExpr
+        | UnionCaseGet (unionExpr, _unionType, _unionCase, _unionCaseField) -> visitExpr handler unionExpr
+        | UnionCaseTest (unionExpr, _unionType, _unionCase) -> visitExpr handler unionExpr
+        | UnionCaseTag (unionExpr, _unionType) -> visitExpr handler unionExpr
+        | ObjectExpr (_objType, baseCallExpr, overrides, interfaceImplementations) ->
+            visitExpr handler baseCallExpr
+            List.iter (visitObjMember handler) overrides
+            List.iter (snd >> List.iter (visitObjMember handler)) interfaceImplementations
+        | TraitCall (_sourceTypes, _traitName, _typeArgs, _typeInstantiation, _argTypes, argExprs) ->
+            visitExprs handler argExprs
+        | ValueSet (_valToSet, valueExpr) -> visitExpr handler valueExpr
+        | WhileLoop (guardExpr, bodyExpr, _debugPointAtWhile) ->
+            visitExpr handler guardExpr
+            visitExpr handler bodyExpr
+        | BaseValue _baseType -> ()
+        | DefaultValue _defaultType -> ()
+        | ThisValue _thisType -> ()
+        | Const (_constValueObj, _constType) -> ()
+        | Value _valueToGet -> ()
+        | _ -> ()
+
+    and visitExprs f exprs = List.iter (visitExpr f) exprs
+
+    and visitObjArg f objOpt = Option.iter (visitExpr f) objOpt
+
+    and visitObjMember f memb = visitExpr f memb.Body
+
+    let rec visitDeclaration f d =
+        match d with
+        | FSharpImplementationFileDeclaration.Entity (_e, subDecls) ->
+            for subDecl in subDecls do
+                visitDeclaration f subDecl
+        | FSharpImplementationFileDeclaration.MemberOrFunctionOrValue (_v, _vs, e) -> visitExpr f e
+        | FSharpImplementationFileDeclaration.InitAction e -> visitExpr f e

tests/FSharp.Analyzers.Tests/FSharp.Analyzers.Tests.fsproj

@@ -12,6 +12,7 @@
   <ItemGroup>
     <Compile Include="Testing.fs" />
     <Compile Include="StringAnalyzerTests.fs" />
+    <Compile Include="JsonSerializerOptionsAnalyzerTests.fs" />
   </ItemGroup>
 
   <ItemGroup>

tests/FSharp.Analyzers.Tests/JsonSerializerOptionsAnalyzerTests.fs

@@ -0,0 +1,58 @@
+namespace ``G-Research``.FSharp.Analyzers.Tests
+
+module JsonSerializerOptionsAnalyzerTests =
+
+    open System.Collections
+    open System.IO
+    open NUnit.Framework
+    open FSharp.Compiler.CodeAnalysis
+    open FSharp.Analyzers.SDK.Testing
+    open ``G-Research``.FSharp.Analyzers
+    open Testing
+
+    let mutable projectOptions : FSharpProjectOptions = FSharpProjectOptions.zero
+
+    [<SetUp>]
+    let Setup () =
+        task {
+            let! options =
+                mkOptionsFromProject
+                    "net7.0"
+                    [
+                        {
+                            Name = "System.Text.Json"
+                            Version = "7.0.3"
+                        }
+                    ]
+
+            projectOptions <- options
+        }
+
+    type TestCases() =
+        static member DataFolder = Path.Combine (__SOURCE_DIRECTORY__, "..", "data")
+
+        interface IEnumerable with
+            member _.GetEnumerator () : IEnumerator =
+                let jsonSerializerOptionsTests =
+                    Path.Combine (TestCases.DataFolder, "jsonserializeroptions")
+
+                Directory.EnumerateFiles (jsonSerializerOptionsTests, "*.fs")
+                |> Seq.map (fun f ->
+                    let fileName = Path.GetRelativePath (TestCases.DataFolder, f)
+                    [| fileName :> obj |]
+                )
+                |> fun s -> s.GetEnumerator ()
+
+    [<TestCaseSource(typeof<TestCases>)>]
+    let JsonSerializerOptionsTests (fileName : string) =
+        task {
+            let dataFolder = TestCases.DataFolder
+            let fileName = Path.Combine (dataFolder, fileName)
+
+            let! messages =
+                File.ReadAllText fileName
+                |> getContext projectOptions
+                |> JsonSerializerOptionsAnalyzer.jsonSerializerOptionsAnalyzer
+
+            do! assertExpected fileName messages
+        }

tests/data/jsonserializeroptions/Deserialize call with ctor.fs

@@ -0,0 +1,9 @@
+module M
+
+open System.Text.Json
+
+let f (json: string) (jsonStream: System.IO.Stream) =
+    let _ = JsonSerializer.Deserialize<string>(json, JsonSerializerOptions ())
+    let _ = JsonSerializer.DeserializeAsync<string>(jsonStream, JsonSerializerOptions ())
+    let _ = JsonSerializer.DeserializeAsyncEnumerable<string>(jsonStream, JsonSerializerOptions ())
+    ()

tests/data/jsonserializeroptions/Deserialize call with ctor.fs.expected

@@ -0,0 +1,3 @@
+GRA-JSONOPTS-001 | Warning | (6,12 - 6,78) | JsonSerializerOptions instances should be cached.
+GRA-JSONOPTS-001 | Warning | (7,12 - 7,89) | JsonSerializerOptions instances should be cached.
+GRA-JSONOPTS-001 | Warning | (8,12 - 8,99) | JsonSerializerOptions instances should be cached.

tests/data/jsonserializeroptions/Deserialize calls with new ctor.fs

@@ -0,0 +1,9 @@
+module M
+
+open System.Text.Json
+
+let f (json: string) (jsonStream: System.IO.Stream) =
+    let _ = JsonSerializer.Deserialize<string>(json, new JsonSerializerOptions ())
+    let _ = JsonSerializer.DeserializeAsync<string>(jsonStream, new JsonSerializerOptions ())
+    let _ = JsonSerializer.DeserializeAsyncEnumerable<string>(jsonStream, new JsonSerializerOptions ())
+    ()

tests/data/jsonserializeroptions/Deserialize calls with new ctor.fs.expected

@@ -0,0 +1,3 @@
+GRA-JSONOPTS-001 | Warning | (6,12 - 6,82) | JsonSerializerOptions instances should be cached.
+GRA-JSONOPTS-001 | Warning | (7,12 - 7,93) | JsonSerializerOptions instances should be cached.
+GRA-JSONOPTS-001 | Warning | (8,12 - 8,103) | JsonSerializerOptions instances should be cached.

tests/data/jsonserializeroptions/Serialize call with cached options.fs

@@ -0,0 +1,14 @@
+module M
+
+open System.Text.Json
+
+let cachedOptions = new JsonSerializerOptions ()
+
+let f (json: string) (jsonStream: System.IO.Stream) =
+    let _ = JsonSerializer.Serialize<string>(json, cachedOptions)
+    let _ = JsonSerializer.SerializeAsync<string>(jsonStream, json, cachedOptions)
+    let _ = JsonSerializer.SerializeToDocument<string>(json, cachedOptions)
+    let _ = JsonSerializer.SerializeToElement<string>(json, cachedOptions)
+    let _ = JsonSerializer.SerializeToNode<string>(json, cachedOptions)
+    let _ = JsonSerializer.SerializeToUtf8Bytes<string>(json, cachedOptions)
+    ()

tests/data/jsonserializeroptions/Serialize call with cached options.fs.expected

@@ -0,0 +1 @@
+

tests/data/jsonserializeroptions/Serialize call with ctor.fs

@@ -0,0 +1,12 @@
+module M
+
+open System.Text.Json
+
+let f (json: string) (jsonStream: System.IO.Stream) =
+    let _ = JsonSerializer.Serialize<string>(json, new JsonSerializerOptions ())
+    let _ = JsonSerializer.SerializeAsync<string>(jsonStream, json, new JsonSerializerOptions ())
+    let _ = JsonSerializer.SerializeToDocument<string>(json, new JsonSerializerOptions ())
+    let _ = JsonSerializer.SerializeToElement<string>(json, new JsonSerializerOptions ())
+    let _ = JsonSerializer.SerializeToNode<string>(json, new JsonSerializerOptions ())
+    let _ = JsonSerializer.SerializeToUtf8Bytes<string>(json, new JsonSerializerOptions ())
+    ()

tests/data/jsonserializeroptions/Serialize call with ctor.fs.expected

@@ -0,0 +1,6 @@
+GRA-JSONOPTS-001 | Warning | (6,12 - 6,80) | JsonSerializerOptions instances should be cached.
+GRA-JSONOPTS-001 | Warning | (7,12 - 7,97) | JsonSerializerOptions instances should be cached.
+GRA-JSONOPTS-001 | Warning | (8,12 - 8,90) | JsonSerializerOptions instances should be cached.
+GRA-JSONOPTS-001 | Warning | (9,12 - 9,89) | JsonSerializerOptions instances should be cached.
+GRA-JSONOPTS-001 | Warning | (10,12 - 10,86) | JsonSerializerOptions instances should be cached.
+GRA-JSONOPTS-001 | Warning | (11,12 - 11,91) | JsonSerializerOptions instances should be cached.

tests/data/jsonserializeroptions/Serialize calls with new ctor.fs

@@ -0,0 +1,12 @@
+module M
+
+open System.Text.Json
+
+let f (json: string) (jsonStream: System.IO.Stream) =
+    let _ = JsonSerializer.Serialize<string>(json, new JsonSerializerOptions ())
+    let _ = JsonSerializer.SerializeAsync<string>(jsonStream, json, new JsonSerializerOptions ())
+    let _ = JsonSerializer.SerializeToDocument<string>(json, new JsonSerializerOptions ())
+    let _ = JsonSerializer.SerializeToElement<string>(json, new JsonSerializerOptions ())
+    let _ = JsonSerializer.SerializeToNode<string>(json, new JsonSerializerOptions ())
+    let _ = JsonSerializer.SerializeToUtf8Bytes<string>(json, new JsonSerializerOptions ())
+    ()

tests/data/jsonserializeroptions/Serialize calls with new ctor.fs.expected

@@ -0,0 +1,6 @@
+GRA-JSONOPTS-001 | Warning | (6,12 - 6,80) | JsonSerializerOptions instances should be cached.
+GRA-JSONOPTS-001 | Warning | (7,12 - 7,97) | JsonSerializerOptions instances should be cached.
+GRA-JSONOPTS-001 | Warning | (8,12 - 8,90) | JsonSerializerOptions instances should be cached.
+GRA-JSONOPTS-001 | Warning | (9,12 - 9,89) | JsonSerializerOptions instances should be cached.
+GRA-JSONOPTS-001 | Warning | (10,12 - 10,86) | JsonSerializerOptions instances should be cached.
+GRA-JSONOPTS-001 | Warning | (11,12 - 11,91) | JsonSerializerOptions instances should be cached.