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

Update Fantomas to FCS 33 #568

Merged
merged 5 commits into from
Nov 24, 2019
Merged

Update Fantomas to FCS 33 #568

merged 5 commits into from
Nov 24, 2019

Conversation

baronfel
Copy link
Contributor

This update was a bit tricky, because the member ranges returned from FCS changed a bit due to work that @auduchinok did in the parser. As a result, I had to update the logic around member flag generation to

A) only apply to ranges that are members, and
B) find members that are 'within' the range of this trivia.

I tried to update the trivia range to match the range of the member, but doing that was beyond me. An example:

Sample Code:

[<AbstractClass>]
type Shape2D(x0 : float, y0 : float) =
    let mutable x, y = x0, y0
    let mutable rotAngle = 0.0

    member this.CenterX with get() = x and set xval = x <- xval
    member this.CenterY with get() = y and set yval = y <- yval

    abstract Area : float with get
    abstract Perimeter : float  with get
    abstract Name : string with get

    member this.Move dx dy =
       x <- x + dx
       y <- y + dy

    abstract member Rotate: float -> unit
    default this.Rotate(angle) = rotAngle <- rotAngle + angle

Given this code, the range for:

  • SynMemberDefn.Member for the Rotate override is 19,4-19,61,
  • MemberFlags representing the member is 19,12-9,61, and
  • Trivia for the SynMemberDefn.Member is 19,4-19,61

So the old simple check for trivia.range = rangeOfBindingAndRhs would naturally never hold anymore. I'm not sure my fix is the best fix.

@auduchinok
Copy link
Contributor

We're likely to fix some of the other ranges in compiler AST as well, so the tooling can reason about the tree nodes better. Sorry for that. :)

@nojaf
Copy link
Contributor

nojaf commented Nov 22, 2019

Fixes in ranges can only be welcomed!
@baronfel at first glance the fix seems ok for now.
I'll update AST viewer first and check more in depth.

See https://gitlab.com/jindraivanek/ast-viewer/merge_requests/13

@nojaf nojaf merged commit 26fd9ef into fsprojects:master Nov 24, 2019
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

Successfully merging this pull request may close these issues.

None yet

3 participants