Merge pull request #3671 from nojaf/logging

Revist logging
fable-compiler · Jan 22, 2024 · 2505c1d · 2505c1d
2 parents 686e806 + 862d9d1
commit 2505c1d
Show file tree

Hide file tree

Showing 18 changed files with 338 additions and 146 deletions.
diff --git a/.config/dotnet-tools.json b/.config/dotnet-tools.json
@@ -15,7 +15,7 @@
       ]
     },
     "fsharp-analyzers": {
-      "version": "0.21.0",
+      "version": "0.23.0",
       "commands": [
         "fsharp-analyzers"
       ]

diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -6,6 +6,10 @@ on:
   pull_request:
     branches: [ main ]
 
+permissions:
+    id-token: write
+    security-events: write
+
 jobs:
   # Separate job that verifies if all code was formatted correctly
   # Run `dotnet fantomas .` to format all code.
@@ -200,3 +204,40 @@ jobs:
 
     - name: Fable Tests - Dart
       run: ./build.sh test dart
+
+  # Separate job to run F# analyzers
+  analyzers:
+      runs-on: windows-latest
+
+      steps:
+      - uses: actions/checkout@v4
+
+      - name: Setup .NET
+        uses: actions/setup-dotnet@v4
+
+      - name: Restore tools
+        run: dotnet tool restore
+
+      - name: Build solution
+        run: dotnet build -c Release Fable.sln
+
+      - name: Run analyzers for Fable.AST
+        run: dotnet msbuild /t:AnalyzeFSharpProject src/Fable.AST/Fable.AST.fsproj
+        continue-on-error: true
+
+      - name: Run analyzers for Rust.AST
+        run: dotnet msbuild /t:AnalyzeFSharpProject src/Fable.Transforms/Rust/AST/Rust.AST.fsproj
+        continue-on-error: true
+
+      - name: Run analyzers Fable.Transforms
+        run: dotnet msbuild /t:AnalyzeFSharpProject src/Fable.Transforms/Fable.Transforms.fsproj
+        continue-on-error: true
+
+      - name: Run analyzers Fable.Compiler
+        run: dotnet msbuild /t:AnalyzeFSharpProject src/Fable.Compiler/Fable.Compiler.fsproj
+        continue-on-error: true
+
+      - name: Upload SARIF files
+        uses: github/codeql-action/upload-sarif@v2
+        with:
+            sarif_file: src/reports
diff --git a/Fable.sln b/Fable.sln
@@ -64,6 +64,8 @@ Project("{F2A71F9B-5D33-465A-A702-920D77279786}") = "Fable.Build", "src\Fable.Bu
 EndProject
 Project("{F2A71F9B-5D33-465A-A702-920D77279786}") = "Fable.Compiler", "src\Fable.Compiler\Fable.Compiler.fsproj", "{942DD29B-07C0-4ACF-891E-85C1235A9BE0}"
 EndProject
+Project("{F2A71F9B-5D33-465A-A702-920D77279786}") = "Fable.Analyzers", "src\Fable.Analyzers\Fable.Analyzers.fsproj", "{75B5084B-C267-47EC-B4A1-7764A1F2A2FF}"
+EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution
 		Debug|Any CPU = Debug|Any CPU
@@ -166,6 +168,10 @@ Global
 		{942DD29B-07C0-4ACF-891E-85C1235A9BE0}.Debug|Any CPU.Build.0 = Debug|Any CPU
 		{942DD29B-07C0-4ACF-891E-85C1235A9BE0}.Release|Any CPU.ActiveCfg = Release|Any CPU
 		{942DD29B-07C0-4ACF-891E-85C1235A9BE0}.Release|Any CPU.Build.0 = Release|Any CPU
+		{75B5084B-C267-47EC-B4A1-7764A1F2A2FF}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{75B5084B-C267-47EC-B4A1-7764A1F2A2FF}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{75B5084B-C267-47EC-B4A1-7764A1F2A2FF}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{75B5084B-C267-47EC-B4A1-7764A1F2A2FF}.Release|Any CPU.Build.0 = Release|Any CPU
 	EndGlobalSection
 	GlobalSection(SolutionProperties) = preSolution
 		HideSolutionNode = FALSE
@@ -196,6 +202,7 @@ Global
 		{C90E23AF-4B5B-44A7-ADCC-3BF89547395B} = {DA29278E-3808-42DE-8333-964F129F295D}
 		{F2E323CE-FDF3-4A1E-AE97-B723D2E63763} = {C8CB96CF-68A8-4083-A0F8-319275CF8097}
 		{942DD29B-07C0-4ACF-891E-85C1235A9BE0} = {C8CB96CF-68A8-4083-A0F8-319275CF8097}
+		{75B5084B-C267-47EC-B4A1-7764A1F2A2FF} = {C8CB96CF-68A8-4083-A0F8-319275CF8097}
 	EndGlobalSection
 	GlobalSection(ExtensibilityGlobals) = postSolution
 		SolutionGuid = {58DF9285-8523-4EAC-B598-BE5B02A76A00}

diff --git a/src/Directory.Build.props b/src/Directory.Build.props
@@ -24,11 +24,15 @@
     </ItemGroup>
     <ItemGroup>
         <PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.0.0" PrivateAssets="All" />
-        <PackageReference Include="FSharp.Analyzers.Build" Version="0.2.0">
+        <PackageReference Include="FSharp.Analyzers.Build" Version="0.3.0">
             <PrivateAssets>all</PrivateAssets>
             <IncludeAssets>build</IncludeAssets>
         </PackageReference>
-        <PackageReference Include="G-Research.FSharp.Analyzers" Version="0.4.0">
+        <PackageReference Include="G-Research.FSharp.Analyzers" Version="0.7.0">
+            <PrivateAssets>all</PrivateAssets>
+            <IncludeAssets>analyzers</IncludeAssets>
+        </PackageReference>
+        <PackageReference Include="Ionide.Analyzers" Version="0.7.0">
             <PrivateAssets>all</PrivateAssets>
             <IncludeAssets>analyzers</IncludeAssets>
         </PackageReference>

diff --git a/src/Directory.Build.targets b/src/Directory.Build.targets
@@ -1,11 +1,12 @@
 <Project>
     <PropertyGroup>
+        <CodeRoot>$([System.IO.Path]::GetDirectoryName($(DirectoryBuildTargetsPath)))</CodeRoot>
         <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>
+        <FSharpAnalyzersOtherFlags>$(FSharpAnalyzersOtherFlags) --analyzers-path  &quot;$(CodeRoot)/Fable.Analyzers/bin/Release/net6.0&quot;</FSharpAnalyzersOtherFlags>
         <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>
+        <FSharpAnalyzersOtherFlags>$(FSharpAnalyzersOtherFlags) --exclude-analyzers PartialAppAnalyzer</FSharpAnalyzersOtherFlags>
     </PropertyGroup>
 </Project>
diff --git a/src/Fable.Analyzers/Fable.Analyzers.fsproj b/src/Fable.Analyzers/Fable.Analyzers.fsproj
@@ -0,0 +1,17 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+    <PropertyGroup>
+        <TargetFramework>net6.0</TargetFramework>
+        <GenerateDocumentationFile>true</GenerateDocumentationFile>
+    </PropertyGroup>
+
+    <ItemGroup>
+        <Compile Include="StdOutAnalyzer.fs"/>
+    </ItemGroup>
+
+    <ItemGroup>
+        <PackageReference Update="FSharp.Core" Version="8.0.100"/>
+        <PackageReference Include="FSharp.Analyzers.SDK" Version="0.23.0"/>
+    </ItemGroup>
+
+</Project>
diff --git a/src/Fable.Analyzers/StdOutAnalyzer.fs b/src/Fable.Analyzers/StdOutAnalyzer.fs
@@ -0,0 +1,62 @@
+module Fable.Analyzers.StdOutAnalyzer
+
+open System
+open FSharp.Analyzers.SDK.TASTCollecting
+open FSharp.Compiler.Symbols
+open FSharp.Compiler.Text
+open FSharp.Analyzers.SDK
+
+let namesToAvoid =
+    set
+        [
+            "System.Console.Write"
+            "System.Console.WriteLine"
+            "System.Console.WriteAsync"
+            "System.Console.WriteLineAsync"
+            "Microsoft.FSharp.Core.ExtraTopLevelOperators.printf"
+            "Microsoft.FSharp.Core.ExtraTopLevelOperators.printfn"
+        ]
+
+[<CliAnalyzer "StdOutAnalyzer">]
+let stdOutAnalyzer: Analyzer<CliContext> =
+    fun (context: CliContext) ->
+        async {
+            let usages = ResizeArray<range * string>()
+
+            let walker =
+                { new TypedTreeCollectorBase() with
+                    override _.WalkCall
+                        _
+                        (m: FSharpMemberOrFunctionOrValue)
+                        _
+                        _
+                        (args: FSharpExpr list)
+                        (range: range)
+                        =
+                        match m.DeclaringEntity with
+                        | None -> ()
+                        | Some de ->
+                            let name =
+                                String.Join(".", de.FullName, m.DisplayName)
+
+                            if Set.contains name namesToAvoid then
+                                usages.Add(range, m.DisplayName)
+                }
+
+            Option.iter (walkTast walker) context.TypedTree
+
+            return
+                usages
+                |> Seq.map (fun (m, name) ->
+                    {
+                        Type = "StdOut analyzer"
+                        Message =
+                            $"Writing to the standard output from this location should absolutely be avoided. Replace `%s{name}` with an ILogger call."
+                        Code = "FABLE_001"
+                        Severity = Error
+                        Range = m
+                        Fixes = []
+                    }
+                )
+                |> Seq.toList
+        }
diff --git a/src/Fable.Cli/CHANGELOG.md b/src/Fable.Cli/CHANGELOG.md
@@ -33,6 +33,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * Fixed 'System.Collections.Generic.Queue' bug (by @PierreYvesR)
 * Added support for generic comparers (by @ncave)
 
+### Changed
+
+#### All
+
+* [GH-3671](https://github.com/fable-compiler/Fable/pull/3671) Use `Microsoft.Extensions.Logging` (by @nojaf)
+
 ## 4.9.0 - 2023-12-14
 
 ### Fixed

diff --git a/src/Fable.Cli/Entry.fs b/src/Fable.Cli/Entry.fs
@@ -4,6 +4,7 @@ open System
 open Main
 open Fable
 open Fable.Compiler.Util
+open Microsoft.Extensions.Logging
 
 type CliArgs(args: string list) =
     let argsMap =
@@ -249,6 +250,7 @@ type Runner =
             language: Language,
             rootDir: string,
             runProc: RunProcess option,
+            verbosity: Fable.Verbosity,
             ?fsprojPath: string,
             ?watch,
             ?precompile
@@ -355,13 +357,6 @@ type Runner =
                 else
                     Ok()
 
-            let verbosity =
-                if args.FlagEnabled "--verbose" then
-                    Log.makeVerbose ()
-                    Verbosity.Verbose
-                else
-                    Verbosity.Normal
-
             let configuration =
                 let defaultConfiguration =
                     if watch then
@@ -445,6 +440,7 @@ type Runner =
                         |> Map
                     RunProcess = runProc
                     CompilerOptions = compilerOptions
+                    Verbosity = verbosity
                 }
 
             let watchDelay =
@@ -603,12 +599,29 @@ let main argv =
             | Some rootDir -> File.getExactFullPath rootDir
             | None -> IO.Directory.GetCurrentDirectory()
 
-        do
+        let verbosity =
             match commands with
-            | [ "--version" ] -> ()
+            | [ "--version" ] -> Verbosity.Normal
             | _ ->
-                if args.FlagEnabled "--verbose" then
-                    Log.makeVerbose ()
+                let verbosity =
+                    let level, verbosity =
+                        if args.FlagEnabled "--verbose" then
+                            LogLevel.Debug, Fable.Verbosity.Verbose
+                        else
+                            LogLevel.Information, Fable.Verbosity.Normal
+
+                    use factory =
+                        LoggerFactory.Create(fun builder ->
+                            builder
+                                .SetMinimumLevel(level)
+                                .AddSimpleConsole(fun options ->
+                                    options.SingleLine <- true
+                                )
+                            |> ignore
+                        )
+
+                    Log.setLogger (factory.CreateLogger(""))
+                    verbosity
 
                 let status =
                     match getStatus language with
@@ -636,6 +649,8 @@ let main argv =
                     + "\n"
                 )
 
