Add Type Inference for $_ / $PSItem in catch{ } blocks by vexx32 · Pull Request #8020 · PowerShell/PowerShell

Rain Sallow (/u/ta11ow) (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

Ilya (iSazonov) · 2018-10-15T05:35:54Z

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

Jason Shirk (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.

Rain Sallow (/u/ta11ow) (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.

Jason Shirk (@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.

Ilya (iSazonov) · 2018-10-15T11:20:24Z

Rain Sallow (/u/ta11ow) (@vexx32) Please add test to demo that both cases work.

Rain Sallow (/u/ta11ow) (vexx32) · 2018-10-15T16:51:00Z

Jason Shirk (@lzybkr) Ilya (@iSazonov)

Added tests.

With Patrick Meinecke (@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! 😄

Jason Shirk (lzybkr) · 2018-10-15T17:49:25Z

Clever but simple, perfect. 👍👍👍

Ilya (iSazonov) · 2018-10-16T03:06:34Z

$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.

Ilya (iSazonov) · 2018-10-16T03:07:20Z

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!

Ilya (iSazonov) · 2018-10-16T04:01:10Z

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)

Rain Sallow (/u/ta11ow) (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. 😄

Rain Sallow (/u/ta11ow) (vexx32) requested review from Bruce Payette (BrucePay) and Dongbo Wang (daxian-dbw) as code owners October 13, 2018 18:49

Ilya (iSazonov) self-assigned this Oct 15, 2018

Ilya (iSazonov) requested a review from Jason Shirk (lzybkr) October 15, 2018 05:36

Jason Shirk (lzybkr) approved these changes Oct 15, 2018

View reviewed changes

Ilya (iSazonov) requested review from Steve Lee (SteveL-MSFT) and Aditya Patwardhan (adityapatwardhan) October 15, 2018 15:33

Rain Sallow (/u/ta11ow) (vexx32) force-pushed the TypeInference/Catch_PSItem branch from 9c1c9c3 to 586aae3 Compare October 15, 2018 16:49

Rain Sallow (/u/ta11ow) (vexx32) force-pushed the TypeInference/Catch_PSItem branch from 0e24c59 to dd61d52 Compare October 15, 2018 17:08

Ilya (iSazonov) reviewed Oct 16, 2018

View reviewed changes

Rain Sallow (/u/ta11ow) (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)

Rain Sallow (/u/ta11ow) (vexx32) force-pushed the TypeInference/Catch_PSItem branch from d09fb29 to b9eaea2 Compare October 16, 2018 13:50

Fix tests to use -HaveCount

589d471

Ilya (iSazonov) approved these changes Oct 16, 2018

View reviewed changes

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

Rain Sallow (/u/ta11ow) (vexx32) deleted the TypeInference/Catch_PSItem branch January 22, 2019 13:20

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

Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj/PowerShell that referenced this pull request Aug 6, 2026

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

2d38719

Uh oh!

Conversation

Rain Sallow (/u/ta11ow) (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!

Ilya (iSazonov) commented Oct 15, 2018

Uh oh!

Jason Shirk (lzybkr) left a comment

Choose a reason for hiding this comment

Uh oh!

Rain Sallow (/u/ta11ow) (vexx32) commented Oct 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Ilya (iSazonov) commented Oct 15, 2018

Uh oh!

Rain Sallow (/u/ta11ow) (vexx32) commented Oct 15, 2018

Uh oh!

Jason Shirk (lzybkr) commented Oct 15, 2018

Uh oh!

Ilya (iSazonov) Oct 16, 2018

Choose a reason for hiding this comment

Uh oh!

Ilya (iSazonov) Oct 16, 2018

Choose a reason for hiding this comment

Uh oh!

Rain Sallow (/u/ta11ow) (vexx32) Oct 16, 2018

Choose a reason for hiding this comment

Uh oh!

Ilya (iSazonov) Oct 16, 2018

Choose a reason for hiding this comment

Uh oh!

Rain Sallow (/u/ta11ow) (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!

Ilya (iSazonov) Oct 16, 2018

Choose a reason for hiding this comment

Uh oh!

Rain Sallow (/u/ta11ow) (vexx32) Oct 16, 2018

Choose a reason for hiding this comment

Uh oh!

Rain Sallow (/u/ta11ow) (vexx32) commented Oct 16, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Rain Sallow (/u/ta11ow) (vexx32) commented Oct 13, 2018 •

edited

Loading

Rain Sallow (/u/ta11ow) (vexx32) commented Oct 15, 2018 •

edited

Loading

Rain Sallow (/u/ta11ow) (vexx32) Oct 16, 2018 •

edited

Loading