Skip to content

Commit

Permalink
Merge branch 'main' into add_partialappanalyzer
Browse files Browse the repository at this point in the history
  • Loading branch information
dawedawe committed Oct 4, 2023
2 parents d96ad92 + 606cd93 commit ee2f914
Show file tree
Hide file tree
Showing 11 changed files with 240 additions and 46 deletions.
27 changes: 14 additions & 13 deletions src/FSharp.Analyzers/FSharp.Analyzers.fsproj
Original file line number Diff line number Diff line change
@@ -1,25 +1,26 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>net6.0</TargetFramework>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
<IsPackable>true</IsPackable>
<AssemblyName>G-Research.FSharp.Analyzers</AssemblyName>
<Tailcalls>true</Tailcalls>
</PropertyGroup>
<PropertyGroup>
<TargetFramework>net6.0</TargetFramework>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
<IsPackable>true</IsPackable>
<AssemblyName>G-Research.FSharp.Analyzers</AssemblyName>
<Tailcalls>true</Tailcalls>
</PropertyGroup>

<ItemGroup>
<Compile Include="ASTCollecting.fs"/>
<Compile Include="TASTCollecting.fs" />
<Compile Include="StringAnalyzers.fs"/>
<Compile Include="JsonSerializerOptionsAnalyzer.fs" />
<Compile Include="UnionCaseAnalyzer.fs" />
<Compile Include="PartialAppAnalyzer.fs" />
</ItemGroup>

<ItemGroup>
<PackageReference Update="FSharp.Core"/>
<PackageReference Include="FSharp.Analyzers.SDK" />
<PackageReference Include="FSharp.Compiler.Service"/>
</ItemGroup>
<ItemGroup>
<PackageReference Update="FSharp.Core"/>
<PackageReference Include="FSharp.Analyzers.SDK" />
<PackageReference Include="FSharp.Compiler.Service"/>
</ItemGroup>

</Project>
133 changes: 133 additions & 0 deletions src/FSharp.Analyzers/UnionCaseAnalyzer.fs
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
namespace ``G-Research``.FSharp.Analyzers

open FSharp.Analyzers.SDK
open FSharp.Compiler.CodeAnalysis
open FSharp.Compiler.Symbols
open FSharp.Compiler.Syntax
open FSharp.Compiler.Text

module UnionCaseAnalyzer =

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

let findAllShadowingCases
(ast : ParsedInput)
(checkFileResults : FSharpCheckFileResults)
(sourceText : ISourceText)
: range list
=
let collector = ResizeArray<range> ()

let namesToWarnAbount =
set
[
"Choice1Of2"
"Choice2Of2"
"Choice1Of3"
"Choice2Of3"
"Choice3Of3"
"Choice1Of4"
"Choice2Of4"
"Choice3Of4"
"Choice4Of4"
"Choice1Of5"
"Choice2Of5"
"Choice3Of5"
"Choice4Of5"
"Choice5Of5"
"Choice1Of6"
"Choice2Of6"
"Choice3Of6"
"Choice4Of6"
"Choice5Of6"
"Choice6Of6"
"Choice1Of7"
"Choice2Of7"
"Choice3Of7"
"Choice4Of7"
"Choice5Of7"
"Choice6Of7"
"Choice7Of7"
"None"
"Some"
"ValueNone"
"ValueSome"
"Ok"
"Error"
]

let handleCase (SynUnionCase (ident = (SynIdent (ident, _)))) =
if (namesToWarnAbount |> Set.contains ident.idText) then
collector.Add ident.idRange

()

let walker =
{ new SyntaxCollectorBase() with
override _.WalkTypeDefn
(SynTypeDefn (typeInfo = SynComponentInfo (attributes = synAttributeLists) ; typeRepr = repr))
=
let hasReqQualAccAttribute =
synAttributeLists
|> List.exists (fun lst ->
lst.Attributes
|> Seq.exists (fun (a : SynAttribute) ->
let lineText = sourceText.GetLineString (a.Range.EndLine - 1)
let name = (List.last a.TypeName.LongIdent).idText

let symbolUseOpt =
checkFileResults.GetSymbolUseAtLocation (
a.Range.EndLine,
a.Range.EndColumn,
lineText,
[ name ]
)

match symbolUseOpt with
| Some symbolUse ->
match symbolUse.Symbol with
| :? FSharpMemberOrFunctionOrValue as mfv ->
mfv.FullName = "Microsoft.FSharp.Core.RequireQualifiedAccessAttribute"
| _ -> false
| _ -> false
)
)

if not hasReqQualAccAttribute then
match repr with
| SynTypeDefnRepr.Simple (SynTypeDefnSimpleRepr.Union (unionCases = synUnionCases), _) ->
synUnionCases |> List.iter handleCase
| _ -> ()
else
()
}

walkAst walker ast

collector |> Seq.toList

[<CliAnalyzer "UnionCaseAnalyzer">]
let unionCaseAnalyzer : Analyzer<CliContext> =
fun ctx ->
async {

let ranges =
findAllShadowingCases ctx.ParseFileResults.ParseTree ctx.CheckFileResults ctx.SourceText

let msgs =
ranges
|> List.map (fun r ->
{
Type = "UnionCase analyzer"
Message =
"This discriminated union contains a case with the same name as a case from FSharp.Core. Consider renaming it or applying RequireQualifiedAccess to avoid clashes."
Code = Code
Severity = Warning
Range = r
Fixes = []
}
)

return msgs
}
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 @@ -14,6 +14,7 @@
<Compile Include="StringAnalyzerTests.fs" />
<Compile Include="PartialAppAnalyzerTests.fs" />
<Compile Include="JsonSerializerOptionsAnalyzerTests.fs" />
<Compile Include="UnionCaseAnalyzerTests.fs" />
</ItemGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,24 +29,14 @@ module JsonSerializerOptionsAnalyzerTests =
}

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 ()
constructTestCaseEnumerator [| "jsonserializeroptions" |]

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

let! messages =
Expand Down
23 changes: 3 additions & 20 deletions tests/FSharp.Analyzers.Tests/StringAnalyzerTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -21,23 +21,14 @@ module StringAnalyzerTests =
}

type TestCases() =
static member DataFolder = Path.Combine (__SOURCE_DIRECTORY__, "..", "data")

interface IEnumerable with
member _.GetEnumerator () : IEnumerator =
let endsWithTests = Path.Combine (TestCases.DataFolder, "string", "endswith")

Directory.EnumerateFiles (endsWithTests, "*.fs")
|> Seq.map (fun f ->
let fileName = Path.GetRelativePath (TestCases.DataFolder, f)
[| fileName :> obj |]
)
|> fun s -> s.GetEnumerator ()
constructTestCaseEnumerator [| "string" ; "endswith" |]

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

let! messages =
Expand All @@ -52,20 +43,12 @@ module StringAnalyzerTests =

interface IEnumerable with
member _.GetEnumerator () : IEnumerator =
let endsWithTests =
Path.Combine (TestCases.DataFolder, "string", "endswith", "negative")

Directory.EnumerateFiles (endsWithTests, "*.fs")
|> Seq.map (fun f ->
let fileName = Path.GetRelativePath (TestCases.DataFolder, f)
[| fileName :> obj |]
)
|> fun s -> s.GetEnumerator ()
constructTestCaseEnumerator [| "string" ; "endswith" ; "negative" |]

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

let! messages =
File.ReadAllText fileName
Expand Down
17 changes: 15 additions & 2 deletions tests/FSharp.Analyzers.Tests/Testing.fs
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@

module Testing =

open System
open System.IO
open System.Threading.Tasks
open NUnit.Framework
open FSharp.Analyzers.SDK
open FSharp.Analyzers.SDK.Testing

let shouldUpdateBaseline () =
System.Environment.GetEnvironmentVariable "TEST_UPDATE_BSL"
Environment.GetEnvironmentVariable "TEST_UPDATE_BSL"
|> Option.ofObj
|> Option.map (fun v -> v.Trim () = "1")
|> Option.defaultValue false
Expand All @@ -22,7 +23,7 @@ module Testing =
$"%s{m.Code} | %A{m.Severity} | (%i{m.Range.StartLine},%i{m.Range.StartColumn} - %i{m.Range.EndLine},%i{m.Range.EndColumn}) | %s{m.Message}"
)
|> String.concat "\n"
|> fun contents -> System.String.Concat (contents, "\n")
|> fun contents -> String.Concat (contents, "\n")

let expectedFile = $"%s{sourceFile}.expected"
let actualFile = $"%s{sourceFile}.actual"
Expand All @@ -44,3 +45,15 @@ module Testing =

Assert.AreEqual (expectedContents, actualContents)
}

let dataFolder = Path.Combine (__SOURCE_DIRECTORY__, "..", "data")

let constructTestCaseEnumerator (subDataPath : string array) =
let testsDirectory = Path.Combine (dataFolder, Path.Combine subDataPath)

Directory.EnumerateFiles (testsDirectory, "*.fs")
|> Seq.map (fun f ->
let fileName = Path.GetRelativePath (dataFolder, f)
[| fileName :> obj |]
)
|> fun s -> s.GetEnumerator ()
61 changes: 61 additions & 0 deletions tests/FSharp.Analyzers.Tests/UnionCaseAnalyzerTests.fs
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
namespace ``G-Research``.FSharp.Analyzers.Tests

module UnionCaseAnalyzerTests =

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

[<SetUp>]
let Setup () =
task {
let! options = mkOptionsFromProject "net7.0" []

projectOptions <- options
}

type TestCases() =
static member DataFolder = Path.Combine (__SOURCE_DIRECTORY__, "..", "data")

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

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

let! messages =
File.ReadAllText fileName
|> getContext projectOptions
|> UnionCaseAnalyzer.unionCaseAnalyzer

do! assertExpected fileName messages
}

type NegativeTestCases() =

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

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

let! messages =
File.ReadAllText fileName
|> getContext projectOptions
|> UnionCaseAnalyzer.unionCaseAnalyzer

Assert.IsEmpty messages
}
4 changes: 4 additions & 0 deletions tests/data/unioncase/Option.fs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
type MyU =
| Case1
| None
| Case3
1 change: 1 addition & 0 deletions tests/data/unioncase/Option.fs.expected
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
GRA-UNIONCASE-001 | Warning | (3,6 - 3,10) | This discriminated union contains a case with the same name as a case from FSharp.Core. Consider renaming it or applying RequireQualifiedAccess to avoid clashes.
5 changes: 5 additions & 0 deletions tests/data/unioncase/negative/ReqFullyQualified.fs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[<RequireQualifiedAccess>]
type MyU =
| Case1
| None
| Case3
2 changes: 2 additions & 0 deletions tests/data/unioncase/negative/ReqFullyQualified2.fs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[<FSharp.Core.RequireQualifiedAccess>]
type MyU = | Choice7Of7

0 comments on commit ee2f914

Please sign in to comment.