Add Type Inference for $_ / $PSItem in catch{ } blocks #8020

vexx32 · 2018-10-13T18:49:02Z

PR Summary

Adds type inference results to $_ / $PSItem when it is used to refer to the ErrorRecord in a catch block.
Adds test for the type inference.
Includes type inference for typed catch [ExceptionType] { $_ } blocks such that $_ is inferred as a dummy ErrorRecord<TException> which is identical to ErrorRecord but with a TException-typed Exception member.
- In other words, using typed catch blocks correctly autocompletes the members for $_.Exception.<tab>

Example:

PS> try {throw 'ball'} catch {$_.<tab>
# completes to:
PS> try {throw 'ball'} catch {$_.CategoryInfo

(Compare current behaviour, where tab completion is missing due to a lack of type inference on the $_ / $PSItem variable.)

PR Checklist

iSazonov · 2018-10-15T05:35:54Z

Perhaps we need a test that try {throw 'ball'} catch {$d=Get-Date; $_.CategoryInfo works.

lzybkr

It would be cool to also add special handling for:

try {}
catch [SomeSpecificException] { $_.Exception.<TAB> }

I realize this could be a bit messy though and I'm not sure it's commonly needed, so maybe it's not worth it.

vexx32 · 2018-10-15T10:18:50Z

Can you expand upon that? As-is, the currently available logic handles this as you'd expect, completing the members for a generic exception object.

Are there some exceptions with additional members we would want to try to detect from the exception type handed to the catch block itself? I'm not aware of any, but I'm sure there are probably some or could have some definitely in the case of custom exception types as well.

@lzybkr I don't think this would go quite in the same place on this code (would it?), so if you could point out where it would need to be added, I'll give it a whirl.

iSazonov · 2018-10-15T11:20:24Z

@vexx32 Please add test to demo that both cases work.

vexx32 · 2018-10-15T16:51:00Z

@lzybkr @iSazonov

Added tests.

With @SeeminglyScience's help, also added in some type inference for typed catch blocks. He came up with the idea of a dummy generic ErrorRecord<T> class that could be used just for type inference in order to get the members of the typed exceptions.

Pretty happy with it! 😄

lzybkr · 2018-10-15T17:49:25Z

Clever but simple, perfect. 👍👍👍

iSazonov · 2018-10-16T03:06:34Z

test/powershell/engine/Api/TypeInference.Tests.ps1

$res | Should -HaveCount 1

Could you please address the comment?
Line 941 too.

All done. There are a few tests for other things that should probably use this as well, but I think that might be better left to another PR specifically for test revisions.

iSazonov · 2018-10-16T03:07:20Z

test/powershell/engine/Api/TypeInference.Tests.ps1

Please add more catch-es.

Reformatted this for additional Test Cases with a variety of exception types.

~~Would you like me to add additional tests verifying that constructs like catch [type1], [type2] {} and subsequent catch blocks all work as expected?~~

Upon having this thought, I just went ahead and did it; figured it better be as robust as we can make it!

iSazonov · 2018-10-16T04:01:10Z

test/powershell/engine/Api/TypeInference.Tests.ps1

I meant:

"try {} catch [System.OverflowException] catch [System.TimeoutException] { $_.Exception }"

See additional below tests. I suppose I only test catch [type] {$_.Exception} catch {$_.Exception} but if you want me to go through a couple stacks I'm game. :)

Add dummy generic errorrecord class for type inference Add logic for inferring errorrecord/exception type Add tests for typed catch block inference Tidy Exception<E> class

@SeeminglyScience

Update generic type argument Update tests. (thanks @SeeminglyScience!)

Reformat test strings Remove [type] specifier from params (not needed)

vexx32 · 2018-10-16T13:52:28Z

Appveyor caught something funky, don't think it was related. Rebased to restart it.

Other than that... everything looks like it works as it ought. 😄

vexx32 requested review from BrucePay and daxian-dbw as code owners October 13, 2018 18:49

iSazonov self-assigned this Oct 15, 2018

iSazonov requested a review from lzybkr October 15, 2018 05:36

lzybkr approved these changes Oct 15, 2018

View reviewed changes

iSazonov requested review from SteveL-MSFT and adityapatwardhan October 15, 2018 15:33

vexx32 force-pushed the TypeInference/Catch_PSItem branch from 9c1c9c3 to 586aae3 Compare October 15, 2018 16:49

vexx32 force-pushed the TypeInference/Catch_PSItem branch from 0e24c59 to dd61d52 Compare October 15, 2018 17:08

iSazonov reviewed Oct 16, 2018

View reviewed changes

vexx32 added 8 commits October 16, 2018 09:49

Fix tab completions

a9d4fa4

Add tests for type inference of $_ in catch blocks

0a4f93e

Add test for System.Exception completion from un-typed catch block

61af646

Add dummy generic errorrecord class for type inference Add logic for inferring errorrecord/exception type Add tests for typed catch block inference Tidy Exception<E> class

Reformat constructor for StyleCop

ea2ebe2

Update generic type argument Update tests. (thanks @SeeminglyScience!)

Add testcases to verify that type inference works with common exceptions

c35a78c

Reformat slightly for less confusing syntax

2fa0169

Add further tests just to be absolutely sure

1f0db69

Fix tests

b9eaea2

Reformat test strings Remove [type] specifier from params (not needed)

vexx32 force-pushed the TypeInference/Catch_PSItem branch from d09fb29 to b9eaea2 Compare October 16, 2018 13:50

Fix tests to use -HaveCount

589d471

iSazonov approved these changes Oct 16, 2018

View reviewed changes

iSazonov merged commit 33f2f0f into PowerShell:master Oct 18, 2018

vexx32 deleted the TypeInference/Catch_PSItem branch January 22, 2019 13:20

iSazonov added the CL-Engine Indicates that a PR should be marked as an engine change in the Change Log label Feb 1, 2019

Add Type Inference for $_ / $PSItem in catch{ } blocks #8020

Add Type Inference for $_ / $PSItem in catch{ } blocks #8020

Uh oh!

Conversation

vexx32 commented Oct 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Checklist

Uh oh!

iSazonov commented Oct 15, 2018

Uh oh!

lzybkr left a comment

Choose a reason for hiding this comment

Uh oh!

vexx32 commented Oct 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iSazonov commented Oct 15, 2018

Uh oh!

vexx32 commented Oct 15, 2018

Uh oh!

lzybkr commented Oct 15, 2018

Uh oh!

iSazonov Oct 16, 2018

Choose a reason for hiding this comment

Uh oh!

iSazonov Oct 16, 2018

Choose a reason for hiding this comment

Uh oh!

vexx32 Oct 16, 2018

Choose a reason for hiding this comment

Uh oh!

iSazonov Oct 16, 2018

Choose a reason for hiding this comment

Uh oh!

vexx32 Oct 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iSazonov Oct 16, 2018

Choose a reason for hiding this comment

Uh oh!

vexx32 Oct 16, 2018

Choose a reason for hiding this comment

Uh oh!

vexx32 commented Oct 16, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vexx32 commented Oct 13, 2018 •

edited

Loading

vexx32 commented Oct 15, 2018 •

edited

Loading

vexx32 Oct 16, 2018 •

edited

Loading