fix(graph): Support enclosing range for constructors

[mmanela]

Fixed GRAPH-909

When testing enclosing range, I noticed in the snapshot test the constructor was missing the enclosing data. In order to add the change I had to handle some of the forking constructor logic that is in a few places. Getting the range of a constructor node is special-cased so to get the enclosing range, I made a new enclosing range method explicitly excludes the special casing logic.

(I also checked in the vscode launch config that makes it easier to debug this, happy to revert that if needed)

Test plan

• Validate snapshot
• Run manually on local index and validate

[olafurpg]

Any reason to keep this method when it's equivalent to getRangeFromNode?

[mmanela]

Thanks for the comment, it was confusing and I refactored to simplify this.

[varungandhi-src]

What is a "name node" here? Nothing else in this diff mentions names. More generally, if we have 2-3 similarly named functions, we should have clear doc comments explain the differences between them.

Looking at the code below, it seems weird that we are falling back to node when getFirstToken() is falsey. Is it even possible to hit that case?

[mmanela]

1. Thanks for the feedback, I moved this logic and updated the comment. There is an asymmetry between how function declarations and constructor declarations are handled here. For function declarations the identifierNode is passed into visitSymbolOccurrence while for constructor declarations it's the declaration node itself. This logic (which is now moved) is meant to handle that difference. This logic predates my change.

2. As for the second point on if firstToken can ever be null. I don't think so in this case, however since the type system requires it to be handled (since its nullable) we can either keep the coalescing or throw beforehand. This coalescing behavior was not added as part of my change here but happy to make this cleaner as part of this.