-
Notifications
You must be signed in to change notification settings - Fork 55
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
require/dont suggest removing new
for IAsyncDisposable construction
#589
Comments
I don't think this should be rider feature instead of compiler feature. E.g. things like that should be consistent across all ides. It's not a refactoring feature, it's more of a design space/code style stuff and should be in the compiler. Personally, tho, I always suppress new everywhere as I never found it useful for anything. It just outlines a single case where disposable is created out of 100 possible ways and manages to break some syntax constructs for no reason. I do have a custom plugin that highlights disposables differently (both variables and types) and I find that working much much better for me personally. |
Not suggesting it should be a Rider feature - just that it shouldn't be a misfeature. Right now:
The suggestions and fixes from the plugin are of a consistently high quality, so I highlight all anomalies (inc esoteric ones e.g. #587) I agree that there's a big language level discussion outside of this, and in a prioritization discussion, doing stuff upstream is absolutely the only way here (I want to know a Your plugin sounds like something that would be excellent to have worked into default rendering (and as part of that, supporting IAsyncDisposable and IDisposable equally would be a concern) |
Unfortunately, it's inline with the current language design, i.e. there's a compiler warning about a missing |
The plugin greys out
new
on the construction of anIAsyncDisposable
in https://github.com/bartelink/FSharp.Control.TaskSeq/blob/idisposable/src/FSharp.Control.TaskSeq/TaskSeqInternal.fs#L928I blindly followed that in fsprojects/FSharp.Control.TaskSeq@cfc9624, but
Thankfully @abelbraaksma caught the fact that this was erroneous for me to remove it (i.e.
new
should be mandatory for Disposables inc Async Disposables)In this instance I'll sidestep it by using
IDisposable
in fsprojects/FSharp.Control.TaskSeq#222Ideally, the compiler would trap/complain/warn about the missing
new
, but I'll settle for Rider not suggesting to remove it.The text was updated successfully, but these errors were encountered: