Add new keyword code fix #2077
Add new keyword code fix #2077
Conversation
vasily-kirichenko
commented
Dec 22, 2016
vasily-kirichenko
commented
|
Cool,should it also change let to use? |
| @@ -108,6 +108,7 @@ type internal FSharpAddOpenCodeFixProvider | |||
| assemblyContentProvider: AssemblyContentProvider | |||
| ) = | |||
| inherit CodeFixProvider() | |||
| let fixableDiagnosticIds = ["FS0039"] | |||
There was a problem hiding this comment.
Could you please apply the same pattern to the other fix provider?
There was a problem hiding this comment.
what do you mean? Introduce an extra field for diagnostic codes? why?
There was a problem hiding this comment.
I think both should follow similar pattern.
There was a problem hiding this comment.
It's maybe better to introduce a base class, but, you know...
There was a problem hiding this comment.
I didn't say that ;-)
Just saying if we find a common pattern than this is beneficial.
No. I think it was discussed somewhere on github, in short it's not always feasible to use |
|
Yeah, but if we suggest with reasonable explanation then it can still be useful. And people can ignore suggestions, right? |
| @@ -0,0 +1,33 @@ | |||
| // Copyright (c) Microsoft Corporation. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. | |||
There was a problem hiding this comment.
typo in Filename... should be AddNewKeywordToDisposableConstructorInvocation.fs
There was a problem hiding this comment.
Thanks :)
This require writing an Analyzer which would emit special warning, then a Code Fix should be written, which suggests possible fixes. This PR is so simple because the compiler emits a warning itself. |
|
Another point for making the compiler smart ;-)
Am 22.12.2016 2:29 nachm. schrieb "Vasily Kirichenko" <
notifications@github.com>:
… Yeah, but if we suggest with reasonable explanation then it can still be
useful. And people can ignore suggestions, right?
This require writing an Analyzer which would emit special warning, then a
Code Fix should be written, which suggests possible fixes. This PR is so
simple because the compiler emits a warning itself.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#2077 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AADgNBfbvvEddsL-XrfqHFMSyduyLKjnks5rKntVgaJpZM4LT1mv>
.
|
|
This is fantastic. With respect to This is somewhat related to work that the C#/VB IDE team is doing, where code fixes recognize older patterns and attempt to push people towards adoption of newer language features. |
|
|
||
| override this.RegisterCodeFixesAsync context : Task = | ||
| async { | ||
| let title = "Add \"new\" keyword" |
There was a problem hiding this comment.
Yes. Could you explain how to do it? (maybe it's worth to add a guide somewhere into DEVGUIDE.md?)
There was a problem hiding this comment.
@vasily-kirichenko
I will submit a PR with the fix and then we can have a document that points to commit with the changes ... that seems like the most straightforward approach.
Kevin
|
@cartermp, both use and let are valid and have different semantics so I don't think there is a way to have a single selection. I agree having both proposed (let being first) would make sense. |
|
@smoothdeveloper Right, but this is why I think it's worth having a discussion about how to approach code fixes in general. There are lots of cases where there are multiple valid ways to do something. In some cases it's best to show all options, and in other cases it may be best to show the preferred option. This is something we'll just have to evaluate on a case-by-case basis. |
* "add 'new' keyword" code fix * "add open" code fix returns FS0039 as "fixed" diagnostics (only) * fix a typo in file name
