Keep track of EQUALS range in Syntax Tree

[nojaf]

I'd like to keep track of the range of the equals token in the Syntax tree.
This is useful for Fantomas to reconstruct code comments (trivia) after equal signs.

let foo = // comment
    value

The closest element to assign the comment, in this case, is the equal token and it is beneficial to have this information in the Syntax tree.

I have in mind to add it to:

• SynBinding
• SynExpr.LetOrUseBang
• SynExpr.Record
• SynExpr.AnonRecd
• SynPat.Record
• SynEnumCase
• SynExpr.For
• SynTypeDefn
• SynTypeDefnSig
• NestedModule
• SynMemberDefn.AutoProperty
• SynArgPats.NamePatPairs
• ...

[auduchinok]

Are ranges actually needed? Wouldn't positions be enough? (Also thinking about other similar cases like dots positions in other nodes)

[nojaf]

Yes, because you can't assume the equals sign is on the same line:

type TypeWithLongCurriedMethods() =
    member _.LongMethodWithLotsOfCurriedParams
        (aVeryLongParam: AVeryLongTypeThatYouNeedToUse)
        (aSecondVeryLongParam: AVeryLongTypeThatYouNeedToUse)
        (aThirdVeryLongParam: AVeryLongTypeThatYouNeedToUse)
        = // some great comment
        ()

[auduchinok]

I'd like to keep track of the range of the equals token in the Syntax tree.

Are ranges actually needed? Wouldn't positions be enough?

Yes, because you can't assume the equals sign is on the same line:

I'm sorry, I don't quite get it. 🙂
= is always single-line and its whole range can always be computed from start position.

[nojaf]

Nope, my bad, I misunderstood your question. You are right, we could get away with using the start position.

However, this is a bit less convenient overall.
Currently, in the parser, I have things like let eqM = rhs parseState 4.
This returns a range, of which you could only store the position.
At Fantomas side, I would need to convert that position again to a range because the whole thing is based on ranges.
Note that I do not say this is a good argument of any kind of sorts. It just is yet another small hoop I have to do twice on our site in order for things to play out. So that is why I'm more a fan of just storing the range as we have already from the parser.

Is there a helper function to only get the start pos in pars.fsy? And what do you think about this @dsyme?
Ranges would be more consistent, though Eugene has a point it can be deduced from the start pos only.

[dsyme]

@nojaf Use a range, it's more consistent and simpler I think

@nojaf You've made it optional in SynBinding - when is it None?

[auduchinok]

@nojaf You've made it optional in SynBinding - when is it None?

I'd expect it to be None in some of error recovery cases.

[dsyme]

I'd expect it to be None in some of error recovery cases.

OK, thanks

[nojaf]

You've made it optional in SynBinding - when is it None?

In recovery cases and I believe there is one case where a SynBinding eventually is transformed to a DoExpr.

fsharp/src/fsharp/pars.fsy

Lines 2614 to 2626 in 196a277

	| cPrototype
	{ let bindRange = lhs parseState
	BindingSetPreAttrs(bindRange, false, false, $1, bindRange) }
	/* A 'do ...' statement in the non-#light syntax */
	doBinding:
	| DO typedSequentialExprBlock
	{ let mDoKwd = rhs parseState 1
	let mWhole = unionRanges mDoKwd $2.Range
	// any attributes prior to the 'let' are left free, e.g. become top-level attributes
	// associated with the module, 'main' function or assembly depending on their target
	BindingSetPreAttrs(mDoKwd, false, false, (fun attrs vis -> attrs, [mkSynDoBinding (vis, true, $2, mWhole)]), mWhole) }

[auduchinok]

I believe there is one case where a SynBinding eventually is transformed to a DoExpr

Oh, sometimes I wish we started refactoring such places, so some nodes aren't reused like this. 🙂
extern declarations, top-level do expressions, interface member implementations come to mind.

[nojaf]

@dsyme ready for review.

[dsyme]

Just a few minor things - perhaps go over the diff and apply consistently throughout, e.g. I think we almost always use id=id in pattern case selectors

[nojaf]

@dsyme I've addressed all the feedback.

[dsyme]

This is great work!