Analyzer & code fix for ~IDE0047~ FS3583: remove unnecessary parentheses

src/Compiler/Service/ServiceAnalysis.fsi

@@ -3,6 +3,7 @@
 namespace FSharp.Compiler.EditorServices
 
 open FSharp.Compiler.CodeAnalysis
+open FSharp.Compiler.Syntax
 open FSharp.Compiler.Text
 
 module public UnusedOpens =
@@ -31,3 +32,12 @@ module public UnusedDeclarations =
 
     /// Get all unused declarations in a file
     val getUnusedDeclarations: checkFileResults: FSharpCheckFileResults * isScriptFile: bool -> Async<seq<range>>
+
+module public UnnecessaryParentheses =
+
+    /// Gets the ranges of all unnecessary pairs of parentheses in a file.
+    ///
+    /// Note that this may include pairs of nested ranges each of whose
+    /// lack of necessity depends on the other's presence, such
+    /// that it is valid to remove either set of parentheses but not both.
+    val getUnnecessaryParentheses: getSourceLineStr: (int -> string) -> parsedInput: ParsedInput -> Async<range seq>

src/Compiler/Service/ServiceParseTreeWalk.fs

@@ -297,9 +297,15 @@ module SyntaxTraversal =
                 ignore debugObj
                 None
 
