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

Some parens fixes #1286

Merged
merged 1 commit into from
May 8, 2024
Merged
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
13 changes: 13 additions & 0 deletions src/FsAutoComplete.Core/Utils.fs
Original file line number Diff line number Diff line change
@@ -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
3 changes: 3 additions & 0 deletions src/FsAutoComplete.Core/Utils.fsi
Original file line number Diff line number Diff line change
@@ -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
116 changes: 85 additions & 31 deletions src/FsAutoComplete/CodeFixes/RemoveUnnecessaryParentheses.fs
Original file line number Diff line number Diff line change
@@ -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>]
@@ -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
@@ -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

@@ -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) =
// (……)…
@@ -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
Original file line number Diff line number Diff line change
@@ -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
@@ -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

Unchanged files with check annotations Beta

<Project Sdk="Microsoft.NET.Sdk">

Check failure on line 1 in test/FsAutoComplete.Tests.Lsp/FsAutoComplete.Tests.Lsp.fsproj

GitHub Actions / Build on macos-13 for 6.0

FSAC.lsp.Ionide WorkspaceLoader.xml doc comments.generate xml docs for function

Couldn't generate xml docs: { Code = -32603 Message = "Check aborted (Aborted). Errors: [|"internal error: One or more errors occurred. (Value cannot be null.)"|]" Data = None } at FsAutoComplete.Tests.XmlDocumentationGeneration.tests@57-524.Invoke(String msg) at FsAutoComplete.Tests.XmlDocumentationGeneration.tests@56-523.Invoke(FSharpResult`2 result) in /Users/runner/work/FsAutoComplete/FsAutoComplete/test/FsAutoComplete.Tests.Lsp/XmlGenerationTests.fs:line 57 at Microsoft.FSharp.Control.AsyncPrimitives.CallThenInvokeNoHijackCheck[a,b](AsyncActivation`1 ctxt, b result1, FSharpFunc`2 userCode) in D:\a\_work\1\s\src\FSharp.Core\async.fs:line 528 at Microsoft.FSharp.Control.AsyncPrimitives.OnTaskCompleted[T](Task`1 completedTask, AsyncActivation`1 ctxt) in D:\a\_work\1\s\src\FSharp.Core\async.fs:line 1219 at Microsoft.FSharp.Control.Trampoline.Execute(FSharpFunc`2 firstAction) in D:\a\_work\1\s\src\FSharp.Core\async.fs:line 112

Check failure on line 1 in test/FsAutoComplete.Tests.Lsp/FsAutoComplete.Tests.Lsp.fsproj

GitHub Actions / Build on ubuntu-latest for 6.0

FSAC.lsp.Ionide WorkspaceLoader.CodeFix-tests.AdjustConstant.Convert int-number to other bases.extrema.int (without suffix).can convert System.Int32.MinValue.can convert from Octal.can convert to Decimal

The input sequence was empty. (Parameter 'source')

Check failure on line 1 in test/FsAutoComplete.Tests.Lsp/FsAutoComplete.Tests.Lsp.fsproj

GitHub Actions / Build on ubuntu-latest for 6.0

FSAC.lsp.Ionide WorkspaceLoader.CodeFix-tests.AdjustConstant.Convert int-number to other bases.extrema.int (without suffix).can convert System.Int32.MinValue.can convert from Octal.can convert to Hexadecimal

The input sequence was empty. (Parameter 'source')

Check failure on line 1 in test/FsAutoComplete.Tests.Lsp/FsAutoComplete.Tests.Lsp.fsproj

GitHub Actions / Build on ubuntu-latest for 6.0

FSAC.lsp.Ionide WorkspaceLoader.CodeFix-tests.AdjustConstant.Convert int-number to other bases.extrema.int (without suffix).can convert System.Int32.MinValue.can convert from Octal.cannot convert to Octal

The input sequence was empty. (Parameter 'source')

Check failure on line 1 in test/FsAutoComplete.Tests.Lsp/FsAutoComplete.Tests.Lsp.fsproj

GitHub Actions / Build on ubuntu-latest for 6.0

FSAC.lsp.Ionide WorkspaceLoader.CodeFix-tests.AdjustConstant.Convert int-number to other bases.extrema.int (without suffix).can convert System.Int32.MinValue.can convert from Octal.can convert to Binary

The input sequence was empty. (Parameter 'source')

Check failure on line 1 in test/FsAutoComplete.Tests.Lsp/FsAutoComplete.Tests.Lsp.fsproj

GitHub Actions / Build on ubuntu-latest for 6.0

FSAC.lsp.Ionide WorkspaceLoader.CodeFix-tests.AdjustConstant.Convert int-number to other bases.extrema.int (without suffix).can convert System.Int32.MinValue.can convert from Octal.cleanup

The input sequence was empty. (Parameter 'source')

Check failure on line 1 in test/FsAutoComplete.Tests.Lsp/FsAutoComplete.Tests.Lsp.fsproj

GitHub Actions / Build on ubuntu-latest for 7.0

FSAC.lsp.Ionide WorkspaceLoader.xml doc comments.generate xml docs for function

Couldn't generate xml docs: { Code = -32603 Message = "Check aborted (Aborted). Errors: [|"internal error: One or more errors occurred. (Value cannot be null.)"|]" Data = None } at FsAutoComplete.Tests.XmlDocumentationGeneration.tests@57-524.Invoke(String msg) at FsAutoComplete.Tests.XmlDocumentationGeneration.tests@56-523.Invoke(FSharpResult`2 result) in /home/runner/work/FsAutoComplete/FsAutoComplete/test/FsAutoComplete.Tests.Lsp/XmlGenerationTests.fs:line 57 at Microsoft.FSharp.Control.AsyncPrimitives.CallThenInvokeNoHijackCheck[a,b](AsyncActivation`1 ctxt, b result1, FSharpFunc`2 userCode) in D:\a\_work\1\s\src\FSharp.Core\async.fs:line 528 at <StartupCode$fsautocomplete>.$AdaptiveServerState.clo@1201-118.Invoke(AsyncActivation`1 ctxt) at Microsoft.FSharp.Control.Trampoline.Execute(FSharpFunc`2 firstAction) in D:\a\_work\1\s\src\FSharp.Core\async.fs:line 148
<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFrameworks>net6.0</TargetFrameworks>