Skip to content

Commit

Permalink
Use shouldBeParenthesizedInContext for both parentheses cases.
Browse files Browse the repository at this point in the history
  • Loading branch information
@nojaf
nojaf committed Apr 3, 2024
1 parent fae1095 commit 90f17fa
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 162 deletions.
3 changes: 3 additions & 0 deletions .editorconfig
View file
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
211 changes: 52 additions & 159 deletions src/FsAutoComplete/CodeFixes/NegateBooleanExpression.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) ->
Expand Down Expand Up @@ -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 []
}
6 changes: 3 additions & 3 deletions test/FsAutoComplete.Tests.Lsp/CodeFixTests/NegateBooleanExpressionTests.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down

0 comments on commit 90f17fa

Please sign in to comment.