diff --git a/crates/pyrefly_types/src/callable.rs b/crates/pyrefly_types/src/callable.rs index 7ec216a8f8..0fe1669a76 100644 --- a/crates/pyrefly_types/src/callable.rs +++ b/crates/pyrefly_types/src/callable.rs @@ -607,6 +607,8 @@ pub struct FuncFlags { pub is_overload: bool, pub is_staticmethod: bool, pub is_classmethod: bool, + /// A function decorated with `@no_type_check`. + pub has_no_type_check: bool, /// A function decorated with `@deprecated` pub deprecation: Option, /// Metadata for `@property`, `@foo.setter`, and `@foo.deleter`. diff --git a/pyrefly/lib/alt/function.rs b/pyrefly/lib/alt/function.rs index e606654fb0..7c05274a29 100644 --- a/pyrefly/lib/alt/function.rs +++ b/pyrefly/lib/alt/function.rs @@ -453,6 +453,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { let mut flags = FuncFlags { is_staticmethod: is_dunder_new, is_classmethod: is_dunder_init_subclass, + has_no_type_check: false, is_async: def.is_async, placeholder_body_kind, is_return_inferred, @@ -568,113 +569,117 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { let ret = self .get(&Key::ReturnType(ShortIdentifier::new(&stmt.name))) .arc_clone_ty(); - // `stmt.returns` is always set to None because the binding step calls `mem::take` on it - let has_return_annotation = self.bindings().function_has_return_annotation(&stmt.name); - if !has_return_annotation { - self.error( - errors, - stmt.name.range(), - ErrorKind::UnannotatedReturn, - format!("`{}` is missing a return annotation", stmt.name), - ); - } - // The first parameter of a non-static method is the implicit self/cls - // parameter and does not require an annotation, regardless of its name. - // __new__ is an implicit staticmethod but still takes cls as its first parameter. - // If the first parameter is variadic (e.g. *args), self is passed inside it, - // so there is no separate implicit parameter to skip. let is_dunder_new = def.defining_cls.is_some() && stmt.name.id == dunder::NEW; let has_implicit_self_or_cls_param = def.defining_cls.is_some() && (!def.metadata.flags.is_staticmethod || is_dunder_new); - for (i, p) in stmt.parameters.iter().enumerate() { - // Skip first param if it's implicit self/cls and not variadic - if i == 0 && has_implicit_self_or_cls_param && !p.is_variadic() { - continue; - } - if p.annotation().is_none() { - let name = p.name().as_str(); + let contains_self_type = |ty: &Type| ty.any(|t| matches!(t, Type::SelfType(_))); + + if !def.metadata.flags.has_no_type_check { + // `stmt.returns` is always set to None because the binding step calls `mem::take` on it + let has_return_annotation = self.bindings().function_has_return_annotation(&stmt.name); + if !has_return_annotation { self.error( errors, - p.name().range(), - ErrorKind::ImplicitAnyParameter, - format!( - "`{}` is missing an annotation for parameter `{name}`", - stmt.name - ), + stmt.name.range(), + ErrorKind::UnannotatedReturn, + format!("`{}` is missing a return annotation", stmt.name), ); } - } - // Only validate TypeGuard/TypeIs functions when they have an explicit return annotation. - // Functions that return a TypeGuard value without an explicit annotation should not be - // treated as TypeGuard functions. - if has_return_annotation { - if matches!(&ret, Type::TypeGuard(_) | Type::TypeIs(_)) { - self.validate_type_guard_positional_argument_count( - &def.params, - def.id_range(), - &def.defining_cls, - def.metadata.flags.is_staticmethod, - errors, - ); + + // The first parameter of a non-static method is the implicit self/cls + // parameter and does not require an annotation, regardless of its name. + // __new__ is an implicit staticmethod but still takes cls as its first parameter. + // If the first parameter is variadic (e.g. *args), self is passed inside it, + // so there is no separate implicit parameter to skip. + for (i, p) in stmt.parameters.iter().enumerate() { + // Skip first param if it's implicit self/cls and not variadic + if i == 0 && has_implicit_self_or_cls_param && !p.is_variadic() { + continue; + } + if p.annotation().is_none() { + let name = p.name().as_str(); + self.error( + errors, + p.name().range(), + ErrorKind::ImplicitAnyParameter, + format!( + "`{}` is missing an annotation for parameter `{name}`", + stmt.name + ), + ); + } } - if let Type::TypeIs(ty_narrow) = &ret { - self.validate_type_is_type_narrowing( - &def.params, - stmt, - &def.defining_cls, - def.metadata.flags.is_staticmethod, - ty_narrow, - errors, - ); + // When self/cls has an explicit TypeVar annotation, using Self anywhere in the signature + // (return type or other parameters) is invalid because the TypeVar and Self create + // conflicting type parameterization. + // For classmethods, the annotation is `type[TFoo]`, so we also unwrap `Type::Type(...)`. + if has_implicit_self_or_cls_param + && !def.metadata.flags.is_staticmethod + && let Some(first_param) = def.params.first() + && { + let ty = first_param.as_type(); + ty.is_explicit_type_variable() + || matches!(ty, Type::Type(inner) if inner.is_explicit_type_variable()) + } + { + let signature_contains_self = contains_self_type(&ret) + || def + .params + .iter() + .skip(1) + .any(|p| contains_self_type(p.as_type())); + if signature_contains_self { + self.error( + errors, + stmt.name.range, + ErrorKind::InvalidAnnotation, + format!( + "`Self` cannot be used when `{}` has an explicit TypeVar annotation", + first_param.name().map_or("self", |n| n.as_str()) + ), + ); + } } - } - let contains_self_type = |ty: &Type| ty.any(|t| matches!(t, Type::SelfType(_))); + // Only validate TypeGuard/TypeIs functions when they have an explicit return annotation. + // Functions that return a TypeGuard value without an explicit annotation should not be + // treated as TypeGuard functions. + if has_return_annotation { + if matches!(&ret, Type::TypeGuard(_) | Type::TypeIs(_)) { + self.validate_type_guard_positional_argument_count( + &def.params, + def.id_range(), + &def.defining_cls, + def.metadata.flags.is_staticmethod, + errors, + ); + } - if def.metadata.flags.is_staticmethod && stmt.name.as_str() != dunder::NEW { - // For static methods, the use of `Self` is not allowed. - let signature_contains_self = contains_self_type(&ret) - || def.params.iter().any(|p| contains_self_type(p.as_type())); - if signature_contains_self { - self.error( - errors, - stmt.name.range, - ErrorKind::InvalidAnnotation, - "`Self` cannot be used in a static method".to_owned(), - ); + if let Type::TypeIs(ty_narrow) = &ret { + self.validate_type_is_type_narrowing( + &def.params, + stmt, + &def.defining_cls, + def.metadata.flags.is_staticmethod, + ty_narrow, + errors, + ); + } } - } - // When self/cls has an explicit TypeVar annotation, using Self anywhere in the signature - // (return type or other parameters) is invalid because the TypeVar and Self create - // conflicting type parameterization. - // For classmethods, the annotation is `type[TFoo]`, so we also unwrap `Type::Type(...)`. - if has_implicit_self_or_cls_param - && !def.metadata.flags.is_staticmethod - && let Some(first_param) = def.params.first() - && { - let ty = first_param.as_type(); - ty.is_explicit_type_variable() - || matches!(ty, Type::Type(inner) if inner.is_explicit_type_variable()) - } - { - let signature_contains_self = contains_self_type(&ret) - || def - .params - .iter() - .skip(1) - .any(|p| contains_self_type(p.as_type())); - if signature_contains_self { - self.error( - errors, - stmt.name.range, - ErrorKind::InvalidAnnotation, - format!( - "`Self` cannot be used when `{}` has an explicit TypeVar annotation", - first_param.name().map_or("self", |n| n.as_str()) - ), - ); + if def.metadata.flags.is_staticmethod && stmt.name.as_str() != dunder::NEW { + // For static methods, the use of `Self` is not allowed. + let signature_contains_self = contains_self_type(&ret) + || def.params.iter().any(|p| contains_self_type(p.as_type())); + if signature_contains_self { + self.error( + errors, + stmt.name.range, + ErrorKind::InvalidAnnotation, + "`Self` cannot be used in a static method".to_owned(), + ); + } } } @@ -736,7 +741,14 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { .forall(tparams); ty = self.move_return_tparams_of_type(ty); for (x, range) in def.decorators.iter().rev() { - ty = self.apply_function_decorator(x.clone(), ty, &def.metadata, *range, errors); + ty = self.apply_function_decorator( + x.clone(), + ty, + &def.metadata, + &stmt.name, + *range, + errors, + ); } Arc::new(ty) } @@ -753,6 +765,15 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { return Some(SpecialDecorator::PropertyDeleter(&decorator.ty)); } match decorator.ty.callee_kind() { + Some(CalleeKind::Function(FunctionKind::Def(func_id))) + if func_id.name.as_str() == "no_type_check" + && matches!( + func_id.module.name().as_str(), + "typing" | "typing_extensions" + ) => + { + Some(SpecialDecorator::NoTypeCheck) + } Some(CalleeKind::Function(FunctionKind::Overload)) => Some(SpecialDecorator::Overload), Some(CalleeKind::Class(ClassKind::StaticMethod(name))) => { Some(SpecialDecorator::StaticMethod(name)) @@ -844,6 +865,10 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { flags.has_final_decoration = true; true } + SpecialDecorator::NoTypeCheck => { + flags.has_no_type_check = true; + true + } SpecialDecorator::Deprecated(deprecation) => { flags.deprecation = Some((**deprecation).clone()); true @@ -1596,11 +1621,59 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { } } + fn decorator_missing_injected_parameter_message( + &self, + decorator: &Type, + decoratee_name: &Identifier, + decoratee: &Type, + ) -> Option { + let decorator_name = decorator + .visit_toplevel_func_metadata(&|meta| Some(meta.kind.function_name().to_string()))?; + let expected_decoratee = decorator.callable_first_param(self.heap)?; + let expected_signature = expected_decoratee + .callable_signatures() + .into_iter() + .find(|signature| { + matches!(&signature.params, Params::ParamSpec(prefix, _) if !prefix.is_empty()) + })?; + + let actual_signature = match decoratee { + Type::Function(func) => &func.signature, + Type::Callable(callable) => callable.as_ref(), + _ => return None, + }; + + let Params::ParamSpec(prefix, _) = &expected_signature.params else { + return None; + }; + let Params::List(actual_params) = &actual_signature.params else { + return None; + }; + if actual_params.len() + 1 != prefix.len() { + return None; + } + + let missing = prefix.get(actual_params.len())?; + let missing_ty = match missing { + PrefixParam::PosOnly(_, ty, Required::Required) + | PrefixParam::Pos(_, ty, Required::Required) => ty, + PrefixParam::PosOnly(_, _, Required::Optional(_)) + | PrefixParam::Pos(_, _, Required::Optional(_)) => return None, + }; + Some(format!( + "Function `{}` is missing parameter of type `{}` injected by decorator `{}`", + decoratee_name.as_str(), + self.for_display(missing_ty.clone()), + decorator_name, + )) + } + fn apply_function_decorator( &self, decorator: Type, decoratee: Type, metadata: &FuncMetadata, + decoratee_name: &Identifier, range: TextRange, errors: &ErrorCollector, ) -> Type { @@ -1610,6 +1683,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { { return decoratee; } + let decorator_for_message = decorator.clone(); let application = self.prepare_decorator_application(decorator, decoratee, range, errors); // Run a decorator call, buffering errors so we can decide between the primary // and Self-rewritten fallback without double-reporting. @@ -1638,6 +1712,17 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { && fallback_errors.is_empty() { fallback_return + } else if let Some(message) = self.decorator_missing_injected_parameter_message( + &decorator_for_message, + decoratee_name, + &application.decoratee_arg, + ) { + self.error( + errors, + decoratee_name.range(), + ErrorKind::InvalidDecorator, + message, + ) } else { errors.extend(primary_errors); primary_return diff --git a/pyrefly/lib/alt/types/decorated_function.rs b/pyrefly/lib/alt/types/decorated_function.rs index 3d97bb0c34..9e7b11fd9b 100644 --- a/pyrefly/lib/alt/types/decorated_function.rs +++ b/pyrefly/lib/alt/types/decorated_function.rs @@ -92,6 +92,7 @@ pub enum SpecialDecorator<'a> { EnumMember, Override, Final, + NoTypeCheck, Deprecated(&'a Deprecation), PropertySetter(&'a Type), PropertyDeleter(&'a Type), diff --git a/pyrefly/lib/test/decorators.rs b/pyrefly/lib/test/decorators.rs index e990cfd830..9933a79ac9 100644 --- a/pyrefly/lib/test/decorators.rs +++ b/pyrefly/lib/test/decorators.rs @@ -419,6 +419,23 @@ g(f) "#, ); +testcase!( + test_decorator_missing_injected_parameter, + r#" +from typing import Callable, Concatenate + +def with_current_tenant_id[T, **P, R]( + view: Callable[Concatenate[T, str, P], R], +) -> Callable[Concatenate[T, P], R]: + ... + +class Foo: + @with_current_tenant_id + def get(self) -> int: # E: Function `get` is missing parameter of type `str` injected by decorator `with_current_tenant_id` + return 0 + "#, +); + testcase!( bug = "This error message is confusing, I think we need to be clearer when we are printing the *type* of an argument", test_decorator_error_message, diff --git a/pyrefly/lib/test/untyped_def_behaviors.rs b/pyrefly/lib/test/untyped_def_behaviors.rs index 7b96cc3cb5..02289f6231 100644 --- a/pyrefly/lib/test/untyped_def_behaviors.rs +++ b/pyrefly/lib/test/untyped_def_behaviors.rs @@ -492,6 +492,17 @@ assert_type(f(0), Any) "#, ); +testcase!( + test_no_type_check_without_return_annotation, + r#" +from typing import no_type_check + +@no_type_check +def f(): + pass +"#, +); + testcase!( test_no_type_check_dunder_new_preserves_self_default, r#"