Skip to content

Commit

Permalink
add the LoggingTemplateMissingValuesAnalyzer
Browse files Browse the repository at this point in the history
  • Loading branch information
dawedawe committed Jan 10, 2024
1 parent 51ffd13 commit 3b39b53
Show file tree
Hide file tree
Showing 13 changed files with 280 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
* Add editor support to all analyzers. [#50](https://github.com/G-Research/fsharp-analyzers/pull/50)
* Add fix to VirtualCall Analyzer. [#51](https://github.com/G-Research/fsharp-analyzers/pull/51)
* Add fix to UnionCaseAnalyzer. [#51](https://github.com/G-Research/fsharp-analyzers/pull/51)
* Add new LoggingTemplateMissingValuesAnalyzer. [#53](https://github.com/G-Research/fsharp-analyzers/pull/53)

### Changed
* Update FSharp.Analyzers.SDK to `0.23.0`. [#51](https://github.com/G-Research/fsharp-analyzers/pull/45)
Expand Down
30 changes: 30 additions & 0 deletions docs/analyzers/LoggingTemplateMissingValuesAnalyzer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
---
title: LoggingTemplateMissingValuesAnalyzer
category: analyzers
categoryindex: 1
index: 10
---

# LoggingTemplateMissingValuesAnalyzer

## Problem

As param arrays are loosely typed it's easy to miss an expected templated value or even give too many.

```fsharp
do
use factory = LoggerFactory.Create(fun b -> b.AddConsole() |> ignore)
let logger: ILogger = factory.CreateLogger("Program")
logger.Log(LogLevel.Information, "first {one} second {two}", 23)
```

## Fix

Provide the correct number of values:

```fsharp
do
use factory = LoggerFactory.Create(fun b -> b.AddConsole() |> ignore)
let logger: ILogger = factory.CreateLogger("Program")
logger.Log(LogLevel.Information, "first {one} second {two}", 23, 42)
```
1 change: 1 addition & 0 deletions src/FSharp.Analyzers/FSharp.Analyzers.fsproj
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
<Compile Include="VirtualCallAnalyzer.fs" />
<Compile Include="LoggingArgFuncNotFullyAppliedAnalyzer.fsi" />
<Compile Include="LoggingArgFuncNotFullyAppliedAnalyzer.fs" />
<Compile Include="LoggingTemplateMissingValuesAnalyzer.fs" />
<Compile Include="TypeAnnotateStringFunctionAnalyzer.fs" />
<Compile Include="ImmutableCollectionEqualityAnalyzer.fs" />
<Compile Include="TypedInterpolatedStringsAnalyzer.fs" />
Expand Down
121 changes: 121 additions & 0 deletions src/FSharp.Analyzers/LoggingTemplateMissingValuesAnalyzer.fs
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
module GR.FSharp.Analyzers.LoggingTemplateMissingValuesAnalyzer

open System
open System.Text.RegularExpressions
open FSharp.Analyzers.SDK
open FSharp.Analyzers.SDK.TASTCollecting
open FSharp.Compiler.Symbols
open FSharp.Compiler.Symbols.FSharpExprPatterns
open FSharp.Compiler.Text

[<Literal>]
let Code = "GRA-LOGTEMPLMISSVALS-001"

let (|StringConst|_|) (e : FSharpExpr) =
let name = e.Type.ErasedType.TypeDefinition.TryGetFullName ()

match name, e with
| Some ("System.String"), Const (o, _type) when not (isNull o) -> Some (string o)
| _ -> None


let analyze (typedTree : FSharpImplementationFileContents) =
let state = ResizeArray<range> ()

let namesToWarnAbout =
set
[
"Microsoft.Extensions.Logging.LoggerExtensions.Log"
"Microsoft.Extensions.Logging.LoggerExtensions.LogCritical"
"Microsoft.Extensions.Logging.LoggerExtensions.LogDebug"
"Microsoft.Extensions.Logging.LoggerExtensions.LogError"
"Microsoft.Extensions.Logging.LoggerExtensions.LogInformation"
"Microsoft.Extensions.Logging.LoggerExtensions.LogTrace"
"Microsoft.Extensions.Logging.LoggerExtensions.LogWarning"
]

let pattern = @"(?<opening>{+)[a-zA-Z0-9]*(?<closing>}+)"
let regex = Regex (pattern)

let walker =
{ new TypedTreeCollectorBase() with
override _.WalkCall _ (m : FSharpMemberOrFunctionOrValue) _ _ (args : FSharpExpr list) (range : range) =
let name = String.Join (".", m.DeclaringEntity.Value.FullName, m.DisplayName)
let assemblyName = "Microsoft.Extensions.Logging.Abstractions"

let provided =
if args.Length >= 2 then
match List.tryLast args with
| Some (NewArray (_type, exprs)) -> List.length exprs
| _ -> 0
else
0

let expected =
let logString =
args
|> List.tryPick (
function
| StringConst (s) -> Some s
| _ -> None
)

match logString with
| Some s ->
let matches = regex.Matches (s)

let escapedMatches =
Seq.sumBy
(fun (matchItem : Match) ->
let opening = matchItem.Groups["opening"]
let closing = matchItem.Groups["closing"]
let isEscaped = opening.Value.Length % 2 = 0 && closing.Value.Length % 2 = 0
if isEscaped then 1 else 0
)
matches

matches.Count - escapedMatches
| None -> 0

if
m.Assembly.SimpleName = assemblyName
&& Set.contains name namesToWarnAbout
&& provided <> expected
then
state.Add range
}

walkTast walker typedTree

[
for range in state do
{
Type = "LoggingTemplateMissingValuesAnalyzer"
Message =
"The given values in your call to ILogger.Log{Warning, Error, ...} don't match the expected templated args."
Code = Code
Severity = Warning
Range = range
Fixes = []
}
]


[<Literal>]
let Name = "LoggingTemplateMissingValuesAnalyzer"

[<Literal>]
let ShortDescription =
"Checks if all templated args in a log message have been given values or too many values have been given"

[<Literal>]
let HelpUri =
"https://g-research.github.io/fsharp-analyzers/analyzers/LoggingTemplateMissingValuesAnalyzer.html"

[<CliAnalyzer(Name, ShortDescription, HelpUri)>]
let loggingTemplateMissingValuesCliAnalyzer : Analyzer<CliContext> =
fun (ctx : CliContext) -> async { return ctx.TypedTree |> Option.map analyze |> Option.defaultValue [] }

[<EditorAnalyzer(Name, ShortDescription, HelpUri)>]
let loggingTemplateMissingValuesEditorAnalyzer : Analyzer<EditorContext> =
fun (ctx : EditorContext) -> async { return ctx.TypedTree |> Option.map analyze |> Option.defaultValue [] }
1 change: 1 addition & 0 deletions tests/FSharp.Analyzers.Tests/FSharp.Analyzers.Tests.fsproj
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
<Compile Include="UnionCaseAnalyzerTests.fs" />
<Compile Include="VirtualCallAnalyzerTests.fs" />
<Compile Include="LoggingArgFuncNotFullyAppliedAnalyzerTests.fs" />
<Compile Include="LoggingTemplateMissingValuesAnalyzerTests.fs" />
<Compile Include="TypeAnnotateStringFunctionAnalyzerTests.fs" />
<Compile Include="ImmutableCollectionEqualityAnalyzerTests.fs" />
<Compile Include="TypedInterpolatedStringsAnalyzerTests.fs" />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
namespace GR.FSharp.Analyzers.Tests

module LoggingTemplateMissingValuesAnalyzerTests =

open System.Collections
open System.IO
open NUnit.Framework
open FSharp.Compiler.CodeAnalysis
open FSharp.Analyzers.SDK.Testing
open GR.FSharp.Analyzers
open GR.FSharp.Analyzers.Tests.Common

let mutable projectOptions : FSharpProjectOptions = FSharpProjectOptions.zero

[<SetUp>]
let Setup () =
task {
let! options =
mkOptionsFromProject
"net7.0"
[
{
Name = "Microsoft.Extensions.Logging"
Version = "8.0.0"
}
{
Name = "Microsoft.Extensions.Logging.Console"
Version = "8.0.0"
}
]

projectOptions <- options
}

type TestCases() =

interface IEnumerable with
member _.GetEnumerator () : IEnumerator =
constructTestCaseEnumerator [| "loggingtemplatemissingvalues" |]

[<TestCaseSource(typeof<TestCases>)>]
let Tests (fileName : string) =
task {
let fileName = Path.Combine (dataFolder, fileName)

let! messages =
File.ReadAllText fileName
|> getContext projectOptions
|> LoggingTemplateMissingValuesAnalyzer.loggingTemplateMissingValuesCliAnalyzer

do! assertExpected fileName messages
}

type NegativeTestCases() =

interface IEnumerable with
member _.GetEnumerator () : IEnumerator =
constructTestCaseEnumerator [| "loggingtemplatemissingvalues" ; "negative" |]

[<TestCaseSource(typeof<NegativeTestCases>)>]
let NegativeTests (fileName : string) =
task {
let fileName = Path.Combine (dataFolder, fileName)

let! messages =
File.ReadAllText fileName
|> getContext projectOptions
|> LoggingTemplateMissingValuesAnalyzer.loggingTemplateMissingValuesCliAnalyzer

Assert.That (messages, Is.Empty)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module M

open Microsoft.Extensions.Logging

let testlog () =
use factory = LoggerFactory.Create(fun b -> b.AddConsole() |> ignore)
let logger: ILogger = factory.CreateLogger("Program")

logger.Log(LogLevel.Information, "xxx {one} yyy {two}", 23)
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
GRA-LOGTEMPLMISSVALS-001 | Warning | (9,8 - 9,67) | The given values in your call to ILogger.Log{Warning, Error, ...} don't match the expected templated args. | []
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module M

open Microsoft.Extensions.Logging

let testlog () =
use factory = LoggerFactory.Create(fun b -> b.AddConsole() |> ignore)
let logger: ILogger = factory.CreateLogger("Program")

logger.Log(LogLevel.Information, "xxx {{esc1}} {{{{esc2}}}} {{{{{{esc3}}}}}} yyy {two}", 2)
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module M

open Microsoft.Extensions.Logging

let testlog () =
use factory = LoggerFactory.Create(fun b -> b.AddConsole() |> ignore)
let logger: ILogger = factory.CreateLogger("Program")

logger.Log(LogLevel.Information, $"xxx {123}")
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module M

open Microsoft.Extensions.Logging

let testlog () =
use factory = LoggerFactory.Create(fun b -> b.AddConsole() |> ignore)
let logger: ILogger = factory.CreateLogger("Program")

logger.Log(LogLevel.Information, "xxx")
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module M

open Microsoft.Extensions.Logging

let testlog () =
use factory = LoggerFactory.Create(fun b -> b.AddConsole() |> ignore)
let logger: ILogger = factory.CreateLogger("Program")

logger.Log(LogLevel.Information, "xxx {one} yyy {two}", 23, 42)
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module M

open Microsoft.Extensions.Logging

let testlog () =
use factory = LoggerFactory.Create(fun b -> b.AddConsole() |> ignore)
let logger: ILogger = factory.CreateLogger("Program")

logger.Log(LogLevel.Information, "xxx {one}", obj())

0 comments on commit 3b39b53

Please sign in to comment.