ruff icon indicating copy to clipboard operation
ruff copied to clipboard

Ruff fails to reject invalid syntax in annotation scopes

Open JelleZijlstra opened this issue 1 year ago • 3 comments

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:

  • https://docs.astral.sh/ruff/rules/yield-outside-function/
  • https://docs.astral.sh/ruff/rules/await-outside-async/

JelleZijlstra avatar Apr 24 '24 00:04 JelleZijlstra

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.

JelleZijlstra avatar Apr 24 '24 00:04 JelleZijlstra

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" 😛

AlexWaygood avatar Apr 24 '24 00:04 AlexWaygood

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 avatar Apr 24 '24 05:04 dhruvmanila