Review impact of pattern matching enhancements on existing rules

[sharwell]

https://docs.microsoft.com/en-us/dotnet/csharp/whats-new/csharp-8#more-patterns-in-more-places

• Switch expressions
• Property patterns
• Tuple patterns
• Positional patterns

[DavidArno]

@sharwell,

Did this review occur? I've recently started using the stylecop analyzers and I'm hitting the issue that SA1012 and SA1013 object the “empty” property pattern (patterns of the form x is {} y), so I'm looking for a way to have those rules apply to non pattern cases only. Is that possible, or would that require changes to the analyzers? It's likely work that I'd be happy to tackle if it does need changes.

Thanks.

[sharwell]

SA1012 and SA1013 already appeared handled the empty pattern correctly, so no changes to the implementation were required. It would be nice to add some tests for empty pattern cases.

[DavidArno]

Oh, OK. Maybe I'm configuring things wrongly. I want those analyzers to complain if I eg write:

public MyClass(T x) : base(x) {}

as I want a space between { and } there. But The C# team's docs on the empty property pattern have no space and I wanted to follow that convention, so I don't want those analyzers to complain if I do:

if (x is {} nonNullX) ...

But for me they are complaining.

[sharwell]

But The C# team's docs on the empty property pattern have no space and I wanted to follow that convention

This is a documentation bug, not an intentional convention. If you try to write {}, you'll also see IDE0055 (Fix formatting). Can you point us to the page where you are seeing this?

[DavidArno]

Sure, it's https://devblogs.microsoft.com/dotnet/do-more-with-patterns-in-c-8-0/

[sharwell]

Thanks for the link! I checked with the author and the blog post was written before we finalized the formatting recommendation. We'll go back and edit it to add a space within the circled instance of {} to help avoid confusion:

[DavidArno]

Thanks, Sam. I'll therefore just adjust my own approach to start adding a space to fit in with MS's formatting recommendations here.

Thanks for your help with this.

[benrr101]

Not sure if there's another issue open for this, but I figure necrobumping is better than creating spam. SA1013 gets falsey triggered on the new syntax as per the following:

switch (obj)
{
    // Raises SA1013 for closing brace not being followed by space
    case TypeWithSizeProperty { Size: >123 }:

    // Raises SA1024 for colon with space before it
    case TypeWithSizeProperty { Size: <=123} :
        break;
}

[bjornhellander]

@benrr101 I am unable to reproduce this with 1.2.0-beta.507. Which version are you using? Note that 1.1.118 is too old for newer language constructs, so a beta version is required to not get false positives like this.

[benrr101]

@bjornhellander Yep, turns out we're using 1.1.118. I'll give it a shot in a beta version. Though I'm not entirely sure if we'll be allowed to use it for production builds (our build pipelines are weird). Is there a timeline to making the beta release not-beta? No worries if not. Thanks for looking into the issue for me 👍

[bjornhellander]

@benrr101 It seems it will take time. See for example #3500.

[sharwell]

Though I'm not entirely sure if we'll be allowed to use it for production builds

There are no shipping components for StyleCop Analyzers, so it shouldn't impact production builds (and NuGet will not warn for a correctly-configured use of StyleCop Analyzers even if you are using it during the production of a non-beta package). If you have an artificial policy impacted the use of a beta build even though it's designed to fit within the design requirements of the packaging system, you can use StyleCop.Analyzers.Unstable instead. It's the same package, just with a different name to work around policies.

[vgriph]

I'm not sure if this is the right place to discuss this, but with 1.2.0-beta.507, the enforced spacing around braces in index patterns fells wrong to me.

I would like to write a pattern like this:

value is SqlSimpleCaseExpression
    {
        TestExpression: SqlScalarRefExpression test,
        WhenClauses:
        [
            {
                WhenExpression: SqlLiteralExpression
                {
                    Type: LiteralValueType.Integer,
                    Value: { } when
                },
                ThenExpression: SqlScalarRefExpression reference,
            }
        ],
        ElseExpression: SqlLiteralExpression { Type: LiteralValueType.Null }
    }

But 1.2.0-beta.507 force me to write it

value is SqlSimpleCaseExpression
    {
        TestExpression: SqlScalarRefExpression test,
        WhenClauses:
        [{
                WhenExpression: SqlLiteralExpression
                {
                    Type: LiteralValueType.Integer,
                    Value: { } when
                },
                ThenExpression: SqlScalarRefExpression reference,
            }

        ],
        ElseExpression: SqlLiteralExpression { Type: LiteralValueType.Null }
    }

or

value is SqlSimpleCaseExpression
    {
        TestExpression: SqlScalarRefExpression test,
        WhenClauses:
        [{
                WhenExpression: SqlLiteralExpression
                {
                    Type: LiteralValueType.Integer,
                    Value: { } when
                },
                ThenExpression: SqlScalarRefExpression reference,
            },
        ],
        ElseExpression: SqlLiteralExpression { Type: LiteralValueType.Null }
    }

Due to the SA1012: Opening brace should not be preceded by space and SA1513: Closing brace should be followed by blank line.

Is this the right place to bring this up, or should I open a separate issue for it?