Fix cast simplification issues#57066
Merged
CyrusNajmabadi merged 22 commits intodotnet:mainfrom Oct 11, 2021
Merged
Conversation
CyrusNajmabadi
commented
Oct 10, 2021
| [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnnecessaryCast)] | ||
| public async Task TestUnintendedReferenceComparison1() | ||
| { | ||
| await VerifyCS.VerifyCodeFixAsync( |
Contributor
Author
There was a problem hiding this comment.
these are things that we do not want to report diagnostics on removing the cast will cause a warning from teh compiler.
CyrusNajmabadi
commented
Oct 10, 2021
| public async Task DoNotRemoveEqualityWarningSilencingCast1() | ||
| { | ||
| var source = | ||
| @" |
Contributor
Author
There was a problem hiding this comment.
these tests are similar to what we already have, except these test against interfaces, not just reference types.
9c7c62f to
533195f
Compare
174569c to
834800e
Compare
CyrusNajmabadi
commented
Oct 11, 2021
| public async Task DoNotRemoveCastFromStringTypeToObjectInReferenceEquality() | ||
| { | ||
| await TestInRegularAndScriptAsync( | ||
| await TestMissingInRegularAndScriptAsync( |
Contributor
Author
There was a problem hiding this comment.
removing this cast causes a warning. so we need to preserve it.
CyrusNajmabadi
commented
Oct 11, 2021
| return false; | ||
|
|
||
| if (CastRemovalWouldCauseUnintendedReferenceComparisonWarning(rewrittenExpression, rewrittenSemanticModel, cancellationToken)) | ||
| return false; |
Contributor
Author
There was a problem hiding this comment.
fixes issue where removing the cast has no semantic/IL impact, but would cause a warning.
CyrusNajmabadi
commented
Oct 11, 2021
|
|
||
| // If the types of the expressions are the same, then we can remove safely. | ||
| if (Equals(originalConvertedType, rewrittenConvertedType)) | ||
| if (originalConvertedType.Equals(rewrittenConvertedType, SymbolEqualityComparer.IncludeNullability)) |
Contributor
Author
There was a problem hiding this comment.
fixes issue where we'd remove a necessary NRT cast.
Contributor
Author
|
@davidwengier ptal. thanks! |
davidwengier
approved these changes
Oct 11, 2021
...es/SharedUtilitiesAndExtensions/Compiler/CSharp/Simplification/Simplifiers/CastSimplifier.cs
Outdated
Show resolved
Hide resolved
Member
|
…don't execute user code).
…mplification/Simplifiers/CastSimplifier.cs Co-authored-by: David Wengier <david.wengier@microsoft.com>
…i/roslyn into castSimplifierWork
16 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #57065
Fixes #57064
Fixes #57063
Also fixes several cases in #56938