EnC rude edit for local function attributes #49027
Conversation
|
FYI @tmat if you get a chance to take a look before you go, that would be great. |
src/Features/Core/Portable/EditAndContinue/AbstractEditAndContinueAnalyzer.cs
Outdated
Show resolved
Hide resolved
src/EditorFeatures/CSharpTest/EditAndContinue/TopLevelEditingTests.cs
Outdated
Show resolved
Hide resolved
src/Features/Core/Portable/EditAndContinue/AbstractEditAndContinueAnalyzer.cs
Outdated
Show resolved
Hide resolved
src/Features/Core/Portable/EditAndContinue/AbstractEditAndContinueAnalyzer.cs
Outdated
Show resolved
Hide resolved
src/EditorFeatures/CSharpTest/EditAndContinue/TopLevelEditingTests.cs
Outdated
Show resolved
Hide resolved
src/Features/Core/Portable/EditAndContinue/AbstractEditAndContinueAnalyzer.cs
Outdated
Show resolved
Hide resolved
src/Features/Core/Portable/EditAndContinue/AbstractEditAndContinueAnalyzer.cs
Outdated
Show resolved
Hide resolved
src/EditorFeatures/CSharpTest/EditAndContinue/CSharpEditAndContinueAnalyzerTests.cs
Outdated
Show resolved
Hide resolved
src/EditorFeatures/CSharpTest/EditAndContinue/StatementEditingTests.cs
Outdated
Show resolved
Hide resolved
src/Features/CSharp/Portable/EditAndContinue/StatementSyntaxComparer.cs
Outdated
Show resolved
Hide resolved
src/Features/CSharp/Portable/EditAndContinue/CSharpEditAndContinueAnalyzer.cs
Show resolved
Hide resolved
src/Features/CSharp/Portable/EditAndContinue/CSharpEditAndContinueAnalyzer.cs
Outdated
Show resolved
Hide resolved
|
I've updated this to now use ClassifyEdit to report the rude edits. I like the approach in general, because it will be easy to add type parameter rude edits, and modifier rude edits etc. but there is an issue that I need to look into (and hence have to talk to you about first 😛) I've left this in a state that should at least leave the tests green though. |
src/Features/Core/Portable/EditAndContinue/AbstractEditAndContinueAnalyzer.cs
Outdated
Show resolved
Hide resolved
| if (!_isTopLevelEdit) | ||
| { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Why do we not report reordering of type parameters, e.g. for local functions?
| if (!_isTopLevelEdit) | ||
| { | ||
| return; | ||
| } |
| else | ||
| } | ||
|
|
||
| private IEnumerable<SyntaxNode> GetRootChildren(SyntaxNode node) |
There was a problem hiding this comment.
GetRootChildren [](start = 40, length = 15)
Looks good. I'd just rename this method to EnumerateChildren - GetRootChildren suggests that node is a root. But it is not expected to be a root.
Also, can we now merge ENumerateNonRootChildren and EnumerateRootChildren? Seems like we have one extra layer there.
src/Features/CSharp/Portable/EditAndContinue/CSharpEditAndContinueAnalyzer.cs
Show resolved
Hide resolved
| @@ -1860,6 +1874,7 @@ protected override string GetSuspensionPointDisplayName(SyntaxNode node, EditKin | |||
| _kind = kind; | |||
| _span = span; | |||
| _match = match; | |||
| _isTopLevelEdit = isTopLevelEdit; | |||
There was a problem hiding this comment.
_isTopLevelEdit [](start = 16, length = 15)
I'm not seeing why is this flag necessary. Wouldn't the syntax kinds that are not found in local functions be just ignored?
There was a problem hiding this comment.
Yeah, I think this can be removed now, just haven't gone back to check yet. This is from previous attempts where the method body itself was classified, and I needed a way to ignore more node types.
There was a problem hiding this comment.
So this is still an issue because the EditClassifier is now seeing edits for nodes that StatementSyntaxComparer cares about, but TopSyntaxComparer doesn't, and it was written for the latter I guess.
- Change
_isTopLevelEditto_throwOnUnexpectedNodeand have it just control thethrow ExceptionUtilities.UnexpectedValuecalls - Check
HasLabelbefore throwing unexpected value - Add all of the missing syntax kinds to the EditClassifier and just return without doing anything
- Leave as is
Thoughts? Or have I missed something about when to class the EditClassifier?
There was a problem hiding this comment.
I'd check HasLabel before calling the classifier for a specific edit.
Fixes #48636