[syntax-errors] Store to or delete __debug__
#16984
Conversation
Summary -- Detect setting or deleting `__debug__`. Assigning to `__debug__` was a `SyntaxError` on the earliest version I tested (3.8). Deleting `__debug__` was made a `SyntaxError` in [BPO 45000], which said it was resolved in Python 3.10. However, `del __debug__` was also a runtime error (`NameError`) when I tested in Python 3.9.6, so I thought it was worth including 3.9 in this check. I don't think it was ever a *good* idea to try `del __debug__`, so I think there's also an argument for not making this version-dependent at all. That would only simplify the implementation very slightly, though. [BPO 45000]: python/cpython#89163 Test Plan -- New inline tests. This also required adding a `PythonVersion` field to the `TestContext` that could be taken from the inline `ParseOptions` and making the version field on the options accessible.
ntBre
added
rule
|
I'd be in favor of just making it version independent but happy to hear others thoughts on this. |
| // # parse_options: {"target-version": "3.8"} | ||
| // del __debug__ | ||
|
|
||
| // test_ok read_from_debug |
There was a problem hiding this comment.
I think there are more cases that need handling where we don't use an ExprName in the AST
from x import __debug__
import __debug__
match x:
__debug__: ...
f(__debug__ = b)Finding the Identifier nodes is somewhat annoying because our visitor doesn't traverse Identifier nodes. You can take a look at where I believe I identified all Identifier nodes
There was a problem hiding this comment.
Ah good catch. I think I should have covered all of these now.
For match patterns, I moved the check into the MultipleCaseAssignmentVisitor, so it might be worth renaming that to something more generic like MatchPatternVisitor or just PatternVisitor now too.
| if type_param.name() == "__debug__" { | ||
| Self::add_error( | ||
| ctx, | ||
| SemanticSyntaxErrorKind::WriteToDebug(WriteToDebugKind::Store), | ||
| type_param.name().range, | ||
| ); | ||
| } |
There was a problem hiding this comment.
should we add a check_identifier function that handles the if name == "__debug__" handling to avoid some repetition?
Summary
Detect setting or deleting
__debug__. Assigning to__debug__was aSyntaxErroron the earliest version I tested (3.8). Deleting__debug__was made aSyntaxErrorin BPO 45000, which said it was resolved in Python 3.10. However,del __debug__was also a runtime error (NameError) when I tested in Python 3.9.6, so I thought it was worth including 3.9 in this check.I don't think it was ever a good idea to try
del __debug__, so I think there's also an argument for not making this version-dependent at all. That would only simplify the implementation very slightly, though.Test Plan
New inline tests. This also required adding a
PythonVersionfield to theTestContextthat could be taken from the inlineParseOptionsand making the version field on the options accessible.