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

[BUG] Quick Fix Suggestion does not match to the specified framework/langVersion #419

Closed
Tracked by #492 ...
sharpchen opened this issue Aug 22, 2023 · 15 comments
Closed
Tracked by #492 ...
Assignees
Labels
area-roslyn bug Something isn't working triaged The issue has been triaged
Milestone

Comments

@sharpchen
Copy link

Describe the Issue

I am using .NET 7 and the .csproj is defined as:

<PropertyGroup>
        <TargetFramework>net7.0</TargetFramework>
        <Nullable>enable</Nullable>
        <LangVersion>latest</LangVersion>
        <AvaloniaUseCompiledBindingsByDefault>true</AvaloniaUseCompiledBindingsByDefault>
</PropertyGroup>

BUT the Quick Fix shows the currently preview feature suggestion of Collection Expression in .NET 8 or C# 12 shall I say? How come? This will lead to build error.
image

Steps To Reproduce

I don't know how to reproduce, it seems not all array initializations can raise this suggestion.

Expected Behavior

No response

Environment Information

  • OS: Windows10 22H2

  • VSCode 1.81.1

Name: C#
Id: ms-dotnettools.csharp
Description: Base language support for C#
Version: 2.0.399
Publisher: Microsoft
VS Marketplace Link: https://marketplace.visualstudio.com/items?itemName=ms-dotnettools.csharp

Name: C# Dev Kit
Id: ms-dotnettools.csdevkit
Description: Official C# extension from Microsoft
Version: 0.3.21
Publisher: Microsoft
VS Marketplace Link: https://marketplace.visualstudio.com/items?itemName=ms-dotnettools.csdevkit

@sharpchen sharpchen added the bug Something isn't working label Aug 22, 2023
@webreidi webreidi added this to the GA milestone Aug 22, 2023
@sharpchen sharpchen changed the title [BUG] Quick Fix Suggestion does not match to the specified [BUG] Quick Fix Suggestion does not match to the specified framework/langVersion Aug 23, 2023
@arunchndr
Copy link
Member

Tagging @lifengl and @Michael-Eng on the .NET preview dependency.

@Michael-Eng
Copy link
Member

@lifengl is this us or Roslyn?

@lifengl
Copy link
Member

lifengl commented Aug 28, 2023

It is unclear to me whether it is a bug in C# Dev Kit side, or Roslyn side, or it can be by design, based on the description here. + @jasonmalinowski , who has more context here.

The problem here is that it is a misunderstanding the version of the C# language is related to the version of the framework this project targets. The project clearly declared that it wanted the latest C# syntax. I am quite sure a NET 8 SDK preview has been installed on the machine (which might be brought to the machine by VS updates). In the earlier report, it doesn't mention this project is using any global.json to pin down to a specific version of SDK, so I guess it might not, which means the version of the language is depending on the specific machine environment. It may get new syntax when there is a newer version of SDK, or an older version on other machines/environment.

If the suggestion and build error appears in the same session inside VS code, it is more likely a bug. If the build failure happens in a command line, or in VS, it can be questionable. The problem is SDK selection is based on the folder where 'build' command is executed, but not where the project is located. Also, VS will not pick up a preview version of SDK by default, which is different from the behavior in vs code, which can lead more confusions. But if they happen in the same session inside vs code, it is more likely a bug in Roslyn.

@UPIMMUNITY : maybe you can help to provide more information of your environment to help us narrow down what might be the source of this problem.

@sharpchen
Copy link
Author

  1. These are the result of dotnet --list-sdks and dotnet --list-runtimes using command line:

6.0.200 [C:\Program Files\dotnet\sdk]
7.0.100 [C:\Program Files\dotnet\sdk]
7.0.302 [C:\Program Files\dotnet\sdk]
7.0.304 [C:\Program Files\dotnet\sdk]

Microsoft.AspNetCore.App 6.0.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 6.0.16 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 7.0.5 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 7.0.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.NETCore.App 3.1.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 6.0.2 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 6.0.16 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 7.0.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 7.0.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.WindowsDesktop.App 3.1.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 6.0.2 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 6.0.16 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 7.0.5 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 7.0.7 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

If I checked it wrong, please let me know.

  1. I've tried building and run the application by right-clicking the solution explorer instead of using terminal with collection expression, it still failed.

@sharpchen
Copy link
Author

I think there is a preview version in my machine, I can use primary constructor in my another project when LangVersion is preview, but preview still failed for collection expression.

@jasonmalinowski
Copy link
Member

So I see a version check here that should be checking for minimum versions:

https://github.com/CyrusNajmabadi/roslyn/blob/3463acb0426f9131c381b08eec68a347ee9600a1/src/Analyzers/CSharp/Analyzers/UseCollectionExpression/AbstractCSharpUseCollectionExpressionDiagnosticAnalyzer.cs#L55-L56

