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

Adds exclusion config for analyzers #1120

Merged
merged 1 commit into from
Jun 21, 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
40 changes: 40 additions & 0 deletions src/FsAutoComplete/LspHelpers.fs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ open System.Collections.Generic
open Ionide.ProjInfo.ProjectSystem
open FSharp.Compiler.Diagnostics
open FSharp.Compiler.EditorServices
open System.Text.RegularExpressions
open FsAutoComplete.Logging


type FcsRange = FSharp.Compiler.Text.Range
Expand Down Expand Up @@ -640,8 +642,11 @@ type FSharpConfigDto =
InterfaceStubGenerationMethodBody: string option
AddPrivateAccessModifier: bool option
UnusedOpensAnalyzer: bool option
UnusedOpensAnalyzerExclusions: string array option
UnusedDeclarationsAnalyzer: bool option
UnusedDeclarationsAnalyzerExclusions: string array option
SimplifyNameAnalyzer: bool option
SimplifyNameAnalyzerExclusions: string array option
ResolveNamespaces: bool option
EnableReferenceCodeLens: bool option
EnableAnalyzers: bool option
Expand Down Expand Up @@ -738,6 +743,15 @@ type DebugConfig =
LogDurationBetweenCheckFiles = false
LogCheckFileDuration = false }

let logger = lazy (LogProvider.getLoggerByName "LspHelpers")

let tryCreateRegex (pattern: string) =
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Safe creation of these Regex since the config could be bad regex.

try
Some(Regex(pattern))
with e ->
logger.Value.warn (Log.setMessageI $"Invalid regex pattern: {pattern}" >> Log.addExn e)
None

type FSharpConfig =
{ AutomaticWorkspaceInit: bool
WorkspaceModePeekDeepLevel: int
Expand All @@ -759,8 +773,11 @@ type FSharpConfig =
InterfaceStubGenerationMethodBody: string
AddPrivateAccessModifier: bool
UnusedOpensAnalyzer: bool
UnusedOpensAnalyzerExclusions: Regex array
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we already have FSharp.unusedOpensAnalyzer it would be a breaking change to make it a nested object and all client configs would need to update for this. Could be renamed to whatever makes more sense though.

UnusedDeclarationsAnalyzer: bool
UnusedDeclarationsAnalyzerExclusions: Regex array
SimplifyNameAnalyzer: bool
SimplifyNameAnalyzerExclusions: Regex array
ResolveNamespaces: bool
EnableReferenceCodeLens: bool
EnableAnalyzers: bool
Expand Down Expand Up @@ -801,8 +818,11 @@ type FSharpConfig =
InterfaceStubGenerationMethodBody = "failwith \"Not Implemented\""
AddPrivateAccessModifier = false
UnusedOpensAnalyzer = false
UnusedOpensAnalyzerExclusions = [||]
UnusedDeclarationsAnalyzer = false
UnusedDeclarationsAnalyzerExclusions = [||]
SimplifyNameAnalyzer = false
SimplifyNameAnalyzerExclusions = [||]
ResolveNamespaces = false
EnableReferenceCodeLens = false
EnableAnalyzers = false
Expand Down Expand Up @@ -841,8 +861,15 @@ type FSharpConfig =
defaultArg dto.InterfaceStubGenerationMethodBody "failwith \"Not Implemented\""
AddPrivateAccessModifier = defaultArg dto.AddPrivateAccessModifier false
UnusedOpensAnalyzer = defaultArg dto.UnusedOpensAnalyzer false
UnusedOpensAnalyzerExclusions = defaultArg dto.UnusedOpensAnalyzerExclusions [||] |> Array.choose tryCreateRegex
UnusedDeclarationsAnalyzer = defaultArg dto.UnusedDeclarationsAnalyzer false
UnusedDeclarationsAnalyzerExclusions =
defaultArg dto.UnusedDeclarationsAnalyzerExclusions [||]
|> Array.choose tryCreateRegex
SimplifyNameAnalyzer = defaultArg dto.SimplifyNameAnalyzer false
SimplifyNameAnalyzerExclusions =
defaultArg dto.SimplifyNameAnalyzerExclusions [||]
|> Array.choose tryCreateRegex
ResolveNamespaces = defaultArg dto.ResolveNamespaces false
EnableReferenceCodeLens = defaultArg dto.EnableReferenceCodeLens false
EnableAnalyzers = defaultArg dto.EnableAnalyzers false
Expand Down Expand Up @@ -932,8 +959,21 @@ type FSharpConfig =
defaultArg dto.InterfaceStubGenerationMethodBody x.InterfaceStubGenerationMethodBody
AddPrivateAccessModifier = defaultArg dto.AddPrivateAccessModifier x.AddPrivateAccessModifier
UnusedOpensAnalyzer = defaultArg dto.UnusedOpensAnalyzer x.UnusedOpensAnalyzer
UnusedOpensAnalyzerExclusions =
defaultArg
(dto.UnusedOpensAnalyzerExclusions |> Option.map (Array.choose tryCreateRegex))
x.UnusedOpensAnalyzerExclusions
UnusedDeclarationsAnalyzer = defaultArg dto.UnusedDeclarationsAnalyzer x.UnusedDeclarationsAnalyzer
UnusedDeclarationsAnalyzerExclusions =
defaultArg
(dto.UnusedDeclarationsAnalyzerExclusions
|> Option.map (Array.choose tryCreateRegex))
x.UnusedDeclarationsAnalyzerExclusions
SimplifyNameAnalyzer = defaultArg dto.SimplifyNameAnalyzer x.SimplifyNameAnalyzer
SimplifyNameAnalyzerExclusions =
defaultArg
(dto.SimplifyNameAnalyzerExclusions |> Option.map (Array.choose tryCreateRegex))
x.SimplifyNameAnalyzerExclusions
ResolveNamespaces = defaultArg dto.ResolveNamespaces x.ResolveNamespaces
EnableReferenceCodeLens = defaultArg dto.EnableReferenceCodeLens x.EnableReferenceCodeLens
EnableAnalyzers = defaultArg dto.EnableAnalyzers x.EnableAnalyzers
Expand Down
45 changes: 34 additions & 11 deletions src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ open FsAutoComplete.LspHelpers
open FsAutoComplete.UnionPatternMatchCaseGenerator
open System.Collections.Concurrent
open System.Diagnostics
open System.Text.RegularExpressions

[<RequireQualifiedAccess>]
type WorkspaceChosen =
Expand Down Expand Up @@ -251,16 +252,23 @@ type AdaptiveFSharpLspServer(workspaceLoader: IWorkspaceLoader, lspClient: FShar

let builtInCompilerAnalyzers config (file: VolatileFile) (tyRes: ParseAndCheckResults) =
let filePath = file.FileName
let filePathUntag = UMX.untag filePath
let source = file.Lines
let version = file.Version


let inline getSourceLine lineNo =
(source: ISourceText).GetLineString(lineNo - 1)

let checkUnusedOpens =
async {
try
let! ct = Async.CancellationToken
use progress = new ServerProgressReport(lspClient)
do! progress.Begin("Checking unused opens...", message = filePathUntag)
Comment on lines +266 to +267
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adds ProgressReport for users to help identify files which might benefit from disabling.


let! unused = UnusedOpens.getUnusedOpens (tyRes.GetCheckResults, getSourceLine)

let! unused =
UnusedOpens.getUnusedOpens (tyRes.GetCheckResults, (fun i -> (source: ISourceText).GetLineString(i - 1)))
let! ct = Async.CancellationToken

notifications.Trigger(NotificationEvent.UnusedOpens(filePath, (unused |> List.toArray)), ct)
with e ->
Expand All @@ -270,11 +278,14 @@ type AdaptiveFSharpLspServer(workspaceLoader: IWorkspaceLoader, lspClient: FShar
let checkUnusedDeclarations =
async {
try
let! ct = Async.CancellationToken
let isScript = Utils.isAScript (UMX.untag filePath)
use progress = new ServerProgressReport(lspClient)
do! progress.Begin("Checking unused declarations...", message = filePathUntag)

let isScript = Utils.isAScript (filePathUntag)
let! unused = UnusedDeclarations.getUnusedDeclarations (tyRes.GetCheckResults, isScript)
let unused = unused |> Seq.toArray

let! ct = Async.CancellationToken
notifications.Trigger(NotificationEvent.UnusedDeclarations(filePath, unused), ct)
with e ->
logger.error (Log.setMessage "checkUnusedDeclarations failed" >> Log.addExn e)
Expand All @@ -283,26 +294,35 @@ type AdaptiveFSharpLspServer(workspaceLoader: IWorkspaceLoader, lspClient: FShar
let checkSimplifiedNames =
async {
try
let getSourceLine lineNo =
(source: ISourceText).GetLineString(lineNo - 1)
use progress = new ServerProgressReport(lspClient)
do! progress.Begin("Checking simplifing of names...", message = filePathUntag)

let! ct = Async.CancellationToken
let! simplified = SimplifyNames.getSimplifiableNames (tyRes.GetCheckResults, getSourceLine)
let simplified = Array.ofSeq simplified
let! ct = Async.CancellationToken
notifications.Trigger(NotificationEvent.SimplifyNames(filePath, simplified), ct)
with e ->
logger.error (Log.setMessage "checkSimplifiedNames failed" >> Log.addExn e)
}

let inline isNotExcluded (exclusions: Regex array) =
exclusions |> Array.exists (fun r -> r.IsMatch filePathUntag) |> not

let analyzers =
[
// if config.Linter then
// commands.Lint filePath |> Async .Ignore
if config.UnusedOpensAnalyzer then
if config.UnusedOpensAnalyzer && isNotExcluded config.UnusedOpensAnalyzerExclusions then
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exclusion check

checkUnusedOpens
if config.UnusedDeclarationsAnalyzer then
if
config.UnusedDeclarationsAnalyzer
&& isNotExcluded config.UnusedDeclarationsAnalyzerExclusions
then
checkUnusedDeclarations
if config.SimplifyNameAnalyzer then
if
config.SimplifyNameAnalyzer
&& isNotExcluded config.SimplifyNameAnalyzerExclusions
then
checkSimplifiedNames ]

async {
Expand All @@ -322,6 +342,9 @@ type AdaptiveFSharpLspServer(workspaceLoader: IWorkspaceLoader, lspClient: FShar
let file = volatileFile.FileName

try
use progress = new ServerProgressReport(lspClient)
do! progress.Begin("Running analyzers...", message = UMX.untag file)

Loggers.analyzers.info (
Log.setMessage "begin analysis of {file}"
>> Log.addContextDestructured "file" file
Expand Down
3 changes: 3 additions & 0 deletions test/FsAutoComplete.Tests.Lsp/Helpers.fs
Original file line number Diff line number Diff line change
Expand Up @@ -236,8 +236,11 @@ let defaultConfigDto: FSharpConfigDto =
RecordStubGenerationBody = None
AddPrivateAccessModifier = None
UnusedOpensAnalyzer = None
UnusedOpensAnalyzerExclusions = None
UnusedDeclarationsAnalyzer = None
UnusedDeclarationsAnalyzerExclusions = None
SimplifyNameAnalyzer = None
SimplifyNameAnalyzerExclusions = None
ResolveNamespaces = None
EnableReferenceCodeLens = None
EnableAnalyzers = None
Expand Down