Skip to content

Commit f7fcbcd

Browse files
brianrourkebollbrianrourkeboll
committed
Some parens fixes
See dotnet/fsharp#16901 * Keep parens around outlaw `match` exprs (where the first `|` is leftward of the start of the `match` keyword). * Ignore single-line comments when determining offsides lines. * Don't add a space when removing parens when doing so would result in reparsing an infix op as a prefix op.
1 parent e9ad5eb commit f7fcbcd

File tree

4 files changed

+164
-33
lines changed

4 files changed

+164
-33
lines changed

‎src/FsAutoComplete.Core/Utils.fs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -515,6 +515,19 @@ type ReadOnlySpanExtensions =
515515

516516
if found then i else -1
517517

518+
[<Extension>]
519+
static member IndexOfAnyExcept(span: ReadOnlySpan<char>, values: ReadOnlySpan<char>) =
520+
let mutable i = 0
521+
let mutable found = false
522+
523+
while not found && i < span.Length do
524+
if values.IndexOf span[i] < 0 then
525+
found <- true
526+
else
527+
i <- i + 1
528+
529+
if found then i else -1
530+
518531
[<Extension>]
519532
static member LastIndexOfAnyExcept(span: ReadOnlySpan<char>, value0: char, value1: char) =
520533
let mutable i = span.Length - 1

‎src/FsAutoComplete.Core/Utils.fsi‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,9 @@ type ReadOnlySpanExtensions =
185185
[<Extension>]
186186
static member IndexOfAnyExcept: span: ReadOnlySpan<char> * value0: char * value1: char -> int
187187

188+
[<Extension>]
189+
static member IndexOfAnyExcept: span: ReadOnlySpan<char> * values: ReadOnlySpan<char> -> int
190+
188191
[<Extension>]
189192
static member LastIndexOfAnyExcept: span: ReadOnlySpan<char> * value0: char * value1: char -> int
190193
#endif

‎src/FsAutoComplete/CodeFixes/RemoveUnnecessaryParentheses.fs‎

Lines changed: 85 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,35 @@ open FsAutoComplete.CodeFix.Types
77
open FsToolkit.ErrorHandling
88
open FsAutoComplete
99
open FsAutoComplete.LspHelpers
10+
open FSharp.Compiler.Text
1011

1112
let title = "Remove unnecessary parentheses"
1213

1314
[<AutoOpen>]
1415
module private Patterns =
1516
let inline toPat f x = if f x then ValueSome() else ValueNone
1617

18+
/// Starts with //.
19+
[<return: Struct>]
20+
let (|StartsWithSingleLineComment|_|) (s: string) =
21+
if s.AsSpan().TrimStart(' ').StartsWith("//".AsSpan()) then
22+
ValueSome StartsWithSingleLineComment
23+
else
24+
ValueNone
25+
26+
/// Starts with match, e.g.,
27+
///
28+
/// (match … with
29+
/// | … -> …)
30+
[<return: Struct>]
31+
let (|StartsWithMatch|_|) (s: string) =
32+
let s = s.AsSpan().TrimStart ' '
33+
34+
if s.StartsWith("match".AsSpan()) && (s.Length = 5 || s[5] = ' ') then
35+
ValueSome StartsWithMatch
36+
else
37+
ValueNone
38+
1739
[<AutoOpen>]
1840
module Char =
1941
[<return: Struct>]
@@ -90,8 +112,8 @@ let fix (getFileLines: GetFileLines) : CodeFix =
90112
| None -> id
91113

92114
let (|ShiftLeft|NoShift|ShiftRight|) (sourceText: IFSACSourceText) =
93-
let startLineNo = range.StartLine
94-
let endLineNo = range.EndLine
115+
let startLineNo = Line.toZ range.StartLine
116+
let endLineNo = Line.toZ range.EndLine
95117

96118
if startLineNo = endLineNo then
97119
NoShift
@@ -105,11 +127,17 @@ let fix (getFileLines: GetFileLines) : CodeFix =
105127
match line.AsSpan(startCol).IndexOfAnyExcept(' ', ')') with
106128
| -1 -> loop innerOffsides (lineNo + 1) 0
107129
| i ->
108-
match innerOffsides with
109-
| NoneYet -> loop (FirstLine(i + startCol)) (lineNo + 1) 0
110-
| FirstLine innerOffsides -> loop (FollowingLine(innerOffsides, i + startCol)) (lineNo + 1) 0
111-
| FollowingLine(firstLine, innerOffsides) ->
112-
loop (FollowingLine(firstLine, min innerOffsides (i + startCol))) (lineNo + 1) 0
130+
match line[i + startCol ..] with
131+
| StartsWithMatch
132+
| StartsWithSingleLineComment -> loop innerOffsides (lineNo + 1) 0
133+
| _ ->
134+
match innerOffsides with
135+
| NoneYet -> loop (FirstLine(i + startCol)) (lineNo + 1) 0
136+
137+
| FirstLine innerOffsides -> loop (FollowingLine(innerOffsides, i + startCol)) (lineNo + 1) 0
138+
139+
| FollowingLine(firstLine, innerOffsides) ->
140+
loop (FollowingLine(firstLine, min innerOffsides (i + startCol))) (lineNo + 1) 0
113141
else
114142
innerOffsides
115143

@@ -133,24 +161,27 @@ let fix (getFileLines: GetFileLines) : CodeFix =
133161

134162
let newText =
135163
let (|ShouldPutSpaceBefore|_|) (s: string) =
136-
// ……(……)
137-
// ↑↑ ↑
138-
(sourceText.TryGetChar(range.Start.IncColumn -1), sourceText.TryGetChar range.Start)
139-
||> Option.map2 (fun twoBefore oneBefore ->
140-
match twoBefore, oneBefore, s[0] with
141-
| _, _, ('\n' | '\r') -> None
142-
| '[', '|', (Punctuation | LetterOrDigit) -> None
143-
| _, '[', '<' -> Some ShouldPutSpaceBefore
144-
| _, ('(' | '[' | '{'), _ -> None
145-
| _, '>', _ -> Some ShouldPutSpaceBefore
146-
| ' ', '=', _ -> Some ShouldPutSpaceBefore
147-
| _, '=', ('(' | '[' | '{') -> None
148-
| _, '=', (Punctuation | Symbol) -> Some ShouldPutSpaceBefore
149-
| _, LetterOrDigit, '(' -> None
150-
| _, (LetterOrDigit | '`'), _ -> Some ShouldPutSpaceBefore
151-
| _, (Punctuation | Symbol), (Punctuation | Symbol) -> Some ShouldPutSpaceBefore
152-
| _ -> None)
153-
|> Option.flatten
164+
match s with
165+
| StartsWithMatch -> None
166+
| _ ->
167+
// ……(……)
168+
// ↑↑ ↑
169+
(sourceText.TryGetChar(range.Start.IncColumn -1), sourceText.TryGetChar range.Start)
170+
||> Option.map2 (fun twoBefore oneBefore ->
171+
match twoBefore, oneBefore, s[0] with
172+
| _, _, ('\n' | '\r') -> None
173+
| '[', '|', (Punctuation | LetterOrDigit) -> None
174+
| _, '[', '<' -> Some ShouldPutSpaceBefore
175+
| _, ('(' | '[' | '{'), _ -> None
176+
| _, '>', _ -> Some ShouldPutSpaceBefore
177+
| ' ', '=', _ -> Some ShouldPutSpaceBefore
178+
| _, '=', ('(' | '[' | '{') -> None
179+
| _, '=', (Punctuation | Symbol) -> Some ShouldPutSpaceBefore
180+
| _, LetterOrDigit, '(' -> None
181+
| _, (LetterOrDigit | '`'), _ -> Some ShouldPutSpaceBefore
182+
| _, (Punctuation | Symbol), (Punctuation | Symbol) -> Some ShouldPutSpaceBefore
183+
| _ -> None)
184+
|> Option.flatten
154185

155186
let (|ShouldPutSpaceAfter|_|) (s: string) =
156187
// (……)…
@@ -160,22 +191,45 @@ let fix (getFileLines: GetFileLines) : CodeFix =
160191
match s[s.Length - 1], endChar with
161192
| '>', ('|' | ']') -> Some ShouldPutSpaceAfter
162193
| _, (')' | ']' | '[' | '}' | '.' | ';' | ',' | '|') -> None
194+
| _, ('+' | '-' | '%' | '&' | '!' | '~') -> None
163195
| (Punctuation | Symbol), (Punctuation | Symbol | LetterOrDigit) -> Some ShouldPutSpaceAfter
164196
| LetterOrDigit, LetterOrDigit -> Some ShouldPutSpaceAfter
165197
| _ -> None)
166198

