Fixing some semantic model and refactoring issues with deconstruction

[jcouv]

There are few issues fixed:

• code getting swallowed while extracting method

// comment
var (x1, x2) = (1, 2);
var x3 = 3; // extract method on "3" causes the two lines above to disappear

• value of d-declaration not picked up for extract method and extract local

var (x1, x2) = [| (1, 2) |]; // not extract method or extract local refactoring available

• when inlining a temporary variable into a deconstruction declaration, superfluous parens were left around:

// var temp = new C();
var (x1, x2) = (new C()); // after inlining temp, there are extra parens

CC @dotnet/roslyn-ide @dotnet/roslyn-compiler for review.
Related to #11299

[jcouv]

I wish to remove this marker before merging, but I'm not sure if additional scenarios need to be covered where variables need to be removed.
I'll check with the @dotnet/roslyn-ide team.

[jcouv]

@heejaechang Let me know if you could review this PR and in particular the ExtractMethod refactoring question above.
I'm happy to stop by and provide some context.

[CyrusNajmabadi]

I don't see the benefit of the "PROTOTYPE" comment.

[jcouv]

@CyrusNajmabadi I intend to remove this PROTOTYPE marker before checking in. The question here is whether this behavior is sufficient, or if I should add logic for removing variables.
I'm not familiar enough with scenarios involving removing variables, so I'm not sure if it could be applicable to deconstructions. I'm guessing it's not applicable.

[rchande]

👍 for tests and inline temp. You might want @heejaechang to look at the extract method changes.

[jcouv]

@gafter Could you review the compiler changes in this small PR?
The change is that bound nodes for deconstruction steps (that don't directly correspond to syntax) should not be added to the node map. This allows GetTypeInfo to work properly on deconstructions.
They are part of what we discussed Wednesday.

[cston]

Compiler changes LGTM.

[CyrusNajmabadi]

Don't set compareTokens:false unless you are specifically testing formatting for the feature.

[jcouv]

Fixed. Thanks

[VSadov]

other than question about BoundLocalDeconstructionDeclaration, and we can come back to that later, LGTM

[jcouv]

@heejaechang @CyrusNajmabadi Let me know if you have any other feedback. I plan to merge before lunch (this is blocking merging the feature back into the master branch). Thanks

[heejaechang]

👍

@balajikris extract method has its own syntax node replace since it was written before we had SyntaxNode.Replace API. at some point, probably need to replace extract method's own syntax node replace implementation to just official SyntaxNode.Replace

[CyrusNajmabadi]

I'm good with the IDE side.

[jcouv]

Thanks all for the review and feedback!
I'll go ahead with the merge.