Correct ranges for nested modules in signature files. #12585
Conversation
| { let needsCheck, (kind, decls) = $1 | ||
| let objectModelRange = | ||
| match decls with | ||
| | [] -> lhs parseState |
There was a problem hiding this comment.
@nojaf Would it be hard to extract successfully ranges here too? Sorry, I should've added comment here too. 🙂
There was a problem hiding this comment.
Would that be replacing it with rhs parseState 1?
There was a problem hiding this comment.
From a glance, it seems it may require passing parsed ranges via tyconClassSpfn and other rules. Otherwise recovered ranges may still span further than they should. Not sure if you'd want to do that as a part of this PR. 🙂
It'd be great to do this systematically, the same as we've been fixing ranges of the things.
There was a problem hiding this comment.
Hmm, I see what you mean. I'm not going to push my luck in this current PR 😅.
I'll let it be while the PR is still green.
There was a problem hiding this comment.
@nojaf , I will merge this as is, can you prepare a PR with the discussed modification and additional tests to cover that additional scenario please?
Thanks
Kevin
There was a problem hiding this comment.
Hey Kevin, thanks for merging this.
I can circle back to this in the future, @auduchinok any specific examples you can think of to cover this?
There was a problem hiding this comment.
@nojaf Nope, I couldn't easily find an example. I'm going to look at members parser recovery later, and maybe I'll find an example. What happened in #12224 was it started returning intersecting ranges only after a better recovery was added to match clauses and patterns parsing, so it could probably happen here too.
4 participants
Fixes #2094.
The main problem was that the
lhs parseStateis unreliable if members are involved.I tried to take a correct starting position and calculate the range by including all the member ranges.
While I was at it I noticed that there is a discrepancy between types in signature files and implementation files.
In signature files SynTypeDefnSig did include the
typekeyword in its range. While the counterpart SynTypeDefn does not.The type keyword should (I think) only be included in SynModuleDecl.Types and SynModuleSigDecl.Types
//cc @dsyme