Merge pull request #876 from nojaf/use-analyzers

Use analyzers

Author: nojaf

.config/dotnet-tools.json

@@ -7,6 +7,12 @@
       "commands": [
         "fantomas"
       ]
+    },
+    "fsharp-analyzers": {
+      "version": "0.20.2",
+      "commands": [
+        "fsharp-analyzers"
+      ]
     }
   }
 }

.github/workflows/pull-requests.yml

@@ -5,6 +5,10 @@ on:
     branches:
     - main
 
+permissions:
+  id-token: write
+  security-events: write
+
 jobs:
   build:
 
@@ -20,3 +24,14 @@ jobs:
       uses: actions/setup-dotnet@v3
     - name: Run CI
       run: dotnet fsi build.fsx
+  
+    - name: Analyze Solution
+      if: matrix.os == 'ubuntu-latest'
+      run: dotnet msbuild /t:AnalyzeSolution
+      continue-on-error: true
+
+    - name: Upload SARIF file
+      uses: github/codeql-action/upload-sarif@v2
+      if: matrix.os == 'ubuntu-latest'
+      with:
+        sarif_file: ./reports

.gitignore

@@ -47,4 +47,7 @@ tests/FSharp.Literate.Tests/output1/
 .vscode/
 .DS_Store
 tests/FSharp.Literate.Tests/output2/
-tests/FSharp.Literate.Tests/previous-next-output/
+tests/FSharp.Literate.Tests/previous-next-output/
+
+# Analyzer
+reports/

Directory.Packages.props

@@ -19,11 +19,14 @@
     <PackageVersion Include="Suave" Version="2.6.2" />
     <PackageVersion Include="System.Memory" Version="4.5.5" />
     <PackageVersion Include="Microsoft.CodeAnalysis.CSharp" Version="4.7.0" />
-    <PackageVersion Include="NUnit" Version="3.13.3" />
+    <PackageVersion Include="NUnit" Version="3.14.0" />
     <PackageVersion Include="FsUnit" Version="5.6.0" />
     <PackageVersion Include="FSharp.Data" Version="6.3.0" />
     <PackageVersion Include="NUnit3TestAdapter" Version="4.5.0" />
-    <PackageVersion Include="Microsoft.NET.Test.Sdk" Version="17.7.2" />
-    <PackageVersion Include="Ionide.KeepAChangelog.Tasks" Version="0.1.8"  />
+    <PackageVersion Include="Microsoft.NET.Test.Sdk" Version="17.8.0" />
+    <PackageVersion Include="Ionide.KeepAChangelog.Tasks" Version="0.1.8" />
+    <PackageVersion Include="FSharp.Analyzers.Build" Version="0.2.0" />
+    <PackageVersion Include="G-Research.FSharp.Analyzers" Version="0.3.1" />
+    <PackageVersion Include="Ionide.Analyzers" Version="0.4.0" />
   </ItemGroup>
 </Project>

Directory.Solution.targets

@@ -0,0 +1,9 @@
+<Project>
+    <ItemGroup>
+        <ProjectsToAnalyze Include="src/**/*.fsproj" />
+    </ItemGroup>
+
+    <Target Name="AnalyzeSolution">
+        <MSBuild Projects="@(ProjectsToAnalyze)" Targets="AnalyzeFSharpProject" />
+    </Target>
+</Project>

README.md

@@ -18,6 +18,17 @@ Once built, you can run the command-line tool to self-build the docs for this di
     src\fsdocs-tool\bin\Debug\net6.0\fsdocs.exe watch
     src\fsdocs-tool\bin\Debug\net6.0\fsdocs.exe build --clean
 
+### Pipelines
+
+Run
+    dotnet fsi build.fsx -- --help
+
+to see what other pipelines can be run from `build.fsx`.
+
+    dotnet fsi build.fsx -- -p Verify
+
+Will perform the linting, unit tests and analyzer check.
+This is useful to run locally before submitting your PR.
 
 ## Maintainer(s)
 

build.fsx

@@ -1,4 +1,4 @@
-#r "nuget: Fun.Build, 0.3.8"
+#r "nuget: Fun.Build, 1.0.4"
 #r "nuget: Fake.IO.FileSystem, 6.0.0"
 #r "nuget: Ionide.KeepAChangelog, 0.1.8"
 
@@ -38,12 +38,21 @@ let releaseNugetVersion, _, _ =
 
 let solutionFile = "FSharp.Formatting.sln"
 
