Skip to content

Commit

Permalink
Prevent stroustrup formatting when trivia is present before (#2527)
Browse files Browse the repository at this point in the history
* Prevent stroustrup formatting when trivia is present before

* Update CHANGELOG.md
  • Loading branch information
josh-degraw authored Sep 22, 2022
1 parent dc8b277 commit 86f6453
Show file tree
Hide file tree
Showing 6 changed files with 133 additions and 19 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
# Changelog

## Unreleased
## [5.0.2] - 2022-09-22

### Fixed
* Wrong generic constraint placement in ML-style generic definitions with multiple type parameters. [#1868](https://github.com/fsprojects/fantomas/issues/1868)
* Incorrect async indentation when inside of a match. [#2501](https://github.com/fsprojects/fantomas/issues/2501)
* Conditional directives before expression leads to invalid code with Stroustrup. [#2517](https://github.com/fsprojects/fantomas/issues/2517)

### Documented
* Building Fantomas in Visual Studio for dev errors. [#2447](https://github.com/fsprojects/fantomas/issues/2447)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -319,3 +319,64 @@ type Foo() =
itemFive
|]
"""

[<Test>]
let ``doesn't trigger stroustrup styling when an expression has trivia before it`` () =
formatSourceString
false
"""
let inline skipNoFail count (source: seq<_>) =
//if FABLE_COMPILER
seq {
yield "Hello"
}
"""
config
|> prepend newline
|> should
equal
"""
let inline skipNoFail count (source: seq<_>) =
//if FABLE_COMPILER
seq { yield "Hello" }
"""

[<Test>]
let ``conditional directives before expression, 2517`` () =
formatSourceString
false
"""
let inline skipNoFail count (source: seq<_>) =
#if FABLE_COMPILER
seq {
let mutable i = 0
let e = source.GetEnumerator ()
while e.MoveNext () do
if i < count
then
i <- i + 1
else
yield e.Current
}
#else
Enumerable.Skip(source, count)
#endif
"""
config
|> prepend newline
|> should
equal
"""
let inline skipNoFail count (source: seq<_>) =
#if FABLE_COMPILER
seq {
let mutable i = 0
let e = source.GetEnumerator()
while e.MoveNext() do
if i < count then i <- i + 1 else yield e.Current
}
#else
Enumerable.Skip(source, count)
#endif
"""
Original file line number Diff line number Diff line change
Expand Up @@ -445,3 +445,42 @@ match x with
yield! visitSynExpr inheritArgs
]
"""

[<Test>]
let ``async indentation when inside of a match, 2501`` () =
formatSourceString
false
"""
let x =
match packageRegistrationUrl with
| Some packageRegistrationUrl ->
async {
let response = ""
return ""
}
| None -> failwith "Package registration url not found"
"""
config
|> prepend newline
|> should
equal
"""
let x =
match packageRegistrationUrl with
| Some packageRegistrationUrl ->
async {
let response = ""
return ""
}
| None -> failwith "Package registration url not found"
"""
31 changes: 29 additions & 2 deletions src/Fantomas.Core/Context.fs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ open Fantomas.Core
open Fantomas.Core.ISourceTextExtensions
open Fantomas.Core.FormatConfig
open Fantomas.Core.TriviaTypes
open Fantomas.Core.SourceTransformer

type WriterEvent =
| Write of string
Expand Down Expand Up @@ -1176,9 +1177,32 @@ let addExtraNewlineIfLeadingWasMultiline leading sepNlnConsideringTriviaContentB
leadingExpressionIsMultiline leading (fun ml ->
sepNln +> onlyIf ml sepNlnConsideringTriviaContentBefore +> continuation)

let addParenIfAutoNln synExpr f =
let expr = f synExpr
expressionFitsOnRestOfLine expr (ifElse (hasParenthesis synExpr) (sepOpenT +> expr +> sepCloseT) expr)

let addParenForTupleWhen f synExpr ctx =
let condition e =
match e with
| SourceParser.ElIf _
| SynExpr.Lambda _ -> true
| _ -> false // "if .. then .. else" have precedence over ","

let expr = f synExpr
ifElse (condition synExpr) (sepOpenT +> expr +> sepCloseT) expr ctx

let private hasTriviaBeforeExpression ctx e =
let astType, range = synExprToFsAstType e

Map.tryFindOrEmptyList astType ctx.TriviaBefore
|> List.exists (fun ti -> RangeHelpers.rangeEq range ti.Range)

let autoIndentAndNlnExpressUnlessStroustrup (f: SynExpr -> Context -> Context) (e: SynExpr) (ctx: Context) =
match e with
| SourceParser.StroustrupStyleExpr ctx.Config.ExperimentalStroustrupStyle e -> f e ctx
| SourceParser.StroustrupStyleExpr ctx.Config.ExperimentalStroustrupStyle e when
(not (hasTriviaBeforeExpression ctx e))
->
f e ctx
| _ -> indentSepNlnUnindent (f e) ctx

let autoIndentAndNlnIfExpressionExceedsPageWidthUnlessStroustrup
Expand All @@ -1187,7 +1211,10 @@ let autoIndentAndNlnIfExpressionExceedsPageWidthUnlessStroustrup
(ctx: Context)
=
match e with
| SourceParser.StroustrupStyleExpr ctx.Config.ExperimentalStroustrupStyle e -> f e ctx
| SourceParser.StroustrupStyleExpr ctx.Config.ExperimentalStroustrupStyle e when
(not (hasTriviaBeforeExpression ctx e))
->
f e ctx
| _ -> autoIndentAndNlnIfExpressionExceedsPageWidth (f e) ctx

type internal ColMultilineItem =
Expand Down
2 changes: 1 addition & 1 deletion src/Fantomas.Core/Fantomas.Core.fsproj
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@
<Compile Include="Defines.fs" />
<Compile Include="Trivia.fsi" />
<Compile Include="Trivia.fs" />
<Compile Include="Context.fs" />
<Compile Include="SourceTransformer.fs" />
<Compile Include="Context.fs" />
<Compile Include="CodePrinter.fsi" />
<Compile Include="CodePrinter.fs" />
<Compile Include="CodeFormatterImpl.fsi" />
Expand Down
15 changes: 0 additions & 15 deletions src/Fantomas.Core/SourceTransformer.fs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ module internal Fantomas.Core.SourceTransformer

open FSharp.Compiler.Syntax
open FSharp.Compiler.Text
open Fantomas.Core.Context
open Fantomas.Core.SourceParser
open Fantomas.Core.TriviaTypes

Expand Down Expand Up @@ -125,20 +124,6 @@ let (|LongGetMember|_|) =
Some(SynBinding(ao, kind, isInline, isMutable, ats, px, valData, pat, ri, e, bindingRange, dp, trivia))
| _ -> None

let addParenIfAutoNln synExpr f =
let expr = f synExpr
expressionFitsOnRestOfLine expr (ifElse (hasParenthesis synExpr) (sepOpenT +> expr +> sepCloseT) expr)

let addParenForTupleWhen f synExpr ctx =
let condition e =
match e with
| ElIf _
| SynExpr.Lambda _ -> true
| _ -> false // "if .. then .. else" have precedence over ","

let expr = f synExpr
ifElse (condition synExpr) (sepOpenT +> expr +> sepCloseT) expr ctx

let synModuleDeclToFsAstType =
function
| SynModuleDecl.Expr _ -> SynModuleDecl_Expr
Expand Down

0 comments on commit 86f6453

Please sign in to comment.