Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Foreach loop using | % {} syntax doesn't support continue #3879

Open
rdbartram opened this Issue May 30, 2017 · 10 comments

Comments

Projects
None yet
8 participants
@rdbartram
Copy link

rdbartram commented May 30, 2017

Steps to reproduce

@("1","2","3") | % { if($_ -ne "2") { continue }; $_ }

This happens in both PS Core and Win PS

Expected behavior

2 Should be output as it would if written so.
foreach($item in @("1","2","3")) { if($item -ne "2") { continue }; $item }

Actual behavior

Nothing is output

Environment data

PSVersion 6.0.0-beta
PSEdition Core
BuildVersion 3.0.0.0
CLRVersion
GitCommitId v6.0.0-beta.1
OS Microsoft Windows 10.0.15063
Platform Win32NT
PSCompatibleVersions {1.0, 2.0, 3.0, 4.0...}
PSRemotingProtocolVersion 2.3
SerializationVersion 1.1.0.1
WSManStackVersion 3.0

@powercode

This comment has been minimized.

Copy link
Collaborator

powercode commented May 30, 2017

Well, it isn't a loop.
You are using foreach-object cmdlet in a pipeline.

If continue should be valid there, it should also be valid in

& {
   if($a -eq 0){
       continue
   }
}

And that is not obviously correct.

@lzybkr

This comment has been minimized.

Copy link
Member

lzybkr commented May 30, 2017

break and continue work dynamically - meaning the break/continue statement searches for an appropriate loop to break from at runtime. For example:

function foo {
    break outer # No loop in this function!
}
:outer foreach ($i in 1..100) {
    $i
    foreach ($j in 100..1000) {
        foo
    }
}

This will output a single number, breaking out of the outer loop.

The unfortunate thing is that break sort of works as people expect with ForEach-Object, but in my opinion, it's sort of an accident that falls out from the implementation.

Under the covers, the break turns into an exception (always V2 or earlier, V3 onwards if not lexically within a loop statement), the exception is always silent (because you don't really want to think of break as an exception, so it's silent even if we don't find a matching loop.

@powercode

This comment has been minimized.

Copy link
Collaborator

powercode commented May 30, 2017

Cool, and really unexpected :)

@mklement0

This comment has been minimized.

Copy link
Contributor

mklement0 commented May 30, 2017

@rdbartram:

In a % (ForEach-Object) script block in a pipeline, return is the equivalent of continue:

> @("1","2","3") | % { if($_ -ne "2") { return }; $_ }
2

You can think of a script block as an (anonymous) function, so you can always return from it to proceed to the next object in the pipeline.

However, there is currently no analog to break for prematurely exiting a pipeline.

Overcoming this limitation is the subject of #3821.

Using the behavior described by @lzybkr, you can use the following (cumbersome) workaround:

# Exit the pipeline after the first 2 items.
do {
  @("1","2","3") | % { $_; if($_ -eq "2") { break } }
} while ($false)  # dummy loop to break out of

Note that you don't need the dummy loop on the command line, but if you neglected to use it in a script, it would break out of whatever enclosing loop there is, if any.
If there is none, the entire script is exited.

@rdbartram

This comment has been minimized.

Copy link
Author

rdbartram commented May 30, 2017

Thanks for all your inputs. I didn't even consider using return. It's good to know that another issue has been raised regarding this though and hopefully someone can add this little improvement to the language. thanks

@lzybkr

This comment has been minimized.

Copy link
Member

lzybkr commented May 31, 2017

It's possible somebody is relying on the existing semantics, so changing that might not be a good idea.

I've considered a strict mode error when break or continue is used in a way that turns into the non-local goto - the error message could suggest alternatives like return.

@rkeithhill

This comment has been minimized.

Copy link
Contributor

rkeithhill commented May 31, 2017

I've considered a strict mode error when break or continue is used in a way that turns into the non-local goto

@lzybkr Might also be good to have a PSSA rule for that - if it doesn't already exist.

cc @kapilmb

@GeeLaw

This comment has been minimized.

Copy link

GeeLaw commented May 31, 2017

You should regard this as Linq ForEach or JavaScript forEach(...). What you feed to ForEach-Object cmdlet is a ScriptBlock (delegate or function). Just as you won't

SomeCollection.ForEach((x) =>
{
    if (x != 2) continue;
    ResultCollection.Add(x);
});

You don't continue or break in a bare ScriptBlock. The scope is isolated.

@RobR14

This comment has been minimized.

Copy link

RobR14 commented Dec 11, 2018

I'm a little late to the party looking at the date (I came past this thread while looking for something else), but thought I might throw this in for the benefit of those on future expeditions...

My suggestion would be to look past the more terse lingual subtext (C#-like constructs) and use the pipeline in a more "intent-wise fashion" = i.e. "script-like"; I have found personally that while PS is sitting on .NET - not everything translates smoothly in the interpreter to a C# equivalent. So sometimes stepping back and "saying the same thing differently" can help expedite.

@("1","2","3") | ?{$_ -notmatch "2"} | out-host

This outputs 1 and 3 as expected (in response to the original "questioned" construct); and in this case, -ne works as well if preferred over the Regex equivalent shown in the example.

@vexx32

This comment has been minimized.

Copy link
Contributor

vexx32 commented Dec 11, 2018

Is it worth discussing whether behaviour of keywords like continue and break should be scope-restricted? After all, currently they literally cannot be handled whatsoever as several folks here have stated.

This is... unsettling, that calling a function from a module you've just downloaded could conceivably halt all running scripts right up to the global level, with no possible way of being able to account for it. This is what actual terminating errors are for in PowerShell, and we're (sensibly) given ways to handle them appropriately.

break and continue are the exception (badum/tss) here, and I don't think it's in a good way. Wouldn't it make a lot more sense for it to simply terminate the parent script block, function, or script -- but only the immediate level of execution? Searching beyond the parent function, script block, or script for something to break seems rife for exploitability and just generally breaking an awful lot of code that would and should otherwise be perfectly capable of handling similar terminating errors. 😦

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.