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

C# 11 required properties do not compile on dotnet 6 #68478

Closed
Varorbc opened this issue Jun 6, 2023 · 16 comments · Fixed by #68577
Closed

C# 11 required properties do not compile on dotnet 6 #68478

Varorbc opened this issue Jun 6, 2023 · 16 comments · Fixed by #68577
Assignees
Labels
Area-IDE New Feature - Required Members Required properties and fields untriaged Issues and PRs which have not yet been triaged by a lead

Comments

@Varorbc
Copy link

Varorbc commented Jun 6, 2023

[Update(jcouv):] The ask is that required properties not be recommended by the IDE on .NET 6 or lower.

Describe the bug

When I want to use required properties in a project targeting net6.0 with LangVersion set to latest I face some compile errors.

To Reproduce

<Project Sdk="Microsoft.NET.Sdk">

    <PropertyGroup>
        <TargetFramework>net6.0</TargetFramework>
        <ImplicitUsings>enable</ImplicitUsings>
        <Nullable>enable</Nullable>
        <LangVersion>latest</LangVersion>
    </PropertyGroup>

</Project>

namespace Csharp11OnDotnet6;

public class Person
{
    public required string Name { get; init; }
}

2.dotnet build

Exceptions (if any)

error CS0656: Missing compiler required member 'System.Runtime.CompilerServices.RequiredMemberAttribute..ctor'
error CS0656: Missing compiler required member 'System.Runtime.CompilerServices.CompilerFeatureRequi
redAttribute..ctor'

Expected Behavior: build successful

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-NetSDK untriaged Issues and PRs which have not yet been triaged by a lead labels Jun 6, 2023
@marcpopMSFT marcpopMSFT transferred this issue from dotnet/sdk Jun 7, 2023
@jcouv
Copy link
Member

jcouv commented Jun 9, 2023

Those errors are expected. The "required members" feature is part of C# 11 and is only supported on .NET 7 and up.

@jcouv jcouv closed this as not planned Won't fix, can't repro, duplicate, stale Jun 9, 2023
@jcouv jcouv added Area-Compilers Resolution-By Design The behavior reported in the issue matches the current design New Feature - Required Members Required properties and fields and removed untriaged Issues and PRs which have not yet been triaged by a lead Area-NetSDK labels Jun 9, 2023
@jcouv jcouv added this to the 17.7 milestone Jun 9, 2023
@Varorbc
Copy link
Author

Varorbc commented Jun 9, 2023

@jcouv

1.use The "required members" feature is part of C# 11 and is only supported on .NET 7 and up instead of Missing compiler required member 'System.Runtime.CompilerServices.RequiredMemberAttribute..ctor' for the error

2.In .net 6 projects, IntelliSense should not recommend "required members"

@CyrusNajmabadi
Copy link
Member

1.use The "required members" feature is part of C# 11 and is only supported on .NET 7 and up instead of

That's not the case here though. That's what we support (and so we don't do additional work in non-supported scenarios). If you added that attribute somehow, it would work (though we still dont' support that scenario), we don't block it. So the error you're getting is correct. The issue here is that those attributes are missing.

2.In .net 6 projects, IntelliSense should not recommend "required members"

Could you clarify what you mean? We def show things in the completion list to help speed up typing, regardless of version. We don't keep things out of the completion list as that can both be confusing, and it means if you try to type this, you can end up with some other piece of completed text.

@Varorbc
Copy link
Author

Varorbc commented Jun 10, 2023

So the error you're getting is correct. The issue here is that those attributes are missing.

Although I am aware of missing RequiredMemberAttribute, I am not aware that the root cause is that the "required members" feature is part of C# 11 and is only supported on .NET 7 and up, and if this can be explained, it would be better rather than asking questions

Could you clarify what you mean?

image

@Varorbc
Copy link
Author

Varorbc commented Jun 12, 2023

@CyrusNajmabadi @jcouv Please re-open

@jcouv
Copy link
Member

jcouv commented Jun 12, 2023

Added a summary to OP: "The ask is that required properties not be recommended by the IDE on .NET 6 or lower." and assigned to the IDE team to take a look.

@jcouv jcouv reopened this Jun 12, 2023
@jcouv jcouv added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead and removed Area-Compilers Resolution-By Design The behavior reported in the issue matches the current design labels Jun 12, 2023
@jcouv jcouv removed this from the 17.7 milestone Jun 12, 2023
@CyrusNajmabadi
Copy link
Member

The ide behavior is expected here. This is a viable way to fix the issue. And if we don't offer it, it could be confusing to the user (I believe we had this Convo when creating the feature). The user is not told to pick this option. They choose it, if appropriate, from the options offered. And if they want this, that means they're ok with moving forward.

@CyrusNajmabadi CyrusNajmabadi closed this as not planned Won't fix, can't repro, duplicate, stale Jun 12, 2023
@Varorbc
Copy link
Author

Varorbc commented Jun 13, 2023

when cleaning up code, should use Declare as nullable. not Make Property 'required'

@CyrusNajmabadi
Copy link
Member

That is one of the options offered :-)

@Varorbc
Copy link
Author

Varorbc commented Jun 13, 2023

The cleanup worked fine when I hadn't updated vs to the latest version, but now that I've updated vs to the latest version, the project has a build error after the cleanup because required is used

@CyrusNajmabadi
Copy link
Member

Can you give an example of what you mean? What was your code, and which fix did you pick?

@Varorbc
Copy link
Author

Varorbc commented Jun 13, 2023

<Project Sdk="Microsoft.NET.Sdk">

    <PropertyGroup>
        <TargetFramework>net6.0</TargetFramework>
        <ImplicitUsings>enable</ImplicitUsings>
        <Nullable>enable</Nullable>
        <LangVersion>latest</LangVersion>
    </PropertyGroup>

</Project>

namespace Csharp11OnDotnet6;

public class Person
{
    public string Name { get; init; }
}

2.image

  1. run code clean(Profile 1)

@CyrusNajmabadi
Copy link
Member

@mavasani Do you know how 'code cleanup' does this work?

@mavasani
Copy link
Contributor

Also adding @sharwell

Choosing "Fix all warnings and errors set in EditorConfig" in the code cleanup profile leads to code cleanup sequentially attempting to fix all analyzer and compiler warnings and errors in code and considers all code fixer as potential candidates for providing a fix. It seems that we may need to add logic such that a code fixer can declaratively state that it shouldn't be included in code cleanup, and all fixers that fall in this bucket of "application introduces further compiler errors" should be marked for exclusion in code cleanup. @sharwell thoughts?

@CyrusNajmabadi
Copy link
Member

Another option is to make this low-pri. Or order after "declare as nullable".

That said, neither of these is truly better than the other. So it's unclear to me what would be hte right behavior here as it depends on hte user.

@sharwell
Copy link
Member

sharwell commented Jun 13, 2023

<TargetFramework>net6.0</TargetFramework>
<LangVersion>latest</LangVersion>

This is explicitly instructing the compiler to ignore the official support matrix and force required members to be allowed for a mismatched target framework, and as such is not an officially supported scenario. As a result, the analyzers related to required members may report diagnostics, and those diagnostics would be fixed if the configuration instructs it to. The change expected here would be the analyzer(s) code fix which suggests the use of required members should only do so when the necessary attribute is present. It sounds like we already check language version, so only the attribute check would be new.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE New Feature - Required Members Required properties and fields untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants