Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add the LoggingTemplateMissingValuesAnalyzer #53

Merged
merged 3 commits into from
Jan 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Changelog

## Unreleased
## 0.7.0 - 2024-01-10

### Fixed
* Don't report FormattableStrings in TypedInterpolatedStringsAnalyzer. [#46](https://github.com/G-Research/fsharp-analyzers/pull/46)
Expand All @@ -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
120 changes: 120 additions & 0 deletions src/FSharp.Analyzers/LoggingTemplateMissingValuesAnalyzer.fs
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
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 * string> ()

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, m.DisplayName)
}

walkTast walker typedTree

[
for range, name in state do
{
Type = "LoggingTemplateMissingValuesAnalyzer"
Message = $"The given values in your call to ILogger.%s{name} 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.LogInformation("Foo {one} {two}", [| box 1 |])
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
GRA-LOGTEMPLMISSVALS-001 | Warning | (9,8 - 9,61) | The given values in your call to ILogger.LogInformation 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 {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 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.LogInformation("Foo {one} {two}", [| box 1; box 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())