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

Analyzers #3624

Merged
merged 9 commits into from
Dec 6, 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
6 changes: 6 additions & 0 deletions .config/dotnet-tools.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@
"commands": [
"husky"
]
},
"fsharp-analyzers": {
"version": "0.21.0",
"commands": [
"fsharp-analyzers"
]
}
}
}
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -218,3 +218,6 @@ __pycache__/

# Compilation tests
tests/**/*.actual

# Analyzers
*.sarif
8 changes: 8 additions & 0 deletions src/Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,13 @@
</ItemGroup>
<ItemGroup>
<PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.0.0" PrivateAssets="All" />
<PackageReference Include="FSharp.Analyzers.Build" Version="0.2.0">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>build</IncludeAssets>
</PackageReference>
<PackageReference Include="G-Research.FSharp.Analyzers" Version="0.4.0">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>analyzers</IncludeAssets>
</PackageReference>
</ItemGroup>
</Project>
11 changes: 11 additions & 0 deletions src/Directory.Build.targets
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<Project>
<PropertyGroup>
<FSharpAnalyzersOtherFlags>--analyzers-path &quot;$(PkgG-Research_FSharp_Analyzers)/analyzers/dotnet/fs&quot;</FSharpAnalyzersOtherFlags>
<FSharpAnalyzersOtherFlags>$(FSharpAnalyzersOtherFlags) --analyzers-path &quot;$(PkgIonide_Analyzers)/analyzers/dotnet/fs&quot;</FSharpAnalyzersOtherFlags>
<CodeRoot>$([System.IO.Path]::GetDirectoryName($(DirectoryBuildTargetsPath)))</CodeRoot>
<SarifOutput>$(CodeRoot)/reports/</SarifOutput>
<FSharpAnalyzersOtherFlags>$(FSharpAnalyzersOtherFlags) --code-root &quot;$(CodeRoot)&quot;</FSharpAnalyzersOtherFlags>
<FSharpAnalyzersOtherFlags>$(FSharpAnalyzersOtherFlags) --report &quot;$(SarifOutput)$(MSBuildProjectName)-$(TargetFramework).sarif&quot;</FSharpAnalyzersOtherFlags>
<FSharpAnalyzersOtherFlags>$(FSharpAnalyzersOtherFlags) --exclude-analyzer PartialAppAnalyzer</FSharpAnalyzersOtherFlags>
</PropertyGroup>
</Project>
6 changes: 4 additions & 2 deletions src/Fable.AST/Common.fs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
namespace Fable.AST

open System

