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 TypeAnnotateStringFunction analyzer. #34

Merged
merged 7 commits into from
Nov 23, 2023
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
10 changes: 8 additions & 2 deletions .config/dotnet-tools.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,22 @@
"isRoot": true,
"tools": {
"fantomas": {
"version": "6.3.0-alpha-002",
"version": "6.3.0-alpha-003",
"commands": [
"fantomas"
]
},
"fsdocs-tool": {
"version": "20.0.0-alpha-013",
"version": "20.0.0-alpha-014",
"commands": [
"fsdocs"
]
},
"fsharp-analyzers": {
"version": "0.21.0",
"commands": [
"fsharp-analyzers"
]
}
}
}
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
# Changelog

## 0.4.0 - 2023-11-23

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

### Changed
* Update FSharp.Analyzers.SDK to `0.21.0`. [#34](https://github.com/G-Research/fsharp-analyzers/pull/34)

## 0.3.1 - 2023-11-15

### Changed
Expand Down
3 changes: 3 additions & 0 deletions Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@
<ServerGarbageCollection>true</ServerGarbageCollection>
<LangVersion>preview</LangVersion>
<OtherFlags>$(OtherFlags) --test:GraphBasedChecking --test:ParallelOptimization --test:ParallelIlxGen</OtherFlags>
<!-- Set to true and adjust the path to your local repo if you want to use that instead of the nuget packages -->
<UseLocalAnalyzersSDK>false</UseLocalAnalyzersSDK>
<LocalAnalyzersSDKRepo>../../../FSharp.Analyzers.SDK</LocalAnalyzersSDKRepo>
</PropertyGroup>

<PropertyGroup>
Expand Down
8 changes: 4 additions & 4 deletions Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@
</PropertyGroup>
<ItemGroup>
<!-- locking the version of F# Core as FCS does this anyway and in practise all will be using the same version -->
<PackageVersion Include="FSharp.Analyzers.SDK" Version="[0.20.2]" />
<PackageVersion Include="FSharp.Analyzers.SDK.Testing" Version="0.20.2" />
<PackageVersion Include="FSharp.Core" Version="[7.0.400]" />
<PackageVersion Include="FSharp.Compiler.Service" Version="[43.7.400]" />
<PackageVersion Include="FSharp.Analyzers.SDK" Version="[0.21.0]" />
<PackageVersion Include="FSharp.Analyzers.SDK.Testing" Version="[0.21.0]" />
<PackageVersion Include="FSharp.Core" Version="[8.0.100]" />
<PackageVersion Include="FSharp.Compiler.Service" Version="[43.8.100]" />
<PackageVersion Include="Ionide.KeepAChangelog.Tasks" Version="0.1.8" />
<PackageVersion Include="DotNet.ReproducibleBuilds" Version="1.1.1" />
<PackageVersion Include="NUnit3TestAdapter" Version="4.5.0" />
Expand Down
26 changes: 26 additions & 0 deletions docs/analyzers/TypeAnnotateStringFunction.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
---
title: TypeAnnotateStringFunctionAnalyzer
category: analyzers
categoryindex: 1
index: 6
---

# TypeAnnotateStringFunction

## Problem

Using the `string` function can catch you off guard when refactoring types.

```fsharp
let v = 1 // Changing the type won't affect `s`
let s = string v
```

## Fix

Add an explicit type annotation:

```fsharp
let v = 1 // Changing the type will affect `s`
let s = string<int> v
```
9 changes: 8 additions & 1 deletion src/FSharp.Analyzers/FSharp.Analyzers.fsproj
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,19 @@
<Compile Include="PartialAppAnalyzer.fs" />
<Compile Include="VirtualCallAnalyzer.fsi" />
<Compile Include="VirtualCallAnalyzer.fs" />
<Compile Include="TypeAnnotateStringFunction.fs" />
</ItemGroup>

<ItemGroup Condition="'$(UseLocalAnalyzersSDK)' == 'true'">
<ProjectReference Include="$(LocalAnalyzersSDKRepo)/src/FSharp.Analyzers.SDK/FSharp.Analyzers.SDK.fsproj" />
</ItemGroup>

<ItemGroup>
<PackageReference Update="FSharp.Core" />
</ItemGroup>

<ItemGroup Condition="'$(UseLocalAnalyzersSDK)' == 'false'">
<PackageReference Include="FSharp.Analyzers.SDK"/>
<PackageReference Include="FSharp.Compiler.Service"/>
</ItemGroup>

<Target Name="_AddAnalyzersToOutput">
Expand Down
121 changes: 121 additions & 0 deletions src/FSharp.Analyzers/TypeAnnotateStringFunction.fs
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
module GR.FSharp.Analyzers.TypeAnnotateStringFunction

open System.Text
open FSharp.Analyzers.SDK
open FSharp.Analyzers.SDK.TASTCollecting
open FSharp.Compiler.Symbols
open FSharp.Compiler.Syntax
open FSharp.Compiler.Text

type ISourceText with

member x.GetContentAt (range : range) : string =
let startLine = range.StartLine - 1
let line = x.GetLineString startLine

if range.StartLine = range.EndLine then
let length = range.EndColumn - range.StartColumn
line.Substring (range.StartColumn, length)
else
let firstLineContent = line.Substring range.StartColumn
let sb = StringBuilder().AppendLine firstLineContent

(sb, [ range.StartLine .. range.EndLine - 2 ])
||> List.fold (fun sb lineNumber -> sb.AppendLine (x.GetLineString lineNumber))
|> fun sb ->
let lastLine = x.GetLineString (range.EndLine - 1)

sb.Append(lastLine.Substring (0, range.EndColumn)).ToString ()

[<RequireQualifiedAccess>]
type StringApplicationResult =
| NoTypeArgument
| TypeArgument
| Unsure of string

let (|StringFunctionExpr|_|) =
function
| SynExpr.Ident (ident = ident) when ident.idText = "string" -> Some ()
| SynExpr.LongIdent (longDotId = SynLongIdent (id = lid)) ->
List.tryLast lid
|> Option.bind (fun ident -> if ident.idText = "string" then Some () else None)
| _ -> None

[<CliAnalyzer("TypeAnnotateStringFunctionAnalyzer",
"Checks if the `string` function call is type annotated.",
"https://g-research.github.io/fsharp-analyzers/analyzers/TypeAnnotateStringFunctionAnalyzer.html")>]
let typeAnnotateStringFunctionsAnalyzer : Analyzer<CliContext> =
fun (ctx : CliContext) ->
async {
let messages = ResizeArray<Message> ()

let tryFindSynExprApp (m : range) =
let visitor =
{ new SyntaxVisitorBase<StringApplicationResult>() with
override x.VisitExpr (path, traverseSynExpr, defaultTraverse, synExpr) =
if not (Range.equals synExpr.Range m) then
defaultTraverse synExpr
else
match synExpr with
// in this case expression was probably piped into the string function.
| StringFunctionExpr ->
match path with
| SyntaxNode.SynExpr (SynExpr.TypeApp _) :: _ ->
Some StringApplicationResult.TypeArgument
| _ -> Some StringApplicationResult.NoTypeArgument

| SynExpr.App (funcExpr = StringFunctionExpr) ->
Some StringApplicationResult.NoTypeArgument

| SynExpr.App (funcExpr = SynExpr.TypeApp (expr = StringFunctionExpr)) ->
Some StringApplicationResult.TypeArgument
| e ->
let source = ctx.SourceText.GetContentAt e.Range
Some (StringApplicationResult.Unsure source)
}

SyntaxTraversal.Traverse (m.Start, ctx.ParseFileResults.ParseTree, visitor)

let tastCollector =
{ new TypedTreeCollectorBase() with
override x.WalkCall
_
(mfv : FSharpMemberOrFunctionOrValue)
objExprTypeArgs
memberOrFuncTypeArgs
(args : FSharpExpr list)
(m : range)
=
if mfv.FullName = "Microsoft.FSharp.Core.Operators.string" && args.Length = 1 then
match tryFindSynExprApp m with
| Some (StringApplicationResult.Unsure sourceCode) ->
#if DEBUG
printfn $"Could map %A{m} to a string application, source:\n%s{sourceCode}"
#else
ignore sourceCode
#endif
| Some StringApplicationResult.NoTypeArgument ->
messages.Add
{
Type = "TypeAnnotateStringFunctionsAnalyzer"
Message = "Please annotate your type when using the `string` function."
Code = "GRA-TYPE-ANNOTATE-001"
Severity = Severity.Warning
Range = m
Fixes = []
}
| Some StringApplicationResult.TypeArgument -> ()
| None ->
#if DEBUG
printfn $"Could not find application for %A{m}"
#else
()
#endif
}

match ctx.TypedTree with
| None -> return []
| Some typedTree ->
walkTast tastCollector typedTree
return Seq.toList messages
}
3 changes: 3 additions & 0 deletions tests/FSharp.Analyzers.Tests/Common.fs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ let shouldUpdateBaseline () =
|> Option.map (fun v -> v.Trim () = "1")
|> Option.defaultValue false

/// Update by:
/// set TEST_UPDATE_BSL=1
/// $env:TEST_UPDATE_BSL=1
let assertExpected sourceFile messages =
task {
let actualContents =
Expand Down
61 changes: 34 additions & 27 deletions tests/FSharp.Analyzers.Tests/FSharp.Analyzers.Tests.fsproj
Original file line number Diff line number Diff line change
@@ -1,35 +1,42 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>net6.0</TargetFramework>
<PropertyGroup>
<TargetFramework>net6.0</TargetFramework>

<IsPackable>false</IsPackable>
<GenerateProgramFile>false</GenerateProgramFile>
<IsTestProject>true</IsTestProject>
<AssemblyName>G-Research.FSharp.Analyzers.Tests</AssemblyName>
</PropertyGroup>
<IsPackable>false</IsPackable>
<GenerateProgramFile>false</GenerateProgramFile>
<IsTestProject>true</IsTestProject>
<AssemblyName>G-Research.FSharp.Analyzers.Tests</AssemblyName>
</PropertyGroup>

<ItemGroup>
<Compile Include="Common.fs" />
<Compile Include="StringAnalyzerTests.fs" />
<Compile Include="PartialAppAnalyzerTests.fs" />
<Compile Include="JsonSerializerOptionsAnalyzerTests.fs" />
<Compile Include="UnionCaseAnalyzerTests.fs" />
<Compile Include="VirtualCallAnalyzerTests.fs" />
</ItemGroup>
<ItemGroup>
<Compile Include="Common.fs"/>
<Compile Include="StringAnalyzerTests.fs"/>
<Compile Include="PartialAppAnalyzerTests.fs"/>
<Compile Include="JsonSerializerOptionsAnalyzerTests.fs"/>
<Compile Include="UnionCaseAnalyzerTests.fs"/>
<Compile Include="VirtualCallAnalyzerTests.fs"/>
<Compile Include="TypeAnnotateStringFunctionAnalyzerTests.fs"/>
</ItemGroup>

<ItemGroup>
<PackageReference Update="FSharp.Core" />
<PackageReference Include="FSharp.Analyzers.SDK.Testing" />
<PackageReference Include="Microsoft.NET.Test.Sdk" />
<PackageReference Include="NUnit" />
<PackageReference Include="NUnit3TestAdapter" />
<PackageReference Include="NUnit.Analyzers" />
<PackageReference Include="coverlet.collector" />
</ItemGroup>
<ItemGroup>
<PackageReference Update="FSharp.Core"/>
<PackageReference Include="Microsoft.NET.Test.Sdk"/>
<PackageReference Include="NUnit"/>
<PackageReference Include="NUnit3TestAdapter"/>
<PackageReference Include="NUnit.Analyzers"/>
<PackageReference Include="coverlet.collector"/>
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\..\src\FSharp.Analyzers\FSharp.Analyzers.fsproj" />
</ItemGroup>
<ItemGroup Condition="'$(UseLocalAnalyzersSDK)' == 'false'">
<PackageReference Include="FSharp.Analyzers.SDK.Testing"/>
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\..\src\FSharp.Analyzers\FSharp.Analyzers.fsproj"/>
</ItemGroup>

<ItemGroup Condition="'$(UseLocalAnalyzersSDK)' == 'true'">
<ProjectReference Include="$(LocalAnalyzersSDKRepo)/src/FSharp.Analyzers.SDK.Testing/FSharp.Analyzers.SDK.Testing.fsproj" />
</ItemGroup>
</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
module GR.FSharp.Analyzers.Tests.TypeAnnotateStringFunctionAnalyzerTests

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 [| "typeAnnotateStringFunctions" |]

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

let! messages =
File.ReadAllText fileName
|> getContext projectOptions
|> TypeAnnotateStringFunction.typeAnnotateStringFunctionsAnalyzer

do! assertExpected fileName messages
}

type NegativeTestCases() =

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

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

let! messages =
File.ReadAllText fileName
|> getContext projectOptions
|> TypeAnnotateStringFunction.typeAnnotateStringFunctionsAnalyzer

Assert.IsEmpty messages
}
5 changes: 5 additions & 0 deletions tests/data/typeAnnotateStringFunctions/AliasStringFunction.fs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module S

let foo = Microsoft.FSharp.Core.Operators.string
do
foo 9.3
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
GRA-TYPE-ANNOTATE-001 | Warning | (3,10 - 3,48) | Please annotate your type when using the `string` function.
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
module S

type Coordinate = { X : int; Y: int }

let v =
Microsoft.FSharp.Core.Operators.string
{
X = 1
Y = 2
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
GRA-TYPE-ANNOTATE-001 | Warning | (6,4 - 10,9) | Please annotate your type when using the `string` function.
Loading