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 TypedInterpolatedStringsAnalyzer #40

Merged
merged 5 commits into from
Dec 11, 2023
Merged
Show file tree
Hide file tree
Changes from 3 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
4 changes: 2 additions & 2 deletions .config/dotnet-tools.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@
"isRoot": true,
"tools": {
"fantomas": {
"version": "6.3.0-alpha-003",
"version": "6.3.0-alpha-004",
"commands": [
"fantomas"
]
},
"fsdocs-tool": {
"version": "20.0.0-alpha-014",
"version": "20.0.0-alpha-016",
"commands": [
"fsdocs"
]
Expand Down
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# Changelog

## Unreleased

### Added
* Add TypedInterpolatedStringsAnalyzer. [#40](https://github.com/G-Research/fsharp-analyzers/pull/40)

## 0.5.1 - 2023-12-06

### Fixed
Expand Down
26 changes: 26 additions & 0 deletions docs/analyzers/TypedInterpolatedStringsAnalyzer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
---
title: TypedInterpolatedStrings Analyzer
category: analyzers
categoryindex: 1
index: 9
---

# TypedInterpolatedStrings Analyzer

Using interpolated strings can catch you off guard when refactoring types.

## Problem

```fsharp
let v = 1 // Changing the type won't affect `s`
let s = $"{v}"
```

## Fix

Add an explicit type format:

```fsharp
let v = 1 // Changing the type will affect `s`
let s = $"%i{v}"
```
1 change: 1 addition & 0 deletions src/FSharp.Analyzers/FSharp.Analyzers.fsproj
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
<Compile Include="LoggingArgFuncNotFullyAppliedAnalyzer.fs" />
<Compile Include="TypeAnnotateStringFunctionAnalyzer.fs" />
<Compile Include="ImmutableCollectionEqualityAnalyzer.fs" />
<Compile Include="TypedInterpolatedStringsAnalyzer.fs" />
</ItemGroup>
<ItemGroup Condition="'$(UseLocalAnalyzersSDK)' == 'true'">
<ProjectReference Include="$(LocalAnalyzersSDKRepo)/src/FSharp.Analyzers.SDK/FSharp.Analyzers.SDK.fsproj" />
Expand Down
53 changes: 53 additions & 0 deletions src/FSharp.Analyzers/TypedInterpolatedStringsAnalyzer.fs
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
module GR.FSharp.Analyzers.TypedInterpolatedStringsAnalyzer

open System.Text.RegularExpressions
open FSharp.Analyzers.SDK
open FSharp.Analyzers.SDK.ASTCollecting
open FSharp.Compiler.Syntax

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

/// See https://learn.microsoft.com/en-us/dotnet/fsharp/language-reference/plaintext-formatting#format-specifiers-for-printf
let specifierRegex =
Regex (@"\%(\+|\-)?\d*\.?\d*(b|s|c|d|i|u|x|X|o|B|e|E|f|F|g|G|M|O|A)$")

[<CliAnalyzer("TypedInterpolatedStringsAnalyzer",
"TODO: add description.",
dawedawe marked this conversation as resolved.
Show resolved Hide resolved
"https://g-research.github.io/fsharp-analyzers/analyzers/TypedInterpolatedStringsAnalyzer.html")>]
let typedInterpolatedStringsAnalyzer : Analyzer<CliContext> =
fun (ctx : CliContext) ->
async {
let messages = ResizeArray<Message> ()

let walker =
{ new SyntaxCollectorBase() with
override x.WalkExpr (expr : SynExpr) =
match expr with
| SynExpr.InterpolatedString (contents = contents) ->
contents
|> List.pairwise
|> List.iter (fun (p1, p2) ->
match p1, p2 with
| SynInterpolatedStringPart.String (value = s),
SynInterpolatedStringPart.FillExpr (fillExpr = e) ->
if not (isNull s) && not (specifierRegex.IsMatch s) then
messages.Add
{
Type = "TypedInterpolatedStringsAnalyzer"
Message =
"Interpolated hole expression without format detected. Use prefix with the correct % to enforce type safety."
dawedawe marked this conversation as resolved.
Show resolved Hide resolved
Code = Code
Severity = Warning
Range = e.Range
Fixes = []
}
| _ -> ()
)
| _ -> ()
}

walkAst walker ctx.ParseFileResults.ParseTree

return Seq.toList messages
}
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 @@ -17,6 +17,7 @@
<Compile Include="LoggingArgFuncNotFullyAppliedAnalyzerTests.fs" />
<Compile Include="TypeAnnotateStringFunctionAnalyzerTests.fs" />
<Compile Include="ImmutableCollectionEqualityAnalyzerTests.fs" />
<Compile Include="TypedInterpolatedStringsAnalyzerTests.fs" />
</ItemGroup>
<ItemGroup>
<PackageReference Update="FSharp.Core" />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
module GR.FSharp.Analyzers.Tests.TypedInterpolatedStringsAnalyzerTests

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" []
projectOptions <- options
}

type TestCases() =

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

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

let! messages =
File.ReadAllText fileName
|> getContext projectOptions
|> TypedInterpolatedStringsAnalyzer.typedInterpolatedStringsAnalyzer

do! assertExpected fileName messages
}

type NegativeTestCases() =

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

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

let! messages =
File.ReadAllText fileName
|> getContext projectOptions
|> TypedInterpolatedStringsAnalyzer.typedInterpolatedStringsAnalyzer

Assert.IsEmpty messages
}
6 changes: 6 additions & 0 deletions tests/data/typedInterpolatedStrings/Sample.fs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module I

open System

ignore $"Hey {42}"
ignore $"{4.} a {obj()} b {TimeSpan.Zero}"
4 changes: 4 additions & 0 deletions tests/data/typedInterpolatedStrings/Sample.fs.expected
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
GRA-INTERPOLATED-001 | Warning | (5,14 - 5,16) | Interpolated hole expression without format detected. Use prefix with the correct % to enforce type safety.
GRA-INTERPOLATED-001 | Warning | (6,10 - 6,12) | Interpolated hole expression without format detected. Use prefix with the correct % to enforce type safety.
GRA-INTERPOLATED-001 | Warning | (6,17 - 6,22) | Interpolated hole expression without format detected. Use prefix with the correct % to enforce type safety.
GRA-INTERPOLATED-001 | Warning | (6,27 - 6,40) | Interpolated hole expression without format detected. Use prefix with the correct % to enforce type safety.
8 changes: 8 additions & 0 deletions tests/data/typedInterpolatedStrings/negative/Sample.fs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
module I

open System

ignore $"Hey %i{42}"
ignore $"%f{4.} a %A{obj()} b %O{TimeSpan.Zero}"
ignore $"%0.17f{43.2}"
ignore $"%b{false}"