[semantic-syntax-tests] IrrefutableCasePattern, SingleStarredAssignment, WriteToDebug, InvalidExpression#17748
Conversation
|
IrrefutableCasePattern, SingleStarredAssignment, WriteToDebug, and InvalidExpression
IrrefutableCasePattern, SingleStarredAssignment, WriteToDebug, and InvalidExpressionIrrefutableCasePattern, SingleStarredAssignment, WriteToDebug, InvalidExpression
|
ntBre
left a comment
There was a problem hiding this comment.
Thanks! I had one suggestion about the yield from cases and a nit you can ignore if you prefer it this way.
crates/red_knot_python_semantic/resources/mdtest/diagnostics/semantic_syntax_errors.md
Outdated
Show resolved
Hide resolved
...emantic_syntax_error_InvalidExpression_invalid_expression_yield_from_in_base_class_3.10.snap
Outdated
Show resolved
Hide resolved
1325207 to
18132e7
Compare
for "Irrefutable Case Pattern", "Single Starred Assignment", "Write to Debug", and "Invalid Expression"
as tests were renamed to avoid name collision.
18132e7 to
b2374b1
Compare
|
I went with the generic type parameter and pinned the necessary Python version, |
|
If this is good, it also closes #17526, I guess. |
ntBre
left a comment
There was a problem hiding this comment.
Thanks! I was just going to push the small tweak to the parens in the ruff case, but then I noticed the mdtest code block issue. Could you update those? Then this should be ready to go.
crates/ty_python_semantic/resources/mdtest/diagnostics/semantic_syntax_errors.md
Outdated
Show resolved
Hide resolved
instead of python; so the code blocks actually run
crates/ty_python_semantic/resources/mdtest/diagnostics/semantic_syntax_errors.md
Show resolved
Hide resolved
ntBre
left a comment
There was a problem hiding this comment.
Thanks again! The new errors prompted one question, mostly for Alex or someone else on the ty team. I also had one suggestion about the "outside of a function" errors, but this is just about ready to go.
| ```py | ||
| # error: [invalid-type-form] "`yield` expressions are not allowed in type expressions" | ||
| # error: [invalid-syntax] "yield expression cannot be used within a TypeVar bound" | ||
| # error: [invalid-syntax] "`yield` statement outside of a function" |
There was a problem hiding this comment.
nit: I'd probably just wrap this in an outer function to suppress these outside of a function errors since we only intend to test the other errors here. I think a lot of other mdtests use a function named _ to indicate this:
def _():
type X[T: (yield 1)] = intbut it's not a huge deal.
There was a problem hiding this comment.
I think there is still something here. With
def _():
# error: [invalid-type-form] "`yield` expressions are not allowed in type expressions"
# error: [invalid-syntax] "yield expression cannot be used within a TypeVar bound"
type X[T: (yield 1)] = int
def _():
# error: [invalid-type-form] "`yield` expressions are not allowed in type expressions"
# error: [invalid-syntax] "yield expression cannot be used within a type alias"
type Y = (yield 1)I still get the outside of a function errors
semantic_syntax_errors.md - Semantic syntax error diagnostics - Invalid expression
crates/ty_python_semantic/resources/mdtest/diagnostics/semantic_syntax_errors.md:304 unexpected error: 16 [invalid-syntax] "`yield` statement outside of a function"
crates/ty_python_semantic/resources/mdtest/diagnostics/semantic_syntax_errors.md:309 unexpected error: 15 [invalid-syntax] "`yield` statement outside of a function"There was a problem hiding this comment.
Ah, that's a bug, thank you! We can merge this as is, and I'll tackle it in a follow-up.
If you're curious, ty has separate scope kinds for type aliases and annotations, so the check for function scope needs to skip those instead of just checking that the immediate scope is a function. At least that looks like the easy fix to me.
| # error: [invalid-type-form] "`yield` expressions are not allowed in type expressions" | ||
| # error: [invalid-syntax] "yield expression cannot be used within a type alias" |
There was a problem hiding this comment.
This doesn't necessarily need to block this PR since you're just reporting the current state of things, but we might need to open a follow-up issue here to avoid duplicate diagnostics. @AlexWaygood do you know if the type error is separate from the syntax error here, or should we only pick one of these to emit?
There was a problem hiding this comment.
I talked to Alex, and this is a symptom of a more general need to suppress type-checking errors when there are syntax errors present. I'll spin off an issue for this too, so no worries here.
There was a problem hiding this comment.
The issue already exists in the ty repo! astral-sh/ty#119
|
One duplicate test case had slipped in, which i removed in 96aeedc. |
Re: #17526
Summary
Add integration test for semantic syntax for
IrrefutableCasePattern,SingleStarredAssignment,WriteToDebug, andInvalidExpression.Notes
Following @ntBre's suggestion, I will keep the test coming in batches like this over the next few days in separate PRs to keep the review load per PR manageable while also not spamming too many.
I did not add a test for
del __debug__which is one of the examples incrates/ruff_python_parser/src/semantic_errors.rs:1051.For python version
<= 3.8there is no error and for>=3.9the error is notWriteToDebugbutSyntaxError: cannot delete __debug__ on Python 3.9 (syntax was removed in 3.9).The
blacken-docsbypass is necessary because otherwise the test does not pass pre-commit checks; but we want to check for this faulty syntax.Test Plan
This is a test.