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.

Sign up for GitHub

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

Some parens fixes #1286

Merged
merged 1 commit into from
May 8, 2024
Merged

Some parens fixes #1286

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 13 additions & 0 deletions src/FsAutoComplete.Core/Utils.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,19 @@ type ReadOnlySpanExtensions =

if found then i else -1

[<Extension>]
static member IndexOfAnyExcept(span: ReadOnlySpan<char>, values: ReadOnlySpan<char>) =
let mutable i = 0
let mutable found = false

while not found && i < span.Length do
if values.IndexOf span[i] < 0 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
Expand Down
3 changes: 3 additions & 0 deletions src/FsAutoComplete.Core/Utils.fsi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,9 @@ type ReadOnlySpanExtensions =
[<Extension>]
static member IndexOfAnyExcept: span: ReadOnlySpan<char> * value0: char * value1: char -> int

[<Extension>]
static member IndexOfAnyExcept: span: ReadOnlySpan<char> * values: ReadOnlySpan<char> -> int

[<Extension>]
static member LastIndexOfAnyExcept: span: ReadOnlySpan<char> * value0: char * value1: char -> int
#endif
Expand Down
116 changes: 85 additions & 31 deletions src/FsAutoComplete/CodeFixes/RemoveUnnecessaryParentheses.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,35 @@ open FsAutoComplete.CodeFix.Types
open FsToolkit.ErrorHandling
open FsAutoComplete
open FsAutoComplete.LspHelpers
open FSharp.Compiler.Text

let title = "Remove unnecessary parentheses"

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

/// Starts with //.
[<return: Struct>]
let (|StartsWithSingleLineComment|_|) (s: string) =
if s.AsSpan().TrimStart(' ').StartsWith("//".AsSpan()) then
ValueSome StartsWithSingleLineComment
else
ValueNone

/// Starts with match, e.g.,
///
/// (match … with
/// | … -> …)
[<return: Struct>]
let (|StartsWithMatch|_|) (s: string) =
let s = s.AsSpan().TrimStart ' '

if s.StartsWith("match".AsSpan()) && (s.Length = 5 || s[5] = ' ') then
ValueSome StartsWithMatch
else
ValueNone

[<AutoOpen>]
module Char =
[<return: Struct>]
Expand Down Expand Up @@ -90,8 +112,8 @@ let fix (getFileLines: GetFileLines) : CodeFix =
| None -> id

let (|ShiftLeft|NoShift|ShiftRight|) (sourceText: IFSACSourceText) =
let startLineNo = range.StartLine
let endLineNo = range.EndLine
let startLineNo = Line.toZ range.StartLine
let endLineNo = Line.toZ range.EndLine

if startLineNo = endLineNo then
NoShift
Expand All @@ -105,11 +127,17 @@ let fix (getFileLines: GetFileLines) : CodeFix =
match line.AsSpan(startCol).IndexOfAnyExcept(' ', ')') with
| -1 -> loop innerOffsides (lineNo + 1) 0
| i ->
match innerOffsides with
| NoneYet -> loop (FirstLine(i + startCol)) (lineNo + 1) 0
| FirstLine innerOffsides -> loop (FollowingLine(innerOffsides, i + startCol)) (lineNo + 1) 0
| FollowingLine(firstLine, innerOffsides) ->
loop (FollowingLine(firstLine, min innerOffsides (i + startCol))) (lineNo + 1) 0
match line[i + startCol ..] with
| StartsWithMatch
| StartsWithSingleLineComment -> loop innerOffsides (lineNo + 1) 0
| _ ->
match innerOffsides with
| NoneYet -> loop (FirstLine(i + startCol)) (lineNo + 1) 0

| FirstLine innerOffsides -> loop (FollowingLine(innerOffsides, i + startCol)) (lineNo + 1) 0

| FollowingLine(firstLine, innerOffsides) ->
loop (FollowingLine(firstLine, min innerOffsides (i + startCol))) (lineNo + 1) 0
else
innerOffsides

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

