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

Name resolution: report interfaces, report type arguments #15628

Closed

Conversation

auduchinok
Copy link
Member

@auduchinok auduchinok commented Jul 19, 2023

This is the second attempt of #14773.

The goals are:

  • Report interface symbols when they are used in expressions (e.g. in unfinished code)
  • Report type arguments inferred in these symbol usages

Reporting both symbols and type arguments may be useful for IDE features.

There're some issues, though. First, the name resolution and reporting of generic symbols is done earlier than type arguments are checked and available for reporting. If we report the types again later, when the needed info is ready, then we'll get duplicated symbol uses that will be different in the type arguments and range. If we remove the type arguments range (and that's what FCS clients often do in an ad-hoc ways), then we can use a special API to report the symbol usage and to replace the previously reported symbol at the range. This replacing API was previously only used in type providers analysis and it was implemented badly performance-wise: it went through all the reported items and tried to remove items at the range from the reported symbol and method group lists. Using that approach would be a no-go for type checking of all the type applications in a file. I've changed the implementation of the CallNameResolutionSinkReplacing, so it tries to find the last reported item at the range and replaces it in place. It will add a new item if no previously reported item is found.

What we can improve in the reporting in general?

What can be improved for making changes easier:

  • Use the same approach to testing for symbols as in the parser tests, as that would allow:
    • easier automatic updates of baselines
    • no test project rebuild needed on test data/baseline update
    • more info captured without needing to specify parts of it manually in each test case
    • Less resolve conflicts when multiple people/PRs change Symbols.fs tests

We can extract the subsequent work notes to a separate issue.

@auduchinok auduchinok requested a review from a team as a code owner July 19, 2023 09:55
@auduchinok auduchinok force-pushed the nameResolution-symbols-interface branch from d90ea55 to 7a72482 Compare July 19, 2023 10:06
@auduchinok
Copy link
Member Author

An interesting case for item replacing logic is reporting both ctor and type items in:

GenericClass<int>()

For now, I've added a way to pass a function deciding whether an item should be replaced. I don't like this approach, but it looks sufficient for this PR. I'll think about ways to improve things here when doing other things on the list above.

@auduchinok auduchinok force-pushed the nameResolution-symbols-interface branch 2 times, most recently from 4aa0ab3 to a73b207 Compare July 19, 2023 17:06
@auduchinok auduchinok force-pushed the nameResolution-symbols-interface branch from 7dd79e0 to b1aa3c8 Compare July 21, 2023 11:43
@abonie
Copy link
Member

abonie commented Aug 12, 2024

Doing cleanup in PRs - please reopen if you deem it appropriate

@abonie abonie closed this Aug 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants