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

Test plan for "required members" #57046

Closed
54 of 64 tasks
jcouv opened this issue Oct 8, 2021 · 7 comments
Closed
54 of 64 tasks

Test plan for "required members" #57046

jcouv opened this issue Oct 8, 2021 · 7 comments
Assignees
Labels
Milestone

Comments

@jcouv
Copy link
Member

jcouv commented Oct 8, 2021

Championed proposal: dotnet/csharplang#3630
Specification: https://github.com/dotnet/csharplang/blob/main/proposals/csharp-11.0/required-members.md

Compiler

declaration

  • required as contextual keyword ("required" type, type parameter, etc exists in scope)
  • where required modifier is allowed/disallowed
  • disallow required modifier based on LangVer
  • disallow required type name based on LangVer
  • missing attribute type or ctor
  • RequiredMember attribute emitted on type and members
  • Overriding can tighten requirement
  • Cannot hide required member
  • Required member must as accessible as containing type
  • RequiredMember attribute disallowed in source
  • field/property without setter
  • field/property with less accessible setter
  • different kinds of members (field, property, indexer, event, nested type, ...)
  • different kinds of containing types (class, struct, record; disallow interface)
  • required with field/property initializer (allowed)
  • rules with obsolete
  • no definite assignment for SetsRequired
  • must specify SetsRequired when SetsRequired on base or this
  • combination with other modifiers (fixed, ref readonly, ref, const, static disallowed; others such as extern, partial, virtual allowed)
  • record with required positional member declared explicitly
  • modifier can be parsed in different orders

metadata

  • Required attribute cannot be manually applied
  • required members are marked Required and the type too. But not if merely derived from a type with required members.
  • CompilerFeatureRequired with marker string
  • Obsolete with marker string
  • invalid type from metadata
  • SetsRequired set on record copy ctor

usage

  • object initialization
  • nested object initializers new C { Inner = new D { ... } } versus new C { Inner = { ... } }
  • attribute construction with named arguments
  • new() constraint not satisfied

nullability

  • nullable analysis of constructors in presence or absence of [SetsRequiredMembers] with required members
  • initial nullable flow state of required setters, when the type contains some combination of required and non-required non-nullable reference properties.

LDM

  • LDM: should we give a diagnostic for required property with an initializer (useless)? (answer: no, can be useful)
  • LDM: should we do something about ref-returning properties (useless)? (disallow)
  • LDM: should we do something about readonly modifier? (disallow)
  • LDM: what if the required property is obsolete?

BCL

Productivity:

  • typing/completion
  • override completion
  • MetadataAsSource (done)
  • formatting
  • colorization
  • F1/help (done)
  • QuickInfo (P0: should not crash, P2: consider whether to show required) (no change, manually verified)
  • Sorting modifiers (done)
  • completion of object creation should call out required members
  • snippet-style completion?
  • some existing refactorings produce/modify constructors
  • add option to generate required property for refactorings that generate properties
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Oct 8, 2021
@jcouv jcouv removed the untriaged Issues and PRs which have not yet been triaged by a lead label Oct 15, 2021
@jcouv jcouv added this to the 17.1 milestone Oct 15, 2021
@333fred 333fred modified the milestones: 17.1, Compiler.Next Nov 10, 2021
333fred added a commit to 333fred/roslyn that referenced this issue Dec 20, 2021
Implements parsing for required members.

Test plan: dotnet#57046
Spec: dotnet/csharplang#3630
333fred added a commit to 333fred/roslyn that referenced this issue Dec 28, 2021
Adding `required` to a member now results in the type having a `RequiredMembersAttribute` emitted with the name of that member as the contents. Reading this data from metadata is not yet supported, nor is adding the requisite `ObsoleteAttribute` to constructors that depend on such contracts. The rules for when required is allowed and when it is disallowed are documented in dotnet/csharplang#5566, pending LDM review.

Test plan: dotnet#57046
333fred added a commit that referenced this issue Jan 10, 2022
Implements parsing for required members.

Test plan: #57046
Spec: dotnet/csharplang#3630
333fred added a commit to 333fred/roslyn that referenced this issue Jan 20, 2022
Adding `required` to a member now results in the type having a `RequiredMembersAttribute` emitted with the name of that member as the contents. Reading this data from metadata is not yet supported, nor is adding the requisite `ObsoleteAttribute` to constructors that depend on such contracts. The rules for when required is allowed and when it is disallowed are documented in dotnet/csharplang#5566, pending LDM review.

