Fix issue where we were not offering 'use collection expression' in a nullable scenario. #71528
Conversation
ghost
added
Area-IDE
untriaged
CyrusNajmabadi
force-pushed
the
useCollectionConditional
branch
from
ffa0f37 to
2e3f925
Compare
| return CompareAcrossSemanticModels(symbol, newSymbol); | ||
| } | ||
|
|
||
| if (symbol.Equals(newSymbol, SymbolEqualityComparer.IncludeNullability)) |
There was a problem hiding this comment.
@RikkiGibson @cston this is the IDE change i had to make to workaround the issue.
|
|
||
| [Theory, CombinatorialData, WorkItem("https://github.com/dotnet/roslyn/issues/71522")] | ||
| public async Task TestTargetTypedConditional1( | ||
| [CombinatorialValues("", "#nullable enable")] string nullable) |
There was a problem hiding this comment.
these tests demonstrate the problem.
CyrusNajmabadi
changed the title
|
@akhera99 ptal |
|
@RikkiGibson do you want me to hold off on this? I can also merge in, and you can adjust things back if you end up changing the compiler side. |
|
I have no strong opinion here about whether to merge the PR. It will maybe help buy the compiler side time to fix up the public API behavior. I don't have a strong understanding of the code that was changed in this PR. But I thought I would point out that the compiler sometimes internally uses TypeCompareKind.ObliviousNullableModifierMatchesAny. There are times where it's acceptable for either an annotated or non-annotated type to unify with an oblivious type. It's possible that such a behavior is also desirable here, I'm not sure. |
|
@RikkiGibson ok. i'll merge in. you'd be able to repro the issue by keeping the tests added in this pr, but undoing the other code changes. You'll see a call to arraysymbol's .Equals override. |
4 participants
Fixes #71522