dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
18.92k stars 4.02k forks source link

Code fixers should warn users if generated code requires to update LangVersion #26598

Open jcouv opened 6 years ago

jcouv commented 6 years ago

The topic came up in a recent issue.

I think the current behavior is that the fixer will just generate code, then once applied you will get offered to update your project... I wonder if we should have some kind of conflict on the fixer, to warn you that it will require a new LangVersion.

My take is that fixers/refactorings that produce such new constructs should make it visible (maybe like conflicts), so that the users aren't surprised that we immediately ask to upgrade their project's LangVersion. And in some cases, we may want to offer distinct fixes/refactorings (one with conservative code, and the other one with latest features).

FYI @CyrusNajmabadi @sharwell

Pilchie commented 6 years ago

Generally code fixes should be LangVersion aware and either not be offered if they require the new langver, or generate fallback code that works for the current langver.

CyrusNajmabadi commented 6 years ago

I think the idea might be: offer the fix, but in a clear way that indicates you'll need a new language version. that will help guide people to new features and why they might want to upgrade.

Note: fix-all woudl need to not do this as a user might understand this needs a new language version, but only want to fix all the cases in the projects that are on hte new version.

Pilchie commented 6 years ago

I'm not convinced that we want to be doing that - anyone not on the latest major version probably has a reason why they aren't, and I'm not sure we should be pushing people that hard past the latest major onto point releases.

CyrusNajmabadi commented 6 years ago

That's fair. It woudl be nice to have some way to evangelize new language features. But i agree that fixers are not necessarily teh right place for that.

Perhaps there could be an option though for people to opt into that tells them about this sort of thing.

sharwell commented 6 years ago

Generally code fixes should be LangVersion aware and either not be offered if they require the new langver, or generate fallback code that works for the current langver.

⬆️ This is the current design. Any deviation from this in the current code is unintentional (bug).

jcouv commented 6 years ago

I don't think we should close this. Tagging @kuhlenh since she received feedback that it's useful for the IDE to suggest new language constructs. That's a good way for users to discover language features.

My take is that fixers/refactorings that produce such new constructs should make it visible, so that the users aren't surprised that we immediately ask to upgrade their project's LangVersion. And in some cases, we may want to offer distinct fixes/refactorings (one with conservative code, and the other one with latest features).

CyrusNajmabadi commented 6 years ago

I agree that this is a request for a change in the design, and should at least be discussed.

IMO, i think it would be worthwhile for hte IDE to surface in some way knowledge about new language features and how they can improve your code. That said, i think it should be very obvious to users that this is different from normal suggestions and i think users should be able to say they don't want to hear about this.

This will help encourage users who can move forward to do so if they wish, while not being a burden on users who have strong reason to stay on their current version.

sharwell commented 6 years ago

@jcouv The original post above as well as the linked issue relate to the behavior of one of the existing code fixes measured against the current design. If you are interested in reopening the design discussion, please open a new issue with the specific proposal for discussion (as a proposal to change design rather than a question of what the current design is). That will help keep the two discussions focused to reduce confusion.

jcouv commented 6 years ago

@sharwell That is precisely what I opened this issue for. I'll update the description above to clarify proposal.

sharwell commented 6 years ago

For me, the following are requirements for an acceptable final design:

Currently I have not seen an alternative proposal to the current design which meets this minimum acceptance criteria, but I would be interested to read one if identified.

kuhlenh commented 6 years ago

It seems like the underlying problem being discussed here is: Users should have some way to know that new point-release language features have been added

The proposed solution above is to use the lightbulb menu to indicate places where new language features can be used, regardless of the specificied langversion. Couple things:

Possible alternate solutions to helping educate users on the latest point-release features:

CyrusNajmabadi commented 6 years ago

Use the promotion system in Visual Studio

What's "the promotion system"?

jcouv commented 6 years ago

I think the "promotion system" is the gold bar and the newsfeed (start page).

CyrusNajmabadi commented 6 years ago

Newsfeed seems ok. Goldbar seems like abuse :)