Skip to content

CSharpImportAdder should be able to add the intended import if the identifier is a var#59947

Merged
akhera99 merged 4 commits intodotnet:mainfrom
akhera99:fix_import_adder
Mar 7, 2022
Merged

CSharpImportAdder should be able to add the intended import if the identifier is a var#59947
akhera99 merged 4 commits intodotnet:mainfrom
akhera99:fix_import_adder

Conversation

@akhera99
Copy link
Copy Markdown
Member

@akhera99 akhera99 commented Mar 4, 2022

No description provided.

@akhera99 akhera99 requested a review from a team as a code owner March 4, 2022 17:47
@ghost ghost added the Area-IDE label Mar 4, 2022
@Cosifne
Copy link
Copy Markdown
Member

Cosifne commented Mar 4, 2022

Is this ready to be reviewed?
From the commit it looks like a draft?

@akhera99
Copy link
Copy Markdown
Member Author

akhera99 commented Mar 4, 2022

Is this ready to be reviewed? From the commit it looks like a draft?

haha it can be reviewed, im just going back and fixing tests

private void Deconstruct(out object x, out object y)
{
throw new System.NotImplementedException();
throw new NotImplementedException();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me this looks fine, but it would be better to check with the original test author to see the old behavior is not by design.

if (node.IsRightSideOfDotOrArrowOrColonColon())
return;

if (node.IsVar)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, so from my side I don't see any harm for this check

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i would def doc this.

Copy link
Copy Markdown
Contributor

@CyrusNajmabadi CyrusNajmabadi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for breaking this into its own PR.

@akhera99 akhera99 enabled auto-merge March 7, 2022 17:43
@akhera99 akhera99 merged commit 592fd2e into dotnet:main Mar 7, 2022
@ghost ghost added this to the Next milestone Mar 7, 2022
@allisonchou allisonchou modified the milestones: Next, 17.2.P3 Mar 28, 2022
@akhera99 akhera99 deleted the fix_import_adder branch June 18, 2024 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants