[syntax-errors] Named expressions in decorators before Python 3.9

[ntBre]

Summary

This PR detects the relaxed grammar for decorators proposed in PEP 614 on Python 3.8 and lower.

The 3.8 grammar for decorators is here:

decorators                ::=  decorator+
decorator                 ::=  "@" dotted_name ["(" [argument_list [","]] ")"] NEWLINE
dotted_name               ::=  identifier ("." identifier)*

in contrast to the current grammar here

decorators                ::= decorator+
decorator                 ::= "@" assignment_expression NEWLINE
assignment_expression ::= [identifier ":="] expression

Test Plan

New inline parser tests.

[github-actions]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

[codspeed-hq]

CodSpeed Performance Report

Merging #16386 will not alter performance

_{Comparing brent/syntax-decorators-39 (0585337) with main (d062388)}

Summary

✅ 32 untouched benchmarks

[AlexWaygood]

Hmm, I think "named expression" generally refers specifically to walrus expressions (https://peps.python.org/pep-0572 uses the term several times), so I'm not sure it's the best term to use here

[ntBre]

Yeah that makes sense, I was just following PEP 614 and the AST reference (which I see now actually calls them assignment_expressions, but I think that's another synonym for the walrus operator?).

Do you have any ideas for a replacement? I didn't think "relaxed decorator" was much more helpful, but that's closer to the PEP name and the "What's new" entry.

[AlexWaygood]

Maybe something like this?

impl Display for UnsupportedSyntaxError {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        let kind = match self.kind {
            UnsupportedSyntaxErrorKind::Match => "`match` statement",
            UnsupportedSyntaxErrorKind::Walrus => "named assignment expression (`:=`)",
            UnsupportedSyntaxErrorKind::ExceptStar => "`except*`",
            UnsupportedSyntaxErrorKind::RelaxedDecorator => {
                let expression_kind = ...;
                return write!(
                    "Cannot use {expression_kind} expressions in decorators on Python {} (syntax was added in Python 3.9)",
                    self.target_version,
               );   
            }      
        };
        write!(
            f,
            "Cannot use {kind} on Python {} (syntax was added in Python {})",
            self.target_version,
            self.kind.minimum_version(),
        )
    }
}

Then if it was e.g. a subscript expression, the error message would be

Cannot use subscript expressions in decorators on Python 3.8 (syntax was added in Python 3.9)

[ntBre]

I think I'll leave this nicer error reporting for a possible follow-up. It would complicate the code a fair amount to track the expression kind through the whole process just for the error message. Micha also pointed out on Discord that 3.9 is the oldest version receiving security updates, so this message will probably have a fairly limited audience.

I did remove the confusing "named expression" part and went for something closer to pyright's "Expression form not supported for decorator prior to Python 3.9":

Syntax Error: Unsupported expression in decorators on Python 3.8 (syntax was added in Python 3.9)

[AlexWaygood]

Nice, thanks. I still think there's room for improvement but this is definitely good enough for now!

[ntBre]

Absolutely, I meant to say I think your suggestion would make for the ideal error message. Thanks!

[AlexWaygood]

This was the trickiest one of these to detect yet. It seemed like the best approach was to attempt to parse the old version and fall back on the new grammar if anything goes wrong, but I'm not as confident in this approach since it required adding a new method to the parser.

Is it possible that this one would be easier to detect using the "greeter" approach that you proposed for syntax errors emitted by the compiler?

[ntBre]

Is it possible that this one would be easier to detect using the "greeter" approach that you proposed for syntax errors emitted by the compiler?

Hmm, maybe it would be slightly easier. We could store state indicating that we're in a decorator and then check if each visited component of the expression is an attribute access, with an optional top-level ExprCall.

Still, I think the approach here makes sense (at least to me 😆) and lets us keep the detection in the parser. It's just certainly trickier than some of the other errors like "is this a walrus operator." And I wasn't sure if I wrote idiomatic parser code or if this is exactly what the checkpoint system is meant for.

[MichaReiser]

I'd prefer not to use checkpoints here. Checkpoints are expensive.

I think I'd prefer to visit the parsed AST instead and check if any name isn't a named expression. One thing that might be challenging is that the outer-most expression can't be parenthesized, but that's something that we should be able to detect at the parse decorator function

[dhruvmanila]

I don't think it's just named expression. As per the grammar and the tests show this as well, one other example is a subscript expression like @foo[0] (top-level expression is subscript) or @foo[0].bar (top-level expression is an attribute) as both are not allowed.

I agree that it might be simplest to do this using a visitor instead of using speculative parsing.

I quickly looked at what Pyright does and it seems to be doing a basic check on the parsed expression if the target version is < 3.9: https://github.com/microsoft/pyright/blob/1d1b43c17a83927d422c9ff52f98fb595419e329/packages/pyright-internal/src/parser/parser.ts#L2401-L2417. I'm not sure if that covers the entire extent of the grammar but it might be worth exploring.

[ntBre]

It looks like pyright's _isNameOrMemberAccessExpression function called there is recursively checking the decorator expression. I guess that is a minimal visitor. When Alex mentioned the greeter approach, I was picturing holding off on this error until I start working on the separate greeter for compile-time/semantic errors. Should I just have a mini visitor directly in the parser like pyright instead?

[MichaReiser]

I don't mind a mini visitor in the parser, as long as it isn't a full Visitor based visitor (and only runs for < 3.9)

[ntBre]

Of course, I think I can pretty much follow pyright directly here.

[dhruvmanila]

Thanks for looking into it. I also think that's fine.

[AlexWaygood]

It might be interesting to add a test that uses a walrus expression in a decorator with the target version set to py37