@UPIMMUNITY When you tried it, were you getting an error that the feature was unsupported (because you weren't on a preview version) or that the syntax was generally invalid (which would indicate your compiler was too old)?

There's also some extra complexity where for some fixes we also get information from the SDK for the given TargetFramework, since some TFMs also have a max version; for example net472 doesn't officially support newer versions since they require extra types in the framework. It's entirely possible we aren't getting that information due to a project system bug (or we just forgot), but I don't see that being checked here and I wouldn't expect it in this case.

Tagging @akhera99 if she knows we just had a bug here.

@sharpchen
Copy link
Author

There's no inline error shown in vscode editor when I use collection expression, error happens when building. It throws the Compiler Error CS1525.

@lifengl lifengl removed their assignment Aug 31, 2023
@arunchndr arunchndr added the triaged The issue has been triaged label Sep 12, 2023
@jasonmalinowski jasonmalinowski modified the milestones: GA, Post GA Sep 20, 2023
@jasonmalinowski
Copy link
Member

So I think this is similar to the report we got for VS for Windows and @jaredpar can probably confirm that. Since you're saying language version = latest, that means you're going to use the highest language version of whatever SDK is being used. So you have to be careful here: if you have rollforward set in your global.json, different users could have different SDK versions with different language versions. The IDE doesn't necessarily know the version being picked since it just sees "latest" as a literal string.

@jaredpar: if the build task gave some hint of what language version was that it would consider "latest" we could fix this. The targets today set a property for the max language version based on TFM and a compiler API version property...if another was added the IDE could read that and interpret "latest" accordingly. Thoughts?

@jaredpar
Copy link
Member

Since you're saying language version = latest, that means you're going to use the highest language version of whatever SDK is being used

Correct.

The IDE doesn't necessarily know the version being picked since it just sees "latest" as a literal string.

Correct. The IDE doesn't really participate here. This is handled in project evaluation at the build layer. The SDK has a mapping of what latest means in the current SDK.

if the build task gave some hint of what language version was that it would consider "latest" we could fix this.

I'm unsure what we want to be fixed here. If the customer wants a fixed version of features then they can specify a fixed langversion value (or specify nothing and let it be derived from target framework). By using latest or preview the customer is explicitly opting into behaviors that change with the SDK being used.

I believe the full set of knobs are already available here.

@jasonmalinowski
Copy link
Member

By using latest or preview the customer is explicitly opting into behaviors that change with the SDK being used.

@jaredpar I think the "problem" is people are doing that, but don't recognize the impacts of what that means. And we then get bugs. 😄 Not saying we need to do something yet, except we saw a few of these in the last few weeks which is making me wonder if users keep failling into a pit of fail here.

@jaredpar
Copy link
Member

jaredpar commented Sep 25, 2023

I think the "problem" is people are doing that, but don't recognize the impacts of what that means.

The documentation mentions this will give latest by definition of current compiler. If we want to change that to include language about the SDK that's fine by me. But at this point don't think we need another knob here.

@webreidi webreidi mentioned this issue Oct 4, 2023
8 tasks
@breyed
Copy link

breyed commented Oct 30, 2023

By using latest or preview the customer is explicitly opting into behaviors that change with the SDK being used.

The changing definitions are expected. What's unexpected is the inconsistency between the compiler and the Quick Fix suggestions. The compiler interprets latest as "latest stable", i.e. the latest excluding preview. So long as the Quick Fix suggestions play by the same rules, the suggestions will guide users down a success path. Otherwise (as just happened to me), they'll guide users down a path that leads to compiler errors like this:

The feature 'primary constructors' is currently in Preview and unsupported. To use Preview features, use the 'preview' language version.

@jasonmalinowski
Copy link
Member

The compiler interprets latest as "latest stable", i.e. the latest excluding preview. So long as the Quick Fix suggestions play by the same rules, the suggestions will guide users down a success path.

The funky bit here is there's no guarantee that your compiler or your IDE are actually using the same compiler version. The IDE version of the actual analysis binaries is always the newest when that extension ships, where as your actual real compiler is what is being pulled from your SDK. Which could absolutely be different.

Copy link

github-actions bot commented Apr 4, 2024

This issue has been marked as stale after 14 days of inactivity.@[@jasonmalinowski,@akhera99@], could you please take a look?

@arunchndr
Copy link
Member

Lets reopen if this is still an issue after dotnet/roslyn#72811. Closing as dup for now.

@arunchndr arunchndr closed this as not planned Won't fix, can't repro, duplicate, stale Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-roslyn bug Something isn't working triaged The issue has been triaged
Projects
None yet
Development

No branches or pull requests

10 participants