diff --git a/conformance/third_party/conformance.exp b/conformance/third_party/conformance.exp index fc70a6729..5aa25c4a1 100644 --- a/conformance/third_party/conformance.exp +++ b/conformance/third_party/conformance.exp @@ -861,6 +861,16 @@ "stop_column": 79, "stop_line": 18 }, + { + "code": -2, + "column": 51, + "concise_description": "`GoodAlias4` is uninitialized", + "description": "`GoodAlias4` is uninitialized", + "line": 19, + "name": "unbound-name", + "stop_column": 61, + "stop_line": 19 + }, { "code": -2, "column": 79, @@ -871,6 +881,16 @@ "stop_column": 83, "stop_line": 19 }, + { + "code": -2, + "column": 40, + "concise_description": "`GoodAlias5` is uninitialized", + "description": "`GoodAlias5` is uninitialized", + "line": 22, + "name": "unbound-name", + "stop_column": 50, + "stop_line": 22 + }, { "code": -2, "column": 17, @@ -981,6 +1001,26 @@ "stop_column": 65, "stop_line": 45 }, + { + "code": -2, + "column": 40, + "concise_description": "`BadAlias4` is uninitialized", + "description": "`BadAlias4` is uninitialized", + "line": 46, + "name": "unbound-name", + "stop_column": 49, + "stop_line": 46 + }, + { + "code": -2, + "column": 44, + "concise_description": "`BadAlias5` is uninitialized", + "description": "`BadAlias5` is uninitialized", + "line": 47, + "name": "unbound-name", + "stop_column": 53, + "stop_line": 47 + }, { "code": -2, "column": 72, @@ -990,6 +1030,16 @@ "name": "bad-argument-type", "stop_column": 76, "stop_line": 47 + }, + { + "code": -2, + "column": 40, + "concise_description": "`BadAlias7` is uninitialized", + "description": "`BadAlias7` is uninitialized", + "line": 48, + "name": "unbound-name", + "stop_column": 49, + "stop_line": 48 } ], "aliases_variance.py": [], @@ -6002,6 +6052,26 @@ "stop_column": 27, "stop_line": 18 }, + { + "code": -2, + "column": 7, + "concise_description": "`T` is uninitialized", + "description": "`T` is uninitialized", + "line": 35, + "name": "unbound-name", + "stop_column": 8, + "stop_line": 35 + }, + { + "code": -2, + "column": 17, + "concise_description": "`T` is uninitialized", + "description": "`T` is uninitialized", + "line": 44, + "name": "unbound-name", + "stop_column": 18, + "stop_line": 44 + }, { "code": -2, "column": 17, diff --git a/conformance/third_party/conformance.result b/conformance/third_party/conformance.result index 08128d4d4..9d8249855 100644 --- a/conformance/third_party/conformance.result +++ b/conformance/third_party/conformance.result @@ -41,8 +41,6 @@ ], "aliases_typealiastype.py": [ "Line 44: Expected 1 errors", - "Line 46: Expected 1 errors", - "Line 48: Expected 1 errors", "Line 52: Expected 1 errors", "Line 53: Expected 1 errors", "Line 54: Expected 1 errors", @@ -58,7 +56,8 @@ "Line 64: Expected 1 errors", "Line 17: Unexpected errors ['Argument `tuple[type[TypeVar[T]]]` is not assignable to parameter `type_params` with type `tuple[ParamSpec | TypeVar | TypeVarTuple, ...]` in function `typing.TypeAliasType.__new__`']", "Line 18: Unexpected errors ['Argument `tuple[type[TypeVar[S]], type[TypeVar[T]]]` is not assignable to parameter `type_params` with type `tuple[ParamSpec | TypeVar | TypeVarTuple, ...]` in function `typing.TypeAliasType.__new__`']", - "Line 19: Unexpected errors ['Argument `tuple[type[TypeVar[T]]]` is not assignable to parameter `type_params` with type `tuple[ParamSpec | TypeVar | TypeVarTuple, ...]` in function `typing.TypeAliasType.__new__`']", + "Line 19: Unexpected errors ['`GoodAlias4` is uninitialized', 'Argument `tuple[type[TypeVar[T]]]` is not assignable to parameter `type_params` with type `tuple[ParamSpec | TypeVar | TypeVarTuple, ...]` in function `typing.TypeAliasType.__new__`']", + "Line 22: Unexpected errors ['`GoodAlias5` is uninitialized']", "Line 23: Unexpected errors ['Argument `tuple[type[TypeVar[S]], type[TypeVar[TStr]], type[ParamSpec[P]], type[TypeVarTuple[Ts]]]` is not assignable to parameter `type_params` with type `tuple[ParamSpec | TypeVar | TypeVarTuple, ...]` in function `typing.TypeAliasType.__new__`']", "Line 35: Unexpected errors ['Expected a type form, got instance of `GenericAlias`']", "Line 36: Unexpected errors ['Expected a type form, got instance of `GenericAlias`']", @@ -263,7 +262,6 @@ "generics_syntax_declarations.py": [], "generics_syntax_infer_variance.py": [], "generics_syntax_scoping.py": [ - "Line 35: Expected 1 errors", "Line 92: Expected 1 errors", "Line 95: Expected 1 errors", "Line 98: Expected 1 errors", diff --git a/conformance/third_party/results.json b/conformance/third_party/results.json index 563fd5b29..1ead1d839 100644 --- a/conformance/third_party/results.json +++ b/conformance/third_party/results.json @@ -3,7 +3,7 @@ "pass": 70, "fail": 66, "pass_rate": 0.51, - "differences": 293, + "differences": 291, "passing": [ "aliases_explicit.py", "aliases_newtype.py", @@ -80,7 +80,7 @@ "aliases_implicit.py": 6, "aliases_recursive.py": 17, "aliases_type_statement.py": 8, - "aliases_typealiastype.py": 25, + "aliases_typealiastype.py": 24, "aliases_variance.py": 4, "annotations_forward_refs.py": 7, "annotations_generators.py": 3, @@ -109,7 +109,7 @@ "generics_self_basic.py": 3, "generics_self_protocols.py": 1, "generics_self_usage.py": 12, - "generics_syntax_scoping.py": 7, + "generics_syntax_scoping.py": 6, "generics_typevartuple_basic.py": 1, "generics_typevartuple_concat.py": 1, "generics_typevartuple_specialization.py": 4, diff --git a/pyrefly/lib/binding/expr.rs b/pyrefly/lib/binding/expr.rs index caa691d37..2a538a690 100644 --- a/pyrefly/lib/binding/expr.rs +++ b/pyrefly/lib/binding/expr.rs @@ -217,6 +217,7 @@ impl<'a> BindingsBuilder<'a> { &mut self, name: &Identifier, value: Result, + used_in_static_type: bool, ) -> Idx { let key = Key::BoundName(ShortIdentifier::new(name)); if name.is_empty() { @@ -236,7 +237,7 @@ impl<'a> BindingsBuilder<'a> { // Don't check flow for global/nonlocal lookups if let Some(error_message) = self .scopes - .get_flow_style(&name.id) + .get_flow_style(&name.id, used_in_static_type) .uninitialized_error_message(name) { self.error( @@ -634,7 +635,11 @@ impl<'a> BindingsBuilder<'a> { let binding = self .lookup_name_usage(Hashed::new(&name.id), usage) .map(Binding::Forward); - self.ensure_name(&name, binding); + self.ensure_name( + &name, + binding, + matches!(usage, Usage::StaticTypeInformation), + ); } Expr::Yield(x) => { self.record_yield(x.clone()); @@ -671,7 +676,7 @@ impl<'a> BindingsBuilder<'a> { .lookup_name(Hashed::new(&name.id), LookupKind::Regular) .map(Binding::Forward), }; - self.ensure_name(&name, binding); + self.ensure_name(&name, binding, true); } Expr::Subscript(ExprSubscript { value, .. }) if self.as_special_export(value) == Some(SpecialExport::Literal) => diff --git a/pyrefly/lib/binding/scope.rs b/pyrefly/lib/binding/scope.rs index 6a8d9ee99..5786d4f39 100644 --- a/pyrefly/lib/binding/scope.rs +++ b/pyrefly/lib/binding/scope.rs @@ -798,10 +798,47 @@ impl Scopes { None } - pub fn get_flow_style(&self, name: &Name) -> &FlowStyle { + fn get_static_info(&self, name: &Name, should_skip_current_scope: bool) -> Option<&StaticInfo> { + let name = Hashed::new(name); + let mut iter = self.iter_rev(); + if should_skip_current_scope { + iter.next(); + } + for scope in iter { + if let Some(info) = scope.stat.0.get_hashed(name) { + return Some(info); + } + } + None + } + + /// Get the flow style for `name`, depending on whether `name` is used in a + /// static type. + /// + /// If we can find a flow info for `name`, return its style. Otherwise, we + /// check the static type information to see if we have a uninitialized + /// binding, in which case, `FlowStyle::Uninitialized` is returned. + /// Otherwise we return `FlowStyle::Other` to indicate no information + /// available. + pub fn get_flow_style(&self, name: &Name, used_in_static_type: bool) -> &FlowStyle { match self.get_flow_info(name) { Some(flow) => &flow.style, - None => &FlowStyle::Other, + None => { + // If the name is used for static type information, we can look + // at the current scope. + // Otherwise, we should skip the current scope, because it may + // permit a name to be used before it is defined. + if self.get_static_info(name, !used_in_static_type).is_some() { + // If we have a static binding, then we are in a scope where + // the name is defined, so we can return Other. + &FlowStyle::Other + } else { + // If we don't have a static binding, then we are in a scope + // where the name is not defined, so we return + // Uninitialized. + &FlowStyle::Uninitialized + } + } } } diff --git a/pyrefly/lib/binding/stmt.rs b/pyrefly/lib/binding/stmt.rs index 5332afb8c..6fad9e26a 100644 --- a/pyrefly/lib/binding/stmt.rs +++ b/pyrefly/lib/binding/stmt.rs @@ -153,12 +153,16 @@ impl<'a> BindingsBuilder<'a> { }) } - pub fn ensure_mutable_name(&mut self, x: &ExprName) -> Idx { + pub fn ensure_mutable_name(&mut self, x: &ExprName, usage: &mut Usage) -> Idx { let name = Ast::expr_name_identifier(x.clone()); let binding = self .lookup_name(Hashed::new(&name.id), LookupKind::Mutable) .map(Binding::Forward); - self.ensure_name(&name, binding) + self.ensure_name( + &name, + binding, + matches!(usage, Usage::StaticTypeInformation), + ) } fn define_nonlocal_name(&mut self, name: &Identifier) { @@ -271,7 +275,7 @@ impl<'a> BindingsBuilder<'a> { for target in &mut x.targets { let mut delete_link = self.declare_current_idx(Key::UsageLink(target.range())); if let Expr::Name(name) = target { - let idx = self.ensure_mutable_name(name); + let idx = self.ensure_mutable_name(name, delete_link.usage()); self.scopes.upsert_flow_info( Hashed::new(&name.id), idx, @@ -527,7 +531,7 @@ impl<'a> BindingsBuilder<'a> { .declare_current_idx(Key::Definition(ShortIdentifier::expr_name(name))); // Ensure the target name, which must already be in scope (it is part of the implicit dunder method call // used in augmented assignment). - self.ensure_mutable_name(name); + self.ensure_mutable_name(name, assigned.usage()); self.ensure_expr(&mut x.value, assigned.usage()); // TODO(stroxler): Should we really be using `bind_key` here? This will update the // flow info to define the name, even if it was not previously defined. diff --git a/pyrefly/lib/test/assign.rs b/pyrefly/lib/test/assign.rs index 68fd0c337..8c3323791 100644 --- a/pyrefly/lib/test/assign.rs +++ b/pyrefly/lib/test/assign.rs @@ -625,10 +625,17 @@ x3: B ); testcase!( - bug = "False negative", + test_use_before_write, + r#" +y # E: `y` is uninitialized +y = 42 + "#, +); + +testcase!( test_read_before_write, r#" -x = y # this should be an error +x = y # E: `y` is uninitialized y = 42 "#, ); @@ -757,7 +764,7 @@ def f(x): def f(x): x = 3 - x = "None" + x = "None" "#, ); diff --git a/pyrefly/lib/test/flow.rs b/pyrefly/lib/test/flow.rs index fdb7990bf..8f4cafc0d 100644 --- a/pyrefly/lib/test/flow.rs +++ b/pyrefly/lib/test/flow.rs @@ -1177,7 +1177,7 @@ def foo() -> list[int]: results: list[int] = [1, 2, 3] for i, x in enumerate(results): results[i] = x * 10 - return results + return results "#, ); @@ -1201,7 +1201,7 @@ bar: str = "bar" def func(): foo: str | None = None - + for x in []: for y in []: pass @@ -1226,7 +1226,7 @@ from typing import assert_type, Literal if 0.1: vari = "test" raise SystemExit -assert_type(vari, Literal["test"]) +assert_type(vari, Literal["test"]) # E: `vari` is uninitialized "#, ); diff --git a/pyrefly/lib/test/scope.rs b/pyrefly/lib/test/scope.rs index 38833eefc..543f91048 100644 --- a/pyrefly/lib/test/scope.rs +++ b/pyrefly/lib/test/scope.rs @@ -75,7 +75,7 @@ def f(): x += "a" # E: `x` is not mutable from the current scope def g(): global x - x += "a" + x += "a" def h0(): global x def h1(): @@ -173,12 +173,12 @@ def f(): global c1 global c2 # Should be permitted, the resulting operation is in-place - c0 += C() + c0 += C() # Should be permitted, the resulting operation returns a new C which is okay - c1 -= C() + c1 -= C() # Should *not* be permitted, this changes the type of the global in a way # that is incompatible with static analysis of the global scope - c2 *= C() + c2 *= C() f() # This shows what would go wrong if we allow the aug assign on `c2` assert_type(c2, C) @@ -234,7 +234,7 @@ def outer(): x += "a" # E: `x` is not mutable from the current scope def g(): nonlocal x - x += "a" + x += "a" def h0(): nonlocal x def h1(): @@ -258,7 +258,7 @@ def outer(): # A minor variation on f(), relevant to specific implementation bugs in our scope analysis def g(): nonlocal x - del x + del x f() f() # This will crash at runtime! "#, @@ -437,3 +437,13 @@ def f(arg: int) -> None: z # E: Could not find name `z` "#, ); + +testcase!( + test_forward_reference_ok, + r#" +def foo(): + x = y + +y = 42 +"#, +);