199+
let (|WouldTurnInfixIntoPrefix|_|) (s: string) =
200+
// (……)…
201+
// ↑ ↑
202+
sourceText.TryGetChar(range.End.IncColumn 1)
203+
|> Option.bind (fun endChar ->
204+
match s[s.Length - 1], endChar with
205+
| (Punctuation | Symbol), ('+' | '-' | '%' | '&' | '!' | '~') ->
206+
match sourceText.GetLine range.End with
207+
| None -> None
208+
| Some line ->
209+
// (……)+…
210+
// ↑
211+
match line.AsSpan(range.EndColumn).IndexOfAnyExcept("*/%-+:^@><=!|$.?".AsSpan()) with
212+
| -1 -> None
213+
| i when line[range.EndColumn + i] <> ' ' -> Some WouldTurnInfixIntoPrefix
214+
| _ -> None
215+
| _ -> None)
216+
167217
match adjusted with
168-
| ShouldPutSpaceBefore & ShouldPutSpaceAfter -> " " + adjusted + " "
169-
| ShouldPutSpaceBefore -> " " + adjusted
170-
| ShouldPutSpaceAfter -> adjusted + " "
171-
| adjusted -> adjusted
218+
| WouldTurnInfixIntoPrefix -> ValueNone
219+
| ShouldPutSpaceBefore & ShouldPutSpaceAfter -> ValueSome(" " + adjusted + " ")
220+
| ShouldPutSpaceBefore -> ValueSome(" " + adjusted)
221+
| ShouldPutSpaceAfter -> ValueSome(adjusted + " ")
222+
| adjusted -> ValueSome adjusted
172223

173224
return
174-
[ { Edits = [| { Range = d.Range; NewText = newText } |]
225+
newText
226+
|> ValueOption.map (fun newText ->
227+
{ Edits = [| { Range = d.Range; NewText = newText } |]
175228
File = codeActionParams.TextDocument
176229
Title = title
177230
SourceDiagnostic = Some d
178-
Kind = FixKind.Fix } ]
231+
Kind = FixKind.Fix })
232+
|> ValueOption.toList
179233

180234
| _notParens -> return []
181235
})

‎test/FsAutoComplete.Tests.Lsp/CodeFixTests/Tests.fs‎

Lines changed: 63 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1606,7 +1606,7 @@ let private generateXmlDocumentationTests state =
16061606
/// <summary></summary>
16071607
type MyRecord = { Foo: int }
16081608
"""
1609-
1609+
16101610
testCaseAsync "documentation for record type with multiple attribute lists"
16111611
<| CodeFix.check
16121612
server
@@ -3436,7 +3436,68 @@ let private removeUnnecessaryParenthesesTests state =
34363436
longFunctionName
34373437
longVarName1
34383438
longVarName2
3439-
""" ])
3439+
"""
3440+
3441+
testCaseAsync "Handles outlaw match exprs"
3442+
<| CodeFix.check
3443+
server
3444+
"""
3445+
3 > (match x with
3446+
| 1
3447+
| _ -> 3)$0
3448+
"""
3449+
(Diagnostics.expectCode "FSAC0004")
3450+
selector
3451+
"""
3452+
3 > match x with
3453+
| 1
3454+
| _ -> 3
3455+
"""
3456+
3457+
testCaseAsync "Handles even more outlaw match exprs"
3458+
<| CodeFix.check
3459+
server
3460+
"""
3461+
3 > ( match x with
3462+
| 1
3463+
| _ -> 3)$0
3464+
"""
3465+
(Diagnostics.expectCode "FSAC0004")
3466+
selector
3467+
"""
3468+
3 > match x with
3469+
| 1
3470+
| _ -> 3
3471+
"""
3472+
3473+
testCaseAsync "Handles single-line comments"
3474+
<| CodeFix.check
3475+
server
3476+
"""
3477+
3 > (match x with
3478+
// Lol.
3479+
| 1
3480+
| _ -> 3)$0
3481+
"""
3482+
(Diagnostics.expectCode "FSAC0004")
3483+
selector
3484+
"""
3485+
3 > match x with
3486+
// Lol.
3487+
| 1
3488+
| _ -> 3
3489+
"""
3490+
3491+
testCaseAsync "Keep parens when removal would cause reparse of infix as prefix"
3492+
<| CodeFix.checkNotApplicable
3493+
server
3494+
"""
3495+
""+(Unchecked.defaultof<string>)$0+""
3496+
"""
3497+
(Diagnostics.expectCode "FSAC0004")
3498+
selector
3499+
3500+
])
34403501

34413502
let tests textFactory state =
34423503
testList

0 commit comments

Comments
 (0)