From cff5a262a2bdd3801e27b641dec8c0f4321c486f Mon Sep 17 00:00:00 2001 From: Mangesh Raut Date: Wed, 28 Oct 2026 03:45:18 +0000 Subject: [PATCH 1/4] Diagnose instance methods missing a positional receiver Report bad-function-definition when a class instance method has no positional parameter and no *args to accept the implicit self. Skip classmethods, staticmethods, @no_type_check, and methods with unresolved custom decorators that may reshape the signature. Fixes #5295 --- pyrefly/lib/alt/function.rs | 26 +++++++++++ pyrefly/lib/test/abstract_methods.rs | 4 +- pyrefly/lib/test/attributes.rs | 67 +++++++++++++++++++++++++++- pyrefly/lib/test/callable.rs | 2 +- pyrefly/lib/test/class_overrides.rs | 4 +- pyrefly/lib/test/constructors.rs | 4 +- pyrefly/lib/test/descriptors.rs | 4 +- pyrefly/lib/test/narrow.rs | 4 +- 8 files changed, 103 insertions(+), 12 deletions(-) diff --git a/pyrefly/lib/alt/function.rs b/pyrefly/lib/alt/function.rs index 3d91c68246..b94a04f385 100644 --- a/pyrefly/lib/alt/function.rs +++ b/pyrefly/lib/alt/function.rs @@ -606,6 +606,32 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { self_type = None; } + // Instance methods expect an implicit receiver. When there is no positional + // parameter and no `*args` to accept it, report a definition error. Skip + // classmethods (see #5294), staticmethods, `@no_type_check`, and methods + // wrapped by unresolved custom decorators that may reshape the signature. + // See https://github.com/facebook/pyrefly/issues/5295. + if defining_cls.is_some() + && self_type.is_some() + && !flags.is_classmethod + && !flags.is_staticmethod + && !flags.has_no_type_check + && decorators.is_empty() + && def.parameters.posonlyargs.is_empty() + && def.parameters.args.is_empty() + && def.parameters.vararg.is_none() + { + self.error( + errors, + def.name.range(), + ErrorKind::BadFunctionDefinition, + format!( + "Instance method `{}` must accept a positional argument", + def.name + ), + ); + } + // The `self`/`cls` receiver of a method is supplied implicitly at call time, so a // default value on it is unreachable and almost always a mistake (e.g. `def m(self=1)`). // `self_type` is `Some` exactly when there is an implicit receiver (instance methods, diff --git a/pyrefly/lib/test/abstract_methods.rs b/pyrefly/lib/test/abstract_methods.rs index f405834708..650bfbb5eb 100644 --- a/pyrefly/lib/test/abstract_methods.rs +++ b/pyrefly/lib/test/abstract_methods.rs @@ -809,11 +809,11 @@ from abc import ABC, abstractmethod class A1(ABC): @abstractmethod - def a(): ... + def a(): ... # E: Instance method `a` must accept a positional argument class B1(ABC): @abstractmethod - def b(): ... + def b(): ... # E: Instance method `b` must accept a positional argument class C(A1, B1): ... diff --git a/pyrefly/lib/test/attributes.rs b/pyrefly/lib/test/attributes.rs index a7733fafa1..14c0c0d184 100644 --- a/pyrefly/lib/test/attributes.rs +++ b/pyrefly/lib/test/attributes.rs @@ -584,12 +584,77 @@ testcase!( test_missing_self_parameter, r#" class C: - def f(): + def f(): # E: Instance method `f` must accept a positional argument pass C().f() # E: Expected 0 positional arguments, got 1 (including implicit `self`) "#, ); +// https://github.com/facebook/pyrefly/issues/5295 +testcase!( + test_instance_method_missing_receiver, + r#" +class Foo: + def fn() -> None: ... # E: Instance method `fn` must accept a positional argument + + def kwargs_only(**kwargs: object) -> None: ... # E: Instance method `kwargs_only` must accept a positional argument + + def keyword_only(*, x: object = None) -> None: ... # E: Instance method `keyword_only` must accept a positional argument + + def conventional(self) -> None: ... + + def unconventional(this) -> None: ... + + def positional_only(this, /) -> None: ... + + def variadic(*args: object) -> None: ... + + @staticmethod + def static() -> None: ... + + def outer(self) -> None: + def inner() -> None: ... + inner() + +Foo().conventional() +Foo().unconventional() +Foo().positional_only() +Foo().variadic(1) +Foo.static() + "#, +); + +testcase!( + test_instance_method_missing_receiver_decorators, + r#" +from typing import Any, Callable, final +from abc import ABC, abstractmethod + +def add_receiver(f: Callable[[], None]) -> Callable[[object], None]: + def wrapper(self: object) -> None: + f() + return wrapper + +unknown: Any = lambda f: f + +class C(ABC): + @final + def plain() -> None: ... # E: Instance method `plain` must accept a positional argument + + @abstractmethod + def abstract() -> None: ... # E: Instance method `abstract` must accept a positional argument + + @add_receiver + def wrapped() -> None: ... + + @unknown + def uncertain() -> None: ... + + @property + def prop() -> int: ... # E: Instance method `prop` must accept a positional argument + "#, +); + testcase!( test_generic_instance_method, r#" diff --git a/pyrefly/lib/test/callable.rs b/pyrefly/lib/test/callable.rs index 17b60f3c4d..2e28303a68 100644 --- a/pyrefly/lib/test/callable.rs +++ b/pyrefly/lib/test/callable.rs @@ -518,7 +518,7 @@ testcase!( test_missing_self_and_kwonly, r#" class A: - def f(*, x): ... + def f(*, x): ... # E: Instance method `f` must accept a positional argument A().f(1) # E: Expected argument `x` to be passed by name # E: Expected 0 positional arguments, got 2 (including implicit `self`) "#, ); diff --git a/pyrefly/lib/test/class_overrides.rs b/pyrefly/lib/test/class_overrides.rs index 2ba780991a..f6316f02a2 100644 --- a/pyrefly/lib/test/class_overrides.rs +++ b/pyrefly/lib/test/class_overrides.rs @@ -2025,11 +2025,11 @@ testcase!( test_override_method_without_self, r#" class A: - def foo() -> int: + def foo() -> int: # E: Instance method `foo` must accept a positional argument return 42 class B(A): - def foo() -> int: + def foo() -> int: # E: Instance method `foo` must accept a positional argument return 100 "#, ); diff --git a/pyrefly/lib/test/constructors.rs b/pyrefly/lib/test/constructors.rs index 0e9e235e9d..68ccbaa045 100644 --- a/pyrefly/lib/test/constructors.rs +++ b/pyrefly/lib/test/constructors.rs @@ -852,9 +852,9 @@ from typing import overload class C: @overload - def __init__(): ... # E: Overloaded function must have an implementation + def __init__(): ... # E: Overloaded function must have an implementation # E: Instance method `__init__` must accept a positional argument @overload - def __init__(): ... + def __init__(): ... # E: Instance method `__init__` must accept a positional argument C() # E: This method has no `self` parameter to receive the implicit instance argument "#, diff --git a/pyrefly/lib/test/descriptors.rs b/pyrefly/lib/test/descriptors.rs index ae867427a9..8ea75fbe15 100644 --- a/pyrefly/lib/test/descriptors.rs +++ b/pyrefly/lib/test/descriptors.rs @@ -1517,7 +1517,7 @@ testcase!( test_self_referential_descriptor_get_no_crash, r#" class C: - def d() -> C: ... + def d() -> C: ... # E: Instance method `d` must accept a positional argument @d # E: Expected 0 positional arguments, got 1 in function `C.d` def __get__(): pass @@ -1676,7 +1676,7 @@ testcase!( test_self_referential_descriptor_set_no_crash, r#" class C: - def d() -> C: ... + def d() -> C: ... # E: Instance method `d` must accept a positional argument @d # E: Expected 0 positional arguments, got 1 in function `C.d` def __get__(): pass diff --git a/pyrefly/lib/test/narrow.rs b/pyrefly/lib/test/narrow.rs index acdde509ff..78d0cd7a49 100644 --- a/pyrefly/lib/test/narrow.rs +++ b/pyrefly/lib/test/narrow.rs @@ -3073,7 +3073,7 @@ class C: return True class D: - def guard_missing_self() -> TypeGuard[int]: # E: Type guard functions must accept at least one positional argument + def guard_missing_self() -> TypeGuard[int]: # E: Type guard functions must accept at least one positional argument # E: Instance method `guard_missing_self` must accept a positional argument return True "#, ); @@ -3141,7 +3141,7 @@ class C: return True class D: - def guard_missing_self() -> TypeIs[int]: # E: Type guard functions must accept at least one positional argument + def guard_missing_self() -> TypeIs[int]: # E: Type guard functions must accept at least one positional argument # E: Instance method `guard_missing_self` must accept a positional argument return True "#, ); From f3b56925d79482646c54972905fb74216a6e979a Mon Sep 17 00:00:00 2001 From: Mangesh Raut <95149253+mangeshraut712@users.noreply.github.com> Date: Wed, 28 Oct 2026 04:07:24 +0000 Subject: [PATCH 2/4] Fix fixtures and bench for missing-self instance method check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add self to hover_docstring and class-scope fixtures that used bare def f(): (now bad-function-definition). Use Foo.f attribute access in attribute_test to avoid MissingArgument. Bump nested_scopes bench expected errors 20→23 for class-nested bare defs at depths 4/8/12. --- pyrefly/benches/micro.rs | 2 +- pyrefly/lib/test/lsp/hover_docstring.rs | 10 +++++----- pyrefly/lib/test/scope.rs | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/pyrefly/benches/micro.rs b/pyrefly/benches/micro.rs index 5e76cfac0e..0b0ac58d32 100644 --- a/pyrefly/benches/micro.rs +++ b/pyrefly/benches/micro.rs @@ -945,7 +945,7 @@ fn suggestion_nested(c: &mut Criterion) { let code = Arc::new(FileContents::from_source(nested_scopes(16, 60, 20))); assert_eq!( check_module(code.dupe(), "bench", BENCH_FILE), - 20, + 23, "benchmark `suggestion_whole_file/nested_scopes` produced an unexpected error count" ); let mut group = c.benchmark_group("suggestion_whole_file"); diff --git a/pyrefly/lib/test/lsp/hover_docstring.rs b/pyrefly/lib/test/lsp/hover_docstring.rs index 5695fbe07d..f95d407a93 100644 --- a/pyrefly/lib/test/lsp/hover_docstring.rs +++ b/pyrefly/lib/test/lsp/hover_docstring.rs @@ -152,7 +152,7 @@ Docstring Result: `Test docstring` fn attribute_itself_test() { let code = r#" class Foo: - def f(): + def f(self): # ^ """Test docstring""" "#; @@ -160,7 +160,7 @@ class Foo: assert_eq!( r#" # main.py -3 | def f(): +3 | def f(self): ^ Docstring Result: `Test docstring` "# @@ -195,16 +195,16 @@ Docstring Result: `Test docstring` fn attribute_test() { let code = r#" class Foo: - def f(): + def f(self): """Test docstring""" -print(Foo.f()) +print(Foo.f) # ^ "#; let report = get_batched_lsp_operations_report(&[("main", code)], test_report_factory(code)); assert_eq!( r#" # main.py -5 | print(Foo.f()) +5 | print(Foo.f) ^ Docstring Result: `Test docstring` "# diff --git a/pyrefly/lib/test/scope.rs b/pyrefly/lib/test/scope.rs index 7ec88002d1..77dbb9a640 100644 --- a/pyrefly/lib/test/scope.rs +++ b/pyrefly/lib/test/scope.rs @@ -1243,7 +1243,7 @@ from typing import assert_type, Literal x = 'string' class A: x = 42 - def f(): + def f(self): assert_type(x, str) lambda_f = lambda: assert_type(x, str) class B: From 365d55a2047d4548879beeeb68b94a2b4d46ce7e Mon Sep 17 00:00:00 2001 From: Mangesh Raut Date: Tue, 3 Nov 2026 16:07:49 +0530 Subject: [PATCH 3/4] Merge classmethod and instance missing-receiver checks; flag __new__ without cls The two near-duplicate bad-function-definition checks shared the same parameter-shape gate, so fold them into one path. __new__ is an implicit staticmethod that still needs cls, matching mypy and pyright. The nested_scopes bench count stays at 23: three class-nested defs without self add three errors on top of the twenty missing-name lookups. Co-authored-by: Cursor --- pyrefly/benches/micro.rs | 3 ++ pyrefly/lib/alt/function.rs | 57 +++++++++++++++------------------- pyrefly/lib/test/attributes.rs | 14 +++++++++ 3 files changed, 42 insertions(+), 32 deletions(-) diff --git a/pyrefly/benches/micro.rs b/pyrefly/benches/micro.rs index 0b0ac58d32..1a941a2e33 100644 --- a/pyrefly/benches/micro.rs +++ b/pyrefly/benches/micro.rs @@ -943,6 +943,9 @@ fn nested_scopes(depth: usize, per_scope: usize, missing: usize) -> String { fn suggestion_nested(c: &mut Criterion) { let code = Arc::new(FileContents::from_source(nested_scopes(16, 60, 20))); + // nested_scopes inserts a class every fourth level; the three `def`s nested + // directly under those classes have no `self`, so the missing-receiver check + // adds three bad-function-definition errors on top of the 20 missing-name ones. assert_eq!( check_module(code.dupe(), "bench", BENCH_FILE), 23, diff --git a/pyrefly/lib/alt/function.rs b/pyrefly/lib/alt/function.rs index 7ebe2d820d..64cba3cee5 100644 --- a/pyrefly/lib/alt/function.rs +++ b/pyrefly/lib/alt/function.rs @@ -645,49 +645,42 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { self_type = None; } - if defining_cls.is_some() - && flags.is_classmethod - && !flags.is_staticmethod - && flags.property_metadata.is_none() + // Methods with an implicit receiver need a positional parameter (or `*args`) + // for it. Covers classmethods, instance methods, and `__new__` (an implicit + // staticmethod that still takes `cls`). Skip `@no_type_check` for ordinary + // instance methods, properties/staticmethods other than `__new__`, and + // unresolved custom decorators that may reshape the signature. + // See https://github.com/facebook/pyrefly/issues/5295 and #5294. + let missing_receiver_param = defining_cls.is_some() && decorators.is_empty() && def.parameters.posonlyargs.is_empty() && def.parameters.args.is_empty() && def.parameters.vararg.is_none() - { - self.error( - errors, - def.name.range(), - ErrorKind::BadFunctionDefinition, + && (is_dunder_new + || (flags.is_classmethod + && !flags.is_staticmethod + && flags.property_metadata.is_none()) + || (self_type.is_some() + && !flags.is_classmethod + && !flags.is_staticmethod + && !flags.has_no_type_check)); + if missing_receiver_param { + let message = if flags.is_classmethod || is_dunder_new { format!( "Class method `{}` must accept the class as its first positional parameter", def.name - ), - ); - } - - // Instance methods expect an implicit receiver. When there is no positional - // parameter and no `*args` to accept it, report a definition error. Skip - // classmethods (handled above), staticmethods, `@no_type_check`, and methods - // wrapped by unresolved custom decorators that may reshape the signature. - // See https://github.com/facebook/pyrefly/issues/5295. - if defining_cls.is_some() - && self_type.is_some() - && !flags.is_classmethod - && !flags.is_staticmethod - && !flags.has_no_type_check - && decorators.is_empty() - && def.parameters.posonlyargs.is_empty() - && def.parameters.args.is_empty() - && def.parameters.vararg.is_none() - { + ) + } else { + format!( + "Instance method `{}` must accept a positional argument", + def.name + ) + }; self.error( errors, def.name.range(), ErrorKind::BadFunctionDefinition, - format!( - "Instance method `{}` must accept a positional argument", - def.name - ), + message, ); } diff --git a/pyrefly/lib/test/attributes.rs b/pyrefly/lib/test/attributes.rs index 2133023daf..6cd82fc2f8 100644 --- a/pyrefly/lib/test/attributes.rs +++ b/pyrefly/lib/test/attributes.rs @@ -1419,6 +1419,20 @@ Foo.static() "#, ); +testcase!( + test_new_missing_cls, + r#" +class A: + def __new__() -> "A": ... # E: Class method `__new__` must accept the class as its first positional parameter + +class B: + def __new__(cls) -> "B": ... + +class C: + def __new__(*args: object) -> "C": ... + "#, +); + testcase!( test_instance_method_missing_receiver_decorators, r#" From 090886ff94e92375e900e585d4fe7f9b9f3b27c7 Mon Sep 17 00:00:00 2001 From: Mangesh Raut Date: Wed, 4 Nov 2026 00:04:17 +0530 Subject: [PATCH 4/4] Give class-nested nested_scopes defs a self parameter The missing-receiver check was counting three extra errors on the suggestion nested_scopes bench. Emit `def f(self)` under the every- fourth-level classes so the expected error count stays at the twenty missing-name lookups the bench measures. Co-authored-by: Cursor --- pyrefly/benches/micro.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/pyrefly/benches/micro.rs b/pyrefly/benches/micro.rs index 1a941a2e33..2726ff2d72 100644 --- a/pyrefly/benches/micro.rs +++ b/pyrefly/benches/micro.rs @@ -926,6 +926,10 @@ fn nested_scopes(depth: usize, per_scope: usize, missing: usize) -> String { let pad = " ".repeat(level); if level % 4 == 3 && level + 1 < depth { let _ = writeln!(source, "{pad}class C{level}:"); + } else if level > 0 && (level - 1) % 4 == 3 { + // Directly under a class inserted above: take `self` so the + // missing-receiver check does not inflate the expected error count. + let _ = writeln!(source, "{pad}def f{level}(self):"); } else { let _ = writeln!(source, "{pad}def f{level}():"); } @@ -943,12 +947,9 @@ fn nested_scopes(depth: usize, per_scope: usize, missing: usize) -> String { fn suggestion_nested(c: &mut Criterion) { let code = Arc::new(FileContents::from_source(nested_scopes(16, 60, 20))); - // nested_scopes inserts a class every fourth level; the three `def`s nested - // directly under those classes have no `self`, so the missing-receiver check - // adds three bad-function-definition errors on top of the 20 missing-name ones. assert_eq!( check_module(code.dupe(), "bench", BENCH_FILE), - 23, + 20, "benchmark `suggestion_whole_file/nested_scopes` produced an unexpected error count" ); let mut group = c.benchmark_group("suggestion_whole_file");