Skip to content

Add option to toggle unused declarations analyzer #4074

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

Merged
merged 2 commits into from
Jan 3, 2018
Merged

Add option to toggle unused declarations analyzer #4074

merged 2 commits into from
Jan 3, 2018

Conversation

cartermp
Copy link
Contributor

@cartermp cartermp commented Dec 7, 2017

Workaround for #3056 and configuration we should probably just have.

Because of how the editor works, I think, you need to either:

  • Type something new to trigger the analyzer
  • Close and re-open the document

to see changes in your settings.

Since locale-based text is now in this repo, there are more files touched than before. All the text is still in English. I'm not sure if I like using a translation tool to make the other languages good, but maybe that can be done...

Anyways, I don't feel too strongly about how I named it in the options, so I welcome feedback on the wording.

demo

@vasily-kirichenko
Copy link
Contributor

The new way to do localization looks like a lot of meaningless work now :(

@cartermp
Copy link
Contributor Author

cartermp commented Dec 8, 2017

It's a bit annoying, but it is better than what we had before where nobody outside of MS could really have a say in what the text was. Localization work is just plain not fun, period :)

@cartermp
Copy link
Contributor Author

cartermp commented Dec 8, 2017

Actually, I think since they're tagged as having a state new, our system will translate them automatically. So really only the ones with grossly inaccurate wording would be modified manually. Is that right, @brettfo?

@vasily-kirichenko
Copy link
Contributor

What about compiler generated unused values warnings (they are handled in the same way as our own analyzer)? I think the problem is still reproduced on them.

@cartermp
Copy link
Contributor Author

cartermp commented Dec 8, 2017

Good catch! We can get the code fixes not to register for compiler generated warnings easily (add an Option.guard call in RegisterCodeFixesAsync), but the tooltip for an unused value will still show if someone using --warnon:1182. But I think that's okay.

VS 2015 - doesn't show type info:
warn-2015

VS 2017 - shows type info:
warn-2017

This will ensure that if someone uses warnon:1182, we won't suggest
fixes if they've turned off the feature.
@vasily-kirichenko
Copy link
Contributor

@cartermp I don't think it's the code fix that hides the signature, it's the diagnostics analyzer and we cannot turn off DocumentDiagnosticsAnalyzer entirely. Have you tested this fix having R# installed and active?

@cartermp
Copy link
Contributor Author

cartermp commented Dec 8, 2017

Correct, with R# installed it will still be an issue if you use --warnon:1182, but if you don't, then this does work around the problem because the analyzer won't run if you turn it off here. And it's still in a better state today than it is in VS2015, where it won't show the type info regardless of an R# installation.

@KevinRansom KevinRansom merged commit 939050e into dotnet:master Jan 3, 2018
@urbanhop
Copy link

urbanhop commented Jan 3, 2018

Disabling compiler warning 1182 sounds promising, but does not do it here.

I edit fsx files in Visual Studio.
Adding #nowarn 1182 at the begin of the file does not remove the "unused" warnings which prevent inspecting the type of the expression. Am I doing something wrong?

All I want is to be able to inspect the types of the expression that I freshly write (they are certainly unused)

@vasily-kirichenko
Copy link
Contributor

@urbanhop prefix them with a single underscore (_). Works in Rider as well (yes, it does not show type info for unused values either :)))))))))))))) /cc @auduchinok

@urbanhop
Copy link

urbanhop commented Jan 3, 2018

@vasily-kirichenko thx for your fast reply and working remedy.
would love to see @catermp 's setting above coming to vs2017. then i could use it.

@cartermp cartermp deleted the unused-decl-settings branch January 3, 2018 17:06
forki pushed a commit to forki/visualfsharp that referenced this pull request Jan 20, 2018
* Add option to toggle unused declarations analyzer (dotnet#4074)

* Add option to toggle unused declarations analyzer

* Better name and handle registering code fixes.

This will ensure that if someone uses warnon:1182, we won't suggest
fixes if they've turned off the feature.

* Revert dotnet#1650 (and dotnet#3366) (dotnet#4173)

* Fix error logging in brace matching code (dotnet#4140)

* Remove error logger pushing code

* Update service.fs

* Fix dotnet#4200: Vsix: fix empty "New file" window for web projects (dotnet#4202)

* LOC CHECKIN | visualfsharp - master | 20180112 (dotnet#4194)

* Fixed FCS netcore tests (dotnet#4180)

* Remove ambiguous resolution error FS0332 (dotnet#4170)

* Add IsInteractive to parsing options for script load closures (dotnet#4169)

* Add IsInteractive to FSharpParsingOptions

* Add test

* Set serializable bit for all serializable types (dotnet#4211)

* Minor fix (dotnet#4195)

on string 58.

*  Symbols API: add Index to active pattern case, Name to pattern group (dotnet#4222)

* Symbols API: add Index to active pattern case, Name to pattern group

* Symbols API: add active pattern case use tests

* don't rebuild (dotnet#4230)

* Optimize in FCS

* Transport tcConfig

* Cleanup

* Replace more ILAsm in Exprs

* More ILAsm replacements

* update resource name

* Added some tests
KevinRansom pushed a commit that referenced this pull request Feb 17, 2018
* Optimize in FCS

* Transport tcConfig

* Cleanup

* Replace more ILAsm in Exprs

* More ILAsm replacements

* update resource name

* Added some tests (#40)

* Add option to toggle unused declarations analyzer (#4074)

* Add option to toggle unused declarations analyzer

* Better name and handle registering code fixes.

This will ensure that if someone uses warnon:1182, we won't suggest
fixes if they've turned off the feature.

* Revert #1650 (and #3366) (#4173)

* Fix error logging in brace matching code (#4140)

* Remove error logger pushing code

* Update service.fs

* Fix #4200: Vsix: fix empty "New file" window for web projects (#4202)

* LOC CHECKIN | visualfsharp - master | 20180112 (#4194)

* Fixed FCS netcore tests (#4180)

* Remove ambiguous resolution error FS0332 (#4170)

* Add IsInteractive to parsing options for script load closures (#4169)

* Add IsInteractive to FSharpParsingOptions

* Add test

* Set serializable bit for all serializable types (#4211)

* Minor fix (#4195)

on string 58.

*  Symbols API: add Index to active pattern case, Name to pattern group (#4222)

* Symbols API: add Index to active pattern case, Name to pattern group

* Symbols API: add active pattern case use tests

* don't rebuild (#4230)

* Optimize in FCS

* Transport tcConfig

* Cleanup

* Replace more ILAsm in Exprs

* More ILAsm replacements

* update resource name

* Added some tests

* test conditions update

* test update

* test condition update

* test update

* review update

* added checked operators

* fixed dual conversions

* review fixes

* more targeted replacements

* adapt to latest

* added more tests

* added more tests

* review fixes

* fixed warnings
nosami pushed a commit to xamarin/visualfsharp that referenced this pull request Jan 26, 2022
* Add option to toggle unused declarations analyzer

* Better name and handle registering code fixes.

This will ensure that if someone uses warnon:1182, we won't suggest
fixes if they've turned off the feature.
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.

4 participants