From 4bc5cf418b6714274413ebccddbce921b942961f Mon Sep 17 00:00:00 2001 From: Christian Findlay <16697547+MelbourneDeveloper@users.noreply.github.com> Date: Tue, 2 Jun 2026 22:30:22 +1000 Subject: [PATCH] feat(checker): BSK-E0153 constructor-to-callable conversion (PEP conformance constructors_callable PASS) Implements the typing-spec rule 'Converting a constructor to callable'. A class object flowing through an identity-over-callable function (def f(cb: Callable[P, R]) -> Callable[P, R]) gains the class's constructor-to-callable signature; calls to the bound variable are now validated against it (arity, keyword names, and list[T] TypeVar binding consistency). Signature derivation priority: metaclass __call__, then __new__ (when it returns a non-Self/non-class type), then __init__. Flips conformance constructors_callable.py to PASS (12/12, 0 FP), raising the suite to 136/146 (93.15%); conformance threshold ratcheted 92 -> 93. Also extracts shared::method_name_map and migrates e0041/e0074/e0111/e0144 to it, removing 4 copies of the (class, method) -> Vec<&FunctionInfo> builder (net DRY win). --- conformance/conformance_status.csv | 2 +- coverage-thresholds.json | 4 +- crates/basilisk-checker/src/rules/e0041.rs | 10 +- crates/basilisk-checker/src/rules/e0074.rs | 11 +- .../basilisk-checker/src/rules/e0111/mod.rs | 11 +- .../basilisk-checker/src/rules/e0144/mod.rs | 11 +- crates/basilisk-checker/src/rules/e0153.rs | 468 ++++++++++++++++++ crates/basilisk-checker/src/rules/mod.rs | 2 + crates/basilisk-checker/src/rules/shared.rs | 22 +- crates/basilisk-checker/tests/e0153_tests.rs | 296 +++++++++++ docs/plans/CHECKER-PEP-CONFORMANCE-PLAN.md | 7 +- docs/specs/CHECKER-ARCHITECTURE-SPEC.md | 27 + website/src/docs/rules/index.md | 1 + website/src/zh/docs/rules/index.md | 1 + 14 files changed, 827 insertions(+), 46 deletions(-) create mode 100644 crates/basilisk-checker/src/rules/e0153.rs create mode 100644 crates/basilisk-checker/tests/e0153_tests.rs diff --git a/conformance/conformance_status.csv b/conformance/conformance_status.csv index 7fd3c2c5..efa96539 100644 --- a/conformance/conformance_status.csv +++ b/conformance/conformance_status.csv @@ -28,7 +28,7 @@ BSK-E0111|BSK-E0128,constructors_call_init.py,constructors,PASS,5,0,0 BSK-E0013|BSK-E0041,constructors_call_metaclass.py,constructors,PASS,2,0,1 BSK-E0013|BSK-E0074,constructors_call_new.py,constructors,PASS,2,0,2 BSK-E0144,constructors_call_type.py,constructors,PASS,8,0,0 -,constructors_callable.py,constructors,FAIL,0,12,0 +BSK-E0153,constructors_callable.py,constructors,PASS,12,0,0 ,constructors_consistency.py,constructors,PASS,0,0,0 ,dataclasses_descriptors.py,dataclasses,PASS,0,0,0 BSK-E0054,dataclasses_final.py,dataclasses,PASS,5,0,1 diff --git a/coverage-thresholds.json b/coverage-thresholds.json index b4853b14..b2968e0b 100644 --- a/coverage-thresholds.json +++ b/coverage-thresholds.json @@ -41,7 +41,7 @@ } }, "conformance": { - "_doc": "Minimum PEP conformance pass percentage (files passing / total files). Ratchet UP only. Current: 135/146 = 92.46% (pinned to python/typing@268d0c4e). 11 files still fail (missed required diagnostics): constructors_callable, dataclasses_transform_converter, generics_defaults_specialization, generics_paramspec_components, generics_paramspec_semantics, generics_paramspec_specialization, generics_typevartuple_args, generics_typevartuple_basic, protocols_definition, typeddicts_extra_items, typeddicts_readonly_inheritance.", - "threshold": 92 + "_doc": "Minimum PEP conformance pass percentage (files passing / total files). Ratchet UP only. Current: 136/146 = 93.15% (pinned to python/typing@268d0c4e). 10 files still fail (missed required diagnostics): dataclasses_transform_converter, generics_defaults_specialization, generics_paramspec_components, generics_paramspec_semantics, generics_paramspec_specialization, generics_typevartuple_args, generics_typevartuple_basic, protocols_definition, typeddicts_extra_items, typeddicts_readonly_inheritance.", + "threshold": 93 } } diff --git a/crates/basilisk-checker/src/rules/e0041.rs b/crates/basilisk-checker/src/rules/e0041.rs index 701ab1ac..fd0a769b 100644 --- a/crates/basilisk-checker/src/rules/e0041.rs +++ b/crates/basilisk-checker/src/rules/e0041.rs @@ -313,15 +313,7 @@ fn check_constructor_calls(module: &ResolvedModule, diagnostics: &mut Vec> = HashMap::new(); - for func in &module.functions { - if let Some(ref cls_name) = func.class_name { - method_map - .entry((cls_name.as_str(), func.name.as_str())) - .or_default() - .push(func); - } - } + let method_map = super::shared::method_name_map(&module.functions); for call in &module.calls { // Only process constructor calls (callee matches a class name) diff --git a/crates/basilisk-checker/src/rules/e0074.rs b/crates/basilisk-checker/src/rules/e0074.rs index 130c20dd..55449cca 100644 --- a/crates/basilisk-checker/src/rules/e0074.rs +++ b/crates/basilisk-checker/src/rules/e0074.rs @@ -55,16 +55,7 @@ impl Rule for ConstructorCallNewMismatch { basilisk_resolver::name_lookup(&module.classes); // Build method map: (class_name, method_name) -> Vec<&FunctionInfo> - let mut method_map: HashMap<(&str, &str), Vec<&basilisk_resolver::FunctionInfo>> = - HashMap::new(); - for func in &module.functions { - if let Some(ref class_name) = func.class_name { - method_map - .entry((class_name.as_str(), func.name.as_str())) - .or_default() - .push(func); - } - } + let method_map = super::shared::method_name_map(&module.functions); // Re-parse source to get AST for walking call expressions. let Ok(parsed) = basilisk_parser::parse_source(source.clone(), path.clone()) else { diff --git a/crates/basilisk-checker/src/rules/e0111/mod.rs b/crates/basilisk-checker/src/rules/e0111/mod.rs index 4515649f..7f566831 100644 --- a/crates/basilisk-checker/src/rules/e0111/mod.rs +++ b/crates/basilisk-checker/src/rules/e0111/mod.rs @@ -48,16 +48,7 @@ impl Rule for ConstructorCallError { basilisk_resolver::name_lookup(&module.classes); // Build method map: (class_name, method_name) -> Vec<&FunctionInfo> - let mut method_map: HashMap<(&str, &str), Vec<&basilisk_resolver::FunctionInfo>> = - HashMap::new(); - for func in &module.functions { - if let Some(ref class_name) = func.class_name { - method_map - .entry((class_name.as_str(), func.name.as_str())) - .or_default() - .push(func); - } - } + let method_map = super::shared::method_name_map(&module.functions); // Collect module-level TypeVar names for class-scoped TypeVar detection. let typevar_names: Vec<&str> = basilisk_resolver::collect_names(&module.typevar_calls); diff --git a/crates/basilisk-checker/src/rules/e0144/mod.rs b/crates/basilisk-checker/src/rules/e0144/mod.rs index 642bffe2..0b8f665e 100644 --- a/crates/basilisk-checker/src/rules/e0144/mod.rs +++ b/crates/basilisk-checker/src/rules/e0144/mod.rs @@ -48,16 +48,7 @@ impl Rule for TypeCallConstructorViolation { let class_map: HashMap<&str, &basilisk_resolver::ClassInfo> = basilisk_resolver::name_lookup(&module.classes); - let mut method_map: HashMap<(&str, &str), Vec<&basilisk_resolver::FunctionInfo>> = - HashMap::new(); - for func in &module.functions { - if let Some(ref class_name) = func.class_name { - method_map - .entry((class_name.as_str(), func.name.as_str())) - .or_default() - .push(func); - } - } + let method_map = super::shared::method_name_map(&module.functions); // Collect TypeVar names (module-level). let typevar_names: Vec<&str> = basilisk_resolver::collect_names(&module.typevar_calls); diff --git a/crates/basilisk-checker/src/rules/e0153.rs b/crates/basilisk-checker/src/rules/e0153.rs new file mode 100644 index 00000000..eb2ad5eb --- /dev/null +++ b/crates/basilisk-checker/src/rules/e0153.rs @@ -0,0 +1,468 @@ +//! Implements [BSK-E0153] from [CHKARCH-DIAG-CTOR-CALLABLE]. See docs/specs/CHECKER-ARCHITECTURE-SPEC.md#chkarch-diag-ctor-callable +//! BSK-E0153: Invalid call to a constructor-derived callable. +//! +//! Implements the typing spec rule "Converting a constructor to callable" +//! (). +//! +//! When a class object flows through an identity-over-callable function such as +//! +//! ```python +//! def accepts_callable(cb: Callable[P, R]) -> Callable[P, R]: +//! return cb +//! +//! r1 = accepts_callable(Class1) # r1 has Class1's constructor signature +//! ``` +//! +//! the bound variable (`r1`) gains the *constructor-to-callable* signature of +//! the class. Calls to that variable must match the synthesized signature: +//! +//! ```python +//! r1() # E0153: missing required argument `x` +//! r1(y=1) # E0153: unexpected keyword argument `y` +//! ``` +//! +//! The synthesized signature is derived (in priority order) from the +//! metaclass `__call__`, then `__new__` (when it returns a type other than the +//! class / `Self`), then `__init__`, mirroring runtime construction. + +use std::collections::HashMap; + +use basilisk_resolver::{ClassInfo, FunctionInfo, ParameterInfo, ResolvedModule, Span}; +use ruff_python_ast::{Expr, ExprCall, Number, Stmt}; +use ruff_text_size::Ranged as _; + +use crate::diagnostic::{error_diagnostic_owned, Diagnostic, ErrorCode}; + +use super::Rule; + +const CODE: ErrorCode = ErrorCode { + code: "BSK-E0153", + docs_url: "https://www.basilisk-python.dev/errors/BSK-E0153", +}; + +/// Method-map key: `(class_name, method_name)` → method definitions. +type MethodMap<'a> = HashMap<(&'a str, &'a str), Vec<&'a FunctionInfo>>; + +/// Emits BSK-E0153 for invalid calls to constructor-derived callables. +pub(crate) struct ConstructorCallableMisuse; + +impl Rule for ConstructorCallableMisuse { + fn check(&self, module: &ResolvedModule, diagnostics: &mut Vec) { + let source = &module.source; + let Ok(parsed) = basilisk_parser::parse_source(source.clone(), module.path.clone()) else { + return; + }; + + let class_map = super::shared::class_name_map(&module.classes); + let method_map = super::shared::method_name_map(&module.functions); + let identity_fns = collect_identity_callables(&module.functions, source); + let var_to_class = map_vars_to_classes(&parsed.ast.body, &identity_fns, &class_map); + if var_to_class.is_empty() { + return; + } + let typevars = basilisk_resolver::collect_names(&module.typevar_calls); + + basilisk_resolver::visit_calls(&parsed.ast.body, &mut |call| { + let Expr::Name(callee) = call.func.as_ref() else { + return; + }; + let Some(class_name) = var_to_class.get(callee.id.as_str()) else { + return; + }; + let sig = build_converted_callable(class_name, &class_map, &method_map, source); + validate_call(call, class_name, &sig, &typevars, &module.path, diagnostics); + }); + } +} + +/// The constructor-to-callable signature synthesized for a class. +struct ConvertedCallable<'a> { + /// Parameters after dropping the implicit `cls`/`self`. + params: Vec<&'a ParameterInfo>, + /// `true` when the controlling method accepts `*args`. + has_var_positional: bool, + /// `true` when the controlling method accepts `**kwargs`. + has_var_keyword: bool, +} + +/// Collect module-level identity-over-callable functions. +/// +/// A function `f` is an identity callable when it has a single parameter +/// annotated `Callable[...]` and an identical return annotation, e.g. +/// `def f(cb: Callable[P, R]) -> Callable[P, R]`. Calling such a function with +/// a class argument yields a value with that class's constructor signature. +fn collect_identity_callables<'a>(functions: &'a [FunctionInfo], source: &str) -> Vec<&'a str> { + functions + .iter() + .filter(|f| is_identity_callable(f, source)) + .map(|f| f.name.as_str()) + .collect() +} + +/// Returns `true` when `func` is a single-argument identity-over-callable. +fn is_identity_callable(func: &FunctionInfo, source: &str) -> bool { + if func.class_name.is_some() + || func.vararg.is_some() + || func.kwarg.is_some() + || func.parameters.len() != 1 + { + return false; + } + let Some(param_ann) = func + .parameters + .first() + .and_then(|p| p.annotation_text.as_deref()) + else { + return false; + }; + let Some(return_text) = func + .return_annotation_span + .and_then(|span| span.slice_source(source)) + else { + return false; + }; + let param = param_ann.trim(); + param == return_text.trim() && is_callable_type_text(param) +} + +/// Returns `true` when `text` denotes a `Callable[...]` type expression. +fn is_callable_type_text(text: &str) -> bool { + text.starts_with("Callable[") || text.contains(".Callable[") +} + +/// Build the map from a bound variable to the class whose constructor it wraps. +/// +/// Only top-level `var = identity_fn(KnownClass)` assignments are recognized. +fn map_vars_to_classes<'a>( + body: &[Stmt], + identity_fns: &[&str], + class_map: &HashMap<&'a str, &'a ClassInfo>, +) -> HashMap { + let mut map = HashMap::new(); + for stmt in body { + let Stmt::Assign(assign) = stmt else { + continue; + }; + let [Expr::Name(target)] = assign.targets.as_slice() else { + continue; + }; + if let Some(class) = wrapped_class(assign.value.as_ref(), identity_fns, class_map) { + let _ = map.insert(target.id.to_string(), class.to_owned()); + } + } + map +} + +/// Extract the class name from `identity_fn(KnownClass)`, if the RHS matches. +fn wrapped_class<'a>( + value: &Expr, + identity_fns: &[&str], + class_map: &HashMap<&'a str, &'a ClassInfo>, +) -> Option<&'a str> { + let Expr::Call(call) = value else { + return None; + }; + let Expr::Name(callee) = call.func.as_ref() else { + return None; + }; + if !identity_fns.contains(&callee.id.as_str()) || !call.arguments.keywords.is_empty() { + return None; + } + let [Expr::Name(arg)] = call.arguments.args.as_ref() else { + return None; + }; + class_map + .get_key_value(arg.id.as_str()) + .map(|(name, _)| *name) +} + +/// Synthesize the constructor-to-callable signature for `class_name`. +/// +/// Priority: a custom metaclass `__call__`, then `__new__` (when its return +/// type is neither `Self` nor the class), then `__init__`. A class with no +/// controlling method synthesizes a zero-argument callable. +fn build_converted_callable<'a>( + class_name: &str, + class_map: &HashMap<&'a str, &'a ClassInfo>, + method_map: &MethodMap<'a>, + source: &str, +) -> ConvertedCallable<'a> { + if let Some(call_fn) = metaclass_call(class_name, class_map, method_map) { + return from_method(call_fn); + } + + let new_impl = method_map + .get(&(class_name, "__new__")) + .and_then(|v| pick_impl(v)); + let init_impl = method_map + .get(&(class_name, "__init__")) + .and_then(|v| pick_impl(v)); + + let controlling = match (new_impl, init_impl) { + (Some(new_fn), _) if new_controls(new_fn, class_name, source) => Some(new_fn), + (_, Some(init_fn)) => Some(init_fn), + (Some(new_fn), None) => Some(new_fn), + (None, None) => None, + }; + + controlling.map_or_else( + || ConvertedCallable { + params: Vec::new(), + has_var_positional: false, + has_var_keyword: false, + }, + from_method, + ) +} + +/// Return the custom metaclass `__call__` controlling construction, if any. +/// +/// A class whose declared metaclass defines `__call__` has that method govern +/// the call signature (e.g. a `__call__` with `*args, **kwargs` accepts any +/// arguments). Classes without an explicit metaclass — or whose metaclass only +/// inherits `type.__call__` — fall through to `__new__`/`__init__`. +fn metaclass_call<'a>( + class_name: &str, + class_map: &HashMap<&'a str, &'a ClassInfo>, + method_map: &MethodMap<'a>, +) -> Option<&'a FunctionInfo> { + let metaclass = class_map.get(class_name)?.metaclass_name.as_deref()?; + pick_impl(method_map.get(&(metaclass, "__call__"))?) +} + +/// Build a converted signature from a controlling method (drops `cls`/`self`). +fn from_method(method: &FunctionInfo) -> ConvertedCallable<'_> { + ConvertedCallable { + params: method.parameters.iter().skip(1).collect(), + has_var_positional: method.vararg.is_some(), + has_var_keyword: method.kwarg.is_some(), + } +} + +/// Pick the non-`@overload` implementation, falling back to the first entry. +fn pick_impl<'a>(funcs: &[&'a FunctionInfo]) -> Option<&'a FunctionInfo> { + funcs + .iter() + .find(|f| !f.decorators.iter().any(|d| d == "overload")) + .or_else(|| funcs.first()) + .copied() +} + +/// Returns `true` when `__new__`'s return type takes over construction. +/// +/// Per the spec, when `__new__` returns a type that is neither `Self` nor the +/// class itself, `__init__` is ignored and the converted callable uses +/// `__new__`'s signature and return type. +fn new_controls(new_fn: &FunctionInfo, class_name: &str, source: &str) -> bool { + let Some(text) = new_fn + .return_annotation_span + .and_then(|span| span.slice_source(source)) + else { + return false; + }; + let text = text.trim(); + if last_segment(text) == "Self" { + return false; + } + let head = text.split('[').next().unwrap_or(text).trim(); + last_segment(head) != class_name +} + +/// Return the final `.`-separated segment of `s` (`typing.Self` → `Self`). +fn last_segment(s: &str) -> &str { + s.rsplit('.').next().unwrap_or(s) +} + +/// Validate a call against the synthesized signature, emitting one diagnostic. +fn validate_call( + call: &ExprCall, + class_name: &str, + sig: &ConvertedCallable<'_>, + typevars: &[&str], + path: &str, + diagnostics: &mut Vec, +) { + // Conservative: starred positional / `**kwargs` unpacking defeats arity + // analysis, so skip rather than risk a false positive. + if call + .arguments + .args + .iter() + .any(|a| matches!(a, Expr::Starred(_))) + || call.arguments.keywords.iter().any(|k| k.arg.is_none()) + { + return; + } + + let positional = call.arguments.args.len(); + let kw_names: Vec<&str> = call + .arguments + .keywords + .iter() + .filter_map(|k| k.arg.as_ref().map(ruff_python_ast::Identifier::as_str)) + .collect(); + + if let Some(diag) = check_keywords(call, class_name, sig, &kw_names, path) { + diagnostics.push(diag); + } else if let Some(diag) = check_too_many(call, class_name, sig, positional, path) { + diagnostics.push(diag); + } else if let Some(diag) = check_missing(call, class_name, sig, positional, &kw_names, path) { + diagnostics.push(diag); + } else if let Some(diag) = check_typevar_conflict(call, class_name, sig, typevars, path) { + diagnostics.push(diag); + } +} + +/// Flag the first keyword that names no parameter (when no `**kwargs`). +fn check_keywords( + call: &ExprCall, + class_name: &str, + sig: &ConvertedCallable<'_>, + kw_names: &[&str], + path: &str, +) -> Option { + if sig.has_var_keyword { + return None; + } + let unknown = kw_names + .iter() + .find(|name| !sig.params.iter().any(|p| p.name == **name))?; + Some(error_diagnostic_owned( + CODE.clone(), + format!( + "Unexpected keyword argument `{unknown}` in call to the constructor-derived \ + callable for `{class_name}`" + ), + Span::from(call.range()), + path, + Some("Remove the keyword argument or pass it positionally".to_owned()), + None, + )) +} + +/// Flag too many positional arguments (when no `*args`). +fn check_too_many( + call: &ExprCall, + class_name: &str, + sig: &ConvertedCallable<'_>, + positional: usize, + path: &str, +) -> Option { + if sig.has_var_positional || positional <= sig.params.len() { + return None; + } + let max = sig.params.len(); + let extra = positional - max; + Some(error_diagnostic_owned( + CODE.clone(), + format!( + "Call to the constructor-derived callable for `{class_name}` has {extra} too many \ + positional argument{} (expected at most {max}, got {positional})", + if extra == 1 { "" } else { "s" }, + ), + Span::from(call.range()), + path, + None, + None, + )) +} + +/// Flag the first required parameter left unsatisfied by position or keyword. +fn check_missing( + call: &ExprCall, + class_name: &str, + sig: &ConvertedCallable<'_>, + positional: usize, + kw_names: &[&str], + path: &str, +) -> Option { + let missing = sig.params.iter().enumerate().find(|(idx, param)| { + !param.has_default && *idx >= positional && !kw_names.contains(¶m.name.as_str()) + })?; + let required = sig.params.iter().filter(|p| !p.has_default).count(); + Some(error_diagnostic_owned( + CODE.clone(), + format!( + "Call to the constructor-derived callable for `{class_name}` is missing required \ + argument `{}` (expected {required}, got {positional})", + missing.1.name, + ), + Span::from(call.range()), + path, + None, + None, + )) +} + +/// Flag a `TypeVar` bound to two incompatible types across `list[T]` params. +fn check_typevar_conflict( + call: &ExprCall, + class_name: &str, + sig: &ConvertedCallable<'_>, + typevars: &[&str], + path: &str, +) -> Option { + let mut bindings: HashMap<&str, &'static str> = HashMap::new(); + for (idx, param) in sig.params.iter().enumerate() { + let Some(arg) = call.arguments.args.get(idx) else { + break; + }; + let Some(tv) = list_typevar(param.annotation_text.as_deref(), typevars) else { + continue; + }; + let Some(element) = list_literal_element(arg) else { + continue; + }; + if let Some(previous) = bindings.insert(tv, element) { + if previous != element { + return Some(error_diagnostic_owned( + CODE.clone(), + format!( + "Inconsistent binding for type variable `{tv}` in call to the \ + constructor-derived callable for `{class_name}`: `list[{previous}]` \ + vs `list[{element}]`" + ), + Span::from(call.range()), + path, + None, + None, + )); + } + } + } + None +} + +/// If `annotation` is `list[T]` where `T` is a known `TypeVar`, return `T`. +fn list_typevar<'a>(annotation: Option<&'a str>, typevars: &[&str]) -> Option<&'a str> { + let text = annotation?.trim(); + let inner = text.strip_prefix("list[")?.strip_suffix(']')?.trim(); + typevars.contains(&inner).then_some(inner) +} + +/// Return the literal element type of a homogeneous list literal `[lit, ...]`. +fn list_literal_element(arg: &Expr) -> Option<&'static str> { + let Expr::List(list) = arg else { + return None; + }; + let first = literal_type_name(list.elts.first()?)?; + list.elts + .iter() + .all(|elt| literal_type_name(elt) == Some(first)) + .then_some(first) +} + +/// Map a literal expression to its Python type name, or `None` for non-literals. +fn literal_type_name(expr: &Expr) -> Option<&'static str> { + match expr { + Expr::BooleanLiteral(_) => Some("bool"), + Expr::StringLiteral(_) | Expr::FString(_) => Some("str"), + Expr::BytesLiteral(_) => Some("bytes"), + Expr::NumberLiteral(n) => match n.value { + Number::Int(_) => Some("int"), + Number::Float(_) => Some("float"), + Number::Complex { .. } => Some("complex"), + }, + _ => None, + } +} diff --git a/crates/basilisk-checker/src/rules/mod.rs b/crates/basilisk-checker/src/rules/mod.rs index ea4e4591..267020e6 100644 --- a/crates/basilisk-checker/src/rules/mod.rs +++ b/crates/basilisk-checker/src/rules/mod.rs @@ -158,6 +158,7 @@ pub(crate) mod e0149; pub(crate) mod e0150; pub(crate) mod e0151; pub(crate) mod e0152; +pub(crate) mod e0153; pub(crate) mod guards; pub(crate) mod shared; pub(crate) mod w0011; @@ -328,6 +329,7 @@ fn all_rules() -> &'static [&'static dyn Rule] { &e0150::DeadBranchVariable, &e0151::TypeAliasTypeViolation, &e0152::MissingTypeStubs, + &e0153::ConstructorCallableMisuse, &w0011::UndeclaredDependencyImport, &w0012::UnusedDependency, &w0013::StaleLockFile, diff --git a/crates/basilisk-checker/src/rules/shared.rs b/crates/basilisk-checker/src/rules/shared.rs index 4c93e6dc..f96946d7 100644 --- a/crates/basilisk-checker/src/rules/shared.rs +++ b/crates/basilisk-checker/src/rules/shared.rs @@ -7,7 +7,7 @@ use std::collections::{HashMap, HashSet}; use basilisk_parser::ParsedModule; -use basilisk_resolver::{ClassInfo, ResolvedModule, Span, TypeVarCallInfo}; +use basilisk_resolver::{ClassInfo, FunctionInfo, ResolvedModule, Span, TypeVarCallInfo}; use ruff_python_ast::{self as ast, Expr}; // --------------------------------------------------------------------------- @@ -111,6 +111,26 @@ pub(crate) fn class_name_map(classes: &[ClassInfo]) -> HashMap<&str, &ClassInfo> classes.iter().map(|c| (c.name.as_str(), c)).collect() } +/// Build a `(class_name, method_name) -> Vec<&FunctionInfo>` lookup for every +/// method in the module (functions carrying a `class_name`). +/// +/// Multiple definitions sharing a key (e.g. `@overload` signatures plus the +/// implementation) are preserved in declaration order. The returned map borrows +/// from the slice; both must outlive the map. +pub(crate) fn method_name_map( + functions: &[FunctionInfo], +) -> HashMap<(&str, &str), Vec<&FunctionInfo>> { + let mut map: HashMap<(&str, &str), Vec<&FunctionInfo>> = HashMap::new(); + for func in functions { + if let Some(ref class_name) = func.class_name { + map.entry((class_name.as_str(), func.name.as_str())) + .or_default() + .push(func); + } + } + map +} + // --------------------------------------------------------------------------- // TypeVar helpers // --------------------------------------------------------------------------- diff --git a/crates/basilisk-checker/tests/e0153_tests.rs b/crates/basilisk-checker/tests/e0153_tests.rs new file mode 100644 index 00000000..c184fa67 --- /dev/null +++ b/crates/basilisk-checker/tests/e0153_tests.rs @@ -0,0 +1,296 @@ +//! Tests for [BSK-E0153] from [CHKARCH-DIAG-CTOR-CALLABLE]. See docs/specs/CHECKER-ARCHITECTURE-SPEC.md#CHKARCH-DIAG-CTOR-CALLABLE +//! +//! BSK-E0153 validates calls to a variable bound to a class's +//! constructor-to-callable conversion (the typing-spec rule "Converting a +//! constructor to callable"). +#![allow( + clippy::expect_used, + clippy::indexing_slicing, + clippy::panic, + clippy::unwrap_used, + dead_code, + missing_docs +)] + +mod common; + +use common::{messages_for, run}; + +const PRELUDE: &str = "\ +from typing import Any, Callable, Generic, ParamSpec, Self, TypeVar + +P = ParamSpec(\"P\") +R = TypeVar(\"R\") +T = TypeVar(\"T\") + + +def accepts_callable(cb: Callable[P, R]) -> Callable[P, R]: + return cb +"; + +fn check(body: &str) -> Result, Box> { + let source = format!("{PRELUDE}\n{body}"); + let diagnostics = run(&source)?; + Ok(messages_for(&diagnostics, "BSK-E0153") + .into_iter() + .map(str::to_owned) + .collect()) +} + +#[test] +fn invalid_calls_emit_e0153() -> Result<(), Box> { + let messages = check( + "\ +class Class1: + def __init__(self, x: int) -> None: + pass + + +class Class2: + pass + + +class Class9: + def __init__(self, x: list[T], y: list[T]) -> None: + pass + + +r1 = accepts_callable(Class1) +r2 = accepts_callable(Class2) +r9 = accepts_callable(Class9) + +r1() # missing required argument `x` +r1(y=1) # unexpected keyword argument `y` +r2(1) # too many positional arguments +r9([1], [\"\"]) # inconsistent TypeVar binding +", + )?; + + assert_eq!(messages.len(), 4, "{messages:#?}"); + assert!( + messages + .iter() + .any(|m| m.contains("missing required argument `x`")), + "{messages:#?}" + ); + assert!( + messages + .iter() + .any(|m| m.contains("Unexpected keyword argument `y`")), + "{messages:#?}" + ); + assert!( + messages + .iter() + .any(|m| m.contains("too many positional argument")), + "{messages:#?}" + ); + assert!( + messages + .iter() + .any(|m| m.contains("Inconsistent binding for type variable `T`")), + "{messages:#?}" + ); + Ok(()) +} + +#[test] +fn valid_calls_do_not_emit_e0153() -> Result<(), Box> { + let messages = check( + "\ +class Class1: + def __init__(self, x: int) -> None: + pass + + +class Class2: + pass + + +class Class9: + def __init__(self, x: list[T], y: list[T]) -> None: + pass + + +r1 = accepts_callable(Class1) +r2 = accepts_callable(Class2) +r9 = accepts_callable(Class9) + +r1(1) +r2() +r9([\"\"], [\"\"]) +", + )?; + + assert!(messages.is_empty(), "{messages:#?}"); + Ok(()) +} + +#[test] +fn metaclass_call_accepts_any_arguments() -> Result<(), Box> { + // A metaclass `__call__` taking `*args, **kwargs` accepts any call, so no + // E0153 should fire regardless of the class's own `__new__`/`__init__`. + let messages = check( + "\ +class Meta1(type): + def __call__(cls, *args: Any, **kwargs: Any) -> Any: + raise NotImplementedError + + +class Class5(metaclass=Meta1): + def __new__(cls, *args: Any, **kwargs: Any) -> Self: + return super().__new__(cls) + + +r5 = accepts_callable(Class5) +r5() +r5(1, x=1) +", + )?; + + assert!(messages.is_empty(), "{messages:#?}"); + Ok(()) +} + +#[test] +fn new_returning_non_self_controls_signature() -> Result<(), Box> { + // `__new__` returning a type other than `Self`/the class takes over: its + // signature is used and `__init__` is ignored. `Class6.__new__` takes no + // args, so `r6(1)` is too many; `Class4.__new__` requires `x`. + let messages = check( + "\ +class Proxy: + pass + + +class Class6: + def __new__(cls) -> Proxy: + return Proxy() + + def __init__(self, x: int) -> None: + pass + + +class Class4: + def __new__(cls, x: int) -> int: + raise NotImplementedError + + +r6 = accepts_callable(Class6) +r4 = accepts_callable(Class4) + +r6(1) # too many: __new__ takes no args, __init__ ignored +r4() # missing required argument `x` +r6() # ok +r4(1) # ok +", + )?; + + assert_eq!(messages.len(), 2, "{messages:#?}"); + assert!( + messages + .iter() + .any(|m| m.contains("too many positional argument")), + "{messages:#?}" + ); + assert!( + messages + .iter() + .any(|m| m.contains("missing required argument `x`")), + "{messages:#?}" + ); + Ok(()) +} + +#[test] +fn non_identity_wrapper_is_not_tracked() -> Result<(), Box> { + // A wrapper whose return type differs from its parameter is NOT an + // identity-over-callable, so the bound variable is not validated. + let messages = check( + "\ +def changes(cb: Callable[P, int]) -> Callable[P, str]: + return cb # type: ignore + + +class Class1: + def __init__(self, x: int) -> None: + pass + + +w = changes(Class1) +w() +", + )?; + + assert!(messages.is_empty(), "{messages:#?}"); + Ok(()) +} + +#[test] +fn typevar_conflict_detects_float_and_bytes_elements() -> Result<(), Box> { + // Exercises the float/bytes literal element classification in the + // TypeVar-binding consistency check. + let messages = check( + "\ +class Pair: + def __init__(self, x: list[T], y: list[T]) -> None: + pass + + +rp = accepts_callable(Pair) + +rp([1.0], [b\"\"]) # float vs bytes -> inconsistent +rp([1.0], [2.0]) # ok: both float +", + )?; + + assert_eq!(messages.len(), 1, "{messages:#?}"); + assert!( + messages + .iter() + .any(|m| m.contains("list[float]") && m.contains("list[bytes]")), + "{messages:#?}" + ); + Ok(()) +} + +#[test] +fn heterogeneous_list_argument_yields_no_conflict() -> Result<(), Box> { + // A heterogeneous list literal has no single element type, so it cannot + // bind the TypeVar and must not produce a (false) conflict diagnostic. + let messages = check( + "\ +class Pair: + def __init__(self, x: list[T], y: list[T]) -> None: + pass + + +rp = accepts_callable(Pair) + +rp([1, \"x\"], [2]) +", + )?; + + assert!(messages.is_empty(), "{messages:#?}"); + Ok(()) +} + +#[test] +fn direct_class_alias_is_not_handled_by_e0153() -> Result<(), Box> { + // A plain alias `Alias = Class1` is a constructor call site handled by + // other rules; E0153 only covers identity-callable-wrapped class objects. + let messages = check( + "\ +class Class1: + def __init__(self, x: int) -> None: + pass + + +Alias = Class1 +Alias() +", + )?; + + assert!(messages.is_empty(), "{messages:#?}"); + Ok(()) +} diff --git a/docs/plans/CHECKER-PEP-CONFORMANCE-PLAN.md b/docs/plans/CHECKER-PEP-CONFORMANCE-PLAN.md index be6b2136..c2c58e60 100644 --- a/docs/plans/CHECKER-PEP-CONFORMANCE-PLAN.md +++ b/docs/plans/CHECKER-PEP-CONFORMANCE-PLAN.md @@ -1,6 +1,6 @@ # PEP Conformance — Plan -> **Score**: 124/146 (84.9%) +> **Score**: 136/146 (93.15%) > **Tests**: `crates/basilisk-cli/tests/conformance/` > **Status CSV**: `conformance/conformance_status.csv` > **Run**: `make conformance` or `cargo test --test conformance_tests -- --nocapture` @@ -21,6 +21,7 @@ - [x] Generics category: 23/30 (76.7%) - [x] `protocols_generic.py` — generic protocol assignability — FLIPPED - [x] `typeddicts_type_consistency.py` — TypedDict type consistency — FLIPPED +- [x] E0153: Constructor-to-callable conversion + call validation — FLIPPED `constructors_callable.py` ([CHKARCH-DIAG-CTOR-CALLABLE](../specs/CHECKER-ARCHITECTURE-SPEC.md#CHKARCH-DIAG-CTOR-CALLABLE)) ## TODO — 23 failing files remaining @@ -52,9 +53,9 @@ - [ ] `callables_annotation.py` (4 missed) — callable annotation edge cases - [ ] `callables_subtyping.py` (30 missed) — callable subtyping rules -### Constructors (1 file) +### Constructors -- [ ] `constructors_callable.py` (12 missed) — callable as constructor +- [x] `constructors_callable.py` — constructor-to-callable conversion (BSK-E0153) — DONE ### Dataclasses (1 file) diff --git a/docs/specs/CHECKER-ARCHITECTURE-SPEC.md b/docs/specs/CHECKER-ARCHITECTURE-SPEC.md index e0e2d09e..5f3e5780 100644 --- a/docs/specs/CHECKER-ARCHITECTURE-SPEC.md +++ b/docs/specs/CHECKER-ARCHITECTURE-SPEC.md @@ -692,12 +692,39 @@ list — keep it in sync after adding or renaming a rule. | `BSK-E0150` | Variable defined only in dead version/platform branch | | `BSK-E0151` | Invalid `TypeAliasType(...)` call | | `BSK-E0152` | Missing type stubs for installed package | +| `BSK-E0153` | Invalid call to a constructor-derived callable ([CHKARCH-DIAG-CTOR-CALLABLE](#CHKARCH-DIAG-CTOR-CALLABLE)) | | `BSK-W0011` | Undeclared dependency import | | `BSK-W0012` | Unused dependency | | `BSK-W0013` | Stale uv lock file | | `BSK-W0040` | Lambda function missing type annotations | | `BSK-W0050` | Redundant type annotation warning | +#### Constructor-to-callable conversion {#CHKARCH-DIAG-CTOR-CALLABLE} + +`BSK-E0153` implements the typing-spec rule +["Converting a constructor to callable"](https://typing.readthedocs.io/en/latest/spec/constructors.html#converting-a-constructor-to-callable). +When a class object flows through an identity-over-callable function +(`def f(cb: Callable[P, R]) -> Callable[P, R]`), the value it returns gains the +class's *constructor-to-callable* signature. Calls to a variable bound that way +are validated against the synthesized signature. + +The synthesized signature is derived in priority order: + +1. The metaclass `__call__` (when the class declares a metaclass that defines + `__call__`) — e.g. a `__call__` taking `*args, **kwargs` accepts any call. +2. `__new__` when its return type is neither `Self` nor the class itself + (e.g. `-> int`, `-> Proxy`, `-> Any`); `__init__` is then ignored. +3. Otherwise `__init__` (or `__new__` when no `__init__` exists); a class with + neither synthesizes a zero-argument callable returning the instance. + +`BSK-E0153` fires when a call to such a variable supplies too few or too many +positional arguments, names a keyword that is not a parameter, or binds a +function-scoped `TypeVar` inconsistently (e.g. `list[T]` filled by both +`list[int]` and `list[str]`). The analysis is conservative: starred positional +arguments and `**kwargs` unpacking suppress arity checks to avoid false +positives. Implemented in `crates/basilisk-checker/src/rules/e0153.rs`; tests in +`crates/basilisk-checker/tests/e0153_tests.rs`. + #### Planned analyses {#CHKARCH-DIAG-PLANNED} The following anchors are retained for historical/spec-ID continuity. The diff --git a/website/src/docs/rules/index.md b/website/src/docs/rules/index.md index 9f20e1bf..2194e9ac 100644 --- a/website/src/docs/rules/index.md +++ b/website/src/docs/rules/index.md @@ -179,6 +179,7 @@ checker source (`scripts/gen_rules_reference.py`) — it is the authoritative li | `BSK-E0150` | Variable defined only in dead version/platform branch | | `BSK-E0151` | Invalid `TypeAliasType(...)` call | | `BSK-E0152` | Missing type stubs for installed package | +| `BSK-E0153` | Invalid call to a constructor-derived callable | | `BSK-W0011` | Undeclared dependency import | | `BSK-W0012` | Unused dependency | | `BSK-W0013` | Stale uv lock file | diff --git a/website/src/zh/docs/rules/index.md b/website/src/zh/docs/rules/index.md index 021f17ff..a557ef8f 100644 --- a/website/src/zh/docs/rules/index.md +++ b/website/src/zh/docs/rules/index.md @@ -173,6 +173,7 @@ Basilisk 内置 **151 个诊断代码**（146 个错误，5 个警告），覆 | `BSK-E0150` | Variable defined only in dead version/platform branch | | `BSK-E0151` | Invalid `TypeAliasType(...)` call | | `BSK-E0152` | Missing type stubs for installed package | +| `BSK-E0153` | Invalid call to a constructor-derived callable | | `BSK-W0011` | Undeclared dependency import | | `BSK-W0012` | Unused dependency | | `BSK-W0013` | Stale uv lock file |