Remove code for generating constructors from class-decl, it will be provided by another refactoring. #19481
Conversation
…rovided by another fixer.
CyrusNajmabadi
added
the
PR For Personal Review Only
|
Retest windows_release_vs-integration_prtest please |
| @@ -2316,66 +2316,6 @@ public Base(bool val = false) | |||
| }"); | |||
| } | |||
|
|
|||
| [WorkItem(6541, "https://github.com/dotnet/Roslyn/issues/6541")] | |||
There was a problem hiding this comment.
These tests moved.
| @@ -1528,98 +1529,6 @@ End Class") | |||
| End Function | |||
| End Class | |||
|
|
|||
| <WorkItem(6541, "https://github.com/dotnet/Roslyn/issues/6541")> | |||
There was a problem hiding this comment.
These tests moved.
|
Tagging @dotnet/roslyn-ide @sharwell This addresses a bunch of complexity that had crept into the set of components we have that are all responsible for some part of the overall 'generate constructor' feature. Code was misplaced and was making things much more complex. It was also bleeding out to the user in confusing ways (i.e. offering the same sort of constructor in multiple ways, or just generating duplicate constructors at times). I've simplified things, squashed several bugs, and tried to comment things to make it clearer what the overall responsibilities are here. The specific components we have (and which i have cleaned up) are:
|
| @@ -590,7 +591,7 @@ protected Program(SerializationInfo info, StreamingContext context) : base(info, | |||
| { | |||
| } | |||
| }", | |||
| index: 3, | |||
| index: 4, | |||
There was a problem hiding this comment.
Because GenrateDefaultConstructors now offers to generate the no arg constructor, the indices increased here.
| @@ -728,10 +729,6 @@ public Program() | |||
| { | |||
| } | |||
|
|
|||
| public Program() | |||
There was a problem hiding this comment.
This is good. If you expand the code above htis, you'll see we were generating the default constructor twice.
| @@ -27,8 +28,7 @@ Imports System.Collections.Generic | |||
| Imports System.Linq | |||
| Class Program | |||
| Inherits Exception | |||
| Public Sub New(message As String) | |||
| MyBase.New(message) | |||
| Public Sub New() | |||
There was a problem hiding this comment.
these changes are expected given that this feature now will offer to generate the default constructor if the base type has one as well.
| @@ -60,35 +51,6 @@ protected override bool IsClassDeclarationGeneration(SemanticDocument document, | |||
| return false; | |||
| } | |||
|
|
|||
| protected override bool TryInitializeClassDeclarationGenerationState( | |||
There was a problem hiding this comment.
Not needed anymore GenerateConstructor is responsible for the "new MyFoo(x, y, z)" cases, not the "class D : BaseType" cases. Those are handled elsewhere.
|
|
||
| var syntaxTree = document.SyntaxTree; | ||
| var node = document.Root.FindToken(textSpan.Start).GetAncestor<TypeSyntax>(); | ||
| if (node != null) |
There was a problem hiding this comment.
This is the same, just indented differently.
CyrusNajmabadi
removed
the
PR For Personal Review Only
|
Tagging @MattGertz Customer scenario IDE shows too many 'fix' options in some cases, with some of the options being effective duplicates of the others. Bugs this fixes: Fixes #19041 Workarounds, if any None. Risk Low. Performance impact Low. Is this a regression from a previous update? Yes. Root cause analysis: As we added more and more refactorings we ended up duplicating a bit of featureset across a few of them. How was the bug found? Internal usage. |
|
Awesome. Small detail but they all add up 👍 |
Fixes #19041