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

Ignore expression code fix #1253

Merged
merged 9 commits into from
Apr 12, 2024
Merged
Show file tree
Hide file tree
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
3 changes: 3 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,6 @@ fsharp_max_array_or_list_width=80
fsharp_max_dot_get_expression_width=80
fsharp_max_function_binding_width=80
fsharp_max_value_binding_width=80

[src/FsAutoComplete/CodeFixes/*.fs]
fsharp_experimental_keep_indent_in_branch = true
2 changes: 1 addition & 1 deletion paket.dependencies
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ lowest_matching: true

nuget BenchmarkDotNet 0.13.5
nuget Fantomas.Client >= 0.9
nuget FSharp.Compiler.Service >= 43.8.300-preview.24175.1
nuget FSharp.Compiler.Service >= 43.8.300-preview.24205.4
nuget Ionide.Analyzers 0.7.0
nuget FSharp.Analyzers.Build 0.3.0
nuget Ionide.ProjInfo >= 0.62.0
Expand Down
6 changes: 3 additions & 3 deletions paket.lock
Original file line number Diff line number Diff line change
Expand Up @@ -749,16 +749,16 @@ NUGET
FSharp.Core (>= 7.0.200) - restriction: || (== net6.0) (== net7.0) (== net8.0) (&& (== netstandard2.0) (>= net6.0)) (&& (== netstandard2.1) (>= net6.0))
System.Collections.Immutable (>= 6.0) - restriction: || (== net6.0) (== net7.0) (== net8.0) (&& (== netstandard2.0) (>= net6.0)) (&& (== netstandard2.1) (>= net6.0))
remote: https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet8/nuget/v3/index.json
FSharp.Compiler.Service (43.8.300-preview.24175.1)
FSharp.Core (8.0.300-beta.24175.1)
FSharp.Compiler.Service (43.8.300-preview.24205.4)
FSharp.Core (8.0.300-beta.24205.4)
System.Buffers (>= 4.5.1)
System.Collections.Immutable (>= 7.0)
System.Diagnostics.DiagnosticSource (>= 7.0.2)
System.Memory (>= 4.5.5)
System.Reflection.Emit (>= 4.7)
System.Reflection.Metadata (>= 7.0)
System.Runtime.CompilerServices.Unsafe (>= 6.0)
FSharp.Core (8.0.300-beta.24175.1)
FSharp.Core (8.0.300-beta.24205.4)

GROUP Build
STORAGE: NONE
Expand Down
2 changes: 1 addition & 1 deletion src/FsAutoComplete.Core/FCSPatches.fs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ module SyntaxTreeOps =

| SynExpr.TryFinally(tryExpr = e1; finallyExpr = e2) -> walkExpr e1 || walkExpr e2

| SynExpr.Sequential(_, _, e1, e2, _) -> walkExpr e1 || walkExpr e2
| SynExpr.Sequential(expr1 = e1; expr2 = e2) -> walkExpr e1 || walkExpr e2

| SynExpr.SequentialOrImplicitYield(_, e1, e2, _, _) -> walkExpr e1 || walkExpr e2

Expand Down
4 changes: 2 additions & 2 deletions src/FsAutoComplete.Core/TestAdapter.fs
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ let private ModuleWithSuffixType = "ModuleWithSuffix"

let rec private (|Sequentials|_|) =
function
| SynExpr.Sequential(_, _, e, Sequentials es, _) -> Some(e :: es)
| SynExpr.Sequential(_, _, e1, e2, _) -> Some [ e1; e2 ]
| SynExpr.Sequential(expr1 = e; expr2 = Sequentials es) -> Some(e :: es)
| SynExpr.Sequential(expr1 = e1; expr2 = e2) -> Some [ e1; e2 ]
| _ -> None

let getExpectoTests (ast: ParsedInput) : TestAdapterEntry<range> list =
Expand Down
4 changes: 2 additions & 2 deletions src/FsAutoComplete.Core/UntypedAstUtils.fs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ module Syntax =
/// An recursive pattern that collect all sequential expressions to avoid StackOverflowException
let rec (|Sequentials|_|) =
function
| SynExpr.Sequential(_, _, e, Sequentials es, _) -> Some(e :: es)
| SynExpr.Sequential(_, _, e1, e2, _) -> Some [ e1; e2 ]
| SynExpr.Sequential(expr1 = e; expr2 = Sequentials es) -> Some(e :: es)
| SynExpr.Sequential(expr1 = e1; expr2 = e2) -> Some [ e1; e2 ]
| _ -> None

let (|ConstructorPats|) =
Expand Down
104 changes: 104 additions & 0 deletions src/FsAutoComplete/CodeFixes/IgnoreExpression.fs
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
module FsAutoComplete.CodeFix.IgnoreExpression

open FSharp.Compiler.Syntax
open FSharp.Compiler.SyntaxTrivia
open FSharp.Compiler.Text
open FSharp.UMX
open FsToolkit.ErrorHandling
open Ionide.LanguageServerProtocol.Types
open FsAutoComplete.CodeFix.Types
open FsAutoComplete
open FsAutoComplete.LspHelpers

let title = "Ignore expression"

let fix (getParseResultsForFile: GetParseResultsForFile) : CodeFix =
Run.ifDiagnosticByCode (set [ "20" ]) (fun diagnostic (codeActionParams: CodeActionParams) ->
asyncResult {
let fileName = codeActionParams.TextDocument.GetFilePath() |> Utils.normalizePath
let fcsPos = protocolPosToPos codeActionParams.Range.Start
let mDiag = protocolRangeToRange (UMX.untag fileName) diagnostic.Range

// Only do single line for now
if mDiag.StartLine <> mDiag.EndLine then
return []
else

let! (parseAndCheckResults: ParseAndCheckResults, _line: string, sourceText: IFSACSourceText) =
getParseResultsForFile fileName fcsPos

let mExprOpt =
(fcsPos, parseAndCheckResults.GetParseResults.ParseTree)
||> ParsedInput.tryPick (fun path node ->
match node with
| SyntaxNode.SynExpr(e) when Range.equals mDiag e.Range -> Some(path, e)
| _ -> None)

match mExprOpt with
| None -> return []
| Some(path, expr) ->

let needsParentheses =
nojaf marked this conversation as resolved.
Show resolved Hide resolved
let appPipe, appIgnore =
let lineNumber = expr.Range.StartLine

let mkRangeInFile (offset: int) (length: int) : range =
Range.mkRange
expr.Range.FileName
(Position.mkPos lineNumber (expr.Range.EndColumn + offset))
(Position.mkPos lineNumber (expr.Range.EndColumn + offset + length))

let mPipe = mkRangeInFile 2 2
let mIgnore = mkRangeInFile 5 6

let appPipe =
SynExpr.App(
ExprAtomicFlag.NonAtomic,
true,
SynExpr.LongIdent(
false,
SynLongIdent(
[ FSharp.Compiler.Syntax.Ident("op_PipeRight", mPipe) ],
[],
[ Some(IdentTrivia.OriginalNotation "|>") ]
),
None,
mPipe
),
expr, // The expr that will now be piped into ignore.
Range.unionRanges expr.Range mPipe
)

let appIgnore =
SynExpr.App(
ExprAtomicFlag.NonAtomic,
false,
appPipe,
SynExpr.Ident(FSharp.Compiler.Syntax.Ident("ignore", mIgnore)),
Range.unionRanges expr.Range mIgnore
)

appPipe, appIgnore

let pathWithPipeIgnore =
SyntaxNode.SynExpr appPipe :: SyntaxNode.SynExpr appIgnore :: path

SynExpr.shouldBeParenthesizedInContext sourceText.GetLineString pathWithPipeIgnore expr
nojaf marked this conversation as resolved.
Show resolved Hide resolved

let newText =
let currentText = sourceText.GetSubTextFromRange expr.Range

if not needsParentheses then
$"%s{currentText} |> ignore"
else
$"(%s{currentText}) |> ignore"

return
[ { SourceDiagnostic = None
Title = title
File = codeActionParams.TextDocument
Edits =
[| { Range = fcsRangeToLsp expr.Range
NewText = newText } |]
Kind = FixKind.Fix } ]
})
6 changes: 6 additions & 0 deletions src/FsAutoComplete/CodeFixes/IgnoreExpression.fsi
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module FsAutoComplete.CodeFix.IgnoreExpression

open FsAutoComplete.CodeFix.Types

val title: string
val fix: getParseResultsForFile: GetParseResultsForFile -> CodeFix
3 changes: 2 additions & 1 deletion src/FsAutoComplete/LspServers/AdaptiveServerState.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1925,7 +1925,8 @@ type AdaptiveState(lspClient: FSharpLspClient, sourceTextFactory: ISourceTextFac
AddTypeAliasToSignatureFile.fix forceGetFSharpProjectOptions tryGetParseAndCheckResultsForFile
UpdateTypeAbbreviationInSignatureFile.fix tryGetParseAndCheckResultsForFile
AddBindingToSignatureFile.fix forceGetFSharpProjectOptions tryGetParseAndCheckResultsForFile
ReplaceLambdaWithDotLambda.fix getLanguageVersion tryGetParseAndCheckResultsForFile |])
ReplaceLambdaWithDotLambda.fix getLanguageVersion tryGetParseAndCheckResultsForFile
IgnoreExpression.fix tryGetParseAndCheckResultsForFile |])

let forgetDocument (uri: DocumentUri) =
async {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
module private FsAutoComplete.Tests.CodeFixTests.IgnoreExpressionTests

open Expecto
open Helpers
open Utils.ServerTests
open Utils.CursorbasedTests
open FsAutoComplete.CodeFix

let tests state =
serverTestList (nameof IgnoreExpression) state defaultConfigDto None (fun server ->
[ let selectCodeFix = CodeFix.withTitle IgnoreExpression.title

testCaseAsync "ignore constant"
<| CodeFix.check
server
"
let a b =
9$0
null"
Diagnostics.acceptAll
selectCodeFix
"
let a b =
9 |> ignore
nojaf marked this conversation as resolved.
Show resolved Hide resolved
null"

testCaseAsync "ignore infix application"
<| CodeFix.check
server
"
let a b =
0 / 9$0
null"
Diagnostics.acceptAll
selectCodeFix
"
let a b =
0 / 9 |> ignore
null"

testCaseAsync "ignore member invocation"
<| CodeFix.check
server
"
open System.Collections.Generic

let foo () =
let dict = dict []
di$0ct.TryAdd(\"foo\", \"bar\")
()"
Diagnostics.acceptAll
selectCodeFix
"
open System.Collections.Generic

let foo () =
let dict = dict []
dict.TryAdd(\"foo\", \"bar\") |> ignore
()"

testCaseAsync "ignore tuple"
<| CodeFix.check
server
"
let _ =
1, 2$0
null"
Diagnostics.acceptAll
selectCodeFix
"
let _ =
(1, 2) |> ignore
null"

])
3 changes: 2 additions & 1 deletion test/FsAutoComplete.Tests.Lsp/CodeFixTests/Tests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -3436,4 +3436,5 @@ let tests textFactory state =
AddTypeAliasToSignatureFileTests.tests state
UpdateTypeAbbreviationInSignatureFileTests.tests state
AddBindingToSignatureFileTests.tests state
ReplaceLambdaWithDotLambdaTests.tests state ]
ReplaceLambdaWithDotLambdaTests.tests state
IgnoreExpressionTests.tests state ]
Loading