Skip to content

Commit fbfabee

Browse files
fix
1 parent b291170 commit fbfabee

4 files changed

Lines changed: 118 additions & 24 deletions

File tree

‎crates/pyrefly_types/src/function.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,8 @@ pub struct FuncFlags {
242242
/// A method directly inside a `Protocol` class.
243243
pub is_in_protocol_class: bool,
244244
pub is_in_type_checking_block: bool,
245+
/// The generated mock type for a callable resolved from a `unittest.mock.patch` target.
246+
pub unittest_mock_type: Option<Box<Type>>,
245247
/// Set when the definition has both `*args` and `**kwargs` and both are typed
246248
/// `Any` — explicitly, or implicitly because they are unannotated. Per the
247249
/// typing spec such a signature is equivalent to `...`. Captured at definition

‎pyrefly/lib/alt/attr.rs‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -632,6 +632,9 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> {
632632
context: Option<&dyn Fn() -> ErrorContext>,
633633
todo_ctx: &str,
634634
) -> Type {
635+
if let Some(signature) = self.unittest_mock_assertion_signature(base, attr_name) {
636+
return signature;
637+
}
635638
let attr_base = self.as_attribute_base(base.clone());
636639
let lookup_result = attr_base.clone().map_or_else(
637640
|| LookupResult::internal_error(InternalError::AttributeBaseUndefined(base.clone())),
@@ -721,6 +724,34 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> {
721724
}
722725
}
723726

727+
/// Check call assertions against the signature retained from a `mock.patch` target.
728+
fn unittest_mock_assertion_signature(&self, base: &Type, attr_name: &Name) -> Option<Type> {
729+
if !matches!(
730+
attr_name.as_str(),
731+
"assert_called_with"
732+
| "assert_called_once_with"
733+
| "assert_any_call"
734+
| "assert_awaited_with"
735+
| "assert_awaited_once_with"
736+
| "assert_any_await"
737+
) {
738+
return None;
739+
}
740+
let mock = base
741+
.toplevel_func_metadata()?
742+
.flags
743+
.unittest_mock_type
744+
.clone()?;
745+
if attr_name.as_str().contains("await")
746+
&& !matches!(*mock, Type::ClassType(cls) if cls.has_qname("unittest.mock", "AsyncMock"))
747+
{
748+
return None;
749+
}
750+
let mut target = base.clone();
751+
target.transform_toplevel_callable_signatures(|callable, _| callable.ret = Type::None);
752+
Some(target)
753+
}
754+
724755
fn add_class_fields(&self, class: &Class, candidates: &mut SmallSet<Name>) {
725756
let mut add_fields_for = |cls: &Class| {
726757
if let Some(class_fields) = self.get_class_fields(cls) {
@@ -2587,6 +2618,14 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> {
25872618
}
25882619

25892620
fn as_attribute_base1(&self, ty: Type, acc: &mut Vec<AttributeBase1>) {
2621+
// A patched callable keeps its signature but delegates attributes to the generated mock.
2622+
if let Some(mock) = ty
2623+
.toplevel_func_metadata()
2624+
.and_then(|metadata| metadata.flags.unittest_mock_type.clone())
2625+
{
2626+
self.as_attribute_base1(*mock, acc);
2627+
return;
2628+
}
25902629
match ty {
25912630
Type::ClassType(class_type) => {
25922631
if let Some(shaped_array) =

‎pyrefly/lib/alt/call.rs‎

Lines changed: 72 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2587,7 +2587,8 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> {
25872587
)
25882588
} else {
25892589
self.expand_mut(&mut callee_ty);
2590-
self.check_unittest_mock_patch_target(&callee_ty, &x.arguments, errors);
2590+
let unittest_mock_patch_target =
2591+
self.check_unittest_mock_patch_target(&callee_ty, &x.arguments, errors);
25912592

25922593
let call = CallWithTypes::new();
25932594
let (args, kws) = if callee_ty.is_union() {
@@ -2619,6 +2620,16 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> {
26192620
);
26202621
}
26212622
match ty.callee_kind() {
2623+
_ if ty.toplevel_func_metadata().is_some_and(|metadata| metadata.flags.unittest_mock_type.is_some()) =>
2624+
self.freeform_call_infer(
2625+
ty.clone(),
2626+
&args,
2627+
&kws,
2628+
x.func.range(),
2629+
x.arguments.range(),
2630+
hint,
2631+
errors,
2632+
).ty,
26222633
Some(CalleeKind::Function(FunctionKind::AssertType)) => self
26232634
.call_assert_type(
26242635
&x.arguments.args,
@@ -2852,12 +2863,18 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> {
28522863
}
28532864
}});
28542865
// TypeIs and TypeGuard functions return bool at runtime
2855-
match result {
2866+
let result = match result {
28562867
Type::TypeIs(_) | Type::TypeGuard(_) => {
28572868
self.heap.mk_class_type(self.stdlib.bool().clone())
28582869
}
28592870
other => other,
2860-
}
2871+
};
2872+
let uses_default_mock = x.arguments.find_keyword("new").is_none()
2873+
&& x.arguments.find_keyword("new_callable").is_none();
2874+
self.specialize_unittest_mock_patch(
2875+
result,
2876+
unittest_mock_patch_target.filter(|_| uses_default_mock),
2877+
)
28612878
};
28622879

28632880
let result = self.apply_django_annotate_call(result, django_annotate_call);
@@ -2869,33 +2886,31 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> {
28692886
callee_ty: &Type,
28702887
arguments: &Arguments,
28712888
errors: &ErrorCollector,
2872-
) {
2889+
) -> Option<Type> {
28732890
let Type::ClassType(cls) = callee_ty else {
2874-
return;
2891+
return None;
28752892
};
28762893
if !cls.has_qname("unittest.mock", "_patcher") {
2877-
return;
2894+
return None;
28782895
}
28792896
if arguments.args.len() > 1 || arguments.keywords.iter().any(|kw| kw.arg.is_none()) {
2880-
return;
2897+
return None;
28812898
}
28822899
if arguments
28832900
.find_keyword("create")
28842901
.is_some_and(|kw| !matches!(&kw.value, Expr::BooleanLiteral(lit) if !lit.value))
28852902
{
2886-
return;
2903+
return None;
28872904
}
2888-
let Some(target_expr) = arguments.find_argument_value("target", 0) else {
2889-
return;
2890-
};
2905+
let target_expr = arguments.find_argument_value("target", 0)?;
28912906
let Expr::StringLiteral(ExprStringLiteral { value, .. }) = target_expr else {
2892-
return;
2907+
return None;
28932908
};
28942909
let range = target_expr.range();
28952910
let target = value.to_str();
28962911
let parts: Vec<&str> = target.split('.').collect();
28972912
if parts.len() < 2 || parts.iter().any(|p| p.is_empty()) {
2898-
return;
2913+
return None;
28992914
}
29002915

29012916
let Some((module_prefix_len, module)) = (1..parts.len()).rev().find_map(|i| {
@@ -2906,7 +2921,7 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> {
29062921
}) else {
29072922
// The target may be resolved dynamically at runtime, so only check paths rooted in a
29082923
// module that is available in the current environment.
2909-
return;
2924+
return None;
29102925
};
29112926

29122927
let attrs = &parts[module_prefix_len..];
@@ -2927,7 +2942,7 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> {
29272942
.exports
29282943
.is_implicit_reexport(ModuleName::builtins(), &name)
29292944
{
2930-
return;
2945+
return None;
29312946
}
29322947
base_ty = self.type_of_attr_get(
29332948
&base_ty,
@@ -2939,6 +2954,48 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> {
29392954
"unittest.mock.patch target",
29402955
);
29412956
}
2957+
Some(base_ty)
2958+
}
2959+
2960+
/// Retain a resolved patch target's signature on its generated mock result.
2961+
fn specialize_unittest_mock_patch(&self, result: Type, target: Option<Type>) -> Type {
2962+
let Some(mut target) = target else {
2963+
return result;
2964+
};
2965+
let Some(metadata) = target.toplevel_func_metadata_mut() else {
2966+
return result;
2967+
};
2968+
let is_async = metadata.flags.is_async;
2969+
let Type::ClassType(mut patcher) = result else {
2970+
return result;
2971+
};
2972+
if !patcher.has_qname("unittest.mock", "_patch_pass_arg") {
2973+
return Type::ClassType(patcher);
2974+
}
2975+
let [mock] = patcher.targs_mut().as_mut() else {
2976+
unreachable!("unittest.mock._patch_pass_arg has exactly one type argument")
2977+
};
2978+
let Type::Union(mock_union) = mock else {
2979+
unreachable!("the default unittest.mock.patch result is a union of mock types")
2980+
};
2981+
let mock_type = mock_union
2982+
.members
2983+
.iter()
2984+
.find(|ty| {
2985+
matches!(
2986+
ty,
2987+
Type::ClassType(cls)
2988+
if cls.has_qname(
2989+
"unittest.mock",
2990+
if is_async { "AsyncMock" } else { "MagicMock" },
2991+
)
2992+
)
2993+
})
2994+
.cloned()
2995+
.expect("the default patch result contains MagicMock and AsyncMock");
2996+
metadata.flags.unittest_mock_type = Some(Box::new(mock_type));
2997+
*mock = target;
2998+
Type::ClassType(patcher)
29422999
}
29433000

29443001
pub fn freeform_call_infer(

‎pyrefly/lib/test/unittest_mock_patch.rs‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -284,16 +284,15 @@ with patch.multiple("other.missing", attribute=DEFAULT):
284284

285285
// https://github.com/facebook/pyrefly/issues/3913
286286
testcase!(
287-
bug = "Mock assertions do not check the target signature",
288287
test_assert_called_with_checks_target_signature,
289288
TestEnv::one("other", "def foo(x: int, *, y: str) -> None: ..."),
290289
r#"
291290
from unittest.mock import patch
292291
293292
with patch("other.foo") as mock_foo:
294293
mock_foo.assert_called_with(1, y="ok")
295-
mock_foo.assert_called_once_with("bad", y="ok") # BUG: This should report that x requires int, not str.
296-
mock_foo.assert_any_call(1, y=2) # BUG: This should report that y requires str, not int.
294+
mock_foo.assert_called_once_with("bad", y="ok") # E: Argument `Literal['bad']` is not assignable to parameter `x` with type `int` in function `other.foo`
295+
mock_foo.assert_any_call(1, y=2) # E: Argument `Literal[2]` is not assignable to parameter `y` with type `str` in function `other.foo`
297296
"#,
298297
);
299298

@@ -311,36 +310,33 @@ with patch("typing.assert_type") as mock_assert_type:
311310

312311
// https://github.com/facebook/pyrefly/issues/3913
313312
testcase!(
314-
bug = "Mock assertions do not check the target signature",
315313
test_patch_start_checks_target_signature,
316314
TestEnv::one("other", "def foo(x: int) -> None: ..."),
317315
r#"
318316
from unittest.mock import patch
319317
320318
patcher = patch("other.foo")
321319
mock_foo = patcher.start()
322-
mock_foo.assert_called_with("bad") # BUG: This should report that x requires int, not str.
320+
mock_foo.assert_called_with("bad") # E: Argument `Literal['bad']` is not assignable to parameter `x` with type `int` in function `other.foo`
323321
patcher.stop()
324322
"#,
325323
);
326324

327325
// https://github.com/facebook/pyrefly/issues/3913
328326
testcase!(
329-
bug = "Mock assertions do not check the target signature",
330327
test_async_patch_checks_await_assertion,
331328
TestEnv::one("other", "async def foo(x: int) -> None: ..."),
332329
r#"
333330
from unittest.mock import patch
334331
335332
with patch("other.foo") as mock_foo:
336333
mock_foo.assert_awaited_with(1)
337-
mock_foo.assert_awaited_once_with("bad") # BUG: This should report that x requires int, not str.
334+
mock_foo.assert_awaited_once_with("bad") # E: Argument `Literal['bad']` is not assignable to parameter `x` with type `int` in function `other.foo`
338335
"#,
339336
);
340337

341338
// https://github.com/facebook/pyrefly/issues/3913
342339
testcase!(
343-
bug = "Mock assertions do not check the target signature",
344340
test_patch_bound_method_checks_bound_signature,
345341
TestEnv::one(
346342
"other",
@@ -350,7 +346,7 @@ testcase!(
350346
from unittest.mock import patch
351347
352348
with patch("other.instance.method") as mock_method:
353-
mock_method.assert_called_once_with("bad") # BUG: This should report that x requires int, not str.
349+
mock_method.assert_called_once_with("bad") # E: Argument `Literal['bad']` is not assignable to parameter `x` with type `int` in function `other.C.method`
354350
"#,
355351
);
356352

0 commit comments

Comments
 (0)