-    /// traverse an implementation file walking all the way down to SynExpr or TypeAbbrev at a particular location
-    ///
-    let Traverse (pos: pos, parseTree, visitor: SyntaxVisitorBase<'T>) =
+    /// <summary>
+    /// Traverse an implementation file until <paramref name="pick"/> returns <c>Some value</c>.
+    /// </summary>
+    let traverseUntil
+        (pick: pos -> range -> obj -> (range * (unit -> 'T option)) list -> 'T option)
+        (pos: pos)
+        (visitor: SyntaxVisitorBase<'T>)
+        (parseTree: ParsedInput)
+        : 'T option =
         let pick x = pick pos x
 
         let rec traverseSynModuleDecl origPath (decl: SynModuleDecl) =
@@ -575,10 +581,17 @@ module SyntaxTraversal =
 
                     if ok.IsSome then ok else traverseSynExpr synExpr
 
-                | SynExpr.Lambda (args = SynSimplePats.SimplePats (pats = pats); body = synExpr) ->
-                    match traverseSynSimplePats path pats with
-                    | None -> traverseSynExpr synExpr
-                    | x -> x
+                | SynExpr.Lambda (parsedData = parsedData) ->
+                    [
+                        match parsedData with
+                        | Some (pats, body) ->
+                            for pat in pats do
+                                yield dive pat pat.Range traversePat
+
+                            yield dive body body.Range traverseSynExpr
+                        | None -> ()
+                    ]
+                    |> pick expr
 
                 | SynExpr.MatchLambda (matchClauses = synMatchClauseList) ->
                     synMatchClauseList
@@ -719,6 +732,7 @@ module SyntaxTraversal =
                 | SynPat.Ands (ps, _)
                 | SynPat.Tuple (elementPats = ps)
                 | SynPat.ArrayOrList (_, ps, _) -> ps |> List.tryPick (traversePat path)
+                | SynPat.Record (fieldPats = fieldPats) -> fieldPats |> List.tryPick (fun (_, _, p) -> traversePat path p)
                 | SynPat.Attrib (p, attributes, m) ->
                     match traversePat path p with
                     | None -> attributeApplicationDives path attributes |> pick m attributes
@@ -731,6 +745,7 @@ module SyntaxTraversal =
                     match traversePat path p with
                     | None -> traverseSynType path ty
                     | x -> x
+                | SynPat.QuoteExpr (expr, _) -> traverseSynExpr path expr
                 | _ -> None
 
             visitor.VisitPat(origPath, defaultTraverse, pat)
@@ -1077,3 +1092,8 @@ module SyntaxTraversal =
             l
             |> List.map (fun x -> dive x x.Range (traverseSynModuleOrNamespaceSig []))
             |> pick fileRange l
+
+    /// traverse an implementation file walking all the way down to SynExpr or TypeAbbrev at a particular location
+    ///
+    let Traverse (pos: pos, parseTree, visitor: SyntaxVisitorBase<'T>) =
+        traverseUntil pick pos visitor parseTree

src/Compiler/Service/ServiceParseTreeWalk.fsi

@@ -187,4 +187,11 @@ module public SyntaxTraversal =
     val internal pick:
         pos: pos -> outerRange: range -> debugObj: obj -> diveResults: (range * (unit -> 'a option)) list -> 'a option
 
+    val internal traverseUntil:
+        pick: (pos -> range -> obj -> (range * (unit -> 'T option)) list -> 'T option) ->
+        pos: pos ->
+        visitor: SyntaxVisitorBase<'T> ->
+        parseTree: ParsedInput ->
+            'T option
+
     val Traverse: pos: pos * parseTree: ParsedInput * visitor: SyntaxVisitorBase<'T> -> 'T option

vsintegration/src/FSharp.Editor/CodeFixes/CodeFixHelpers.fs

@@ -142,11 +142,11 @@ module internal CodeFixExtensions =
 
 [<AutoOpen>]
 module IFSharpCodeFixProviderExtensions =
-    type IFSharpCodeFixProvider with
+    // Cache this no-op delegate.
+    let private registerCodeFix =
+        Action<CodeActions.CodeAction, ImmutableArray<Diagnostic>>(fun _ _ -> ())
 
-        // this is not used anywhere, it's just needed to create the context
-        static member private Action =
-            Action<CodeActions.CodeAction, ImmutableArray<Diagnostic>>(fun _ _ -> ())
+    type IFSharpCodeFixProvider with
 
         member private provider.FixAllAsync (fixAllCtx: FixAllContext) (doc: Document) (allDiagnostics: ImmutableArray<Diagnostic>) =
             cancellableTask {
@@ -163,7 +163,7 @@ module IFSharpCodeFixProviderExtensions =
                     // a proper fix is needed.
                     |> Seq.distinctBy (fun d -> d.Id, d.Location)
                     |> Seq.map (fun diag ->
-                        let context = CodeFixContext(doc, diag, IFSharpCodeFixProvider.Action, token)
+                        let context = CodeFixContext(doc, diag, registerCodeFix, token)
                         provider.GetCodeFixIfAppliesAsync context)
                     |> CancellableTask.whenAll
 
@@ -194,3 +194,10 @@ module IFSharpCodeFixProviderExtensions =
             FixAllProvider.Create(fun fixAllCtx doc allDiagnostics ->
                 provider.FixAllAsync fixAllCtx doc allDiagnostics
                 |> CancellableTask.start fixAllCtx.CancellationToken)
+
+        member provider.RegisterFsharpFixAll filter =
+            FixAllProvider.Create(fun fixAllCtx doc allDiagnostics ->
+                let filteredDiagnostics = filter allDiagnostics
+
+                provider.FixAllAsync fixAllCtx doc filteredDiagnostics
+                |> CancellableTask.start fixAllCtx.CancellationToken)

vsintegration/src/FSharp.Editor/CodeFixes/RemoveUnnecessaryParentheses.fs

@@ -0,0 +1,188 @@
+// Copyright (c) Microsoft Corporation.  All Rights Reserved.  See License.txt in the project root for license information.
+
+namespace Microsoft.VisualStudio.FSharp.Editor
+
+open System
+open System.Collections.Generic
+open System.Collections.Immutable
+open System.Composition
+
+open Microsoft.CodeAnalysis.CodeFixes
+open Microsoft.CodeAnalysis.ExternalAccess.FSharp.Diagnostics
+open Microsoft.CodeAnalysis.Text
+
+open CancellableTasks
+
+#if !NET7_0_OR_GREATER
+open System.Runtime.CompilerServices
+
+[<Sealed; AbstractClass; Extension>]
+type private ReadOnlySpanExtensions =
+    [<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
+#endif
+
+[<AutoOpen>]
+module private Patterns =
+    let inline toPat f x = if f x then ValueSome() else ValueNone
+
+    [<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
+
+module private SourceText =
+    /// Returns true 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), e.g.,
+    ///
+    ///     // Valid.
+    ///     (let x = 2
+    ///      x)
+    ///
+    ///     // Invalid.
+    ///    ←let x = 2
+    ///      x◌
+    ///
+    ///     // Valid.
+    ///     ◌let x = 2
+    ///      x◌
+    let containsSensitiveIndentation (span: TextSpan) (sourceText: SourceText) =
+        let startLinePosition = sourceText.Lines.GetLinePosition span.Start
+        let endLinePosition = sourceText.Lines.GetLinePosition span.End
+        let startLine = startLinePosition.Line
+        let startCol = startLinePosition.Character
+        let endLine = endLinePosition.Line
+
+        if startLine = endLine then
+            false
+        else
+            let rec loop offsides lineNo startCol =
+                if lineNo <= endLine then
+                    let line = sourceText.Lines[ lineNo ].ToString()
+
+                    match offsides with
+                    | ValueNone ->
+                        let i = line.AsSpan(startCol).IndexOfAnyExcept(' ', ')')
+
+                        if i >= 0 then
+                            loop (ValueSome(i + startCol)) (lineNo + 1) 0
+                        else
+                            loop offsides (lineNo + 1) 0
+
+                    | ValueSome offsidesCol ->
+                        let i = line.AsSpan(0, min offsidesCol line.Length).IndexOfAnyExcept(' ', ')')
+                        i <= offsidesCol || loop offsides (lineNo + 1) 0
+                else
+                    false
+
+            loop ValueNone startLine startCol
+
+[<ExportCodeFixProvider(FSharpConstants.FSharpLanguageName, Name = CodeFix.RemoveUnnecessaryParentheses); Shared; Sealed>]
+type internal FSharpRemoveUnnecessaryParenthesesCodeFixProvider [<ImportingConstructor>] () =
+    inherit CodeFixProvider()
+
+    static let title = SR.RemoveUnnecessaryParentheses()
+    static let fixableDiagnosticIds = ImmutableArray.Create "IDE0047" // TODO: FSharpIDEDiagnosticIds.RemoveUnnecessaryParentheses
+
+    /// IDE0047: Remove unnecessary parentheses.
+    ///
+    /// https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/style-rules/ide0047-ide0048
+    override _.FixableDiagnosticIds = fixableDiagnosticIds
+
+    override this.RegisterCodeFixesAsync context = context.RegisterFsharpFix this
+
+    override this.GetFixAllProvider() =
+        this.RegisterFsharpFixAll(fun diagnostics ->
+            // There may be pairs of diagnostics with nested spans
+            // for which it would be valid to apply either but not both.
+            let builder = ImmutableArray.CreateBuilder diagnostics.Length
+
+            let spans =
+                SortedSet
+                    { new IComparer<TextSpan> with
+                        member _.Compare(x, y) =
+                            if x.IntersectsWith y then 0 else x.CompareTo y
+                    }
+
+            for i in 0 .. diagnostics.Length - 1 do
+                let diagnostic = diagnostics[i]
+
+                if spans.Add diagnostic.Location.SourceSpan then
+                    builder.Add diagnostic
+
+            builder.ToImmutable())
+
+    interface IFSharpCodeFixProvider with
+        member _.GetCodeFixIfAppliesAsync context =
+            assert (context.Span.Length >= 3) // (…)
+
+            cancellableTask {
+                let! sourceText = context.Document.GetTextAsync context.CancellationToken
+                let txt = sourceText.ToString(TextSpan(context.Span.Start, context.Span.Length))
+
+                let firstChar = txt[0]
+                let lastChar = txt[txt.Length - 1]
+
+                match firstChar, lastChar with
+                | '(', ')' ->
+                    let (|ShouldPutSpaceBefore|_|) (s: string) =
+                        // "……(……)"
+                        //  ↑↑ ↑
+                        match sourceText[max (context.Span.Start - 2) 0], sourceText[max (context.Span.Start - 1) 0], s[1] with
+                        | _, _, ('\n' | '\r') -> None
+                        | _, ('(' | '[' | '{'), _ -> None
+                        | _, '>', _ -> Some ShouldPutSpaceBefore
+                        | ' ', '=', _ -> Some ShouldPutSpaceBefore
+                        | _, '=', ('(' | '[' | '{') -> None
+                        | _, '=', (Punctuation | Symbol) -> Some ShouldPutSpaceBefore
+                        | _, LetterOrDigit, '(' -> None
+                        | _, (LetterOrDigit | '`'), _ -> Some ShouldPutSpaceBefore
+                        | _, (Punctuation | Symbol), (Punctuation | Symbol) -> Some ShouldPutSpaceBefore
+                        | _ when SourceText.containsSensitiveIndentation context.Span sourceText -> Some ShouldPutSpaceBefore
+                        | _ -> None
+
+                    let (|ShouldPutSpaceAfter|_|) (s: string) =
+                        // "(……)…"
+                        //    ↑ ↑
+                        match s[s.Length - 2], sourceText[min context.Span.End (sourceText.Length - 1)] with
+                        | _, (')' | ']' | '}' | '.' | ';') -> None
+                        | (Punctuation | Symbol), (Punctuation | Symbol | LetterOrDigit) -> Some ShouldPutSpaceAfter
+                        | LetterOrDigit, LetterOrDigit -> Some ShouldPutSpaceAfter
+                        | _ -> None
+
+                    let newText =
+                        match txt with
+                        | ShouldPutSpaceBefore & ShouldPutSpaceAfter -> " " + txt[1 .. txt.Length - 2] + " "
+                        | ShouldPutSpaceBefore -> " " + txt[1 .. txt.Length - 2]
+                        | ShouldPutSpaceAfter -> txt[1 .. txt.Length - 2] + " "
+                        | _ -> txt[1 .. txt.Length - 2]
+
+                    return
+                        ValueSome
+                            {
+                                Name = CodeFix.RemoveUnnecessaryParentheses
+                                Message = title
+                                Changes = [ TextChange(context.Span, newText) ]
+                            }
+
+                | notParens ->
+                    System.Diagnostics.Debug.Fail $"%A{notParens} <> ('(', ')')"
+                    return ValueNone
+            }

vsintegration/src/FSharp.Editor/Common/Constants.fs

@@ -205,3 +205,6 @@ module internal CodeFix =
 
     [<Literal>]
     let RemoveSuperfluousCapture = "RemoveSuperfluousCapture"
+
+    [<Literal>]
+    let RemoveUnnecessaryParentheses = "RemoveUnnecessaryParentheses"

vsintegration/src/FSharp.Editor/Diagnostics/DocumentDiagnosticAnalyzer.fs

@@ -13,6 +13,7 @@ open Microsoft.CodeAnalysis.Text
 open Microsoft.CodeAnalysis.ExternalAccess.FSharp.Diagnostics
 
 open FSharp.Compiler.Diagnostics
+open FSharp.Compiler.EditorServices
 open CancellableTasks
 open Microsoft.VisualStudio.FSharp.Editor.Telemetry
 
@@ -108,7 +109,40 @@ type internal FSharpDocumentDiagnosticAnalyzer [<ImportingConstructor>] () =
                 for diagnostic in checkResults.Diagnostics do
                     errors.Add(diagnostic) |> ignore
 
-            if errors.Count = 0 then
+            let! unnecessaryParentheses =
+                match diagnosticType with
+                | DiagnosticsType.Semantic -> CancellableTask.singleton ImmutableArray.Empty
+                | DiagnosticsType.Syntax ->
+                    cancellableTask {
+                        let fsharpSourceText = sourceText.ToFSharpSourceText()
+
+                        let! unnecessaryParentheses =
+                            UnnecessaryParentheses.getUnnecessaryParentheses
+                                (FSharp.Compiler.Text.Line.toZ >> fsharpSourceText.GetLineString)
+                                parseResults.ParseTree
+
+                        let descriptor =
+                            let title = "Parentheses can be removed."
+
+                            DiagnosticDescriptor(
+                                "IDE0047",
+                                title,
+                                title,
+                                "Style",
+                                DiagnosticSeverity.Hidden,
+                                isEnabledByDefault = true,
+                                description = null,
+                                helpLinkUri = null
+                            )
+
+                        return
+                            unnecessaryParentheses
+                            |> Seq.map (fun range ->
+                                Diagnostic.Create(descriptor, RoslynHelpers.RangeToLocation(range, sourceText, document.FilePath)))
+                            |> Seq.toImmutableArray
+                    }
+
+            if errors.Count = 0 && unnecessaryParentheses.IsEmpty then
                 return ImmutableArray.Empty
             else
                 let iab = ImmutableArray.CreateBuilder(errors.Count)
@@ -135,6 +169,7 @@ type internal FSharpDocumentDiagnosticAnalyzer [<ImportingConstructor>] () =
                         let location = Location.Create(filePath, correctedTextSpan, linePositionSpan)
                         iab.Add(RoslynHelpers.ConvertError(diagnostic, location))
 
+                iab.AddRange unnecessaryParentheses
                 return iab.ToImmutable()
         }