/// Each Position object consists of a line number (1-indexed) and a column number (0-indexed):
type Position =
{
Expand All @@ -24,7 +26,7 @@ type SourceLocation =
member this.DisplayName =
this.identifierName
|> Option.bind (fun name ->
match name.IndexOf(";file:") with
match name.IndexOf(";file:", StringComparison.Ordinal) with
| -1 -> Some name
| 0 -> None
| i -> name.Substring(0, i) |> Some
Expand All @@ -33,7 +35,7 @@ type SourceLocation =
member this.File =
this.identifierName
|> Option.bind (fun name ->
match name.IndexOf(";file:") with
match name.IndexOf(";file:", StringComparison.Ordinal) with
| -1 -> None
| i -> name.Substring(i + ";file:".Length) |> Some
)
Expand Down
8 changes: 8 additions & 0 deletions src/Fable.Cli/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Unreleased

### Added

#### All

* Overall performance improvements
* [PR 3620](https://github.com/fable-compiler/Fable/pull/3620) Removed double-dictionary lookups (by @Thorium)
* [PR 3624](https://github.com/fable-compiler/Fable/pull/3624) Add G-Research analyzers and fix reported issues (by @nojaf)

### Fixed

#### Python
Expand Down
24 changes: 15 additions & 9 deletions src/Fable.Cli/Entry.fs
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,17 @@ type CliArgs(args: string list) =
let args =
// Assume last arg has true value in case it's a flag
match List.tryLast args with
| Some key when key.StartsWith("-") -> args @ [ "true" ]
| Some key when key.StartsWith('-') -> args @ [ "true" ]
| _ -> args

(Map.empty, List.windowed 2 args)
||> List.fold (fun map pair ->
match pair with
| [ key; value ] when key.StartsWith("-") ->
| [ key; value ] when key.StartsWith('-') ->
let key = key.ToLower()

let value =
if value.StartsWith("-") then
if value.StartsWith('-') then
"true"
else
value
Expand Down Expand Up @@ -279,11 +279,15 @@ type Runner =
|> Seq.toList

files
|> List.filter (fun file -> file.EndsWith(".fsproj"))
|> List.filter (fun file ->
file.EndsWith(".fsproj", StringComparison.Ordinal)
)
|> function
| [] ->
files
|> List.filter (fun file -> file.EndsWith(".fsx"))
|> List.filter (fun file ->
file.EndsWith(".fsx", StringComparison.Ordinal)
)
| candidates -> candidates
|> function
| [] ->
Expand Down Expand Up @@ -388,7 +392,7 @@ type Runner =
let fileExt =
args.Value("-e", "--extension")
|> Option.map (fun e ->
if e.StartsWith(".") then
if e.StartsWith('.') then
e
else
"." + e
Expand Down Expand Up @@ -537,7 +541,7 @@ let clean (args: CliArgs) language rootDir =
"No files have been deleted. If Fable output is in another directory, pass it as argument."
)
else
Log.always ("Clean completed! Files deleted: " + string fileCount)
Log.always ("Clean completed! Files deleted: " + string<int> fileCount)

let getStatus =
function
Expand All @@ -563,7 +567,9 @@ let main argv =
let! argv, runProc =
argv
|> List.ofArray
|> List.splitWhile (fun a -> not (a.StartsWith("--run")))
|> List.splitWhile (fun a ->
not (a.StartsWith("--run", StringComparison.Ordinal))
)
|> function
| argv, flag :: runArgs ->
match flag, runArgs with
Expand All @@ -584,7 +590,7 @@ let main argv =
| ("help" | "--help" | "-h") :: _ -> [ "--help" ], []
| "--version" :: _ -> [ "--version" ], []
| argv ->
argv |> List.splitWhile (fun x -> x.StartsWith("-") |> not)
argv |> List.splitWhile (fun x -> x.StartsWith('-') |> not)

let! args = parseCliArgs args
let! language = argLanguage args
Expand Down
25 changes: 16 additions & 9 deletions src/Fable.Cli/Globbing.fs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
namespace Fable.Cli

open System
open System.Collections.Generic
open System.IO

Expand Down Expand Up @@ -141,7 +142,7 @@ module Globbing =

let globRoot =
// If we dropped "/" from the beginning of the path in the 'Split' call, put it back!
if normPattern.StartsWith("/") then
if normPattern.StartsWith('/') then
"/" + globRoot
else
globRoot
Expand All @@ -163,9 +164,9 @@ module Globbing =
// names (as one folder name could be a substring of the other)
let start =
baseDir.TrimEnd([| Path.DirectorySeparatorChar |])
+ string Path.DirectorySeparatorChar
+ string<char> Path.DirectorySeparatorChar
// See https://github.com/fsharp/FAKE/issues/1925
if input.StartsWith start then
if input.StartsWith(start, StringComparison.Ordinal) then
input.Substring start.Length
else
input
Expand All @@ -183,7 +184,10 @@ module Globbing =

let baseItems =
let start, rest =
if input.StartsWith "\\\\" && splits.Length >= 4 then
if
input.StartsWith("\\\\", StringComparison.Ordinal)
&& splits.Length >= 4
then
let serverName = splits.[2]
let share = splits.[3]

Expand All @@ -198,12 +202,12 @@ module Globbing =
elif
splits.Length >= 2
&& Path.IsPathRooted input
&& input.StartsWith "/"
&& input.StartsWith '/'
then
[ Directory("/") ], splits |> Array.toSeq
else
if Path.IsPathRooted input then
if input.StartsWith "\\" then
if input.StartsWith '\\' then
failwithf
"Please remove the leading '\\' or '/' and replace them with \
'.\\' or './' if you want to use a relative path. Leading \
Expand Down Expand Up @@ -415,13 +419,13 @@ module Globbing =

let included =
this.Includes
|> Seq.exists (fun fileInclude ->
|> List.exists (fun fileInclude ->
Glob.isMatch (fullDir fileInclude) fullPath
)

let excluded =
this.Excludes
|> Seq.exists (fun fileExclude ->
|> List.exists (fun fileExclude ->
Glob.isMatch (fullDir fileExclude) fullPath
)

Expand Down Expand Up @@ -468,7 +472,10 @@ module Globbing =
|> Seq.filter (fun d ->
directoryIncludes
|> Seq.exists (fun p ->
d.StartsWith(p + string Path.DirectorySeparatorChar)
d.StartsWith(
p + string<char> Path.DirectorySeparatorChar,
StringComparison.Ordinal
)
&& p <> d
)
|> not
Expand Down
26 changes: 19 additions & 7 deletions src/Fable.Cli/Main.fs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ module private Util =
}

let isImplementationFile (fileName: string) =
fileName.EndsWith(".fs") || fileName.EndsWith(".fsx")
fileName.EndsWith(".fs", StringComparison.Ordinal)
|| fileName.EndsWith(".fsx", StringComparison.Ordinal)

let caseInsensitiveSet (items: string seq) : ISet<string> =
let s = HashSet(items)
Expand Down Expand Up @@ -312,7 +313,7 @@ module FileWatcherUtil =
// See https://github.com/fable-compiler/Fable/pull/2725#issuecomment-1015123642
|> List.filter (fun file ->
not (
file.EndsWith(".fsproj.fsx")
file.EndsWith(".fsproj.fsx", StringComparison.Ordinal)
// It looks like latest F# compiler puts generated files for resolution of packages
// in scripts in $HOME/.packagemanagement. See #3222
|| file.Contains(".packagemanagement")
Expand All @@ -334,7 +335,8 @@ module FileWatcherUtil =
if
restDirs
|> List.forall (fun d ->
(withTrailingSep d).StartsWith dir'
(withTrailingSep d)
.StartsWith(dir', StringComparison.Ordinal)
)
then
dir
Expand Down Expand Up @@ -800,8 +802,14 @@ and FableCompiler
let filesToCompile =
state.FilesToCompile
|> Array.filter (fun file ->
file.EndsWith(".fs")
|| file.EndsWith(".fsx")
file.EndsWith(
".fs",
StringComparison.Ordinal
)
|| file.EndsWith(
".fsx",
StringComparison.Ordinal
)
)

(state, filesToCompile)
Expand Down Expand Up @@ -1084,7 +1092,8 @@ let private areCompiledFilesUpToDate (state: State) (filesToCompile: string[]) =
let upToDate =
filesToCompile
|> Array.filter (fun file ->
file.EndsWith(".fs") || file.EndsWith(".fsx")
file.EndsWith(".fs", StringComparison.Ordinal)
|| file.EndsWith(".fsx", StringComparison.Ordinal)
)
|> Array.forall (fun source ->
let outPath = getOutPath state.CliArgs pathResolver source
Expand Down Expand Up @@ -1208,7 +1217,10 @@ let private compilationCycle (state: State) (changes: ISet<string>) =
| Some(projCracked, fableCompiler) ->
// For performance reasons, don't crack .fsx scripts for every change
let fsprojChanged =
changes |> Seq.exists (fun c -> c.EndsWith(".fsproj"))
changes
|> Seq.exists (fun c ->
c.EndsWith(".fsproj", StringComparison.Ordinal)
)

if fsprojChanged then
let oldProjCracked = projCracked
Expand Down
10 changes: 5 additions & 5 deletions src/Fable.Cli/Pipeline.fs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ module Js =
let fileExt =
let fileExt = cliArgs.CompilerOptions.FileExtension

if fileExt.EndsWith(".ts") then
if fileExt.EndsWith(".ts", StringComparison.Ordinal) then
Path.ChangeExtension(fileExt, ".js")
else
fileExt
Expand Down Expand Up @@ -172,7 +172,7 @@ module Js =
cliArgs.OutDir
path

if path.EndsWith(".fs") then
if path.EndsWith(".fs", StringComparison.Ordinal) then
let isInFableModules =
Path.Combine(targetDir, path) |> Naming.isInFableModules

Expand Down Expand Up @@ -488,7 +488,7 @@ module Php =
cliArgs.OutDir
path

if path.EndsWith(".fs") then
if path.EndsWith(".fs", StringComparison.Ordinal) then
Path.ChangeExtension(path, fileExt)
else
path
Expand Down Expand Up @@ -546,7 +546,7 @@ module Dart =
cliArgs.OutDir
path

if path.EndsWith(".fs") then
if path.EndsWith(".fs", StringComparison.Ordinal) then
Path.ChangeExtension(path, fileExt)
else
path
Expand Down Expand Up @@ -607,7 +607,7 @@ module Rust =
cliArgs.OutDir
path

if path.EndsWith(".fs") then
if path.EndsWith(".fs", StringComparison.Ordinal) then
Path.ChangeExtension(path, fileExt)
else
path
Expand Down
Loading
Loading