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

Ruff fails to reject invalid syntax in annotation scopes #11118

Open
Tracked by #11934
JelleZijlstra opened this issue Apr 24, 2024 · 3 comments
Open
Tracked by #11934

Ruff fails to reject invalid syntax in annotation scopes #11118

JelleZijlstra opened this issue Apr 24, 2024 · 3 comments
Labels
linter Related to the linter rule Implementing or modifying a lint rule

Comments

@JelleZijlstra
Copy link
Contributor

Ruff produces no errors for these three lines:

type X[T: (yield 1)] = int
type Y = (yield 1)
def _f[T](x: (yield 1)) -> (y := 3): return x

But they are syntax errors in Python:

>>> type X[T: (yield 1)] = int
  File "<stdin>-0", line 1
SyntaxError: yield expression cannot be used within a TypeVar bound
>>> type Y = (yield 1)
  File "<stdin>-1", line 1
SyntaxError: yield expression cannot be used within a type alias
>>> def _f[T](x: (yield 1)) -> (y := 3): return x
... 
  File "<stdin>-2", line 1
SyntaxError: yield expression cannot be used within the definition of a generic

Producing errors for these cases feels low priority, because I don't know why anyone would do this other than to be difficult; I noticed it while looking at the parser. Still, I'd expect Ruff to tell me about all SyntaxErrors in my Python code.

Ruff has a few analogous rules for constructs that produce syntax errors at runtime:

@AlexWaygood AlexWaygood added bug Something isn't working parser Related to the parser labels Apr 24, 2024
@JelleZijlstra
Copy link
Contributor Author

Not sure if the "parser" label is appropriate here. This sort of thing cannot really be detected in the parser itself, because it requires contextual information that is only available once you have a parse tree. In CPython, these errors get detected during symtable construction. In Ruff, I think it makes the most sense to detect them in standard AST-based lint rules, similar to the two rules I mentioned above.

The AST nodes affected are yield, yield from, await, and the walrus. This applies to all forms of annotation scopes: TypeVar bounds/constraints, generic class bases, generic function annotations, and the values of type aliases.

@AlexWaygood
Copy link
Member

Fair point... In that case, I wonder if this is this even a "bug". Perhaps this could be classified as a "feature request for a new rule" 😛

@dhruvmanila
Copy link
Member

Thanks! So, as per the grammar this is a valid syntax which is why the parser doesn't raise a syntax error.

Module(
   body=[
      TypeAlias(
         name=Name(id='X', ctx=Store()),
         type_params=[
            TypeVar(
               name='T',
               bound=Yield(
                  value=Constant(value=1)))],
         value=Name(id='int', ctx=Load()))],
   type_ignores=[])

Correct me if I'm wrong but I believe that in CPython this is being done at compile time. There are a couple more syntax errors which we want to look into, so this should be a new rule. Also, all of these syntax errors which are being raised at compile time would be categorized in a "Correctness" (or similar) category after #1774 is done.

@dhruvmanila dhruvmanila added rule Implementing or modifying a lint rule linter Related to the linter and removed bug Something isn't working labels Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
linter Related to the linter rule Implementing or modifying a lint rule
Projects
None yet
Development

No branches or pull requests

3 participants