+                verbosity
+
         match commands with
         | [ "--help" ] -> return printHelp ()
         | [ "--version" ] -> return Log.always Literals.VERSION
@@ -648,31 +663,49 @@ let main argv =
                     language,
                     rootDir,
                     runProc,
+                    verbosity,
                     fsprojPath = path,
                     watch = true
                 )
         | [ "watch" ] ->
-            return! Runner.Run(args, language, rootDir, runProc, watch = true)
+            return!
+                Runner.Run(
+                    args,
+                    language,
+                    rootDir,
+                    runProc,
+                    verbosity,
+                    watch = true
+                )
         | [ "precompile"; path ] ->
             return!
                 Runner.Run(
                     args,
                     language,
                     rootDir,
                     runProc,
+                    verbosity,
                     fsprojPath = path,
                     precompile = true
                 )
         | [ "precompile" ] ->
             return!
-                Runner.Run(args, language, rootDir, runProc, precompile = true)
+                Runner.Run(
+                    args,
+                    language,
+                    rootDir,
+                    runProc,
+                    verbosity,
+                    precompile = true
+                )
         | [ path ] ->
             return!
                 Runner.Run(
                     args,
                     language,
                     rootDir,
                     runProc,
+                    verbosity,
                     fsprojPath = path,
                     watch = args.FlagEnabled("--watch")
                 )
@@ -683,6 +716,7 @@ let main argv =
                     language,
                     rootDir,
                     runProc,
+                    verbosity,
                     watch = args.FlagEnabled("--watch")
                 )
         | _ ->

diff --git a/src/Fable.Cli/Fable.Cli.fsproj b/src/Fable.Cli/Fable.Cli.fsproj
@@ -52,6 +52,7 @@
   <ItemGroup>
     <PackageReference Include="Buildalyzer" Version="5.0.1" />
     <PackageReference Include="FSharp.SystemTextJson" Version="1.2.42" />
+    <PackageReference Include="Microsoft.Extensions.Logging.Console" Version="8.0.0" />
     <PackageReference Include="source-map-sharp" Version="1.0.9" />
   </ItemGroup>
 </Project>