Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactors INullCoalescingExpression to ICoalesce expression, and prop… #21458

Merged

Conversation

333fred
Copy link
Member

@333fred 333fred commented Aug 11, 2017

…erties to sensible names. Fixes #21290. Tagging @dotnet/analyzer-ioperation @dotnet/roslyn-compiler for review.

@333fred
Copy link
Member Author

333fred commented Aug 11, 2017

@dotnet/roslyn-infrastructure failure appears to be #21347.

@333fred
Copy link
Member Author

333fred commented Aug 11, 2017

@dotnet-bot retest windows_release_unit64_prtest please

Copy link
Member

@genlu genlu left a comment

Choose a reason for hiding this comment

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

Consider adding a VB test for ICoalesceExpression

…oalescingexpression-refactor

* dotnet/features/ioperation:
  Update CSharpReplaceMethodWithPropertyService.cs
  Add VB side of fix.
  Remove unneeded function.
  Improve trivia preservation when converting methods into a property.
  VB side of do-not-simplify-nameof if it changes semantics.
  Do not simplify to an alias in a nameof if it changes the value of hte nameof.
  Include System.Runtime.Serialization.Primitives and System.Security.Cryptography.Csp in PortableFacades CoreXT package. (dotnet#21438)
  Address one more refactoring feedback
  Address PR feedback
  Fix possible race conditions in TestExtensionErrorHandler
  Fix expected test results to properly consider trivia
  Improve messages when tests fail due to expected text
  Default to considering trivia during testing
  Remove RemoveUnneededReferences from LamdaRewriter (dotnet#21367)
  Enable embedding sources to Windows PDBs (dotnet#21391)
  re-enabled assert we have disabled
  Do not insert Microsoft.DiaSymReader.Native (dotnet#21420)
  Resolving merge conflict
  Don't pick a project arbitrarily when navigating to symbols
@@ -286,7 +286,7 @@ Namespace Microsoft.CodeAnalysis.Semantics
End Function

Private Function CreateBoundMeReferenceOperation(boundMeReference As BoundMeReference) As IInstanceReferenceExpression
Dim instanceReferenceKind As InstanceReferenceKind = If(boundMeReference.WasCompilerGenerated, InstanceReferenceKind.Implicit, InstanceReferenceKind.Explicit)
Dim instanceReferenceKind As InstanceReferenceKind = If(boundMeReference.WasCompilerGenerated, instanceReferenceKind.Implicit, instanceReferenceKind.Explicit)
Copy link
Contributor

@AlekseyTs AlekseyTs Aug 15, 2017

Choose a reason for hiding this comment

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

instanceReferenceKind [](start = 107, length = 21)

This looks like an unintentional capitalization change. #Closed

Copy link
Member Author

Choose a reason for hiding this comment

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

I seriously do not know why these keep changing.

@@ -286,7 +286,7 @@ Namespace Microsoft.CodeAnalysis.Semantics
End Function

Private Function CreateBoundMeReferenceOperation(boundMeReference As BoundMeReference) As IInstanceReferenceExpression
Dim instanceReferenceKind As InstanceReferenceKind = If(boundMeReference.WasCompilerGenerated, InstanceReferenceKind.Implicit, InstanceReferenceKind.Explicit)
Dim instanceReferenceKind As InstanceReferenceKind = If(boundMeReference.WasCompilerGenerated, instanceReferenceKind.Implicit, instanceReferenceKind.Explicit)
Copy link
Contributor

@AlekseyTs AlekseyTs Aug 15, 2017

Choose a reason for hiding this comment

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

instanceReferenceKind [](start = 139, length = 21)

Here too #Closed

@@ -1073,7 +1073,7 @@ Namespace Microsoft.CodeAnalysis.Semantics

Private Function CreateBoundGotoStatementOperation(boundGotoStatement As BoundGotoStatement) As IBranchStatement
Dim target As ILabelSymbol = boundGotoStatement.Label
Dim branchKind As BranchKind = BranchKind.GoTo
Dim branchKind As BranchKind = branchKind.GoTo
Copy link
Contributor

@AlekseyTs AlekseyTs Aug 15, 2017

Choose a reason for hiding this comment

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

branchKind [](start = 43, length = 10)

Here too. #Closed

@@ -1083,7 +1083,7 @@ Namespace Microsoft.CodeAnalysis.Semantics

Private Function CreateBoundContinueStatementOperation(boundContinueStatement As BoundContinueStatement) As IBranchStatement
Dim target As ILabelSymbol = boundContinueStatement.Label
Dim branchKind As BranchKind = BranchKind.Continue
Dim branchKind As BranchKind = branchKind.Continue
Copy link
Contributor

@AlekseyTs AlekseyTs Aug 15, 2017

Choose a reason for hiding this comment

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

branchKind [](start = 43, length = 10)

Here too. #Closed

@@ -1093,7 +1093,7 @@ Namespace Microsoft.CodeAnalysis.Semantics

Private Function CreateBoundExitStatementOperation(boundExitStatement As BoundExitStatement) As IBranchStatement
Dim target As ILabelSymbol = boundExitStatement.Label
Dim branchKind As BranchKind = BranchKind.Break
Dim branchKind As BranchKind = branchKind.Break
Copy link
Contributor

@AlekseyTs AlekseyTs Aug 15, 2017

Choose a reason for hiding this comment

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

branchKind [](start = 43, length = 10)

Here too. #Closed

@AlekseyTs
Copy link
Contributor

AlekseyTs commented Aug 15, 2017

Done with review pass. #Closed

@333fred
Copy link
Member Author

333fred commented Aug 15, 2017

@AlekseyTs fixed.

Copy link
Contributor

@AlekseyTs AlekseyTs left a comment

Choose a reason for hiding this comment

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

LGTM

@333fred 333fred merged commit 41574d3 into dotnet:features/ioperation Aug 15, 2017
@333fred 333fred deleted the inullcoalescingexpression-refactor branch August 15, 2017 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants