Skip to content

Fix issue where 'remove parens' would break code with ++/-- operators.#29457

Merged
jasonmalinowski merged 3 commits intodotnet:masterfrom
CyrusNajmabadi:removeParentUnary
Aug 30, 2018
Merged

Fix issue where 'remove parens' would break code with ++/-- operators.#29457
jasonmalinowski merged 3 commits intodotnet:masterfrom
CyrusNajmabadi:removeParentUnary

Conversation

@CyrusNajmabadi
Copy link
Copy Markdown
Contributor

@CyrusNajmabadi CyrusNajmabadi commented Aug 23, 2018

Fixes #29454

Also fixes a related issue i found while looking into this area where we could break code of the form x+(++y).

@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner August 23, 2018 17:29
@CyrusNajmabadi
Copy link
Copy Markdown
Contributor Author

Tagging @jcouv @dotnet/roslyn-ide @jasonmalinowski

@jinujoseph jinujoseph added Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee. labels Aug 23, 2018
@CyrusNajmabadi
Copy link
Copy Markdown
Contributor Author

Tagging @jcouv @dotnet/roslyn-ide @jasonmalinowski

Copy link
Copy Markdown
Member

@jasonmalinowski jasonmalinowski left a comment

Choose a reason for hiding this comment

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

@CyrusNajmabadi one question about some code and how it handles script mode. Might just need another test. Otherwise this looks good.


// Have to be careful if we would remove parens and cause a + and a + to become a ++.
// (same with - as well).
var tokenBeforeParen = node.GetFirstToken().GetPreviousToken();
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.

Does this have problems in script/interactive mode?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fortunately no. GetPreviousToken will simply return default(SyntaxToken). default(SyntaxToken) returns empty-string for .Text, and LastOrDefault will just return 0 in that case.

@jasonmalinowski jasonmalinowski merged commit a107b43 into dotnet:master Aug 30, 2018
@CyrusNajmabadi
Copy link
Copy Markdown
Contributor Author

Thanks!

@CyrusNajmabadi CyrusNajmabadi deleted the removeParentUnary branch August 30, 2018 22:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

False positive IDE0047 : "Parentheses can be removed"

3 participants