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

Default member implementation changed to member during formatting #742

Closed
Szer opened this issue Apr 7, 2020 · 4 comments
Closed

Default member implementation changed to member during formatting #742

Szer opened this issue Apr 7, 2020 · 4 comments

Comments

@Szer
Copy link
Contributor

Szer commented Apr 7, 2020

Fantomas: 3.3.0
FSharp.Compiler.Service: 34.1.0
OS: macOS Catalina 10.15.3

Description

Roundtrip parsing and formatting abstract class code gives code which doesn't compile.
Default member implementation of abstract property is being ignored and changed to usual member which causes compile error in FSC

Repro

#r "/Users/auh002i/.nuget/packages/fsharp.compiler.service/34.1.0/lib/netstandard2.0/FSharp.Compiler.Service.dll"
#r "/Users/auh002i/.nuget/packages/fantomas/3.3.0/lib/netstandard2.0/Fantomas.dll"

open Fantomas
open FSharp.Compiler.SourceCodeServices

let sourceCode = """module A

[<AbstractClass>]
type Foo =
    abstract foo: int
    default __.foo = 1
"""

let checker = FSharpChecker.Create()
let fileName = "abc.fs"

let parsed =
    Fantomas.CodeFormatter.ParseAsync(
         fileName = fileName,
         source = SourceOrigin.SourceString sourceCode,
         parsingOptions = Fantomas.FakeHelpers.createParsingOptionsFromFile fileName,
         checker = checker
    ) |> Async.RunSynchronously
    
let formatted =
    Fantomas.CodeFormatter.FormatASTAsync(
        ast = fst parsed.[0],
        fileName = fileName,
        defines = [],
        source = None,
        config = Fantomas.FormatConfig.FormatConfig.Default
    ) |> Async.RunSynchronously

formatted = sourceCode

Expected

formatted should contain default method foo

Actual

formatted doesn't have default implementation of foo and code doesn't compile

@Szer
Copy link
Contributor Author

Szer commented Apr 7, 2020

Did a little investigation.

If Fantomas doesn't get origin source code on formatting (which is usual on building sources from pure AST), it doesn't get Trivia from it and can't find what particular word (default or override) were used before member with MemberFlag.IsOverrideOrExplicitImpl = true and emits member keyword.

ctx.Trivia
|> List.tryFind(fun { Type = t; Range = r } -> t = MainNode "SynMemberDefn.Member" && RangeHelpers.``range contains`` r rangeOfBindingAndRhs)
|> Option.bind(fun tn ->
tn.ContentBefore
|> List.choose (fun tc ->
match tc with
| Keyword({ Content = kw }) when (kw = "override" || kw = "default") -> Some (!- (sprintf "%s " kw))
| _ -> None)
|> List.tryHead
)
|> Option.defaultValue (!- "member ")

Emitting override keyword as a fallback could solve the problem, because both default and override are the same dispatch slot for compiler (according to spec at least)
image

This change breaks 10 tests with interface being implemented, but everything compiles well:

type IFoo = 
    abstract foo: int

let foo = 
    { new IFoo with
        override __.foo = 1 }

@auduchinok
Copy link
Contributor

It'd be really great to preserve the keyword in AST somehow.

@Szer
Copy link
Contributor Author

Szer commented Apr 7, 2020

@auduchinok currently Fantomas preserves the keyword in Context.Trivia when OriginSourceCode passed to formatter (this is optional). In that case it could restore original keyword without issues.

Problem arise when you are trying to create sources from pure AST which lacks info about what keyword has been used. The only sign that this member is abstract dispatch slot is MemberFlag.IsOverrideOrExplicitImpl = true

Emitting override on every method or property with such flag should produce correct code if we are lacking trivia.

@Szer
Copy link
Contributor Author

Szer commented Apr 8, 2020

fixed in #743

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants