Add unnecessary parens analyzer & codefix

This uses the API exposed in FSC in
dotnet/fsharp#16079.

Author: brianrourkeboll

src/FsAutoComplete.Core/Commands.fs

@@ -80,6 +80,7 @@ type NotificationEvent =
   // | Lint of file: string<LocalPath> * warningsWithCodes: Lint.EnrichedLintWarning list
   | UnusedDeclarations of file: string<LocalPath> * decls: range[] * version: int
   | SimplifyNames of file: string<LocalPath> * names: SimplifyNames.SimplifiableRange[] * version: int
+  | UnnecessaryParentheses of file: string<LocalPath> * ranges: range[] * version: int
   | Canceled of errorMessage: string
   | FileParsed of string<LocalPath>
   | TestDetected of file: string<LocalPath> * tests: TestAdapter.TestAdapterEntry<range>[]

src/FsAutoComplete.Core/Utils.fs

@@ -499,6 +499,38 @@ type ReadOnlySpanExtensions =
 
     line
 
+  #if !NET7_0_OR_GREATER
+  [<Extension>]
+  static member IndexOfAnyExcept(span: ReadOnlySpan<char>, value0: char, value1: char) =
+      let mutable i = 0
+      let mutable found = false
+
+      while not found && i < span.Length do
+          let c = span[i]
+
+          if c <> value0 && c <> value1 then
+              found <- true
+          else
+              i <- i + 1
+
+      if found then i else -1
+
+  [<Extension>]
+  static member LastIndexOfAnyExcept(span: ReadOnlySpan<char>, value0: char, value1: char) =
+      let mutable i = span.Length - 1
+      let mutable found = false
+
+      while not found && i >= 0 do
+          let c = span[i]
+
+          if c <> value0 && c <> value1 then
+              found <- true
+          else
+              i <- i - 1
+
+      if found then i else -1
+#endif
+
 type ConcurrentDictionary<'key, 'value> with
 
   member x.TryFind key =

src/FsAutoComplete.Core/Utils.fsi

@@ -181,6 +181,14 @@ type ReadOnlySpanExtensions =
   [<Extension>]
   static member LastLine: text: ReadOnlySpan<char> -> ReadOnlySpan<char>
 
+#if !NET7_0_OR_GREATER
+  [<Extension>]
+  static member IndexOfAnyExcept: span: ReadOnlySpan<char> * value0: char * value1: char -> int
+
+  [<Extension>]
+  static member LastIndexOfAnyExcept: span: ReadOnlySpan<char> * value0: char * value1: char -> int
+#endif
+
 type ConcurrentDictionary<'key, 'value> with
 
   member TryFind: key: 'key -> 'value option

src/FsAutoComplete/CodeFixes/RemoveUnnecessaryParentheses.fs

@@ -0,0 +1,163 @@
+module FsAutoComplete.CodeFix.RemoveUnnecessaryParentheses
+
+open System
+open Ionide.LanguageServerProtocol.Types
+open FsAutoComplete.CodeFix
+open FsAutoComplete.CodeFix.Types
+open FsToolkit.ErrorHandling
+open FsAutoComplete
+open FsAutoComplete.LspHelpers
+
+let title = "Remove unnecessary parentheses"
+
+[<AutoOpen>]
+module private Patterns =
+  let inline toPat f x = if f x then ValueSome() else ValueNone
+
+  [<AutoOpen>]
+  module Char =
+    [<return: Struct>]
+    let inline (|LetterOrDigit|_|) c = toPat Char.IsLetterOrDigit c
+
+    [<return: Struct>]
+    let inline (|Punctuation|_|) c = toPat Char.IsPunctuation c
+
+    [<return: Struct>]
+    let inline (|Symbol|_|) c = toPat Char.IsSymbol c
+
+  [<AutoOpen>]
+  module SourceText =
+    /// E.g., something like:
+    ///
+    ///     let … = (␤
+    ///     …
+    ///     )
+    [<return: Struct>]
+    let (|TrailingOpen|_|) (range: Range) (sourceText: IFSACSourceText) =
+      let line = sourceText.Lines[range.Start.Line]
+
+      if
+        line.AsSpan(0, range.Start.Character).LastIndexOfAnyExcept(' ', '(') >= 0
+        && line.AsSpan(range.Start.Character).IndexOfAnyExcept('(', ' ') < 0
+      then
+        ValueSome TrailingOpen
+      else
+        ValueNone
+
+    /// Trim only spaces from the start if there is something else
+    /// before the open paren on the same line (or else we could move
+    /// the whole inner expression up a line); otherwise trim all whitespace
+    // from start and end.
+    let (|Trim|) (range: Range) (sourceText: IFSACSourceText) =
+      let line = sourceText.Lines[range.Start.Line]
+
+      if line.AsSpan(0, range.Start.Character).LastIndexOfAnyExcept(' ', '(') >= 0 then
+        fun (s: string) -> s.TrimEnd().TrimStart ' '
+      else
+        fun (s: string) -> s.Trim()
+
+    /// Returns the offsides diff if the given span contains an expression
+    /// whose indentation would be made invalid if the open paren
+    /// were removed (because the offside line would be shifted).
+    [<return: Struct>]
+    let (|OffsidesDiff|_|) (range: Range) (sourceText: IFSACSourceText) =
+      let startLineNo = range.Start.Line
+      let endLineNo = range.End.Line
+
+      if startLineNo = endLineNo then
+        ValueNone
+      else
+        let rec loop innerOffsides lineNo startCol =
+          if lineNo <= endLineNo then
+            let line = sourceText.Lines[lineNo]
+
+            match line.AsSpan(startCol).IndexOfAnyExcept(' ', ')') with
+            | -1 -> loop innerOffsides (lineNo + 1) 0
+            | i -> loop (i + startCol) (lineNo + 1) 0
+          else
+            ValueSome(range.Start.Character - innerOffsides)
+
+        loop range.Start.Character startLineNo (range.Start.Character + 1)
+
+    let (|ShiftLeft|NoShift|ShiftRight|) n =
+      if n < 0 then ShiftLeft -n
+      elif n = 0 then NoShift
+      else ShiftRight n
+
+/// A codefix that removes unnecessary parentheses from the source.
+let fix (getFileLines: GetFileLines) : CodeFix =
+  Run.ifDiagnosticByCode (Set.singleton "FSAC0004") (fun d codeActionParams ->
+    asyncResult {
+      let fileName = codeActionParams.TextDocument.GetFilePath() |> Utils.normalizePath
+
+      let! sourceText = getFileLines fileName
+      let! txt = sourceText.GetText(protocolRangeToRange (string fileName) d.Range)
+
+      let firstChar = txt[0]
+      let lastChar = txt[txt.Length - 1]
+
+      match firstChar, lastChar with
+      | '(', ')' ->
+        let adjusted =
+          match sourceText with
+          | TrailingOpen d.Range -> txt[1 .. txt.Length - 2].TrimEnd()
+
+          | Trim d.Range trim & OffsidesDiff d.Range spaces ->
+            match spaces with
+            | NoShift -> trim txt[1 .. txt.Length - 2]
+            | ShiftLeft spaces -> trim (txt[1 .. txt.Length - 2].Replace("\n" + String(' ', spaces), "\n"))
+            | ShiftRight spaces -> trim (txt[1 .. txt.Length - 2].Replace("\n", "\n" + String(' ', spaces)))
+
+          | _ -> txt[1 .. txt.Length - 2].Trim()
+
+        let newText =
+          let (|ShouldPutSpaceBefore|_|) (s: string) =
+            // "……(……)"
+            //  ↑↑ ↑
+            (sourceText.TryGetChar((protocolPosToPos d.Range.Start).IncColumn -1),
+             sourceText.TryGetChar((protocolPosToPos d.Range.Start)))
+            ||> Option.map2 (fun twoBefore oneBefore ->
+              match twoBefore, oneBefore, s[0] with
+              | _, _, ('\n' | '\r') -> None
+              | '[', '|', (Punctuation | LetterOrDigit) -> None
+              | _, '[', '<' -> Some ShouldPutSpaceBefore
+              | _, ('(' | '[' | '{'), _ -> None
+              | _, '>', _ -> Some ShouldPutSpaceBefore
+              | ' ', '=', _ -> Some ShouldPutSpaceBefore
+              | _, '=', ('(' | '[' | '{') -> None
+              | _, '=', (Punctuation | Symbol) -> Some ShouldPutSpaceBefore
+              | _, LetterOrDigit, '(' -> None
+              | _, (LetterOrDigit | '`'), _ -> Some ShouldPutSpaceBefore
+              | _, (Punctuation | Symbol), (Punctuation | Symbol) -> Some ShouldPutSpaceBefore
+              | _ -> None)
+            |> Option.flatten
+
+          let (|ShouldPutSpaceAfter|_|) (s: string) =
+            // "(……)…"
+            //    ↑ ↑
+            sourceText.TryGetChar((protocolPosToPos d.Range.End).IncColumn 1)
+            |> Option.bind (fun endChar ->
+              match s[s.Length - 1], endChar with
+              | '>', ('|' | ']') -> Some ShouldPutSpaceAfter
+              | _, (')' | ']' | '[' | '}' | '.' | ';' | ',' | '|') -> None
+              | (Punctuation | Symbol), (Punctuation | Symbol | LetterOrDigit) -> Some ShouldPutSpaceAfter
+              | LetterOrDigit, LetterOrDigit -> Some ShouldPutSpaceAfter
+              | _ -> None)
+
+          match adjusted with
+          | ShouldPutSpaceBefore & ShouldPutSpaceAfter -> " " + adjusted + " "
+          | ShouldPutSpaceBefore -> " " + adjusted
+          | ShouldPutSpaceAfter -> adjusted + " "
+          | adjusted -> adjusted
+
+        return
+          [ { Edits = [| { Range = d.Range; NewText = newText } |]
+              File = codeActionParams.TextDocument
+              Title = title
+              SourceDiagnostic = Some d
+              Kind = FixKind.Fix } ]
+
+      | notParens ->
+        System.Diagnostics.Debug.Fail $"%A{notParens} <> ('(', ')')"
+        return []
+    })