let newText =
let (|ShouldPutSpaceBefore|_|) (s: string) =
// ……(……)
// ↑↑ ↑
(sourceText.TryGetChar(range.Start.IncColumn -1), sourceText.TryGetChar 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
match s with
| StartsWithMatch -> None
| _ ->
// ……(……)
// ↑↑ ↑
(sourceText.TryGetChar(range.Start.IncColumn -1), sourceText.TryGetChar 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) =
// (……)…
Expand All @@ -160,22 +191,45 @@ let fix (getFileLines: GetFileLines) : CodeFix =
match s[s.Length - 1], endChar with
| '>', ('|' | ']') -> Some ShouldPutSpaceAfter
| _, (')' | ']' | '[' | '}' | '.' | ';' | ',' | '|') -> None
| _, ('+' | '-' | '%' | '&' | '!' | '~') -> None
| (Punctuation | Symbol), (Punctuation | Symbol | LetterOrDigit) -> Some ShouldPutSpaceAfter
| LetterOrDigit, LetterOrDigit -> Some ShouldPutSpaceAfter
| _ -> None)

let (|WouldTurnInfixIntoPrefix|_|) (s: string) =
// (……)…
// ↑ ↑
sourceText.TryGetChar(range.End.IncColumn 1)
|> Option.bind (fun endChar ->
match s[s.Length - 1], endChar with
| (Punctuation | Symbol), ('+' | '-' | '%' | '&' | '!' | '~') ->
match sourceText.GetLine range.End with
| None -> None
| Some line ->
// (……)+…
// ↑
match line.AsSpan(range.EndColumn).IndexOfAnyExcept("*/%-+:^@><=!|$.?".AsSpan()) with
| -1 -> None
| i when line[range.EndColumn + i] <> ' ' -> Some WouldTurnInfixIntoPrefix
| _ -> None
| _ -> None)

match adjusted with
| ShouldPutSpaceBefore & ShouldPutSpaceAfter -> " " + adjusted + " "
| ShouldPutSpaceBefore -> " " + adjusted
| ShouldPutSpaceAfter -> adjusted + " "
| adjusted -> adjusted
| WouldTurnInfixIntoPrefix -> ValueNone
| ShouldPutSpaceBefore & ShouldPutSpaceAfter -> ValueSome(" " + adjusted + " ")
| ShouldPutSpaceBefore -> ValueSome(" " + adjusted)
| ShouldPutSpaceAfter -> ValueSome(adjusted + " ")
| adjusted -> ValueSome adjusted

return
[ { Edits = [| { Range = d.Range; NewText = newText } |]
newText
|> ValueOption.map (fun newText ->
{ Edits = [| { Range = d.Range; NewText = newText } |]
File = codeActionParams.TextDocument
Title = title
SourceDiagnostic = Some d
Kind = FixKind.Fix } ]
Kind = FixKind.Fix })
|> ValueOption.toList

| _notParens -> return []
})
65 changes: 63 additions & 2 deletions test/FsAutoComplete.Tests.Lsp/CodeFixTests/Tests.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1606,7 +1606,7 @@ let private generateXmlDocumentationTests state =
/// <summary></summary>
type MyRecord = { Foo: int }
"""

testCaseAsync "documentation for record type with multiple attribute lists"
<| CodeFix.check
server
Expand Down Expand Up @@ -3436,7 +3436,68 @@ let private removeUnnecessaryParenthesesTests state =
longFunctionName
longVarName1
longVarName2
""" ])
"""

testCaseAsync "Handles outlaw match exprs"
<| CodeFix.check
server
"""
3 > (match x with
| 1
| _ -> 3)$0
"""
(Diagnostics.expectCode "FSAC0004")
selector
"""
3 > match x with
| 1
| _ -> 3
"""

testCaseAsync "Handles even more outlaw match exprs"
<| CodeFix.check
server
"""
3 > ( match x with
| 1
| _ -> 3)$0
"""
(Diagnostics.expectCode "FSAC0004")
selector
"""
3 > match x with
| 1
| _ -> 3
"""

testCaseAsync "Handles single-line comments"
<| CodeFix.check
server
"""
3 > (match x with
// Lol.
| 1
| _ -> 3)$0
"""
(Diagnostics.expectCode "FSAC0004")
selector
"""
3 > match x with
// Lol.
| 1
| _ -> 3
"""

testCaseAsync "Keep parens when removal would cause reparse of infix as prefix"
<| CodeFix.checkNotApplicable
server
"""
""+(Unchecked.defaultof<string>)$0+""
"""
(Diagnostics.expectCode "FSAC0004")
selector

])

let tests textFactory state =
testList
Expand Down
Loading