diff --git a/src/FSharp.Analyzers/FSharp.Analyzers.fsproj b/src/FSharp.Analyzers/FSharp.Analyzers.fsproj index 148a851..82f2e1e 100644 --- a/src/FSharp.Analyzers/FSharp.Analyzers.fsproj +++ b/src/FSharp.Analyzers/FSharp.Analyzers.fsproj @@ -12,6 +12,7 @@ + diff --git a/src/FSharp.Analyzers/JsonSerializerOptionsAnalyzer.fs b/src/FSharp.Analyzers/JsonSerializerOptionsAnalyzer.fs new file mode 100644 index 0000000..2583da4 --- /dev/null +++ b/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 = + + [] + let Code = "GRA-JSONOPTS-001" + + [] + let jsonSerializerOptionsAnalyzer : Analyzer = + fun ctx -> + async { + let state = ResizeArray () + + 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 + } diff --git a/src/FSharp.Analyzers/TASTCollection.fs b/src/FSharp.Analyzers/TASTCollection.fs new file mode 100644 index 0000000..637f063 --- /dev/null +++ b/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 diff --git a/tests/FSharp.Analyzers.Tests/FSharp.Analyzers.Tests.fsproj b/tests/FSharp.Analyzers.Tests/FSharp.Analyzers.Tests.fsproj index cfe24d9..4795f5d 100644 --- a/tests/FSharp.Analyzers.Tests/FSharp.Analyzers.Tests.fsproj +++ b/tests/FSharp.Analyzers.Tests/FSharp.Analyzers.Tests.fsproj @@ -12,6 +12,7 @@ + diff --git a/tests/FSharp.Analyzers.Tests/JsonSerializerOptionsAnalyzerTests.fs b/tests/FSharp.Analyzers.Tests/JsonSerializerOptionsAnalyzerTests.fs new file mode 100644 index 0000000..24c41c6 --- /dev/null +++ b/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 + + [] + 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 () + + [)>] + 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 + } diff --git a/tests/data/jsonserializeroptions/Deserialize call with ctor.fs b/tests/data/jsonserializeroptions/Deserialize call with ctor.fs new file mode 100644 index 0000000..b5a9d23 --- /dev/null +++ b/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(json, JsonSerializerOptions ()) + let _ = JsonSerializer.DeserializeAsync(jsonStream, JsonSerializerOptions ()) + let _ = JsonSerializer.DeserializeAsyncEnumerable(jsonStream, JsonSerializerOptions ()) + () diff --git a/tests/data/jsonserializeroptions/Deserialize call with ctor.fs.expected b/tests/data/jsonserializeroptions/Deserialize call with ctor.fs.expected new file mode 100644 index 0000000..ec7df68 --- /dev/null +++ b/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. diff --git a/tests/data/jsonserializeroptions/Deserialize calls with new ctor.fs b/tests/data/jsonserializeroptions/Deserialize calls with new ctor.fs new file mode 100644 index 0000000..60e5ab8 --- /dev/null +++ b/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(json, new JsonSerializerOptions ()) + let _ = JsonSerializer.DeserializeAsync(jsonStream, new JsonSerializerOptions ()) + let _ = JsonSerializer.DeserializeAsyncEnumerable(jsonStream, new JsonSerializerOptions ()) + () \ No newline at end of file diff --git a/tests/data/jsonserializeroptions/Deserialize calls with new ctor.fs.expected b/tests/data/jsonserializeroptions/Deserialize calls with new ctor.fs.expected new file mode 100644 index 0000000..ac9f895 --- /dev/null +++ b/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. diff --git a/tests/data/jsonserializeroptions/Serialize call with cached options.fs b/tests/data/jsonserializeroptions/Serialize call with cached options.fs new file mode 100644 index 0000000..8b08320 --- /dev/null +++ b/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(json, cachedOptions) + let _ = JsonSerializer.SerializeAsync(jsonStream, json, cachedOptions) + let _ = JsonSerializer.SerializeToDocument(json, cachedOptions) + let _ = JsonSerializer.SerializeToElement(json, cachedOptions) + let _ = JsonSerializer.SerializeToNode(json, cachedOptions) + let _ = JsonSerializer.SerializeToUtf8Bytes(json, cachedOptions) + () diff --git a/tests/data/jsonserializeroptions/Serialize call with cached options.fs.expected b/tests/data/jsonserializeroptions/Serialize call with cached options.fs.expected new file mode 100644 index 0000000..8b13789 --- /dev/null +++ b/tests/data/jsonserializeroptions/Serialize call with cached options.fs.expected @@ -0,0 +1 @@ + diff --git a/tests/data/jsonserializeroptions/Serialize call with ctor.fs b/tests/data/jsonserializeroptions/Serialize call with ctor.fs new file mode 100644 index 0000000..3047f2f --- /dev/null +++ b/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(json, new JsonSerializerOptions ()) + let _ = JsonSerializer.SerializeAsync(jsonStream, json, new JsonSerializerOptions ()) + let _ = JsonSerializer.SerializeToDocument(json, new JsonSerializerOptions ()) + let _ = JsonSerializer.SerializeToElement(json, new JsonSerializerOptions ()) + let _ = JsonSerializer.SerializeToNode(json, new JsonSerializerOptions ()) + let _ = JsonSerializer.SerializeToUtf8Bytes(json, new JsonSerializerOptions ()) + () diff --git a/tests/data/jsonserializeroptions/Serialize call with ctor.fs.expected b/tests/data/jsonserializeroptions/Serialize call with ctor.fs.expected new file mode 100644 index 0000000..1c9ae73 --- /dev/null +++ b/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. diff --git a/tests/data/jsonserializeroptions/Serialize calls with new ctor.fs b/tests/data/jsonserializeroptions/Serialize calls with new ctor.fs new file mode 100644 index 0000000..3047f2f --- /dev/null +++ b/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(json, new JsonSerializerOptions ()) + let _ = JsonSerializer.SerializeAsync(jsonStream, json, new JsonSerializerOptions ()) + let _ = JsonSerializer.SerializeToDocument(json, new JsonSerializerOptions ()) + let _ = JsonSerializer.SerializeToElement(json, new JsonSerializerOptions ()) + let _ = JsonSerializer.SerializeToNode(json, new JsonSerializerOptions ()) + let _ = JsonSerializer.SerializeToUtf8Bytes(json, new JsonSerializerOptions ()) + () diff --git a/tests/data/jsonserializeroptions/Serialize calls with new ctor.fs.expected b/tests/data/jsonserializeroptions/Serialize calls with new ctor.fs.expected new file mode 100644 index 0000000..1c9ae73 --- /dev/null +++ b/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.