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

Typed Unused Opens analyzer #3803

Merged
merged 55 commits into from
Oct 30, 2017

Conversation

vasily-kirichenko
Copy link
Contributor

@vasily-kirichenko vasily-kirichenko commented Oct 23, 2017

This is a rewrite of the analyzer. It uses EntityRef list corresponding to each open declaration, gets its content and check if any symbol is used in the open declaration scope. No more long id fiddling and such. It should work precisely, always (no false positives, no missed unused opens).

As a side effect, module usages are now found in open declarations:

image

@KevinRansom
Copy link
Member

@dotnet-bot test this please

@vasily-kirichenko
Copy link
Contributor Author

It needs almost 2 minutes to process TypeChecker.fs.

@vasily-kirichenko
Copy link
Contributor Author

@dotnet-bot test Ubuntu14.04 Release_fcs Build please

@vasily-kirichenko
Copy link
Contributor Author

It's ready. It's slow on large files, but works correctly. As @dsyme implements fsharp/fsharp-compiler-docs#830, it will be very easy to make this analyzer very fast.

We should push Roslyn team to implement priority and parallel execution of analyzers. Any progress on this?

@KevinRansom
Copy link
Member

@dotnet-bot test this please

@KevinRansom
Copy link
Member

I'm not sure whether the _fsc failures are expected to pass?

Certainly pr's today have beem green.

@forki?

@forki
Copy link
Contributor

forki commented Oct 28, 2017

It reports fcs tests errors. Not sure. @vasily-kirichenko could you please rebase and if things are still red we need to take a look at that unit test

@vasily-kirichenko
Copy link
Contributor Author

Fixed.

@forki
Copy link
Contributor

forki commented Oct 28, 2017 via email

@vasily-kirichenko
Copy link
Contributor Author

@forki Yes. It was related to the fact that provided types are not supported on netstandard. Great work :)

@forki
Copy link
Contributor

forki commented Oct 28, 2017 via email

@vasily-kirichenko
Copy link
Contributor Author

@dsyme I cannot see why the FCS repo is needed anymore now.

@KevinRansom
Copy link
Member

@vasily-kirichenko

Thanks for this

Kevin

@dsyme
Copy link
Contributor

dsyme commented Nov 7, 2017

@vasily-kirichenko Re FCS - see fsharp/fsharp-compiler-docs#834 and let's discuss there.

nosami pushed a commit to xamarin/visualfsharp that referenced this pull request Jan 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants