dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
19.02k stars 4.03k forks source link

Proposal: guard statement #11562

Closed gafter closed 8 years ago

gafter commented 8 years ago

When reviewing the scoping rules for out variables and pattern variables, the LDM discovered that there is sometimes an unfortunate interaction with a pattern of coding that we call the guard pattern. The guard pattern is a coding pattern where you test for the exceptional conditions early, handle them, and then bypass the rest of a method by returning, the rest of a loop by continuing, etc. The idea is to not have to deeply nest the "normal" control path. You'd write code like this:

    int parsedValue;
    if (!int.TryParse(s, out parsedValue))
    {
        // handle or report the problem
        // this block does not complete normally
    }

    // use parsedValue here

To take advantage of the out var feature, and avoid having to write the type of the out variable, you'd like to write

    if (!int.TryParse(s, out var parsedValue))
    {
        // handle or report the problem
        return;
    }

    // use parsedValue here

but that doesn't work because parsedValue isn't in scope in the enclosing statement. Similar issues arise when using pattern-matching:

    Tuple<string, int> expression = ...;
    if (!(expression is @(string key, int value))
    {
        // either the tuple was null or the string was null.
        // handle the problem
        continue;
    }

    // use key, value here

(The @ here assumes that tuple patterns are preceded by an @ character)

but that doesn't work because key and value aren't in scope after the if statement.

Swift addresses this category of issue by introducing the guard statement. Translating that into the concepts we've been proposing for C# with tuples and pattern-matching, the equivalent construct would be a new kind of statement:

statement
    : guard_statement
    ;
guard_statement
    : 'guard' '(' expression ')' 'else' embedded_statement
    ;

Open Issue: should we require parens around the expression as part of the syntax?

A guard statement is like an inverted if statement, or an if statement without a "then" block. It has the following semantics:

  1. The expression of a _guardstatement must be a boolean expression.
  2. The _embeddedstatement is reached when the expression is false.
  3. The statement following the _guardstatement is reached when the expression is true.
  4. The _embeddedstatement may not complete normally.
  5. Any pattern variables and out variables declared in expression are in the scope of the block enclosing the _guardstatement.

This would allow you to handle the previous examples as follows:

    guard (int.TryParse(s, out var parsedValue))
    else {
        // handle or report the problem
        return;
    }

    // use parsedValue here
    Tuple<string, int> expression = ...;
    guard (expression is @(string key, int value))
    else {
        // either the tuple was null or the key was null.
        // handle the problem
        continue;
    }

    // use key, value here

/cc @dotnet/ldm

Thaina commented 8 years ago

@qrli Your idea is neat But it not satisfied the proposal to do some work and log something before return or throw

vladd commented 8 years ago

@Thaina Perhaps

let int.TryParse(s, out var parsedValue) else { Log.Something(); return; }
Richiban commented 8 years ago

I don't like this idea that variables can start escaping their scope (or what it looks like their scope should be) if certain keywords were used... I think it makes the code more difficult to read.

I'm not sure it deserves a new language keyword / construct just to avoid having to write

int parsedValue;
if (!int.TryParse(s, out parsedValue))
    return;

// Do something with parsedValue

Now that the language has Tuples, why don't we instead argue that methods with out parameters now appear to return Tuples? (FYI this is what FSharp does.) After all, the only reason out parameters exist is because the language didn't have tuples from the beginning. For example, calls to Int32.TryParse would become:

var (success, parsedValue) = int.TryParse(s);

if(!success) return;

// Do something with parsedValue

So much cleaner and easier to understand, especially with pattern matching (if I can take a guess at the syntax:

switch(int.TryParse(s))
{
    case (false, *) : return;
    case (true, parsedValue) : // Do something with parsedValue
}
HaloFour commented 8 years ago

@Richiban

Per #12597 the conversation is probably mostly moot. Now the scope will sometimes leak out into their enclosing scope based on which constructs you use:

if (!int.TryParse(s, out int parsedValue))
    return;

// use parsedValue here

I personally don't agree with it. It's inconsistent with existing C# behavior, it's inconsistent with itself and it's inconsistent with the rest of the C-family of languages. I'd rather the team did nothing at all and if the user intended to "leak" the scope that they would declare the variable separately, just as you demonstrated.

Richiban commented 8 years ago

@HaloFour that's a shame...

By the way,

I personally don't disagree with it

did you mean "I personally don't agree with it"?

HaloFour commented 8 years ago

@Richiban

did you mean "I personally don't agree with it"?

That I did.

gafter commented 8 years ago

Closing, as this is no longer useful with the new scoping rules.