Look for type check in local declaration and return statements#20240
Look for type check in local declaration and return statements#20240sharwell merged 3 commits intodotnet:masterfrom
Conversation
|
Tagging @dotnet/roslyn-ide for review. |
There was a problem hiding this comment.
indentation got worse.
There was a problem hiding this comment.
what if there are multiple variables?
There was a problem hiding this comment.
There won't, it's checked in analyzer.
There was a problem hiding this comment.
Tagging @mavasani . We have to do this sort of a check in a lot of our analyzers. It seems like it would be nice to avoid having to do all the callbacks for a file if we could do an up front check only once.
|
Ready for review. /cc @Pilchie @CyrusNajmabadi |
|
@CyrusNajmabadi @sharwell Anything else need to be done here? |
|
@alrz Separate Separate for now is good, but if nothing changes by 25 Sep ask again. |
There was a problem hiding this comment.
❗ This is a compiler error (returning value from void-returning method).
There was a problem hiding this comment.
I'll check, but I thought semantic errors were ok in targeted ide tests?
There was a problem hiding this comment.
They are allowed when we are specifically testing the IDE behavior when the code contains errors. In other cases I've seen them allow little bugs to sneak in where the test isn't doing what it looks like. Related to #21550.
There was a problem hiding this comment.
❓ What about other initializers?
var c = o as string;
int a = c != null ? c.Length : 0,
b = c != null ? c.Length + 1 : 0;
There was a problem hiding this comment.
Currently we pick the "leftmost" initializer/expression to simplify definitive assignment check. This can be extended later when we analyze the "flow" rather than just matching a "pattern".
There was a problem hiding this comment.
💭 Seems like this is still missing some (other places where expressions could appear). Not a problem IMO, but I'm thinking at the end of this sequence of PRs we should create an issue to describe the remaining work that would expand this to support "expressions" more generally.
|
I synced with master, this is ready to go in. thanks. |
|
test windows_release_unit32_prtest please |
|
test windows_debug_unit32_prtest please |
|
@MattGertz for ask mode approval |
Follow up: #19907