pyrefly icon indicating copy to clipboard operation
pyrefly copied to clipboard

Allow global before variable is defined

Open ndmitchell opened this issue 1 month ago • 4 comments

Describe the Bug

def main():
    global table
    table = 1
    print(table)

main()

This works fine, but Pyrefly complains Could not find name "table"

Sandbox Link

https://pyrefly.org/sandbox/?project=N4IgZglgNgpgziAXKOBDAdgEwEYHsAeAdAA4CeSIAOljGAAQC2qE6AFAJSLV090DmUXNlRQ6AF1TZY3XhKkw6AXjoBGGT2IAnFmNZzY7atSYsOIADQgArmOhwS5RCADEdAKq2oEMaTpgr6ADGtrjocEY09GC4mkxiAProVgzYMJqs%2BIh0Oux0ALQAfHRwYppc6Lx0mjBiVpoVYJQgAHLJqWV0wPgAvk3UFiBk1WBQpIRiuAxQFK4ACqTDo8UYOAR0gaGQfHWoIeiE1K4AyjAKABZiYsRwiAD0t0O0o4QxfLcw6LeYuIFwtxvoLY7Pa3PwxOioABuzCgklg602EG2ml2EFCdFwxD29moZDEZ1CeUhaTgaIqyiaAGZCCoAEx9dAgbqWVDBCDEgBi0BgFDQWDwRDITKAA

(Only applicable for extension issues) IDE Information

No response

ndmitchell avatar Nov 27 '25 10:11 ndmitchell

Did this come up from an example project?

I brought this up after @yangdanny97 patched an internal file using this pattern, but we weren't confident it was worth implementing based on just one example, it's pretty easy to fix by adding an annotation.

At any rate there are open questions about how to implement it - should table be treated as globally visible, or only available in the scope of main? Should reads of it without a global be considered valid in other functions? Depending what we decide to do, the Definitions phase might be forced to do a full AST scan which has some downsides

Looks like basedpyright allows all reads, unsoundly - this program definitely crashes on a NameError but isn't flagged. Doing it this way would probably require visiting the entire AST in definitions

stroxler avatar Nov 27 '25 15:11 stroxler

I found another internal example with this problem. I've sent you the details. Happy to patch if we consider it rare and bad practice.

ndmitchell avatar Nov 28 '25 11:11 ndmitchell

I've changed my mind on this issue, I think matching basedpyright here makes sense to avoid giving large numbers of false-positives on code that uses this (anti) pattern

yangdanny97 avatar Nov 28 '25 14:11 yangdanny97

I don't have strong opinions either way.

The second internal example is actually using the globals in other functions defined well above the original function, not just as persistent variables across calls - so it looks something like this:

def f():
     print(VAR)

def main():
     global VAR = sys.args[1]

This example does suggest that Definitions has to do a full AST traversal, because there's no way we could know for sure whether f is using an undefined variable without analyzing every token of the module in advance.

stroxler avatar Nov 28 '25 15:11 stroxler