-pipeline "CI" {
+let lintStage =
     stage "Lint" {
         run "dotnet tool restore"
         run $"dotnet fantomas {__SOURCE_FILE__} src tests docs --check"
     }
 
+let testStage =
+    stage "Tests" {
+        run
+            $"dotnet test {solutionFile} --configuration {configuration} --no-build --blame --logger trx --framework net7.0 --results-directory TestResults"
+    }
+
+pipeline "CI" {
+    lintStage
+
     stage "Clean" {
         run (fun _ ->
             !!artifactsDir ++ "temp" |> Shell.cleanDirs
@@ -58,10 +67,7 @@ pipeline "CI" {
 
     stage "NuGet" { run $"dotnet pack {solutionFile} --output \"{artifactsDir}\" --configuration {configuration}" }
 
-    stage "Tests" {
-        run
-            $"dotnet test {solutionFile} --configuration {configuration} --no-build --blame --logger trx --framework net7.0 --results-directory TestResults"
-    }
+    testStage
 
     stage "GenerateDocs" {
         run (fun _ ->
@@ -80,4 +86,11 @@ pipeline "CI" {
     runIfOnlySpecified false
 }
 
+pipeline "Verify" {
+    lintStage
+    testStage
+    stage "Analyzers" { run "dotnet msbuild /t:AnalyzeSolution" }
+    runIfOnlySpecified true
+}
+
 tryPrintPipelineCommandHelp ()

src/Common/Collections.fs

@@ -103,7 +103,7 @@ module internal List =
 
 /// Represents a tree with nodes containing values an a list of children
 ///
-type internal Tree<'T> = Node of 'T * list<Tree<'T>>
+type internal Tree<'T> = Node of self: 'T * children: Tree<'T> list
 
 module internal Tree =
     /// Takes all elements at the specified level and turns them into nodes

src/Common/StringParsing.fs

@@ -23,15 +23,15 @@ module String =
 
     /// Matches when a string starts with the specified sub-string
     let (|StartsWith|_|) (start: string) (text: string) =
-        if text.StartsWith(start) then
+        if text.StartsWith(start, StringComparison.Ordinal) then
             Some(text.Substring(start.Length))
         else
             None
 
     /// Matches when a string starts with the specified sub-string
     /// The matched string is trimmed from all whitespace.
     let (|StartsWithTrim|_|) (start: string) (text: string) =
-        if text.StartsWith(start) then
+        if text.StartsWith(start, StringComparison.Ordinal) then
             Some(text.Substring(start.Length).Trim())
         else
             None
@@ -40,8 +40,8 @@ module String =
     /// with a given value (and returns the rest of it)
     let (|StartsAndEndsWith|_|) (starts: string, ends: string) (s: string) =
         if
-            s.StartsWith(starts)
-            && s.EndsWith(ends)
+            s.StartsWith(starts, StringComparison.Ordinal)
+            && s.EndsWith(ends, StringComparison.Ordinal)
             && s.Length >= starts.Length + ends.Length
         then
             Some(s.Substring(starts.Length, s.Length - starts.Length - ends.Length))
@@ -60,8 +60,8 @@ module String =
     /// For example "[aa]bc" is wrapped in [ and ] pair. Returns the wrapped
     /// text together with the rest.
     let (|StartsWithWrapped|_|) (starts: string, ends: string) (text: string) =
-        if text.StartsWith(starts) then
-            let id = text.IndexOf(ends, starts.Length)
+        if text.StartsWith(starts, StringComparison.Ordinal) then
+            let id = text.IndexOf(ends, starts.Length, StringComparison.Ordinal)
 
             if id >= 0 then
                 let wrapped = text.Substring(starts.Length, id - starts.Length)
@@ -79,7 +79,7 @@ module String =
         let rec tryEol eolList =
             match eolList with
             | h: string :: t ->
-                match text.IndexOf(h) with
+                match text.IndexOf(h, StringComparison.Ordinal) with
                 | i when i < 0 -> tryEol t
                 | i -> text.Substring(i + h.Length)
             | _ -> text
@@ -174,15 +174,15 @@ module StringPosition =
              StartColumn = n.StartColumn + text.Length - trimmed.Length })
 
     /// Matches when a string starts with any of the specified sub-strings
-    let (|StartsWithAny|_|) (starts: seq<string>) (text: string, _n: MarkdownRange) =
-        if starts |> Seq.exists (text.StartsWith) then
+    let (|StartsWithAny|_|) (starts: string seq) (text: string, _n: MarkdownRange) =
+        if starts |> Seq.exists (fun s -> text.StartsWith(s, StringComparison.Ordinal)) then
             Some()
         else
             None
 
     /// Matches when a string starts with the specified sub-string
     let (|StartsWith|_|) (start: string) (text: string, n: MarkdownRange) =
-        if text.StartsWith(start) then
+        if text.StartsWith(start, StringComparison.Ordinal) then
             Some(
                 text.Substring(start.Length),
                 { n with
@@ -194,7 +194,7 @@ module StringPosition =
     /// Matches when a string starts with the specified sub-string
     /// The matched string is trimmed from all whitespace.
     let (|StartsWithTrim|_|) (start: string) (text: string, n: MarkdownRange) =
-        if text.StartsWith(start) then
+        if text.StartsWith(start, StringComparison.Ordinal) then
             Some(
                 text.Substring(start.Length).Trim(),
                 { n with
@@ -207,7 +207,7 @@ module StringPosition =
     /// The matched string is trimmed from all whitespace.
     let (|StartsWithNTimesTrimIgnoreStartWhitespace|_|) (start: string) (text: string, _n: MarkdownRange) =
         if text.Contains(start) then
-            let beforeStart = text.Substring(0, text.IndexOf(start))
+            let beforeStart = text.Substring(0, text.IndexOf(start, StringComparison.Ordinal))
 
             if String.IsNullOrWhiteSpace(beforeStart) then
                 let startAndRest = text.Substring(beforeStart.Length)
@@ -232,8 +232,8 @@ module StringPosition =
     /// with a given value (and returns the rest of it)
     let (|StartsAndEndsWith|_|) (starts: string, ends: string) (s: string, n: MarkdownRange) =
         if
-            s.StartsWith(starts)
-            && s.EndsWith(ends)
+            s.StartsWith(starts, StringComparison.Ordinal)
+            && s.EndsWith(ends, StringComparison.Ordinal)
             && s.Length >= starts.Length + ends.Length
         then
             Some(
@@ -276,8 +276,8 @@ module StringPosition =
     /// For example "[aa]bc" is wrapped in [ and ] pair. Returns the wrapped
     /// text together with the rest.
     let (|StartsWithWrapped|_|) (starts: string, ends: string) (text: string, n: MarkdownRange) =
-        if text.StartsWith(starts) then
-            let id = text.IndexOf(ends, starts.Length)
+        if text.StartsWith(starts, StringComparison.Ordinal) then
+            let id = text.IndexOf(ends, starts.Length, StringComparison.Ordinal)
 
             if id >= 0 then
                 let wrapped = text.Substring(starts.Length, id - starts.Length)
@@ -299,7 +299,7 @@ module StringPosition =
     /// complete repetitions of a specified sub-string.
     let (|EqualsRepeated|_|) (repeated, _n: MarkdownRange) =
         function
-        | StartsWithRepeated repeated (_n, ("", _)) -> Some()
+        | StartsWithRepeated repeated (_n, (v, _)) when (String.IsNullOrWhiteSpace v) -> Some()
         | _ -> None
 
 module List =
@@ -363,7 +363,8 @@ module Lines =
     let (|TakeStartingWithOrBlank|_|) (start: string) (input: string list) =
         match
             input
-            |> List.partitionWhile (fun s -> String.IsNullOrWhiteSpace s || s.StartsWith(start))
+            |> List.partitionWhile (fun s ->
+                String.IsNullOrWhiteSpace s || s.StartsWith(start, StringComparison.Ordinal))
         with
         | matching, rest when matching <> [] -> Some(matching, rest)
         | _ -> None
@@ -405,7 +406,7 @@ module Lines =
         |> List.map (fun (StringPosition.TrimStart s) -> s)
         // Now remove all additional spaces at the end, but keep two spaces if existent
         |> List.map (fun (s, n) ->
-            let endsWithTwoSpaces = s.EndsWith("  ")
+            let endsWithTwoSpaces = s.EndsWith("  ", StringComparison.Ordinal)
 
             let trimmed = s.TrimEnd([| ' ' |]) + if endsWithTwoSpaces then "  " else ""
 

src/Directory.Build.props

@@ -0,0 +1,16 @@
+<Project>
+    <ItemGroup>
+        <PackageReference Include="FSharp.Analyzers.Build">
+            <PrivateAssets>all</PrivateAssets>
+            <IncludeAssets>build</IncludeAssets>
+        </PackageReference>
+        <PackageReference Include="G-Research.FSharp.Analyzers">
+            <PrivateAssets>all</PrivateAssets>
+            <IncludeAssets>analyzers</IncludeAssets>
+        </PackageReference>
+        <PackageReference Include="Ionide.Analyzers">
+            <PrivateAssets>all</PrivateAssets>
+            <IncludeAssets>analyzers</IncludeAssets>
+        </PackageReference>
+    </ItemGroup>
+</Project>