More test coverage for delegate invoke with conditional access #8586
Conversation
.. invocation fix/refactoring.. Notably, in cases where the code has a local declared and if the declaration has multiple variables or if the local is accessed outside of the invocation pattern, we do not touch the declaration in anyway. However we still simplify the delegate invocation pattern.
Renaming a helper, returning appropriately from a TryXXX() method etc., very minor code cleanups.
.. invocation fix/refactoring. Particularly around the pattern we look for - a local, immediately followed by a check for null and invocation. Adding some tests where the local declaration is grouped to the top and not grouped with the pattern itself.
|
@CyrusNajmabadi @dotnet/roslyn-ide |
.. that does not match the delegate invoke pattern we look for..
|
@CyrusNajmabadi : Please glance through the tests as well. I've taken the assumptions made in the code and turned them to unit tests to document current design. Such assumptions may or may not be intended, but I wouldn't know 😄 For e.g: we look for a strict pattern with removing locals -- that the declaration is in the line immediately preceding the null check and invoke pattern. If the local were grouped with other locals at the beginning of the block, we will not touch the declaration. This could be the intended design (it convincingly looks like that is the case), but there could be other cases where it wasn't really intended. So, if you find anything odd with the expectations of the unit tests I've added, Please call them out. Thanks. |
|
👍 |
|
Test only PR. Merging. |
More test coverage for delegate invoke with conditional access
This PR contains work done for #8309.