Skip to content

Commit

Permalink
Don't use module or namespace as trivia candidate. Fixes fsprojects#1649
Browse files Browse the repository at this point in the history
.
  • Loading branch information
nojaf committed Apr 17, 2021
1 parent 26674f9 commit a8c1d02
Show file tree
Hide file tree
Showing 15 changed files with 349 additions and 287 deletions.
4 changes: 3 additions & 1 deletion src/Fantomas.Tests/AttributeTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -266,9 +266,11 @@ let ``comments before attributes should be added correctly, issue 422`` () =
Verified : bool }
"""
config
|> prepend newline
|> should
equal
"""module RecordTypes =
"""
module RecordTypes =
/// Records can also be represented as structs via the 'Struct' attribute.
/// This is helpful in situations where the performance of structs outweighs
Expand Down
79 changes: 79 additions & 0 deletions src/Fantomas.Tests/CommentTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1252,3 +1252,82 @@ let a = 8
let a = 8
// foobar
"""

[<Test>]
let ``file end with newline followed by comment, 1649`` () =
formatSourceString
false
"""
#load "Hi.fsx"
open Something

//// FOO
[
1
2
3
]

//// The end
"""
{ config with
SpaceBeforeUppercaseInvocation = true
SpaceBeforeClassConstructor = true
SpaceBeforeMember = true
SpaceBeforeColon = true
SpaceBeforeSemicolon = true
MultilineBlockBracketsOnSameColumn = true
NewlineBetweenTypeDefinitionAndMembers = true
KeepIfThenInSameLine = true
AlignFunctionSignatureToIndentation = true
AlternativeLongMemberDefinitions = true
MultiLineLambdaClosingNewline = true
KeepIndentInBranch = true }
|> prepend newline
|> should
equal
"""
#load "Hi.fsx"
open Something

//// FOO
[ 1 ; 2 ; 3 ]

//// The end
"""

[<Test>]
let ``meh 123`` () =
formatSourceString
false
"""
let a = 7
let b = 8
// foo
"""
config
|> prepend newline
|> should
equal
"""
let a = 7

let b = 8
// foo
"""

[<Test>]
let ``block comment above let binding`` () =
formatSourceString
false
"""(* meh *)
let a = b
"""
config
|> prepend newline
|> should
equal
"""
(* meh *)
let a = b
"""
55 changes: 52 additions & 3 deletions src/Fantomas.Tests/CompilerDirectivesTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,14 @@ SetupTesting.generateSetupScript __SOURCE_DIRECTORY__
#endif
"""
config
|> prepend newline
|> should
equal
"""#if INTERACTIVE
"""
#if INTERACTIVE
#load "../FSharpx.TypeProviders/SetupTesting.fsx"
SetupTesting.generateSetupScript __SOURCE_DIRECTORY__
#load "__setup__.fsx"
#endif
"""
Expand All @@ -38,12 +41,42 @@ SetupTesting.generateSetupScript __SOURCE_DIRECTORY__
#endif
"""
config
|> prepend newline
|> should
equal
"""#if INTERACTIVE
"""
#if INTERACTIVE
#else
#load "../FSharpx.TypeProviders/SetupTesting.fsx"
SetupTesting.generateSetupScript __SOURCE_DIRECTORY__
#load "__setup__.fsx"
#endif
"""

[<Test>]
let ``should keep compiler directives, idempotent`` () =
formatSourceString
false
"""
#if INTERACTIVE
#else
#load "../FSharpx.TypeProviders/SetupTesting.fsx"
SetupTesting.generateSetupScript __SOURCE_DIRECTORY__
#load "__setup__.fsx"
#endif
"""
config
|> prepend newline
|> should
equal
"""
#if INTERACTIVE
#else
#load "../FSharpx.TypeProviders/SetupTesting.fsx"
SetupTesting.generateSetupScript __SOURCE_DIRECTORY__
#load "__setup__.fsx"
#endif
"""
Expand Down Expand Up @@ -211,7 +244,6 @@ namespace Internal.Utilities.Text.Lexing"""
"""
#nowarn "47"
namespace Internal.Utilities.Text.Lexing
"""

[<Test>]
Expand Down Expand Up @@ -1030,6 +1062,23 @@ let foo = 42
#endif
"""

[<Test>]
let ``no code for inactive define, no defines`` () =
formatSourceStringWithDefines
[]
"""#if SOMETHING
let foo = 42
#endif"""
config
|> prepend newline
|> should
equal
"""
#if SOMETHING
#endif
"""

[<Test>]
let ``#if should not be printed twice, #482`` () =
formatSourceString
Expand Down
15 changes: 15 additions & 0 deletions src/Fantomas.Tests/SynConstTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -504,3 +504,18 @@ long
triple quotes string thing
\"\"\"
"

[<Test>]
let ``collect keyword string as separate trivia`` () =
formatSourceString
false
"""
__SOURCE_DIRECTORY__
"""
config
|> prepend newline
|> should
equal
"""
__SOURCE_DIRECTORY__
"""
24 changes: 12 additions & 12 deletions src/Fantomas.Tests/TriviaTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,9 @@ let ``line comment on same line, is after last AST item`` () =
let triviaNodes = toTrivia source |> List.head

match triviaNodes with
| [ { Type = MainNode SynModuleOrNamespace_AnonModule
| [ { Type = MainNode SynConst_Int32
ContentAfter = [ Comment (LineCommentAfterSourceCode lineComment) ] } ] -> lineComment == "// should be 8"
| _ -> fail ()
| _ -> Assert.Fail(sprintf "Unexpected trivia %A" triviaNodes)

[<Test>]
let ``newline pick up before let binding`` () =
Expand Down Expand Up @@ -316,16 +316,16 @@ doSomething()
let withoutDefine = Map.find [] triviaNodes

match withoutDefine with
| [ { Type = MainNode SynModuleOrNamespace_AnonModule
| [ { Type = MainNode SynModuleDecl_DoExpr
ContentBefore = [ Directive "#if NOT_DEFINED\n#else" ]
ContentAfter = [ Directive "#endif" ] } ] -> pass ()
| _ -> fail ()
| _ -> Assert.Fail(sprintf "Unexpected trivia %A" withoutDefine)

match withDefine with
| [ { Type = MainNode SynModuleOrNamespace_AnonModule
| [ { Type = MainNode LongIdent_
ContentBefore = [ Directive "#if NOT_DEFINED"; Directive "#else"; Directive "#endif" ]
ContentAfter = [] } ] -> pass ()
| _ -> fail ()
| _ -> Assert.Fail(sprintf "Unexpected trivia %A" withDefine)

[<Test>]
let ``directive without else clause`` () =
Expand All @@ -341,19 +341,19 @@ let x = 1
let withoutDefine = Map.find [] triviaNodes

match withoutDefine with
| [ { Type = MainNode SynModuleOrNamespace_AnonModule
| [ { Type = MainNode LongIdent_
ContentAfter = [ Directive "#if NOT_DEFINED\n\n#endif" ]
ContentBefore = [] } ] -> pass ()
| _ -> fail ()
| _ -> Assert.Fail(sprintf "Unexpected trivia %A" withoutDefine)

match withDefine with
| [ { Type = MainNode SynModuleOrNamespace_AnonModule
| [ { Type = MainNode LongIdent_
ContentBefore = [ Directive "#if NOT_DEFINED" ]
ContentAfter = [] };
{ Type = MainNode SynModuleDecl_Let
ContentBefore = []
ContentAfter = [ Directive "#endif" ] } ] -> pass ()
| _ -> fail ()
| _ -> Assert.Fail(sprintf "Unexpected trivia %A" withDefine)

[<Test>]
let ``unreachable directive should be present in trivia`` () =
Expand All @@ -370,7 +370,7 @@ type ExtensibleDumper = A | B
let trivias = Map.find [ "DEBUG" ] triviaNodes

match trivias with
| [ { Type = MainNode Ident_
| [ { Type = MainNode LongIdent_
ContentAfter = [ Directive "#if EXTENSIBLE_DUMPER\n#if DEBUG\n\n#endif\n#endif" ] } ] -> pass ()
| _ -> Assert.Fail(sprintf "Unexpected trivia %A" trivias)

Expand Down Expand Up @@ -400,7 +400,7 @@ let foo = 42
toTriviaWithDefines source |> Map.find []

match trivia with
| [ { Type = MainNode SynModuleOrNamespace_AnonModule
| [ { Type = MainNode LongIdent_
ContentAfter = [ Directive "#if SOMETHING\n\n#endif" ] } ] -> pass ()
| _ -> fail ()

Expand Down
5 changes: 5 additions & 0 deletions src/Fantomas/AstExtensions.fs
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,8 @@ type SynTypeDefnSig with
member this.FullRange : Range =
match this with
| SynTypeDefnSig.TypeDefnSig (comp, _, _, r) -> mkRange r.FileName comp.Range.Start r.End

let longIdentFullRange (li: LongIdent) : Range =
match li with
| [] -> range.Zero
| h :: _ -> unionRanges h.idRange (List.last li).idRange
Loading

0 comments on commit a8c1d02

Please sign in to comment.