From cbd153dd4be37c2fbda8b52dfa33e56fd4e359ab Mon Sep 17 00:00:00 2001 From: nojaf Date: Sun, 24 Mar 2024 20:33:21 +0100 Subject: [PATCH 1/6] NegateBooleanExpression code fix --- .../CodeFixes/NegateBooleanExpression.fs | 164 ++++++++++++++++ .../CodeFixes/NegateBooleanExpression.fsi | 6 + .../LspServers/AdaptiveServerState.fs | 3 +- .../NegateBooleanExpressionTests.fs | 183 ++++++++++++++++++ .../CodeFixTests/Tests.fs | 3 +- 5 files changed, 357 insertions(+), 2 deletions(-) create mode 100644 src/FsAutoComplete/CodeFixes/NegateBooleanExpression.fs create mode 100644 src/FsAutoComplete/CodeFixes/NegateBooleanExpression.fsi create mode 100644 test/FsAutoComplete.Tests.Lsp/CodeFixTests/NegateBooleanExpressionTests.fs diff --git a/src/FsAutoComplete/CodeFixes/NegateBooleanExpression.fs b/src/FsAutoComplete/CodeFixes/NegateBooleanExpression.fs new file mode 100644 index 000000000..3e53979da --- /dev/null +++ b/src/FsAutoComplete/CodeFixes/NegateBooleanExpression.fs @@ -0,0 +1,164 @@ +module FsAutoComplete.CodeFix.NegateBooleanExpression + +open FSharp.Compiler.Symbols +open FSharp.Compiler.Syntax +open FSharp.Compiler.SyntaxTrivia +open FSharp.Compiler.Text +open FsToolkit.ErrorHandling +open Ionide.LanguageServerProtocol.Types +open FsAutoComplete.CodeFix.Types +open FsAutoComplete +open FsAutoComplete.LspHelpers + +let title = "Negate boolean expression" + +let booleanOperators = set [ "||"; "&&"; "=" ] // TODO: probably some others + +[] +let (|BooleanOperator|_|) = + function + | SynExpr.LongIdent( + longDotId = SynLongIdent(id = [ operatorIdent ]; trivia = [ Some(IdentTrivia.OriginalNotation operatorText) ])) -> + if booleanOperators.Contains operatorText then + ValueSome operatorIdent + else + ValueNone + | _ -> ValueNone + +[] +let (|LastIdentFromSynLongIdent|_|) (SynLongIdent(id = id)) = + match List.tryLast id with + | None -> ValueNone + | Some ident -> ValueSome ident + +type FixData = + { Expr: SynExpr + Path: SyntaxVisitorPath + Ident: Ident + NeedsParensAfterNot: bool } + +let mkFix (codeActionParams: CodeActionParams) (sourceText: ISourceText) fixData (returnType: FSharpType) = + if + returnType.IsFunctionType + || returnType.BasicQualifiedName <> "Microsoft.FSharp.Core.bool" + then + [] + else + let needsOuterParens = + SynExpr.shouldBeParenthesizedInContext sourceText.GetLineString fixData.Path fixData.Expr + + let lpr, rpr = if fixData.NeedsParensAfterNot then "(", ")" else "", "" + + let newText = + $"not %s{lpr}%s{sourceText.GetSubTextFromRange fixData.Expr.Range}%s{rpr}" + |> (if not needsOuterParens then id else sprintf "(%s)") + + + [ { SourceDiagnostic = None + Title = title + File = codeActionParams.TextDocument + Edits = + [| { Range = fcsRangeToLsp fixData.Expr.Range + NewText = newText } |] + Kind = FixKind.Fix } ] + +let fix (getParseResultsForFile: GetParseResultsForFile) : CodeFix = + fun (codeActionParams: CodeActionParams) -> + asyncResult { + // Most code fixes have some general setup. + // We initially want to detect the state of the current code and whether we can propose any text edits to the user. + + let fileName = codeActionParams.TextDocument.GetFilePath() |> Utils.normalizePath + // The converted LSP start position to an FCS start position. + let fcsPos = protocolPosToPos codeActionParams.Range.Start + // The syntax tree and typed tree, current line and sourceText of the current file. + let! (parseAndCheckResults: ParseAndCheckResults, _line: string, sourceText: IFSACSourceText) = + getParseResultsForFile fileName fcsPos + + let optFixData = + ParsedInput.tryNode fcsPos parseAndCheckResults.GetParseResults.ParseTree + |> Option.bind (fun (node, path) -> + + match node with + | SyntaxNode.SynExpr e -> + match e, path with + // && + | BooleanOperator operator, + SyntaxNode.SynExpr(SynExpr.App(isInfix = true)) :: SyntaxNode.SynExpr(SynExpr.App(isInfix = false) as e) :: rest -> + Some + { Expr = e + Ident = operator + Path = rest + NeedsParensAfterNot = true } + + // $0X().Y + | SynExpr.Ident _, + SyntaxNode.SynExpr(SynExpr.App _) :: SyntaxNode.SynExpr(SynExpr.DotGet( + longDotId = LastIdentFromSynLongIdent ident) as e) :: rest -> + Some + { Expr = e + Ident = ident + Path = rest + NeedsParensAfterNot = true } + + // X$0() + | SynExpr.Ident ident, SyntaxNode.SynExpr(SynExpr.App _ as e) :: rest -> + Some + { Expr = e + Ident = ident + Path = rest + NeedsParensAfterNot = true } + + // X()$0 + | (SynExpr.Const(constant = SynConst.Unit) | SynExpr.Paren _), + SyntaxNode.SynExpr(SynExpr.App(funcExpr = SynExpr.Ident ident) as e) :: rest -> + Some + { Expr = e + Ident = ident + Path = rest + NeedsParensAfterNot = true } + + // X().Y$0 + | SynExpr.DotGet(longDotId = LastIdentFromSynLongIdent ident), path -> + Some + { Expr = e + Ident = ident + Path = path + NeedsParensAfterNot = true } + + // a.Y + | SynExpr.LongIdent(isOptional = false; longDotId = LastIdentFromSynLongIdent ident), path -> + Some + { Expr = e + Ident = ident + Path = path + NeedsParensAfterNot = false } + // a + | SynExpr.Ident ident, path -> + Some + { Expr = e + Ident = ident + Path = path + NeedsParensAfterNot = false } + + | _ -> None + | _ -> None) + + match optFixData with + | Some fixData -> + let mExpr = fixData.Expr.Range + + if mExpr.StartLine <> mExpr.EndLine then + // Only process single line expressions for now + return [] + else + match parseAndCheckResults.TryGetSymbolUseFromIdent sourceText fixData.Ident with + | None -> return [] + | Some symbolUse -> + match symbolUse.Symbol with + | :? FSharpField as ff -> return mkFix codeActionParams sourceText fixData ff.FieldType + | :? FSharpMemberOrFunctionOrValue as mfv -> + return mkFix codeActionParams sourceText fixData mfv.ReturnParameter.Type + | _ -> return [] + | _ -> return [] + } diff --git a/src/FsAutoComplete/CodeFixes/NegateBooleanExpression.fsi b/src/FsAutoComplete/CodeFixes/NegateBooleanExpression.fsi new file mode 100644 index 000000000..7fc11fedf --- /dev/null +++ b/src/FsAutoComplete/CodeFixes/NegateBooleanExpression.fsi @@ -0,0 +1,6 @@ +module FsAutoComplete.CodeFix.NegateBooleanExpression + +open FsAutoComplete.CodeFix.Types + +val title: string +val fix: getParseResultsForFile: GetParseResultsForFile -> CodeFix diff --git a/src/FsAutoComplete/LspServers/AdaptiveServerState.fs b/src/FsAutoComplete/LspServers/AdaptiveServerState.fs index 2ccff0d76..c9ddfe4b9 100644 --- a/src/FsAutoComplete/LspServers/AdaptiveServerState.fs +++ b/src/FsAutoComplete/LspServers/AdaptiveServerState.fs @@ -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 + NegateBooleanExpression.fix tryGetParseAndCheckResultsForFile |]) let forgetDocument (uri: DocumentUri) = async { diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests/NegateBooleanExpressionTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests/NegateBooleanExpressionTests.fs new file mode 100644 index 000000000..7f13cda49 --- /dev/null +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests/NegateBooleanExpressionTests.fs @@ -0,0 +1,183 @@ +module private FsAutoComplete.Tests.CodeFixTests.NegateBooleanExpressionTests + +open Expecto +open Helpers +open Utils.ServerTests +open Utils.CursorbasedTests +open FsAutoComplete.CodeFix + +let tests state = + serverTestList + (nameof NegateBooleanExpression) + state + { defaultConfigDto with + EnableAnalyzers = Some true } + None + (fun server -> + [ let selectCodeFix = CodeFix.withTitle NegateBooleanExpression.title + + ftestCaseAsync "negate single identifier" + <| CodeFix.check + server + "let a = false +let b = a$0" + Diagnostics.acceptAll + selectCodeFix + "let a = false +let b = not a" + + ftestCaseAsync "negate boolean expression" + <| CodeFix.check + server + "let a = false +let b = a $0|| false" + Diagnostics.acceptAll + selectCodeFix + "let a = false +let b = not (a || false)" + + ftestCaseAsync "negate longdotident expression" + <| CodeFix.check + server + " +module A = + let a = false + +let b = $0A.a" + Diagnostics.acceptAll + selectCodeFix + " +module A = + let a = false + +let b = not A.a" + + ftestCaseAsync "negate record field" + <| CodeFix.check + server + " +type X = { Y: bool } + +let a = { Y = true } +let b = $0a.Y" + Diagnostics.acceptAll + selectCodeFix + " +type X = { Y: bool } + +let a = { Y = true } +let b = not a.Y" + + ftestCaseAsync "negate class property" + <| CodeFix.check + server + " +type X() = + member val Y : bool = false + +let b = X().Y$0" + Diagnostics.acceptAll + selectCodeFix + " +type X() = + member val Y : bool = false + +let b = not (X().Y)" + + ftestCaseAsync "negate class property, cursor at start" + <| CodeFix.check + server + " +type X() = + member val Y : bool = false + +let b = $0X().Y" + Diagnostics.acceptAll + selectCodeFix + " +type X() = + member val Y : bool = false + +let b = not (X().Y)" + + ftestCaseAsync "negate unit function call" + <| CodeFix.check + server + " +let a () = false +let b = a$0 ()" + Diagnostics.acceptAll + selectCodeFix + " +let a () = false +let b = not (a ())" + + ftestCaseAsync "negate unit function call, cursor at end" + <| CodeFix.check + server + " +let a () = false +let b = a ()$0" + Diagnostics.acceptAll + selectCodeFix + " +let a () = false +let b = not (a ())" + + ftestCaseAsync "negate unit function call, cursor at end" + <| CodeFix.check + server + " +let a _ = false +let b = a 4$0" + Diagnostics.acceptAll + selectCodeFix + " +let a _ = false +let b = not (a 4)" + + ftestCaseAsync "negate unit function call, cursor at end" + <| CodeFix.check + server + " +let a _ = false +let b = a 4$0" + Diagnostics.acceptAll + selectCodeFix + " +let a _ = false +let b = not (a 4)" + + ftestCaseAsync "negate unit member invocation" + <| CodeFix.check + server + " +type X() = + member x.Y () : bool = false + +let b = $0X().Y()" + Diagnostics.acceptAll + selectCodeFix + " +type X() = + member x.Y () : bool = false + +let b = not (X().Y())" + + ftestCaseAsync "negate unit member invocation, cursor at end" + <| CodeFix.check + server + " +type X() = + member x.Y () : bool = false + +let b = X().Y()$0" + Diagnostics.acceptAll + selectCodeFix + " +type X() = + member x.Y () : bool = false + +let b = not (X().Y())" + + ]) diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests/Tests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests/Tests.fs index 04760bc77..dccb520ef 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests/Tests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests/Tests.fs @@ -3436,4 +3436,5 @@ let tests textFactory state = AddTypeAliasToSignatureFileTests.tests state UpdateTypeAbbreviationInSignatureFileTests.tests state AddBindingToSignatureFileTests.tests state - ReplaceLambdaWithDotLambdaTests.tests state ] + ReplaceLambdaWithDotLambdaTests.tests state + NegateBooleanExpressionTests.tests state ] From 97d9398e34bbfa092cda9a83fc74d5642c0372e9 Mon Sep 17 00:00:00 2001 From: nojaf Date: Mon, 25 Mar 2024 08:45:08 +0100 Subject: [PATCH 2/6] Traverse from the outside in. --- .../CodeFixes/NegateBooleanExpression.fs | 50 ++++++------------- .../NegateBooleanExpressionTests.fs | 28 +++++------ 2 files changed, 30 insertions(+), 48 deletions(-) diff --git a/src/FsAutoComplete/CodeFixes/NegateBooleanExpression.fs b/src/FsAutoComplete/CodeFixes/NegateBooleanExpression.fs index 3e53979da..bd50fcc54 100644 --- a/src/FsAutoComplete/CodeFixes/NegateBooleanExpression.fs +++ b/src/FsAutoComplete/CodeFixes/NegateBooleanExpression.fs @@ -12,7 +12,7 @@ open FsAutoComplete.LspHelpers let title = "Negate boolean expression" -let booleanOperators = set [ "||"; "&&"; "=" ] // TODO: probably some others +let booleanOperators = set [ "||"; "&&"; "="; "<>" ] [] let (|BooleanOperator|_|) = @@ -76,50 +76,32 @@ let fix (getParseResultsForFile: GetParseResultsForFile) : CodeFix = getParseResultsForFile fileName fcsPos let optFixData = - ParsedInput.tryNode fcsPos parseAndCheckResults.GetParseResults.ParseTree - |> Option.bind (fun (node, path) -> - + (fcsPos, parseAndCheckResults.GetParseResults.ParseTree) + ||> ParsedInput.tryPick (fun path node -> match node with | SyntaxNode.SynExpr e -> - match e, path with - // && - | BooleanOperator operator, - SyntaxNode.SynExpr(SynExpr.App(isInfix = true)) :: SyntaxNode.SynExpr(SynExpr.App(isInfix = false) as e) :: rest -> + match e with + // a && b + | SynExpr.App(isInfix = false; funcExpr = SynExpr.App(isInfix = true; funcExpr = BooleanOperator operator)) -> Some { Expr = e Ident = operator - Path = rest - NeedsParensAfterNot = true } - - // $0X().Y - | SynExpr.Ident _, - SyntaxNode.SynExpr(SynExpr.App _) :: SyntaxNode.SynExpr(SynExpr.DotGet( - longDotId = LastIdentFromSynLongIdent ident) as e) :: rest -> - Some - { Expr = e - Ident = ident - Path = rest + Path = path NeedsParensAfterNot = true } - // X$0() - | SynExpr.Ident ident, SyntaxNode.SynExpr(SynExpr.App _ as e) :: rest -> - Some - { Expr = e - Ident = ident - Path = rest - NeedsParensAfterNot = true } + // X().Y() + | SynExpr.App(funcExpr = SynExpr.DotGet(longDotId = LastIdentFromSynLongIdent ident)) - // X()$0 - | (SynExpr.Const(constant = SynConst.Unit) | SynExpr.Paren _), - SyntaxNode.SynExpr(SynExpr.App(funcExpr = SynExpr.Ident ident) as e) :: rest -> + // X().Y + | SynExpr.DotGet(longDotId = LastIdentFromSynLongIdent ident) -> Some { Expr = e Ident = ident - Path = rest + Path = path NeedsParensAfterNot = true } - // X().Y$0 - | SynExpr.DotGet(longDotId = LastIdentFromSynLongIdent ident), path -> + // X() + | SynExpr.App(funcExpr = SynExpr.Ident ident) -> Some { Expr = e Ident = ident @@ -127,14 +109,14 @@ let fix (getParseResultsForFile: GetParseResultsForFile) : CodeFix = NeedsParensAfterNot = true } // a.Y - | SynExpr.LongIdent(isOptional = false; longDotId = LastIdentFromSynLongIdent ident), path -> + | SynExpr.LongIdent(isOptional = false; longDotId = LastIdentFromSynLongIdent ident) -> Some { Expr = e Ident = ident Path = path NeedsParensAfterNot = false } // a - | SynExpr.Ident ident, path -> + | SynExpr.Ident ident -> Some { Expr = e Ident = ident diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests/NegateBooleanExpressionTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests/NegateBooleanExpressionTests.fs index 7f13cda49..b493f0e4f 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests/NegateBooleanExpressionTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests/NegateBooleanExpressionTests.fs @@ -16,7 +16,7 @@ let tests state = (fun server -> [ let selectCodeFix = CodeFix.withTitle NegateBooleanExpression.title - ftestCaseAsync "negate single identifier" + testCaseAsync "negate single identifier" <| CodeFix.check server "let a = false @@ -26,7 +26,7 @@ let b = a$0" "let a = false let b = not a" - ftestCaseAsync "negate boolean expression" + testCaseAsync "negate boolean expression" <| CodeFix.check server "let a = false @@ -36,7 +36,7 @@ let b = a $0|| false" "let a = false let b = not (a || false)" - ftestCaseAsync "negate longdotident expression" + testCaseAsync "negate longdotident expression" <| CodeFix.check server " @@ -52,7 +52,7 @@ module A = let b = not A.a" - ftestCaseAsync "negate record field" + testCaseAsync "negate record field" <| CodeFix.check server " @@ -68,7 +68,7 @@ type X = { Y: bool } let a = { Y = true } let b = not a.Y" - ftestCaseAsync "negate class property" + testCaseAsync "negate class property" <| CodeFix.check server " @@ -84,7 +84,7 @@ type X() = let b = not (X().Y)" - ftestCaseAsync "negate class property, cursor at start" + testCaseAsync "negate class property, cursor at start" <| CodeFix.check server " @@ -100,7 +100,7 @@ type X() = let b = not (X().Y)" - ftestCaseAsync "negate unit function call" + testCaseAsync "negate unit function call" <| CodeFix.check server " @@ -112,7 +112,7 @@ let b = a$0 ()" let a () = false let b = not (a ())" - ftestCaseAsync "negate unit function call, cursor at end" + testCaseAsync "negate unit function call, cursor at end" <| CodeFix.check server " @@ -124,7 +124,7 @@ let b = a ()$0" let a () = false let b = not (a ())" - ftestCaseAsync "negate unit function call, cursor at end" + testCaseAsync "negate non-unit function call, cursor at end" <| CodeFix.check server " @@ -136,19 +136,19 @@ let b = a 4$0" let a _ = false let b = not (a 4)" - ftestCaseAsync "negate unit function call, cursor at end" + testCaseAsync "negate non-unit function call, cursor in middle" <| CodeFix.check server " let a _ = false -let b = a 4$0" +let b = a $0 4" Diagnostics.acceptAll selectCodeFix " let a _ = false -let b = not (a 4)" +let b = not (a 4)" - ftestCaseAsync "negate unit member invocation" + testCaseAsync "negate unit member invocation" <| CodeFix.check server " @@ -164,7 +164,7 @@ type X() = let b = not (X().Y())" - ftestCaseAsync "negate unit member invocation, cursor at end" + testCaseAsync "negate unit member invocation, cursor at end" <| CodeFix.check server " From 96c0fa1e39797cd46258d11b6a4c72af67e5c1c9 Mon Sep 17 00:00:00 2001 From: nojaf Date: Mon, 25 Mar 2024 08:48:11 +0100 Subject: [PATCH 3/6] Remove some scaffold comments --- src/FsAutoComplete/CodeFixes/NegateBooleanExpression.fs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/FsAutoComplete/CodeFixes/NegateBooleanExpression.fs b/src/FsAutoComplete/CodeFixes/NegateBooleanExpression.fs index bd50fcc54..133f53f12 100644 --- a/src/FsAutoComplete/CodeFixes/NegateBooleanExpression.fs +++ b/src/FsAutoComplete/CodeFixes/NegateBooleanExpression.fs @@ -65,13 +65,9 @@ let mkFix (codeActionParams: CodeActionParams) (sourceText: ISourceText) fixData let fix (getParseResultsForFile: GetParseResultsForFile) : CodeFix = fun (codeActionParams: CodeActionParams) -> asyncResult { - // Most code fixes have some general setup. - // We initially want to detect the state of the current code and whether we can propose any text edits to the user. - let fileName = codeActionParams.TextDocument.GetFilePath() |> Utils.normalizePath - // The converted LSP start position to an FCS start position. let fcsPos = protocolPosToPos codeActionParams.Range.Start - // The syntax tree and typed tree, current line and sourceText of the current file. + let! (parseAndCheckResults: ParseAndCheckResults, _line: string, sourceText: IFSACSourceText) = getParseResultsForFile fileName fcsPos From 3db68a3d8399aa5a7b101a376f446cb5995b5fa5 Mon Sep 17 00:00:00 2001 From: nojaf Date: Mon, 25 Mar 2024 09:27:51 +0100 Subject: [PATCH 4/6] negate instance member invocation --- .../CodeFixes/NegateBooleanExpression.fs | 3 +++ .../NegateBooleanExpressionTests.fs | 20 +++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/src/FsAutoComplete/CodeFixes/NegateBooleanExpression.fs b/src/FsAutoComplete/CodeFixes/NegateBooleanExpression.fs index 133f53f12..f96de4eee 100644 --- a/src/FsAutoComplete/CodeFixes/NegateBooleanExpression.fs +++ b/src/FsAutoComplete/CodeFixes/NegateBooleanExpression.fs @@ -85,6 +85,9 @@ let fix (getParseResultsForFile: GetParseResultsForFile) : CodeFix = Path = path NeedsParensAfterNot = true } + // X.Y() + | SynExpr.App(funcExpr = SynExpr.LongIdent(longDotId = LastIdentFromSynLongIdent ident)) + // X().Y() | SynExpr.App(funcExpr = SynExpr.DotGet(longDotId = LastIdentFromSynLongIdent ident)) diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests/NegateBooleanExpressionTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests/NegateBooleanExpressionTests.fs index b493f0e4f..16be05b69 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests/NegateBooleanExpressionTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests/NegateBooleanExpressionTests.fs @@ -180,4 +180,24 @@ type X() = let b = not (X().Y())" + testCaseAsync "negate instance member invocation" + <| CodeFix.check + server + " +open System.Collections.Generic + +let foo () = + let dict = dict [] + dict.TryAdd$0(\"foo\", \"bar\") + ()" + Diagnostics.acceptAll + selectCodeFix + " +open System.Collections.Generic + +let foo () = + let dict = dict [] + (not (dict.TryAdd(\"foo\", \"bar\"))) + ()" + ]) From fae10958cd4e62ae65247fa44b1e095e9517090d Mon Sep 17 00:00:00 2001 From: nojaf Date: Tue, 2 Apr 2024 08:51:16 +0200 Subject: [PATCH 5/6] Use SynExpr.shouldBeParenthesizedInContext to detect parent between not and expr. --- .../CodeFixes/NegateBooleanExpression.fs | 141 +++++++++++++++++- .../NegateBooleanExpressionTests.fs | 2 +- 2 files changed, 139 insertions(+), 4 deletions(-) diff --git a/src/FsAutoComplete/CodeFixes/NegateBooleanExpression.fs b/src/FsAutoComplete/CodeFixes/NegateBooleanExpression.fs index f96de4eee..c3d277412 100644 --- a/src/FsAutoComplete/CodeFixes/NegateBooleanExpression.fs +++ b/src/FsAutoComplete/CodeFixes/NegateBooleanExpression.fs @@ -37,6 +37,110 @@ type FixData = Ident: Ident NeedsParensAfterNot: bool } +/// Update the range of an expression. +let updateRangeOfExpr (range: range) (expr: SynExpr) : SynExpr = + match expr with + | SynExpr.AddressOf(i, e, opRange, _) -> SynExpr.AddressOf(i, e, opRange, range) + | SynExpr.Paren(expr, leftParenRange, rightParenRange, _) -> + SynExpr.Paren(expr, leftParenRange, rightParenRange, range) + | SynExpr.Quote(operator, isRaw, quotedExpr, isFromQueryExpression, _) -> + SynExpr.Quote(operator, isRaw, quotedExpr, isFromQueryExpression, range) + | SynExpr.Const(constant, _) -> SynExpr.Const(constant, range) + | SynExpr.Typed(expr, targetType, _) -> SynExpr.Typed(expr, targetType, range) + | SynExpr.Tuple(isStruct, exprs, commaRanges, _) -> SynExpr.Tuple(isStruct, exprs, commaRanges, range) + | SynExpr.AnonRecd(isStruct, copyInfo, recordFields, _, trivia) -> + SynExpr.AnonRecd(isStruct, copyInfo, recordFields, range, trivia) + | SynExpr.ArrayOrList(isArray, exprs, _) -> SynExpr.ArrayOrList(isArray, exprs, range) + | SynExpr.Record(baseInfo, copyInfo, recordFields, _) -> SynExpr.Record(baseInfo, copyInfo, recordFields, range) + | SynExpr.New(isProtected, targetType, expr, _) -> SynExpr.New(isProtected, targetType, expr, range) + | SynExpr.ObjExpr(objType, argOptions, withKeyword, bindings, members, extraImpls, newExprRange, _) -> + SynExpr.ObjExpr(objType, argOptions, withKeyword, bindings, members, extraImpls, newExprRange, range) + | SynExpr.While(whileDebugPoint, whileExpr, doExpr, _) -> SynExpr.While(whileDebugPoint, whileExpr, doExpr, range) + | SynExpr.For(forDebugPoint, toDebugPoint, ident, equalsRange, identBody, direction, toBody, doBody, _) -> + SynExpr.For(forDebugPoint, toDebugPoint, ident, equalsRange, identBody, direction, toBody, doBody, range) + | SynExpr.ForEach(forDebugPoint, inDebugPoint, seqExprOnly, isFromSource, pat, enumExpr, bodyExpr, _) -> + SynExpr.ForEach(forDebugPoint, inDebugPoint, seqExprOnly, isFromSource, pat, enumExpr, bodyExpr, range) + | SynExpr.ArrayOrListComputed(isArray, expr, _) -> SynExpr.ArrayOrListComputed(isArray, expr, range) + | SynExpr.IndexRange(expr1, opm, expr2, range1, range2, _) -> + SynExpr.IndexRange(expr1, opm, expr2, range1, range2, range) + | SynExpr.IndexFromEnd(expr, _) -> SynExpr.IndexFromEnd(expr, range) + | SynExpr.ComputationExpr(hasSeqBuilder, expr, _) -> SynExpr.ComputationExpr(hasSeqBuilder, expr, range) + | SynExpr.Lambda(fromMethod, inLambdaSeq, args, body, parsedData, _, trivia) -> + SynExpr.Lambda(fromMethod, inLambdaSeq, args, body, parsedData, range, trivia) + | SynExpr.MatchLambda(isExnMatch, keywordRange, matchClauses, matchDebugPoint, _) -> + SynExpr.MatchLambda(isExnMatch, keywordRange, matchClauses, matchDebugPoint, range) + | SynExpr.Match(matchDebugPoint, expr, clauses, _, trivia) -> + SynExpr.Match(matchDebugPoint, expr, clauses, range, trivia) + | SynExpr.Do(expr, _) -> SynExpr.Do(expr, range) + | SynExpr.Assert(expr, _) -> SynExpr.Assert(expr, range) + | SynExpr.App(flag, isInfix, funcExpr, argExpr, _) -> SynExpr.App(flag, isInfix, funcExpr, argExpr, range) + | SynExpr.TypeApp(expr, lessRange, typeArgs, commaRanges, greaterRange, typeArgsRange, _) -> + SynExpr.TypeApp(expr, lessRange, typeArgs, commaRanges, greaterRange, typeArgsRange, range) + | SynExpr.LetOrUse(isRecursive, isUse, bindings, body, _, trivia) -> + SynExpr.LetOrUse(isRecursive, isUse, bindings, body, range, trivia) + | SynExpr.TryWith(tryExpr, withCases, _, tryDebugPoint, withDebugPoint, trivia) -> + SynExpr.TryWith(tryExpr, withCases, range, tryDebugPoint, withDebugPoint, trivia) + | SynExpr.TryFinally(tryExpr, finallyExpr, _, tryDebugPoint, finallyDebugPoint, trivia) -> + SynExpr.TryFinally(tryExpr, finallyExpr, range, tryDebugPoint, finallyDebugPoint, trivia) + | SynExpr.Lazy(expr, _) -> SynExpr.Lazy(expr, range) + | SynExpr.Sequential(debugPoint, isTrueSeq, expr1, expr2, _) -> + SynExpr.Sequential(debugPoint, isTrueSeq, expr1, expr2, range) + | SynExpr.IfThenElse(ifExpr, thenExpr, elseExpr, spIfToThen, isFromErrorRecovery, _, trivia) -> + SynExpr.IfThenElse(ifExpr, thenExpr, elseExpr, spIfToThen, isFromErrorRecovery, range, trivia) + | SynExpr.Typar(typar, _) -> SynExpr.Typar(typar, range) + | SynExpr.Ident(ident) -> SynExpr.Ident(FSharp.Compiler.Syntax.Ident(ident.idText, range)) + | SynExpr.LongIdent(isOptional, longDotId, altNameRefCell, _) -> + SynExpr.LongIdent(isOptional, longDotId, altNameRefCell, range) + | SynExpr.LongIdentSet(longDotId, expr, _) -> SynExpr.LongIdentSet(longDotId, expr, range) + | SynExpr.DotGet(expr, rangeOfDot, longDotId, _) -> SynExpr.DotGet(expr, rangeOfDot, longDotId, range) + | SynExpr.DotLambda(expr, _, trivia) -> SynExpr.DotLambda(expr, range, trivia) + | SynExpr.DotSet(targetExpr, longDotId, rhsExpr, _) -> SynExpr.DotSet(targetExpr, longDotId, rhsExpr, range) + | SynExpr.Set(targetExpr, rhsExpr, _) -> SynExpr.Set(targetExpr, rhsExpr, range) + | SynExpr.DotIndexedGet(objectExpr, indexArgs, dotRange, _) -> + SynExpr.DotIndexedGet(objectExpr, indexArgs, dotRange, range) + | SynExpr.DotIndexedSet(objectExpr, indexArgs, valueExpr, leftOfSetRange, dotRange, _) -> + SynExpr.DotIndexedSet(objectExpr, indexArgs, valueExpr, leftOfSetRange, dotRange, range) + | SynExpr.NamedIndexedPropertySet(longDotId, expr1, expr2, _) -> + SynExpr.NamedIndexedPropertySet(longDotId, expr1, expr2, range) + | SynExpr.DotNamedIndexedPropertySet(targetExpr, longDotId, argExpr, rhsExpr, _) -> + SynExpr.DotNamedIndexedPropertySet(targetExpr, longDotId, argExpr, rhsExpr, range) + | SynExpr.TypeTest(expr, targetType, _) -> SynExpr.TypeTest(expr, targetType, range) + | SynExpr.Upcast(expr, targetType, _) -> SynExpr.Upcast(expr, targetType, range) + | SynExpr.Downcast(expr, targetType, _) -> SynExpr.Downcast(expr, targetType, range) + | SynExpr.InferredUpcast(expr, _) -> SynExpr.InferredUpcast(expr, range) + | SynExpr.InferredDowncast(expr, _) -> SynExpr.InferredDowncast(expr, range) + | SynExpr.Null _ -> SynExpr.Null range + | SynExpr.TraitCall(supportTys, traitSig, argExpr, _) -> SynExpr.TraitCall(supportTys, traitSig, argExpr, range) + | SynExpr.JoinIn(lhsExpr, lhsRange, rhsExpr, _) -> SynExpr.JoinIn(lhsExpr, lhsRange, rhsExpr, range) + | SynExpr.ImplicitZero _ -> SynExpr.ImplicitZero range + | SynExpr.SequentialOrImplicitYield(debugPoint, expr1, expr2, ifNotStmt, _) -> + SynExpr.SequentialOrImplicitYield(debugPoint, expr1, expr2, ifNotStmt, range) + | SynExpr.YieldOrReturn(flags, expr, _) -> SynExpr.YieldOrReturn(flags, expr, range) + | SynExpr.YieldOrReturnFrom(flags, expr, _) -> SynExpr.YieldOrReturnFrom(flags, expr, range) + | SynExpr.LetOrUseBang(bindDebugPoint, isUse, isFromSource, pat, rhs, andBangs, body, _, trivia) -> + SynExpr.LetOrUseBang(bindDebugPoint, isUse, isFromSource, pat, rhs, andBangs, body, range, trivia) + | SynExpr.MatchBang(matchDebugPoint, expr, clauses, _, trivia) -> + SynExpr.MatchBang(matchDebugPoint, expr, clauses, range, trivia) + | SynExpr.DoBang(expr, _) -> SynExpr.DoBang(expr, range) + | SynExpr.WhileBang(whileDebugPoint, whileExpr, doExpr, _) -> + SynExpr.WhileBang(whileDebugPoint, whileExpr, doExpr, range) + | SynExpr.LibraryOnlyILAssembly(ilCode, typeArgs, args, retTy, _) -> + SynExpr.LibraryOnlyILAssembly(ilCode, typeArgs, args, retTy, range) + | SynExpr.LibraryOnlyStaticOptimization(constraints, expr, optimizedExpr, _) -> + SynExpr.LibraryOnlyStaticOptimization(constraints, expr, optimizedExpr, range) + | SynExpr.LibraryOnlyUnionCaseFieldGet(expr, longId, fieldNum, _) -> + SynExpr.LibraryOnlyUnionCaseFieldGet(expr, longId, fieldNum, range) + | SynExpr.LibraryOnlyUnionCaseFieldSet(expr, longId, fieldNum, rhsExpr, _) -> + SynExpr.LibraryOnlyUnionCaseFieldSet(expr, longId, fieldNum, rhsExpr, range) + | SynExpr.ArbitraryAfterError(debugStr, _) -> SynExpr.ArbitraryAfterError(debugStr, range) + | SynExpr.FromParseError(expr, _) -> SynExpr.FromParseError(expr, range) + | SynExpr.DiscardAfterMissingQualificationAfterDot(expr, dotRange, _) -> + SynExpr.DiscardAfterMissingQualificationAfterDot(expr, dotRange, range) + | SynExpr.Fixed(expr, _) -> SynExpr.Fixed(expr, range) + | SynExpr.InterpolatedString(contents, synStringKind, _) -> SynExpr.InterpolatedString(contents, synStringKind, range) + | SynExpr.DebugPoint(debugPoint, isControlFlow, innerExpr) -> SynExpr.DebugPoint(debugPoint, isControlFlow, innerExpr) + | SynExpr.Dynamic(funcExpr, qmark, argExpr, _) -> SynExpr.Dynamic(funcExpr, qmark, argExpr, range) + let mkFix (codeActionParams: CodeActionParams) (sourceText: ISourceText) fixData (returnType: FSharpType) = if returnType.IsFunctionType @@ -44,10 +148,41 @@ let mkFix (codeActionParams: CodeActionParams) (sourceText: ISourceText) fixData then [] else - let needsOuterParens = - SynExpr.shouldBeParenthesizedInContext sourceText.GetLineString fixData.Path fixData.Expr - let lpr, rpr = if fixData.NeedsParensAfterNot then "(", ")" else "", "" + let mExpr = fixData.Expr.Range + + let needsParensAfterNot = + let path, exprAfter = + // This is the rang of the expression moved 4 spaces so the `not` function can be applied. + let mExprAfter = + Range.mkRange + mExpr.FileName + (Position.mkPos mExpr.StartLine (mExpr.StartColumn + 4)) + (Position.mkPos mExpr.StartLine (mExpr.EndColumn + 4)) + + let exprAfter = updateRangeOfExpr mExprAfter fixData.Expr + + let mNot = + Range.mkRange mExpr.FileName mExpr.Start (Position.mkPos mExpr.StartLine (mExpr.StartColumn + 3)) + + let notExpr = SynExpr.Ident(FSharp.Compiler.Syntax.Ident("not", mNot)) + + let appExpr = + SynExpr.App( + ExprAtomicFlag.NonAtomic, + false, + notExpr, + exprAfter, + Range.unionRanges notExpr.Range exprAfter.Range + ) + + SyntaxNode.SynExpr appExpr :: fixData.Path, exprAfter + + SynExpr.shouldBeParenthesizedInContext sourceText.GetLineString path exprAfter + + let lpr, rpr = if needsParensAfterNot then "(", ")" else "", "" + + let needsOuterParens = false // TODO let newText = $"not %s{lpr}%s{sourceText.GetSubTextFromRange fixData.Expr.Range}%s{rpr}" diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests/NegateBooleanExpressionTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests/NegateBooleanExpressionTests.fs index 16be05b69..1b664d2db 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests/NegateBooleanExpressionTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests/NegateBooleanExpressionTests.fs @@ -7,7 +7,7 @@ open Utils.CursorbasedTests open FsAutoComplete.CodeFix let tests state = - serverTestList + fserverTestList (nameof NegateBooleanExpression) state { defaultConfigDto with From 90f17fa82e59a6ff42efd1b702bcfbf7fb66a913 Mon Sep 17 00:00:00 2001 From: nojaf Date: Wed, 3 Apr 2024 13:34:00 +0200 Subject: [PATCH 6/6] Use shouldBeParenthesizedInContext for both parentheses cases. --- .editorconfig | 3 + .../CodeFixes/NegateBooleanExpression.fs | 211 +++++------------- .../NegateBooleanExpressionTests.fs | 6 +- 3 files changed, 58 insertions(+), 162 deletions(-) diff --git a/.editorconfig b/.editorconfig index 8d1e1bb03..bfa5488e6 100644 --- a/.editorconfig +++ b/.editorconfig @@ -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 diff --git a/src/FsAutoComplete/CodeFixes/NegateBooleanExpression.fs b/src/FsAutoComplete/CodeFixes/NegateBooleanExpression.fs index c3d277412..a0988e636 100644 --- a/src/FsAutoComplete/CodeFixes/NegateBooleanExpression.fs +++ b/src/FsAutoComplete/CodeFixes/NegateBooleanExpression.fs @@ -37,110 +37,6 @@ type FixData = Ident: Ident NeedsParensAfterNot: bool } -/// Update the range of an expression. -let updateRangeOfExpr (range: range) (expr: SynExpr) : SynExpr = - match expr with - | SynExpr.AddressOf(i, e, opRange, _) -> SynExpr.AddressOf(i, e, opRange, range) - | SynExpr.Paren(expr, leftParenRange, rightParenRange, _) -> - SynExpr.Paren(expr, leftParenRange, rightParenRange, range) - | SynExpr.Quote(operator, isRaw, quotedExpr, isFromQueryExpression, _) -> - SynExpr.Quote(operator, isRaw, quotedExpr, isFromQueryExpression, range) - | SynExpr.Const(constant, _) -> SynExpr.Const(constant, range) - | SynExpr.Typed(expr, targetType, _) -> SynExpr.Typed(expr, targetType, range) - | SynExpr.Tuple(isStruct, exprs, commaRanges, _) -> SynExpr.Tuple(isStruct, exprs, commaRanges, range) - | SynExpr.AnonRecd(isStruct, copyInfo, recordFields, _, trivia) -> - SynExpr.AnonRecd(isStruct, copyInfo, recordFields, range, trivia) - | SynExpr.ArrayOrList(isArray, exprs, _) -> SynExpr.ArrayOrList(isArray, exprs, range) - | SynExpr.Record(baseInfo, copyInfo, recordFields, _) -> SynExpr.Record(baseInfo, copyInfo, recordFields, range) - | SynExpr.New(isProtected, targetType, expr, _) -> SynExpr.New(isProtected, targetType, expr, range) - | SynExpr.ObjExpr(objType, argOptions, withKeyword, bindings, members, extraImpls, newExprRange, _) -> - SynExpr.ObjExpr(objType, argOptions, withKeyword, bindings, members, extraImpls, newExprRange, range) - | SynExpr.While(whileDebugPoint, whileExpr, doExpr, _) -> SynExpr.While(whileDebugPoint, whileExpr, doExpr, range) - | SynExpr.For(forDebugPoint, toDebugPoint, ident, equalsRange, identBody, direction, toBody, doBody, _) -> - SynExpr.For(forDebugPoint, toDebugPoint, ident, equalsRange, identBody, direction, toBody, doBody, range) - | SynExpr.ForEach(forDebugPoint, inDebugPoint, seqExprOnly, isFromSource, pat, enumExpr, bodyExpr, _) -> - SynExpr.ForEach(forDebugPoint, inDebugPoint, seqExprOnly, isFromSource, pat, enumExpr, bodyExpr, range) - | SynExpr.ArrayOrListComputed(isArray, expr, _) -> SynExpr.ArrayOrListComputed(isArray, expr, range) - | SynExpr.IndexRange(expr1, opm, expr2, range1, range2, _) -> - SynExpr.IndexRange(expr1, opm, expr2, range1, range2, range) - | SynExpr.IndexFromEnd(expr, _) -> SynExpr.IndexFromEnd(expr, range) - | SynExpr.ComputationExpr(hasSeqBuilder, expr, _) -> SynExpr.ComputationExpr(hasSeqBuilder, expr, range) - | SynExpr.Lambda(fromMethod, inLambdaSeq, args, body, parsedData, _, trivia) -> - SynExpr.Lambda(fromMethod, inLambdaSeq, args, body, parsedData, range, trivia) - | SynExpr.MatchLambda(isExnMatch, keywordRange, matchClauses, matchDebugPoint, _) -> - SynExpr.MatchLambda(isExnMatch, keywordRange, matchClauses, matchDebugPoint, range) - | SynExpr.Match(matchDebugPoint, expr, clauses, _, trivia) -> - SynExpr.Match(matchDebugPoint, expr, clauses, range, trivia) - | SynExpr.Do(expr, _) -> SynExpr.Do(expr, range) - | SynExpr.Assert(expr, _) -> SynExpr.Assert(expr, range) - | SynExpr.App(flag, isInfix, funcExpr, argExpr, _) -> SynExpr.App(flag, isInfix, funcExpr, argExpr, range) - | SynExpr.TypeApp(expr, lessRange, typeArgs, commaRanges, greaterRange, typeArgsRange, _) -> - SynExpr.TypeApp(expr, lessRange, typeArgs, commaRanges, greaterRange, typeArgsRange, range) - | SynExpr.LetOrUse(isRecursive, isUse, bindings, body, _, trivia) -> - SynExpr.LetOrUse(isRecursive, isUse, bindings, body, range, trivia) - | SynExpr.TryWith(tryExpr, withCases, _, tryDebugPoint, withDebugPoint, trivia) -> - SynExpr.TryWith(tryExpr, withCases, range, tryDebugPoint, withDebugPoint, trivia) - | SynExpr.TryFinally(tryExpr, finallyExpr, _, tryDebugPoint, finallyDebugPoint, trivia) -> - SynExpr.TryFinally(tryExpr, finallyExpr, range, tryDebugPoint, finallyDebugPoint, trivia) - | SynExpr.Lazy(expr, _) -> SynExpr.Lazy(expr, range) - | SynExpr.Sequential(debugPoint, isTrueSeq, expr1, expr2, _) -> - SynExpr.Sequential(debugPoint, isTrueSeq, expr1, expr2, range) - | SynExpr.IfThenElse(ifExpr, thenExpr, elseExpr, spIfToThen, isFromErrorRecovery, _, trivia) -> - SynExpr.IfThenElse(ifExpr, thenExpr, elseExpr, spIfToThen, isFromErrorRecovery, range, trivia) - | SynExpr.Typar(typar, _) -> SynExpr.Typar(typar, range) - | SynExpr.Ident(ident) -> SynExpr.Ident(FSharp.Compiler.Syntax.Ident(ident.idText, range)) - | SynExpr.LongIdent(isOptional, longDotId, altNameRefCell, _) -> - SynExpr.LongIdent(isOptional, longDotId, altNameRefCell, range) - | SynExpr.LongIdentSet(longDotId, expr, _) -> SynExpr.LongIdentSet(longDotId, expr, range) - | SynExpr.DotGet(expr, rangeOfDot, longDotId, _) -> SynExpr.DotGet(expr, rangeOfDot, longDotId, range) - | SynExpr.DotLambda(expr, _, trivia) -> SynExpr.DotLambda(expr, range, trivia) - | SynExpr.DotSet(targetExpr, longDotId, rhsExpr, _) -> SynExpr.DotSet(targetExpr, longDotId, rhsExpr, range) - | SynExpr.Set(targetExpr, rhsExpr, _) -> SynExpr.Set(targetExpr, rhsExpr, range) - | SynExpr.DotIndexedGet(objectExpr, indexArgs, dotRange, _) -> - SynExpr.DotIndexedGet(objectExpr, indexArgs, dotRange, range) - | SynExpr.DotIndexedSet(objectExpr, indexArgs, valueExpr, leftOfSetRange, dotRange, _) -> - SynExpr.DotIndexedSet(objectExpr, indexArgs, valueExpr, leftOfSetRange, dotRange, range) - | SynExpr.NamedIndexedPropertySet(longDotId, expr1, expr2, _) -> - SynExpr.NamedIndexedPropertySet(longDotId, expr1, expr2, range) - | SynExpr.DotNamedIndexedPropertySet(targetExpr, longDotId, argExpr, rhsExpr, _) -> - SynExpr.DotNamedIndexedPropertySet(targetExpr, longDotId, argExpr, rhsExpr, range) - | SynExpr.TypeTest(expr, targetType, _) -> SynExpr.TypeTest(expr, targetType, range) - | SynExpr.Upcast(expr, targetType, _) -> SynExpr.Upcast(expr, targetType, range) - | SynExpr.Downcast(expr, targetType, _) -> SynExpr.Downcast(expr, targetType, range) - | SynExpr.InferredUpcast(expr, _) -> SynExpr.InferredUpcast(expr, range) - | SynExpr.InferredDowncast(expr, _) -> SynExpr.InferredDowncast(expr, range) - | SynExpr.Null _ -> SynExpr.Null range - | SynExpr.TraitCall(supportTys, traitSig, argExpr, _) -> SynExpr.TraitCall(supportTys, traitSig, argExpr, range) - | SynExpr.JoinIn(lhsExpr, lhsRange, rhsExpr, _) -> SynExpr.JoinIn(lhsExpr, lhsRange, rhsExpr, range) - | SynExpr.ImplicitZero _ -> SynExpr.ImplicitZero range - | SynExpr.SequentialOrImplicitYield(debugPoint, expr1, expr2, ifNotStmt, _) -> - SynExpr.SequentialOrImplicitYield(debugPoint, expr1, expr2, ifNotStmt, range) - | SynExpr.YieldOrReturn(flags, expr, _) -> SynExpr.YieldOrReturn(flags, expr, range) - | SynExpr.YieldOrReturnFrom(flags, expr, _) -> SynExpr.YieldOrReturnFrom(flags, expr, range) - | SynExpr.LetOrUseBang(bindDebugPoint, isUse, isFromSource, pat, rhs, andBangs, body, _, trivia) -> - SynExpr.LetOrUseBang(bindDebugPoint, isUse, isFromSource, pat, rhs, andBangs, body, range, trivia) - | SynExpr.MatchBang(matchDebugPoint, expr, clauses, _, trivia) -> - SynExpr.MatchBang(matchDebugPoint, expr, clauses, range, trivia) - | SynExpr.DoBang(expr, _) -> SynExpr.DoBang(expr, range) - | SynExpr.WhileBang(whileDebugPoint, whileExpr, doExpr, _) -> - SynExpr.WhileBang(whileDebugPoint, whileExpr, doExpr, range) - | SynExpr.LibraryOnlyILAssembly(ilCode, typeArgs, args, retTy, _) -> - SynExpr.LibraryOnlyILAssembly(ilCode, typeArgs, args, retTy, range) - | SynExpr.LibraryOnlyStaticOptimization(constraints, expr, optimizedExpr, _) -> - SynExpr.LibraryOnlyStaticOptimization(constraints, expr, optimizedExpr, range) - | SynExpr.LibraryOnlyUnionCaseFieldGet(expr, longId, fieldNum, _) -> - SynExpr.LibraryOnlyUnionCaseFieldGet(expr, longId, fieldNum, range) - | SynExpr.LibraryOnlyUnionCaseFieldSet(expr, longId, fieldNum, rhsExpr, _) -> - SynExpr.LibraryOnlyUnionCaseFieldSet(expr, longId, fieldNum, rhsExpr, range) - | SynExpr.ArbitraryAfterError(debugStr, _) -> SynExpr.ArbitraryAfterError(debugStr, range) - | SynExpr.FromParseError(expr, _) -> SynExpr.FromParseError(expr, range) - | SynExpr.DiscardAfterMissingQualificationAfterDot(expr, dotRange, _) -> - SynExpr.DiscardAfterMissingQualificationAfterDot(expr, dotRange, range) - | SynExpr.Fixed(expr, _) -> SynExpr.Fixed(expr, range) - | SynExpr.InterpolatedString(contents, synStringKind, _) -> SynExpr.InterpolatedString(contents, synStringKind, range) - | SynExpr.DebugPoint(debugPoint, isControlFlow, innerExpr) -> SynExpr.DebugPoint(debugPoint, isControlFlow, innerExpr) - | SynExpr.Dynamic(funcExpr, qmark, argExpr, _) -> SynExpr.Dynamic(funcExpr, qmark, argExpr, range) - let mkFix (codeActionParams: CodeActionParams) (sourceText: ISourceText) fixData (returnType: FSharpType) = if returnType.IsFunctionType @@ -149,53 +45,48 @@ let mkFix (codeActionParams: CodeActionParams) (sourceText: ISourceText) fixData [] else - let mExpr = fixData.Expr.Range - - let needsParensAfterNot = - let path, exprAfter = - // This is the rang of the expression moved 4 spaces so the `not` function can be applied. - let mExprAfter = - Range.mkRange - mExpr.FileName - (Position.mkPos mExpr.StartLine (mExpr.StartColumn + 4)) - (Position.mkPos mExpr.StartLine (mExpr.EndColumn + 4)) - - let exprAfter = updateRangeOfExpr mExprAfter fixData.Expr - - let mNot = - Range.mkRange mExpr.FileName mExpr.Start (Position.mkPos mExpr.StartLine (mExpr.StartColumn + 3)) - - let notExpr = SynExpr.Ident(FSharp.Compiler.Syntax.Ident("not", mNot)) - - let appExpr = - SynExpr.App( - ExprAtomicFlag.NonAtomic, - false, - notExpr, - exprAfter, - Range.unionRanges notExpr.Range exprAfter.Range - ) - - SyntaxNode.SynExpr appExpr :: fixData.Path, exprAfter - - SynExpr.shouldBeParenthesizedInContext sourceText.GetLineString path exprAfter - - let lpr, rpr = if needsParensAfterNot then "(", ")" else "", "" - - let needsOuterParens = false // TODO - - let newText = - $"not %s{lpr}%s{sourceText.GetSubTextFromRange fixData.Expr.Range}%s{rpr}" - |> (if not needsOuterParens then id else sprintf "(%s)") - - - [ { SourceDiagnostic = None - Title = title - File = codeActionParams.TextDocument - Edits = - [| { Range = fcsRangeToLsp fixData.Expr.Range - NewText = newText } |] - Kind = FixKind.Fix } ] + let mExpr = fixData.Expr.Range + let expr = fixData.Expr + let notExpr = SynExpr.Ident(FSharp.Compiler.Syntax.Ident("not", mExpr.StartRange)) + + let appExpr = + SynExpr.App(ExprAtomicFlag.NonAtomic, false, notExpr, expr, expr.Range) + + let negatedPath, negatedExpr = SyntaxNode.SynExpr appExpr :: fixData.Path, expr + + // not (expr) + let needsParensAfterNot = + SynExpr.shouldBeParenthesizedInContext sourceText.GetLineString negatedPath negatedExpr + + let lpr, rpr = if needsParensAfterNot then "(", ")" else "", "" + + // (not expr) + let needsOuterParens = + let e = + if not needsParensAfterNot then + negatedExpr + else + SynExpr.App( + ExprAtomicFlag.NonAtomic, + false, + notExpr, + SynExpr.Paren(expr, expr.Range.StartRange, Some expr.Range.EndRange, expr.Range), + expr.Range + ) + + SynExpr.shouldBeParenthesizedInContext sourceText.GetLineString fixData.Path e + + let newText = + $"not %s{lpr}%s{sourceText.GetSubTextFromRange fixData.Expr.Range}%s{rpr}" + |> (if not needsOuterParens then id else sprintf "(%s)") + + [ { SourceDiagnostic = None + Title = title + File = codeActionParams.TextDocument + Edits = + [| { Range = fcsRangeToLsp fixData.Expr.Range + NewText = newText } |] + Kind = FixKind.Fix } ] let fix (getParseResultsForFile: GetParseResultsForFile) : CodeFix = fun (codeActionParams: CodeActionParams) -> @@ -268,13 +159,15 @@ let fix (getParseResultsForFile: GetParseResultsForFile) : CodeFix = // Only process single line expressions for now return [] else - match parseAndCheckResults.TryGetSymbolUseFromIdent sourceText fixData.Ident with - | None -> return [] - | Some symbolUse -> - match symbolUse.Symbol with - | :? FSharpField as ff -> return mkFix codeActionParams sourceText fixData ff.FieldType - | :? FSharpMemberOrFunctionOrValue as mfv -> - return mkFix codeActionParams sourceText fixData mfv.ReturnParameter.Type - | _ -> return [] + + match parseAndCheckResults.TryGetSymbolUseFromIdent sourceText fixData.Ident with + | None -> return [] + | Some symbolUse -> + + match symbolUse.Symbol with + | :? FSharpField as ff -> return mkFix codeActionParams sourceText fixData ff.FieldType + | :? FSharpMemberOrFunctionOrValue as mfv -> + return mkFix codeActionParams sourceText fixData mfv.ReturnParameter.Type + | _ -> return [] | _ -> return [] } diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests/NegateBooleanExpressionTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests/NegateBooleanExpressionTests.fs index 1b664d2db..365006bcb 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests/NegateBooleanExpressionTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests/NegateBooleanExpressionTests.fs @@ -24,7 +24,7 @@ let b = a$0" Diagnostics.acceptAll selectCodeFix "let a = false -let b = not a" +let b = not (a)" testCaseAsync "negate boolean expression" <| CodeFix.check @@ -50,7 +50,7 @@ let b = $0A.a" module A = let a = false -let b = not A.a" +let b = not (A.a)" testCaseAsync "negate record field" <| CodeFix.check @@ -66,7 +66,7 @@ let b = $0a.Y" type X = { Y: bool } let a = { Y = true } -let b = not a.Y" +let b = not (a.Y)" testCaseAsync "negate class property" <| CodeFix.check