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

Provide actual name ranges for FSharpSymbolUse #3920

Closed
auduchinok opened this issue Nov 10, 2017 · 10 comments
Closed

Provide actual name ranges for FSharpSymbolUse #3920

auduchinok opened this issue Nov 10, 2017 · 10 comments

Comments

@auduchinok
Copy link
Member

Consider a use of FromString below:

let lexbuf = Lexing.LexBuffer<_>.FromString(str)
//           |----------------------------|
// current range is bigger than actual name usage

We could keep start pos of symbol use as well.

@dsyme
Copy link
Contributor

dsyme commented Nov 10, 2017

@auduchinok I suppose providing just the range FromString would be simpler and provide for overall accuracy. I don't really know why we provided the long names historically. Do you have any use for them?

My guess is that a fair bit of client FCS code is sensitive to this however.

@auduchinok
Copy link
Member Author

auduchinok commented Nov 10, 2017

@dsyme No, there's no uses of the full resolved ranges in our code, we trim them.
Talking about client code I've seen (and as we do), there're usually methods that trim these ranges and look error-prone (e.g. looking for dot separator that may be a part of operator).

@dsyme
Copy link
Contributor

dsyme commented Nov 10, 2017

Talking about client code I've seen, there're usually methods that trim these ranges and look error-prone (e.g. looking for dot separator that may be a part of operator).

Yeah, I've seen that code too.

I suppose we can do a major version upgrade and put out some info to major consumers that the ranges have changed

@vasily-kirichenko
Copy link
Contributor

I confirm that we don’t rely on long range and trimm them. If we need full long ident, we use QuickParse. I think we should add LongIdent into FSharpSymbolUse (however, we cannot remove QP entirely because we need to parse partially typed symbols in complition).

@dsyme
Copy link
Contributor

dsyme commented Nov 10, 2017

@vasily-kirichenko Thanks for checking!

@vasily-kirichenko
Copy link
Contributor

Implemented in #3922. I think there will be a storm of failing tests :)

@vasily-kirichenko
Copy link
Contributor

@auduchinok

image

image

@auduchinok
Copy link
Member Author

@vasily-kirichenko This is great!
Looking at how LexBuffer is highlighted it looks like <_> is still in its range.

@vasily-kirichenko
Copy link
Contributor

Yes.

@dsyme
Copy link
Contributor

dsyme commented Apr 5, 2022

There is a duplicate of this error where "<_>" is still included. I will close this.

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

No branches or pull requests

4 participants