Test plan: dotnet#57046
@jcouv jcouv self-assigned this Feb 2, 2022
@jcouv jcouv added this to Language in Compiler: Julien's umbrellas Feb 3, 2022
@jcouv jcouv added the New Feature - Required Members Required properties and fields label Feb 4, 2022
333fred added a commit that referenced this issue Feb 4, 2022
Adding `required` to a member now results in the type having a `RequiredMemberAttribute` emitted on the member and the containing type. Reading this data from metadata is not yet supported, nor is adding the requisite `ObsoleteAttribute` to constructors that depend on such contracts. The rules for when required is allowed and when it is disallowed are documented in dotnet/csharplang#5566.

Test plan: #57046
333fred added a commit to 333fred/roslyn that referenced this issue Mar 11, 2022
Implements reading the required members list of a type, and enforces that required members are all set by an initializer on the constructor. Required members must be initialized with values, not with nested object initializers.

Test plan dotnet#57046.
Specification https://github.com/dotnet/csharplang/blob/main/proposals/required-members.md
333fred added a commit that referenced this issue Mar 17, 2022
Implements reading the required members list of a type, and enforces that required members are all set by an initializer on the constructor. Required members must be initialized with values, not with nested object initializers.

Test plan #57046.
Specification https://github.com/dotnet/csharplang/blob/main/proposals/required-members.md
333fred added a commit to 333fred/roslyn that referenced this issue Mar 25, 2022
The SetsRequiredMembersAttribute prevents the compiler from checking the required member list of a type when calling that constructor, and suppresses any errors from a base type's list being invalid.

Specification: https://github.com/dotnet/csharplang/blob/main/proposals/required-members.md
Test plan: dotnet#57046
333fred added a commit to 333fred/roslyn that referenced this issue Mar 25, 2022
The SetsRequiredMembersAttribute prevents the compiler from checking the required member list of a type when calling that constructor, and suppresses any errors from a base type's list being invalid.

Specification: https://github.com/dotnet/csharplang/blob/main/proposals/required-members.md
Test plan: dotnet#57046
333fred added a commit that referenced this issue Apr 15, 2022
The SetsRequiredMembersAttribute prevents the compiler from checking the required member list of a type when calling that constructor, and suppresses any errors from a base type's list being invalid.

Specification: https://github.com/dotnet/csharplang/blob/main/proposals/required-members.md
Test plan: #57046
@jcouv jcouv added this to the C# 11.0 milestone Apr 28, 2022
333fred added a commit that referenced this issue Apr 28, 2022
Support emitting ObsoleteAttribute on the constructor of types with required members.

Specification: https://github.com/dotnet/csharplang/blob/main/proposals/required-members.md
Test plan: #57046
333fred added a commit to 333fred/roslyn that referenced this issue May 3, 2022
Adds support for decoding and reporting errors when `CompilerFeatureRequiredAttribute` is encountered on metadata type symbols. We also block applying the attribute by hand in both C# and VB.

Test plan: dotnet#57046
333fred added a commit to 333fred/roslyn that referenced this issue May 3, 2022
Adds support for decoding and reporting errors when `CompilerFeatureRequiredAttribute` is encountered on metadata type symbols. We also block applying the attribute by hand in both C# and VB.

Test plan: dotnet#57046
@jaredpar jaredpar modified the milestones: C# 11.0, 17.4 Jun 24, 2022
@hez2010
Copy link

hez2010 commented Aug 21, 2022

Any updates for record with required positional member declared explicitly? Will this land in C# 11?

@333fred
Copy link
Member

333fred commented Aug 21, 2022

I'm not sure what you're asking about. This issue is tracking adding tests.

@SunnieShine

This comment was marked as outdated.

@hez2010
Copy link

hez2010 commented Aug 22, 2022

I'm not sure what you're asking about. This issue is tracking adding tests.

See https://sharplab.io/#v2:EYLgtghglgdgPgAQEwEYCwAoTAnApgYwHtsATAAgDFDCAKPARwFco9zYAXMgDQEoBuIA

We still cannot use required in primary constructor of records.

@333fred
Copy link
Member

333fred commented Aug 22, 2022

We still cannot use required in primary constructor of records.

For new additions to the feature, please use csharplang.

However, for this particular request, I'll save you some time by telling you that you don't really want it. If you mark a property as required, it must be set with a property initializer. No ifs, ands, or buts. This would create a primary constructor that takes a parameter to set the property, and you must then use an object initializer to set the property again.

@jcouv
Copy link
Member Author

jcouv commented Sep 30, 2022

@jasonmalinowski I think we're ready to close this test plan from the compiler side. The follow-up compiler work is tracked by separate issues.
Can you create follow-up issues for bullets that still need IDE work?

@jcouv
Copy link
Member Author

jcouv commented Oct 18, 2022

@333fred will follow-up with runtime team on possibility of Reflection API and file an issue if there's interest.
Chatted with @jasonmalinowski about remaining IDE items. I think all the basics are covered and there's no urgent follow-up.
I'll go ahead and close this test plan as completed. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

No branches or pull requests

7 participants