Skip to content

Make minor fixes in Compiler to properly handle void type expression#5764

Merged
daxian-dbw merged 2 commits intoPowerShell:masterfrom
daxian-dbw:fixarrayliteral
Jan 2, 2018
Merged

Make minor fixes in Compiler to properly handle void type expression#5764
daxian-dbw merged 2 commits intoPowerShell:masterfrom
daxian-dbw:fixarrayliteral

Conversation

@daxian-dbw
Copy link
Copy Markdown
Member

PR Summary

Make some minor fixes in Compiler to properly handle void type expressions.

Before Fix

> [void]1,2,3
An error occurred while creating the pipeline.
+ CategoryInfo          : NotSpecified: (:) [], ParentContainsErrorRecordException
+ FullyQualifiedErrorId : RuntimeException
> [void]1 | Measure-Object
An error occurred while creating the pipeline.
+ CategoryInfo          : NotSpecified: (:) [], ParentContainsErrorRecordException
+ FullyQualifiedErrorId : RuntimeException
## The generated code for `[void](mkdir c:\test)` is discarded
> $s = @([void](mkdir c:\test))
> Test-Path c:\test
False

After Fix

> $a = [void]1,2,3
> $a.Count
3
> [void]1 | Measure-Object

Count    : 0
Average  :
Sum      :
Maximum  :
Minimum  :
Property :
> $s = @([void](mkdir c:\test))
> $s.Count
0
> Test-Path C:\test
True

PR Checklist

Note: Please mark anything not applicable to this PR NA.


It "@([void](New-Item)) should create file" {
try {
$testFile = "$TestDrive\test.txt"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to see more reliable tests - could we use the file name based on guid or do $testFile | Should Not Exist?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't the contract with Pester to create a new directory for every It block? Assuming that's the case, reliability shouldn't be a concern.

Copy link
Copy Markdown
Collaborator

@iSazonov iSazonov Dec 31, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I see tons of files and directories in my temp folder I always wonder this though a debugger and Ctrl-C are the simplest ways to create trash. :-)
From docs:

  • TestDrive is a PowerShell PSDrive for file activity limited to the scope of a single Describe or Context block.

Also sometimes we have bugs with not closed and blocked files.

Copy link
Copy Markdown
Collaborator

@iSazonov iSazonov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leave a comment

Copy link
Copy Markdown
Contributor

@lzybkr lzybkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might consider testing the cast to [void] when it's not the first element, or all elements.

You might also consider testing a non-void method invocation cast to void as well as a void returning method invocation.

@iSazonov
Copy link
Copy Markdown
Collaborator

@daxian-dbw Is the PR ready to merge or you will continue?

@daxian-dbw
Copy link
Copy Markdown
Member Author

@iSazonov Sorry for the late response. I will address Jason's and your comments.

@daxian-dbw
Copy link
Copy Markdown
Member Author

@iSazonov @lzybkr Your comments have been addressed. Thank you!

@iSazonov
Copy link
Copy Markdown
Collaborator

iSazonov commented Jan 2, 2018

LGTM.

@daxian-dbw daxian-dbw merged commit ad231a8 into PowerShell:master Jan 2, 2018
@daxian-dbw daxian-dbw deleted the fixarrayliteral branch January 2, 2018 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants