Add some EE and EnC tests for primary constructors #66677
Conversation
This change also flags as rude EnC edits that might result in changes around captured primary constructor parameters: - Adding a capture - Removing a capture - Changing type of a captured parameter - Etc. The check is not meant to be able to accurately detect “safe” edits. The primary goal is to not let “unsafe” edits through.
|
@cston, @dotnet/roslyn-compiler For the second review. |
| { | ||
| var container = symbol.ContainingSymbol; | ||
|
|
||
| if (container is IMethodSymbol { IsImplicitlyDeclared: false, MethodKind: MethodKind.Constructor }) |
There was a problem hiding this comment.
There was a problem hiding this comment.
Any edit to a class with primary ctor is a rude edit for now.
There was a problem hiding this comment.
Is this treating any parameter of an explicit constructor as a rude edit when containing type has a primary constructor?
No. Only for a primary constructor.
Is that expected for a parameter of a constructor other than the primary constructor?
It is not and those are ignored. But as the PR comment says, there is no goal to enable any EnC scenarios in a class/struct with a primary constructor. The goal is to error on the safe side and disable EnC in them. So, if the change were to treat as rude any edit for parameter of a constructor other than the primary constructor, that would be fine.
There was a problem hiding this comment.
If this is a temporary restriction, consider adding a PROTOTYPE comment.
The existence of this whole method is temporary. And I have PROTOTYPE comments where it is used. I do not expect it to survive once real EnC support is implemented.
| { | ||
| var container = symbol.ContainingSymbol; | ||
|
|
||
| if (container is { Kind: SymbolKind.NamedType, IsImplicitlyDeclared: false }) |
There was a problem hiding this comment.
Is this treating edits to members as rude edit when the containing type has a primary constructor?
Yes.
If this is a temporary restriction, consider adding a PROTOTYPE comment.
The comments are added at the call sites. The whole method is temporary.
| Diagnostic(RudeEditKind.Update, "long a", FeaturesResources.parameter)); | ||
| } | ||
|
|
||
| #endregion |
There was a problem hiding this comment.
Consider testing edits of explicit constructors other than the primary constructor, and edits of other members such as instance and static methods and fields.
Testing of field initializers is not particularly interesting at the moment because they do not change capturing of primary constructor parameters. Also, at the moment, I do not care if their edits are flagged as rude edits or not. The goal is to error on the safe side.
Same goes for static methods.
Explicit constructors other than the primary constructors cannot refer to primary constructor parameters. So, same as for fields, I do not care what happens for them at the moment.
Addition/removal/edit of instance methods is covered. For example, PrimaryConstructors_01, PrimaryConstructors_02, PrimaryConstructors_03.
This change also flags as rude EnC edits that might result in changes around captured primary constructor parameters:
The check is not meant to be able to accurately detect “safe” edits. The primary goal is to not let “unsafe” edits through.