src/FsAutoComplete/LspHelpers.fs

@@ -657,6 +657,7 @@ type FSharpConfigDto =
     UnusedDeclarationsAnalyzerExclusions: string array option
     SimplifyNameAnalyzer: bool option
     SimplifyNameAnalyzerExclusions: string array option
+    UnnecessaryParenthesesAnalyzer: bool option
     ResolveNamespaces: bool option
     EnableReferenceCodeLens: bool option
     EnableAnalyzers: bool option
@@ -798,6 +799,7 @@ type FSharpConfig =
     UnusedDeclarationsAnalyzerExclusions: Regex array
     SimplifyNameAnalyzer: bool
     SimplifyNameAnalyzerExclusions: Regex array
+    UnnecessaryParenthesesAnalyzer: bool
     ResolveNamespaces: bool
     EnableReferenceCodeLens: bool
     EnableAnalyzers: bool
@@ -846,6 +848,7 @@ type FSharpConfig =
       UnusedDeclarationsAnalyzerExclusions = [||]
       SimplifyNameAnalyzer = false
       SimplifyNameAnalyzerExclusions = [||]
+      UnnecessaryParenthesesAnalyzer = false
       ResolveNamespaces = false
       EnableReferenceCodeLens = false
       EnableAnalyzers = false
@@ -896,6 +899,7 @@ type FSharpConfig =
       SimplifyNameAnalyzerExclusions =
         defaultArg dto.SimplifyNameAnalyzerExclusions [||]
         |> Array.choose tryCreateRegex
+      UnnecessaryParenthesesAnalyzer = defaultArg dto.UnnecessaryParenthesesAnalyzer false
       ResolveNamespaces = defaultArg dto.ResolveNamespaces false
       EnableReferenceCodeLens = defaultArg dto.EnableReferenceCodeLens false
       EnableAnalyzers = defaultArg dto.EnableAnalyzers false
@@ -1003,6 +1007,7 @@ type FSharpConfig =
         defaultArg
           (dto.SimplifyNameAnalyzerExclusions |> Option.map (Array.choose tryCreateRegex))
           x.SimplifyNameAnalyzerExclusions
+      UnnecessaryParenthesesAnalyzer = defaultArg dto.UnnecessaryParenthesesAnalyzer x.UnnecessaryParenthesesAnalyzer
       ResolveNamespaces = defaultArg dto.ResolveNamespaces x.ResolveNamespaces
       EnableReferenceCodeLens = defaultArg dto.EnableReferenceCodeLens x.EnableReferenceCodeLens
       EnableAnalyzers = defaultArg dto.EnableAnalyzers x.EnableAnalyzers

src/FsAutoComplete/LspHelpers.fsi

@@ -287,6 +287,7 @@ type FSharpConfigDto =
     UnusedDeclarationsAnalyzerExclusions: string array option
     SimplifyNameAnalyzer: bool option
     SimplifyNameAnalyzerExclusions: string array option
+    UnnecessaryParenthesesAnalyzer: bool option
     ResolveNamespaces: bool option
     EnableReferenceCodeLens: bool option
     EnableAnalyzers: bool option
@@ -387,6 +388,7 @@ type FSharpConfig =
     UnusedDeclarationsAnalyzerExclusions: System.Text.RegularExpressions.Regex array
     SimplifyNameAnalyzer: bool
     SimplifyNameAnalyzerExclusions: System.Text.RegularExpressions.Regex array
+    UnnecessaryParenthesesAnalyzer: bool
     ResolveNamespaces: bool
     EnableReferenceCodeLens: bool
     EnableAnalyzers: bool

src/FsAutoComplete/LspServers/AdaptiveServerState.fs

@@ -348,6 +348,24 @@ type AdaptiveState(lspClient: FSharpLspClient, sourceTextFactory: ISourceTextFac
           logger.error (Log.setMessage "checkSimplifiedNames failed" >> Log.addExn e)
       }
 
+    let checkUnnecessaryParentheses =
+      async {
+        try
+          use progress = new ServerProgressReport(lspClient)
+          do! progress.Begin($"Checking for unnecessary parentheses {fileName}...", message = filePathUntag)
+
+          let! unnecessaryParentheses = UnnecessaryParentheses.getUnnecessaryParentheses getSourceLine tyRes.GetAST
+
+          let! ct = Async.CancellationToken
+
+          notifications.Trigger(
+            NotificationEvent.UnnecessaryParentheses(filePath, Array.ofSeq unnecessaryParentheses, file.Version),
+            ct
+          )
+        with e ->
+          logger.error (Log.setMessage "checkUnnecessaryParentheses failed" >> Log.addExn e)
+      }
+
     let inline isNotExcluded (exclusions: Regex array) =
       exclusions |> Array.exists (fun r -> r.IsMatch filePathUntag) |> not
 
@@ -366,7 +384,9 @@ type AdaptiveState(lspClient: FSharpLspClient, sourceTextFactory: ISourceTextFac
           config.SimplifyNameAnalyzer
           && isNotExcluded config.SimplifyNameAnalyzerExclusions
         then
-          checkSimplifiedNames ]
+          checkSimplifiedNames
+        if config.UnnecessaryParenthesesAnalyzer then
+          checkUnnecessaryParentheses ]
 
     async {
       do! analyzers |> Async.parallel75 |> Async.Ignore<unit[]>
@@ -525,6 +545,24 @@ type AdaptiveState(lspClient: FSharpLspClient, sourceTextFactory: ISourceTextFac
 
             diagnosticCollections.SetFor(uri, "F# simplify names", version, diags)
 
+          | NotificationEvent.UnnecessaryParentheses(file, ranges, version) ->
+            let uri = Path.LocalPathToUri file
+
+            let diags =
+              ranges
+              |> Array.map (fun range ->
+                { Diagnostic.Range = fcsRangeToLsp range
+                  Code = Some "FSAC0004"
+                  Severity = Some DiagnosticSeverity.Hint
+                  Source = Some "FSAC"
+                  Message = "Parentheses can be removed"
+                  RelatedInformation = Some [||]
+                  Tags = Some [| DiagnosticTag.Unnecessary |]
+                  Data = None
+                  CodeDescription = None })
+
+            diagnosticCollections.SetFor(uri, "F# unnecessary parentheses", version, diags)
+
           // | NotificationEvent.Lint (file, warnings) ->
           //     let uri = Path.LocalPathToUri file
           //     // let fs =
@@ -1848,7 +1886,8 @@ type AdaptiveState(lspClient: FSharpLspClient, sourceTextFactory: ISourceTextFac
          RemovePatternArgument.fix tryGetParseAndCheckResultsForFile
          ToInterpolatedString.fix tryGetParseAndCheckResultsForFile getLanguageVersion
          AdjustConstant.fix tryGetParseAndCheckResultsForFile
-         UpdateValueInSignatureFile.fix tryGetParseAndCheckResultsForFile |])
+         UpdateValueInSignatureFile.fix tryGetParseAndCheckResultsForFile
+         RemoveUnnecessaryParentheses.fix forceFindSourceText |])
 
   let forgetDocument (uri: DocumentUri) =
     async {