diff --git a/crates/ruff_db/src/parsed.rs b/crates/ruff_db/src/parsed.rs index ad241b3ded..9d9604aea0 100644 --- a/crates/ruff_db/src/parsed.rs +++ b/crates/ruff_db/src/parsed.rs @@ -21,7 +21,11 @@ use crate::source::source_text; /// reflected in the changed AST offsets. /// The other reason is that Ruff's AST doesn't implement `Eq` which Salsa requires /// for determining if a query result is unchanged. -#[salsa::tracked(returns(ref), no_eq, heap_size=ruff_memory_usage::heap_size)] +/// +/// The LRU capacity of 200 was picked without any empirical evidence that it's optimal, +/// instead it's a wild guess that it should be unlikely that incremental changes involve +/// more than 200 modules. Parsed ASTs within the same revision are never evicted by Salsa. +#[salsa::tracked(returns(ref), no_eq, heap_size=ruff_memory_usage::heap_size, lru=200)] pub fn parsed_module(db: &dyn Db, file: File) -> ParsedModule { let _span = tracing::trace_span!("parsed_module", ?file).entered(); @@ -92,14 +96,9 @@ impl ParsedModule { self.inner.store(None); } - /// Returns the pointer address of this [`ParsedModule`]. - /// - /// The pointer uniquely identifies the module within the current Salsa revision, - /// regardless of whether particular [`ParsedModuleRef`] instances are garbage collected. - pub fn addr(&self) -> usize { - // Note that the outer `Arc` in `inner` is stable across garbage collection, while the inner - // `Arc` within the `ArcSwap` may change. - Arc::as_ptr(&self.inner).addr() + /// Returns the file to which this module belongs. + pub fn file(&self) -> File { + self.file } } diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index c1d98b6d50..aa9fa839f2 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -747,6 +747,7 @@ impl SemanticSyntaxContext for Checker<'_> { | SemanticSyntaxErrorKind::LoadBeforeNonlocalDeclaration { .. } | SemanticSyntaxErrorKind::NonlocalAndGlobal(_) | SemanticSyntaxErrorKind::AnnotatedGlobal(_) + | SemanticSyntaxErrorKind::TypeParameterDefaultOrder(_) | SemanticSyntaxErrorKind::AnnotatedNonlocal(_) => { self.semantic_errors.borrow_mut().push(error); } diff --git a/crates/ruff_memory_usage/src/lib.rs b/crates/ruff_memory_usage/src/lib.rs index e75c75808e..81444d5197 100644 --- a/crates/ruff_memory_usage/src/lib.rs +++ b/crates/ruff_memory_usage/src/lib.rs @@ -1,17 +1,46 @@ -use std::sync::{LazyLock, Mutex}; +use std::cell::RefCell; use get_size2::{GetSize, StandardTracker}; use ordermap::{OrderMap, OrderSet}; +thread_local! { + pub static TRACKER: RefCell>= const { RefCell::new(None) }; +} + +struct TrackerGuard(Option); + +impl Drop for TrackerGuard { + fn drop(&mut self) { + TRACKER.set(self.0.take()); + } +} + +pub fn attach_tracker(tracker: StandardTracker, f: impl FnOnce() -> R) -> R { + let prev = TRACKER.replace(Some(tracker)); + let _guard = TrackerGuard(prev); + f() +} + +fn with_tracker(f: F) -> R +where + F: FnOnce(Option<&mut StandardTracker>) -> R, +{ + TRACKER.with(|tracker| { + let mut tracker = tracker.borrow_mut(); + f(tracker.as_mut()) + }) +} + /// Returns the memory usage of the provided object, using a global tracker to avoid /// double-counting shared objects. pub fn heap_size(value: &T) -> usize { - static TRACKER: LazyLock> = - LazyLock::new(|| Mutex::new(StandardTracker::new())); - - value - .get_heap_size_with_tracker(&mut *TRACKER.lock().unwrap()) - .0 + with_tracker(|tracker| { + if let Some(tracker) = tracker { + value.get_heap_size_with_tracker(tracker).0 + } else { + value.get_heap_size() + } + }) } /// An implementation of [`GetSize::get_heap_size`] for [`OrderSet`]. diff --git a/crates/ruff_python_ast/src/parenthesize.rs b/crates/ruff_python_ast/src/parenthesize.rs index a7fb1224ce..786ca0572c 100644 --- a/crates/ruff_python_ast/src/parenthesize.rs +++ b/crates/ruff_python_ast/src/parenthesize.rs @@ -11,6 +11,8 @@ use crate::ExprRef; /// Note that without a parent the range can be inaccurate, e.g. `f(a)` we falsely return a set of /// parentheses around `a` even if the parentheses actually belong to `f`. That is why you should /// generally prefer [`parenthesized_range`]. +/// +/// Prefer [`crate::token::parentheses_iterator`] if you have access to [`crate::token::Tokens`]. pub fn parentheses_iterator<'a>( expr: ExprRef<'a>, parent: Option, @@ -57,6 +59,8 @@ pub fn parentheses_iterator<'a>( /// Returns the [`TextRange`] of a given expression including parentheses, if the expression is /// parenthesized; or `None`, if the expression is not parenthesized. +/// +/// Prefer [`crate::token::parenthesized_range`] if you have access to [`crate::token::Tokens`]. pub fn parenthesized_range( expr: ExprRef, parent: AnyNodeRef, diff --git a/crates/ruff_python_ast/src/token.rs b/crates/ruff_python_ast/src/token.rs index fc1b62a366..4b9d98ec5c 100644 --- a/crates/ruff_python_ast/src/token.rs +++ b/crates/ruff_python_ast/src/token.rs @@ -16,8 +16,10 @@ use crate::str_prefix::{ use crate::{AnyStringFlags, BoolOp, Operator, StringFlags, UnaryOp}; use ruff_text_size::{Ranged, TextRange}; +mod parentheses; mod tokens; +pub use parentheses::{parentheses_iterator, parenthesized_range}; pub use tokens::{TokenAt, TokenIterWithContext, Tokens}; #[derive(Clone, Copy, PartialEq, Eq)] diff --git a/crates/ruff_python_ast/src/token/parentheses.rs b/crates/ruff_python_ast/src/token/parentheses.rs new file mode 100644 index 0000000000..c1d6f40650 --- /dev/null +++ b/crates/ruff_python_ast/src/token/parentheses.rs @@ -0,0 +1,58 @@ +use ruff_text_size::{Ranged, TextLen, TextRange}; + +use super::{TokenKind, Tokens}; +use crate::{AnyNodeRef, ExprRef}; + +/// Returns an iterator over the ranges of the optional parentheses surrounding an expression. +/// +/// E.g. for `((f()))` with `f()` as expression, the iterator returns the ranges (1, 6) and (0, 7). +/// +/// Note that without a parent the range can be inaccurate, e.g. `f(a)` we falsely return a set of +/// parentheses around `a` even if the parentheses actually belong to `f`. That is why you should +/// generally prefer [`parenthesized_range`]. +pub fn parentheses_iterator<'a>( + expr: ExprRef<'a>, + parent: Option, + tokens: &'a Tokens, +) -> impl Iterator + 'a { + let after_tokens = if let Some(parent) = parent { + // If the parent is a node that brings its own parentheses, exclude the closing parenthesis + // from our search range. Otherwise, we risk matching on calls, like `func(x)`, for which + // the open and close parentheses are part of the `Arguments` node. + let exclusive_parent_end = if parent.is_arguments() { + parent.end() - ")".text_len() + } else { + parent.end() + }; + + tokens.in_range(TextRange::new(expr.end(), exclusive_parent_end)) + } else { + tokens.after(expr.end()) + }; + + let right_parens = after_tokens + .iter() + .filter(|token| !token.kind().is_trivia()) + .take_while(move |token| token.kind() == TokenKind::Rpar); + + let left_parens = tokens + .before(expr.start()) + .iter() + .rev() + .filter(|token| !token.kind().is_trivia()) + .take_while(|token| token.kind() == TokenKind::Lpar); + + right_parens + .zip(left_parens) + .map(|(right, left)| TextRange::new(left.start(), right.end())) +} + +/// Returns the [`TextRange`] of a given expression including parentheses, if the expression is +/// parenthesized; or `None`, if the expression is not parenthesized. +pub fn parenthesized_range( + expr: ExprRef, + parent: AnyNodeRef, + tokens: &Tokens, +) -> Option { + parentheses_iterator(expr, Some(parent), tokens).last() +} diff --git a/crates/ruff_python_ast_integration_tests/tests/parentheses.rs b/crates/ruff_python_ast_integration_tests/tests/parentheses.rs new file mode 100644 index 0000000000..479c456c39 --- /dev/null +++ b/crates/ruff_python_ast_integration_tests/tests/parentheses.rs @@ -0,0 +1,199 @@ +//! Tests for [`ruff_python_ast::tokens::parentheses_iterator`] and +//! [`ruff_python_ast::tokens::parenthesized_range`]. + +use ruff_python_ast::{ + self as ast, Expr, + token::{parentheses_iterator, parenthesized_range}, +}; +use ruff_python_parser::parse_module; + +#[test] +fn test_no_parentheses() { + let source = "x = 2 + 2"; + let parsed = parse_module(source).expect("should parse valid python"); + let tokens = parsed.tokens(); + let module = parsed.syntax(); + + let stmt = module.body.first().expect("module should have a statement"); + let ast::Stmt::Assign(assign) = stmt else { + panic!("expected `Assign` statement, got {stmt:?}"); + }; + + let result = parenthesized_range(assign.value.as_ref().into(), stmt.into(), tokens); + assert_eq!(result, None); +} + +#[test] +fn test_single_parentheses() { + let source = "x = (2 + 2)"; + let parsed = parse_module(source).expect("should parse valid python"); + let tokens = parsed.tokens(); + let module = parsed.syntax(); + + let stmt = module.body.first().expect("module should have a statement"); + let ast::Stmt::Assign(assign) = stmt else { + panic!("expected `Assign` statement, got {stmt:?}"); + }; + + let result = parenthesized_range(assign.value.as_ref().into(), stmt.into(), tokens); + let range = result.expect("should find parentheses"); + assert_eq!(&source[range], "(2 + 2)"); +} + +#[test] +fn test_double_parentheses() { + let source = "x = ((2 + 2))"; + let parsed = parse_module(source).expect("should parse valid python"); + let tokens = parsed.tokens(); + let module = parsed.syntax(); + + let stmt = module.body.first().expect("module should have a statement"); + let ast::Stmt::Assign(assign) = stmt else { + panic!("expected `Assign` statement, got {stmt:?}"); + }; + + let result = parenthesized_range(assign.value.as_ref().into(), stmt.into(), tokens); + let range = result.expect("should find parentheses"); + assert_eq!(&source[range], "((2 + 2))"); +} + +#[test] +fn test_parentheses_with_whitespace() { + let source = "x = ( 2 + 2 )"; + let parsed = parse_module(source).expect("should parse valid python"); + let tokens = parsed.tokens(); + let module = parsed.syntax(); + + let stmt = module.body.first().expect("module should have a statement"); + let ast::Stmt::Assign(assign) = stmt else { + panic!("expected `Assign` statement, got {stmt:?}"); + }; + + let result = parenthesized_range(assign.value.as_ref().into(), stmt.into(), tokens); + let range = result.expect("should find parentheses"); + assert_eq!(&source[range], "( 2 + 2 )"); +} + +#[test] +fn test_parentheses_with_comments() { + let source = "x = ( # comment\n 2 + 2\n)"; + let parsed = parse_module(source).expect("should parse valid python"); + let tokens = parsed.tokens(); + let module = parsed.syntax(); + + let stmt = module.body.first().expect("module should have a statement"); + let ast::Stmt::Assign(assign) = stmt else { + panic!("expected `Assign` statement, got {stmt:?}"); + }; + + let result = parenthesized_range(assign.value.as_ref().into(), stmt.into(), tokens); + let range = result.expect("should find parentheses"); + assert_eq!(&source[range], "( # comment\n 2 + 2\n)"); +} + +#[test] +fn test_parenthesized_range_multiple() { + let source = "x = (((2 + 2)))"; + let parsed = parse_module(source).expect("should parse valid python"); + let tokens = parsed.tokens(); + let module = parsed.syntax(); + + let stmt = module.body.first().expect("module should have a statement"); + let ast::Stmt::Assign(assign) = stmt else { + panic!("expected `Assign` statement, got {stmt:?}"); + }; + + let result = parenthesized_range(assign.value.as_ref().into(), stmt.into(), tokens); + let range = result.expect("should find parentheses"); + assert_eq!(&source[range], "(((2 + 2)))"); +} + +#[test] +fn test_parentheses_iterator_multiple() { + let source = "x = (((2 + 2)))"; + let parsed = parse_module(source).expect("should parse valid python"); + let tokens = parsed.tokens(); + let module = parsed.syntax(); + + let stmt = module.body.first().expect("module should have a statement"); + let ast::Stmt::Assign(assign) = stmt else { + panic!("expected `Assign` statement, got {stmt:?}"); + }; + + let ranges: Vec<_> = + parentheses_iterator(assign.value.as_ref().into(), Some(stmt.into()), tokens).collect(); + assert_eq!(ranges.len(), 3); + assert_eq!(&source[ranges[0]], "(2 + 2)"); + assert_eq!(&source[ranges[1]], "((2 + 2))"); + assert_eq!(&source[ranges[2]], "(((2 + 2)))"); +} + +#[test] +fn test_call_arguments_not_counted() { + let source = "f(x)"; + let parsed = parse_module(source).expect("should parse valid python"); + let tokens = parsed.tokens(); + let module = parsed.syntax(); + + let stmt = module.body.first().expect("module should have a statement"); + let ast::Stmt::Expr(expr_stmt) = stmt else { + panic!("expected `Expr` statement, got {stmt:?}"); + }; + + let Expr::Call(call) = expr_stmt.value.as_ref() else { + panic!("expected Call expression, got {:?}", expr_stmt.value); + }; + + let arg = call + .arguments + .args + .first() + .expect("call should have an argument"); + let result = parenthesized_range(arg.into(), (&call.arguments).into(), tokens); + // The parentheses belong to the call, not the argument + assert_eq!(result, None); +} + +#[test] +fn test_call_with_parenthesized_argument() { + let source = "f((x))"; + let parsed = parse_module(source).expect("should parse valid python"); + let tokens = parsed.tokens(); + let module = parsed.syntax(); + + let stmt = module.body.first().expect("module should have a statement"); + let ast::Stmt::Expr(expr_stmt) = stmt else { + panic!("expected Expr statement, got {stmt:?}"); + }; + + let Expr::Call(call) = expr_stmt.value.as_ref() else { + panic!("expected `Call` expression, got {:?}", expr_stmt.value); + }; + + let arg = call + .arguments + .args + .first() + .expect("call should have an argument"); + let result = parenthesized_range(arg.into(), (&call.arguments).into(), tokens); + + let range = result.expect("should find parentheses around argument"); + assert_eq!(&source[range], "(x)"); +} + +#[test] +fn test_multiline_with_parentheses() { + let source = "x = (\n 2 + 2 + 2\n)"; + let parsed = parse_module(source).expect("should parse valid python"); + let tokens = parsed.tokens(); + let module = parsed.syntax(); + + let stmt = module.body.first().expect("module should have a statement"); + let ast::Stmt::Assign(assign) = stmt else { + panic!("expected `Assign` statement, got {stmt:?}"); + }; + + let result = parenthesized_range(assign.value.as_ref().into(), stmt.into(), tokens); + let range = result.expect("should find parentheses"); + assert_eq!(&source[range], "(\n 2 + 2 + 2\n)"); +} diff --git a/crates/ruff_python_parser/resources/inline/err/type_parameter_default_order.py b/crates/ruff_python_parser/resources/inline/err/type_parameter_default_order.py new file mode 100644 index 0000000000..91362a5007 --- /dev/null +++ b/crates/ruff_python_parser/resources/inline/err/type_parameter_default_order.py @@ -0,0 +1,3 @@ +class C[T = int, U]: ... +class C[T1, T2 = int, T3, T4]: ... +type Alias[T = int, U] = ... diff --git a/crates/ruff_python_parser/src/semantic_errors.rs b/crates/ruff_python_parser/src/semantic_errors.rs index cd7335bdbe..2c573271e1 100644 --- a/crates/ruff_python_parser/src/semantic_errors.rs +++ b/crates/ruff_python_parser/src/semantic_errors.rs @@ -144,11 +144,16 @@ impl SemanticSyntaxChecker { } } } - Stmt::ClassDef(ast::StmtClassDef { type_params, .. }) - | Stmt::TypeAlias(ast::StmtTypeAlias { type_params, .. }) => { - if let Some(type_params) = type_params { - Self::duplicate_type_parameter_name(type_params, ctx); - } + Stmt::ClassDef(ast::StmtClassDef { + type_params: Some(type_params), + .. + }) + | Stmt::TypeAlias(ast::StmtTypeAlias { + type_params: Some(type_params), + .. + }) => { + Self::duplicate_type_parameter_name(type_params, ctx); + Self::type_parameter_default_order(type_params, ctx); } Stmt::Assign(ast::StmtAssign { targets, value, .. }) => { if let [Expr::Starred(ast::ExprStarred { range, .. })] = targets.as_slice() { @@ -611,6 +616,39 @@ impl SemanticSyntaxChecker { } } + fn type_parameter_default_order( + type_params: &ast::TypeParams, + ctx: &Ctx, + ) { + let mut seen_default = false; + for type_param in type_params.iter() { + let has_default = match type_param { + ast::TypeParam::TypeVar(ast::TypeParamTypeVar { default, .. }) + | ast::TypeParam::TypeVarTuple(ast::TypeParamTypeVarTuple { default, .. }) + | ast::TypeParam::ParamSpec(ast::TypeParamParamSpec { default, .. }) => { + default.is_some() + } + }; + + if seen_default && !has_default { + // test_err type_parameter_default_order + // class C[T = int, U]: ... + // class C[T1, T2 = int, T3, T4]: ... + // type Alias[T = int, U] = ... + Self::add_error( + ctx, + SemanticSyntaxErrorKind::TypeParameterDefaultOrder( + type_param.name().id.to_string(), + ), + type_param.range(), + ); + } + if has_default { + seen_default = true; + } + } + } + fn duplicate_parameter_name( parameters: &ast::Parameters, ctx: &Ctx, @@ -1066,6 +1104,12 @@ impl Display for SemanticSyntaxError { SemanticSyntaxErrorKind::DuplicateTypeParameter => { f.write_str("duplicate type parameter") } + SemanticSyntaxErrorKind::TypeParameterDefaultOrder(name) => { + write!( + f, + "non default type parameter `{name}` follows default type parameter" + ) + } SemanticSyntaxErrorKind::MultipleCaseAssignment(name) => { write!(f, "multiple assignments to name `{name}` in pattern") } @@ -1572,6 +1616,9 @@ pub enum SemanticSyntaxErrorKind { /// Represents a nonlocal statement for a name that has no binding in an enclosing scope. NonlocalWithoutBinding(String), + + /// Represents a default type parameter followed by a non-default type parameter. + TypeParameterDefaultOrder(String), } #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, get_size2::GetSize)] diff --git a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@type_param_param_spec_invalid_default_expr.py.snap b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@type_param_param_spec_invalid_default_expr.py.snap index 0bcbd3cc52..fd2b2f4714 100644 --- a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@type_param_param_spec_invalid_default_expr.py.snap +++ b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@type_param_param_spec_invalid_default_expr.py.snap @@ -375,3 +375,12 @@ Module( 4 | type X[**P = x := int] = int 5 | type X[**P = *int] = int | + + + | +2 | type X[**P = yield x] = int +3 | type X[**P = yield from x] = int +4 | type X[**P = x := int] = int + | ^^^ Syntax Error: non default type parameter `int` follows default type parameter +5 | type X[**P = *int] = int + | diff --git a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@type_param_type_var_invalid_default_expr.py.snap b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@type_param_type_var_invalid_default_expr.py.snap index c3e38b8e9a..0b4041088c 100644 --- a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@type_param_type_var_invalid_default_expr.py.snap +++ b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@type_param_type_var_invalid_default_expr.py.snap @@ -459,3 +459,12 @@ Module( 5 | type X[T = x := int] = int 6 | type X[T: int = *int] = int | + + + | +3 | type X[T = (yield x)] = int +4 | type X[T = yield from x] = int +5 | type X[T = x := int] = int + | ^^^ Syntax Error: non default type parameter `int` follows default type parameter +6 | type X[T: int = *int] = int + | diff --git a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@type_param_type_var_tuple_invalid_default_expr.py.snap b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@type_param_type_var_tuple_invalid_default_expr.py.snap index 4aff137a73..8cd49704ce 100644 --- a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@type_param_type_var_tuple_invalid_default_expr.py.snap +++ b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@type_param_type_var_tuple_invalid_default_expr.py.snap @@ -384,3 +384,11 @@ Module( | ^^^^^^^^^^^^ Syntax Error: yield expression cannot be used within a TypeVarTuple default 5 | type X[*Ts = x := int] = int | + + + | +3 | type X[*Ts = yield x] = int +4 | type X[*Ts = yield from x] = int +5 | type X[*Ts = x := int] = int + | ^^^ Syntax Error: non default type parameter `int` follows default type parameter + | diff --git a/crates/ruff_python_parser/tests/snapshots/invalid_syntax@type_parameter_default_order.py.snap b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@type_parameter_default_order.py.snap new file mode 100644 index 0000000000..a8280e947a --- /dev/null +++ b/crates/ruff_python_parser/tests/snapshots/invalid_syntax@type_parameter_default_order.py.snap @@ -0,0 +1,277 @@ +--- +source: crates/ruff_python_parser/tests/fixtures.rs +input_file: crates/ruff_python_parser/resources/inline/err/type_parameter_default_order.py +--- +## AST + +``` +Module( + ModModule { + node_index: NodeIndex(None), + range: 0..89, + body: [ + ClassDef( + StmtClassDef { + node_index: NodeIndex(None), + range: 0..24, + decorator_list: [], + name: Identifier { + id: Name("C"), + range: 6..7, + node_index: NodeIndex(None), + }, + type_params: Some( + TypeParams { + range: 7..19, + node_index: NodeIndex(None), + type_params: [ + TypeVar( + TypeParamTypeVar { + node_index: NodeIndex(None), + range: 8..15, + name: Identifier { + id: Name("T"), + range: 8..9, + node_index: NodeIndex(None), + }, + bound: None, + default: Some( + Name( + ExprName { + node_index: NodeIndex(None), + range: 12..15, + id: Name("int"), + ctx: Load, + }, + ), + ), + }, + ), + TypeVar( + TypeParamTypeVar { + node_index: NodeIndex(None), + range: 17..18, + name: Identifier { + id: Name("U"), + range: 17..18, + node_index: NodeIndex(None), + }, + bound: None, + default: None, + }, + ), + ], + }, + ), + arguments: None, + body: [ + Expr( + StmtExpr { + node_index: NodeIndex(None), + range: 21..24, + value: EllipsisLiteral( + ExprEllipsisLiteral { + node_index: NodeIndex(None), + range: 21..24, + }, + ), + }, + ), + ], + }, + ), + ClassDef( + StmtClassDef { + node_index: NodeIndex(None), + range: 25..59, + decorator_list: [], + name: Identifier { + id: Name("C"), + range: 31..32, + node_index: NodeIndex(None), + }, + type_params: Some( + TypeParams { + range: 32..54, + node_index: NodeIndex(None), + type_params: [ + TypeVar( + TypeParamTypeVar { + node_index: NodeIndex(None), + range: 33..35, + name: Identifier { + id: Name("T1"), + range: 33..35, + node_index: NodeIndex(None), + }, + bound: None, + default: None, + }, + ), + TypeVar( + TypeParamTypeVar { + node_index: NodeIndex(None), + range: 37..45, + name: Identifier { + id: Name("T2"), + range: 37..39, + node_index: NodeIndex(None), + }, + bound: None, + default: Some( + Name( + ExprName { + node_index: NodeIndex(None), + range: 42..45, + id: Name("int"), + ctx: Load, + }, + ), + ), + }, + ), + TypeVar( + TypeParamTypeVar { + node_index: NodeIndex(None), + range: 47..49, + name: Identifier { + id: Name("T3"), + range: 47..49, + node_index: NodeIndex(None), + }, + bound: None, + default: None, + }, + ), + TypeVar( + TypeParamTypeVar { + node_index: NodeIndex(None), + range: 51..53, + name: Identifier { + id: Name("T4"), + range: 51..53, + node_index: NodeIndex(None), + }, + bound: None, + default: None, + }, + ), + ], + }, + ), + arguments: None, + body: [ + Expr( + StmtExpr { + node_index: NodeIndex(None), + range: 56..59, + value: EllipsisLiteral( + ExprEllipsisLiteral { + node_index: NodeIndex(None), + range: 56..59, + }, + ), + }, + ), + ], + }, + ), + TypeAlias( + StmtTypeAlias { + node_index: NodeIndex(None), + range: 60..88, + name: Name( + ExprName { + node_index: NodeIndex(None), + range: 65..70, + id: Name("Alias"), + ctx: Store, + }, + ), + type_params: Some( + TypeParams { + range: 70..82, + node_index: NodeIndex(None), + type_params: [ + TypeVar( + TypeParamTypeVar { + node_index: NodeIndex(None), + range: 71..78, + name: Identifier { + id: Name("T"), + range: 71..72, + node_index: NodeIndex(None), + }, + bound: None, + default: Some( + Name( + ExprName { + node_index: NodeIndex(None), + range: 75..78, + id: Name("int"), + ctx: Load, + }, + ), + ), + }, + ), + TypeVar( + TypeParamTypeVar { + node_index: NodeIndex(None), + range: 80..81, + name: Identifier { + id: Name("U"), + range: 80..81, + node_index: NodeIndex(None), + }, + bound: None, + default: None, + }, + ), + ], + }, + ), + value: EllipsisLiteral( + ExprEllipsisLiteral { + node_index: NodeIndex(None), + range: 85..88, + }, + ), + }, + ), + ], + }, +) +``` +## Semantic Syntax Errors + + | +1 | class C[T = int, U]: ... + | ^ Syntax Error: non default type parameter `U` follows default type parameter +2 | class C[T1, T2 = int, T3, T4]: ... +3 | type Alias[T = int, U] = ... + | + + + | +1 | class C[T = int, U]: ... +2 | class C[T1, T2 = int, T3, T4]: ... + | ^^ Syntax Error: non default type parameter `T3` follows default type parameter +3 | type Alias[T = int, U] = ... + | + + + | +1 | class C[T = int, U]: ... +2 | class C[T1, T2 = int, T3, T4]: ... + | ^^ Syntax Error: non default type parameter `T4` follows default type parameter +3 | type Alias[T = int, U] = ... + | + + + | +1 | class C[T = int, U]: ... +2 | class C[T1, T2 = int, T3, T4]: ... +3 | type Alias[T = int, U] = ... + | ^ Syntax Error: non default type parameter `U` follows default type parameter + | diff --git a/crates/ty/docs/rules.md b/crates/ty/docs/rules.md index 12eb74d15a..5ac36c4fb9 100644 --- a/crates/ty/docs/rules.md +++ b/crates/ty/docs/rules.md @@ -39,7 +39,7 @@ def test(): -> "int": Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -63,7 +63,7 @@ Calling a non-callable object will raise a `TypeError` at runtime. Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -95,7 +95,7 @@ f(int) # error Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -126,7 +126,7 @@ a = 1 Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -158,7 +158,7 @@ class C(A, B): ... Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -190,7 +190,7 @@ class B(A): ... Default level: error · Preview (since 1.0.0) · Related issues · -View source +View source @@ -218,7 +218,7 @@ type B = A Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -245,7 +245,7 @@ class B(A, A): ... Default level: error · Added in 0.0.1-alpha.12 · Related issues · -View source +View source @@ -357,7 +357,7 @@ def test(): -> "Literal[5]": Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -387,7 +387,7 @@ class C(A, B): ... Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -413,7 +413,7 @@ t[3] # IndexError: tuple index out of range Default level: error · Added in 0.0.1-alpha.12 · Related issues · -View source +View source @@ -502,7 +502,7 @@ an atypical memory layout. Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -529,7 +529,7 @@ func("foo") # error: [invalid-argument-type] Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -557,7 +557,7 @@ a: int = '' Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -591,7 +591,7 @@ C.instance_var = 3 # error: Cannot assign to instance variable Default level: error · Added in 0.0.1-alpha.19 · Related issues · -View source +View source @@ -627,7 +627,7 @@ asyncio.run(main()) Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -651,7 +651,7 @@ class A(42): ... # error: [invalid-base] Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -678,7 +678,7 @@ with 1: Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -707,7 +707,7 @@ a: str Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -751,7 +751,7 @@ except ZeroDivisionError: Default level: error · Added in 0.0.1-alpha.28 · Related issues · -View source +View source @@ -793,7 +793,7 @@ class D(A): Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -826,7 +826,7 @@ class C[U](Generic[T]): ... Default level: error · Added in 0.0.1-alpha.17 · Related issues · -View source +View source @@ -865,7 +865,7 @@ carol = Person(name="Carol", age=25) # typo! Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -900,7 +900,7 @@ def f(t: TypeVar("U")): ... Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -934,7 +934,7 @@ class B(metaclass=f): ... Default level: error · Added in 0.0.1-alpha.20 · Related issues · -View source +View source @@ -1041,7 +1041,7 @@ Correct use of `@override` is enforced by ty's `invalid-explicit-override` rule. Default level: error · Added in 0.0.1-alpha.19 · Related issues · -View source +View source @@ -1095,7 +1095,7 @@ AttributeError: Cannot overwrite NamedTuple attribute _asdict Default level: error · Preview (since 1.0.0) · Related issues · -View source +View source @@ -1125,7 +1125,7 @@ Baz = NewType("Baz", int | str) # error: invalid base for `typing.NewType` Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1175,7 +1175,7 @@ def foo(x: int) -> int: ... Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1201,7 +1201,7 @@ def f(a: int = ''): ... Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1232,7 +1232,7 @@ P2 = ParamSpec("S2") # error: ParamSpec name must match the variable it's assig Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1266,7 +1266,7 @@ TypeError: Protocols can only inherit from other protocols, got Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1315,7 +1315,7 @@ def g(): Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1340,7 +1340,7 @@ def func() -> int: Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1398,7 +1398,7 @@ TODO #14889 Default level: error · Added in 0.0.1-alpha.6 · Related issues · -View source +View source @@ -1425,7 +1425,7 @@ NewAlias = TypeAliasType(get_name(), int) # error: TypeAliasType name mus Default level: error · Added in 0.0.1-alpha.29 · Related issues · -View source +View source @@ -1472,7 +1472,7 @@ Bar[int] # error: too few arguments Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1502,7 +1502,7 @@ TYPE_CHECKING = '' Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1532,7 +1532,7 @@ b: Annotated[int] # `Annotated` expects at least two arguments Default level: error · Added in 0.0.1-alpha.11 · Related issues · -View source +View source @@ -1566,7 +1566,7 @@ f(10) # Error Default level: error · Added in 0.0.1-alpha.11 · Related issues · -View source +View source @@ -1600,7 +1600,7 @@ class C: Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1635,7 +1635,7 @@ T = TypeVar('T', bound=str) # valid bound TypeVar Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1660,7 +1660,7 @@ func() # TypeError: func() missing 1 required positional argument: 'x' Default level: error · Added in 0.0.1-alpha.20 · Related issues · -View source +View source @@ -1693,7 +1693,7 @@ alice["age"] # KeyError Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1722,7 +1722,7 @@ func("string") # error: [no-matching-overload] Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1746,7 +1746,7 @@ Subscripting an object that does not support it will raise a `TypeError` at runt Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1772,7 +1772,7 @@ for i in 34: # TypeError: 'int' object is not iterable Default level: error · Added in 0.0.1-alpha.29 · Related issues · -View source +View source @@ -1805,7 +1805,7 @@ class B(A): Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1832,7 +1832,7 @@ f(1, x=2) # Error raised here Default level: error · Added in 0.0.1-alpha.22 · Related issues · -View source +View source @@ -1890,7 +1890,7 @@ def test(): -> "int": Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1920,7 +1920,7 @@ static_assert(int(2.0 * 3.0) == 6) # error: does not have a statically known tr Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -1949,7 +1949,7 @@ class B(A): ... # Error raised here Default level: error · Preview (since 0.0.1-alpha.30) · Related issues · -View source +View source @@ -1983,7 +1983,7 @@ class F(NamedTuple): Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2010,7 +2010,7 @@ f("foo") # Error raised here Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2038,7 +2038,7 @@ def _(x: int): Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2084,7 +2084,7 @@ class A: Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2111,7 +2111,7 @@ f(x=1, y=2) # Error raised here Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2139,7 +2139,7 @@ A().foo # AttributeError: 'A' object has no attribute 'foo' Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2164,7 +2164,7 @@ import foo # ModuleNotFoundError: No module named 'foo' Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2189,7 +2189,7 @@ print(x) # NameError: name 'x' is not defined Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2226,7 +2226,7 @@ b1 < b2 < b1 # exception raised here Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2254,7 +2254,7 @@ A() + A() # TypeError: unsupported operand type(s) for +: 'A' and 'A' Default level: error · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2279,7 +2279,7 @@ l[1:10:0] # ValueError: slice step cannot be zero Default level: warn · Added in 0.0.1-alpha.20 · Related issues · -View source +View source @@ -2320,7 +2320,7 @@ class SubProto(BaseProto, Protocol): Default level: warn · Added in 0.0.1-alpha.16 · Related issues · -View source +View source @@ -2408,7 +2408,7 @@ a = 20 / 0 # type: ignore Default level: warn · Added in 0.0.1-alpha.22 · Related issues · -View source +View source @@ -2436,7 +2436,7 @@ A.c # AttributeError: type object 'A' has no attribute 'c' Default level: warn · Added in 0.0.1-alpha.22 · Related issues · -View source +View source @@ -2468,7 +2468,7 @@ A()[0] # TypeError: 'A' object is not subscriptable Default level: warn · Added in 0.0.1-alpha.22 · Related issues · -View source +View source @@ -2500,7 +2500,7 @@ from module import a # ImportError: cannot import name 'a' from 'module' Default level: warn · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2527,7 +2527,7 @@ cast(int, f()) # Redundant Default level: warn · Added in 0.0.1-alpha.1 · Related issues · -View source +View source @@ -2551,7 +2551,7 @@ reveal_type(1) # NameError: name 'reveal_type' is not defined Default level: warn · Added in 0.0.1-alpha.15 · Related issues · -View source +View source @@ -2609,7 +2609,7 @@ def g(): Default level: warn · Added in 0.0.1-alpha.7 · Related issues · -View source +View source @@ -2648,7 +2648,7 @@ class D(C): ... # error: [unsupported-base] Default level: warn · Added in 0.0.1-alpha.22 · Related issues · -View source +View source @@ -2711,7 +2711,7 @@ def foo(x: int | str) -> int | str: Default level: ignore · Preview (since 0.0.1-alpha.1) · Related issues · -View source +View source @@ -2735,7 +2735,7 @@ Dividing by zero raises a `ZeroDivisionError` at runtime. Default level: ignore · Added in 0.0.1-alpha.1 · Related issues · -View source +View source diff --git a/crates/ty_ide/src/importer.rs b/crates/ty_ide/src/importer.rs index bf75157147..5ff46a1ae1 100644 --- a/crates/ty_ide/src/importer.rs +++ b/crates/ty_ide/src/importer.rs @@ -327,9 +327,7 @@ impl<'ast> MembersInScope<'ast> { .members_in_scope_at(node) .into_iter() .map(|(name, memberdef)| { - let Some(def) = memberdef.definition else { - return (name, MemberInScope::other(memberdef.ty)); - }; + let def = memberdef.first_reachable_definition; let kind = match *def.kind(db) { DefinitionKind::Import(ref kind) => { MemberImportKind::Imported(AstImportKind::Import(kind.import(parsed))) @@ -1891,13 +1889,13 @@ else: "#); assert_snapshot!( test.import_from("foo", "MAGIC"), @r#" - import foo + from foo import MAGIC if os.getenv("WHATEVER"): from foo import MAGIC else: from bar import MAGIC - (foo.MAGIC) + (MAGIC) "#); } @@ -2108,13 +2106,13 @@ except ImportError: "); assert_snapshot!( test.import_from("foo", "MAGIC"), @r" - import foo + from foo import MAGIC try: from foo import MAGIC except ImportError: from bar import MAGIC - (foo.MAGIC) + (MAGIC) "); } diff --git a/crates/ty_ide/src/symbols.rs b/crates/ty_ide/src/symbols.rs index 01b177619e..a2278f40e6 100644 --- a/crates/ty_ide/src/symbols.rs +++ b/crates/ty_ide/src/symbols.rs @@ -379,6 +379,16 @@ pub(crate) fn symbols_for_file_global_only(db: &dyn Db, file: File) -> FlatSymbo global_only: true, }; visitor.visit_body(&module.syntax().body); + + if file + .path(db) + .as_system_path() + .is_none_or(|path| !db.project().is_file_included(db, path)) + { + // Eagerly clear ASTs of third party files. + parsed.clear(); + } + FlatSymbols { symbols: visitor.symbols, } diff --git a/crates/ty_project/src/db.rs b/crates/ty_project/src/db.rs index 8f6ec20c95..9c8eab8204 100644 --- a/crates/ty_project/src/db.rs +++ b/crates/ty_project/src/db.rs @@ -7,6 +7,7 @@ pub use self::changes::ChangeResult; use crate::CollectReporter; use crate::metadata::settings::file_settings; use crate::{ProgressReporter, Project, ProjectMetadata}; +use get_size2::StandardTracker; use ruff_db::Db as SourceDb; use ruff_db::diagnostic::Diagnostic; use ruff_db::files::{File, Files}; @@ -129,7 +130,10 @@ impl ProjectDatabase { /// Returns a [`SalsaMemoryDump`] that can be use to dump Salsa memory usage information /// to the CLI after a typechecker run. pub fn salsa_memory_dump(&self) -> SalsaMemoryDump { - let memory_usage = ::memory_usage(self); + let memory_usage = ruff_memory_usage::attach_tracker(StandardTracker::new(), || { + ::memory_usage(self) + }); + let mut ingredients = memory_usage .structs .into_iter() diff --git a/crates/ty_python_semantic/resources/mdtest/dataclasses/dataclasses.md b/crates/ty_python_semantic/resources/mdtest/dataclasses/dataclasses.md index ba5151fa61..fa6f76de75 100644 --- a/crates/ty_python_semantic/resources/mdtest/dataclasses/dataclasses.md +++ b/crates/ty_python_semantic/resources/mdtest/dataclasses/dataclasses.md @@ -1462,3 +1462,57 @@ def test_c(): c = C(1) c.__lt__ = Mock() ``` + +## Imperatively calling `dataclasses.dataclass` + +While we do not currently recognize the special behaviour of `dataclasses.dataclass` if it is called +imperatively, we recognize that it can be called imperatively and do not emit any false-positive +diagnostics on such calls: + +```py +from dataclasses import dataclass +from typing_extensions import TypeVar, dataclass_transform + +U = TypeVar("U") + +@dataclass_transform(kw_only_default=True) +def sequence(cls: type[U]) -> type[U]: + d = dataclass( + repr=False, + eq=False, + match_args=False, + kw_only=True, + )(cls) + reveal_type(d) # revealed: type[U@sequence] & Any + return d + +@dataclass_transform(kw_only_default=True) +def sequence2(cls: type) -> type: + d = dataclass( + repr=False, + eq=False, + match_args=False, + kw_only=True, + )(cls) + reveal_type(d) # revealed: type & Any + return d + +@dataclass_transform(kw_only_default=True) +def sequence3(cls: type[U]) -> type[U]: + # TODO: should reveal `type[U@sequence3]` + return reveal_type(dataclass(cls)) # revealed: Unknown + +@dataclass_transform(kw_only_default=True) +def sequence4(cls: type) -> type: + # TODO: should reveal `type` + return reveal_type(dataclass(cls)) # revealed: Unknown + +class Foo: ... + +ordered_foo = dataclass(order=True)(Foo) +reveal_type(ordered_foo) # revealed: type[Foo] & Any +# TODO: should be `Foo & Any` +reveal_type(ordered_foo()) # revealed: @Todo(Type::Intersection.call) +# TODO: should be `Any` +reveal_type(ordered_foo() < ordered_foo()) # revealed: @Todo(Type::Intersection.call) +``` diff --git a/crates/ty_python_semantic/resources/mdtest/final.md b/crates/ty_python_semantic/resources/mdtest/final.md index 91c6f9273f..fc5233e246 100644 --- a/crates/ty_python_semantic/resources/mdtest/final.md +++ b/crates/ty_python_semantic/resources/mdtest/final.md @@ -488,11 +488,110 @@ class C(A): pass if coinflip(): - def method2(self) -> None: ... # TODO: should emit [override-of-final-method] + def method2(self) -> None: ... # error: [override-of-final-method] else: - def method2(self) -> None: ... # TODO: should emit [override-of-final-method] + def method2(self) -> None: ... if coinflip(): def method3(self) -> None: ... # error: [override-of-final-method] - def method4(self) -> None: ... # error: [override-of-final-method] + + # TODO: we should emit Liskov violations here too: + if coinflip(): + method4 = 42 # error: [override-of-final-method] + else: + method4 = 56 +``` + +## Definitions in statically known branches + +```toml +[environment] +python-version = "3.10" +``` + +```py +import sys +from typing_extensions import final + +class Parent: + if sys.version_info >= (3, 10): + @final + def foo(self) -> None: ... + @final + def foooo(self) -> None: ... + @final + def baaaaar(self) -> None: ... + else: + @final + def bar(self) -> None: ... + @final + def baz(self) -> None: ... + @final + def spam(self) -> None: ... + +class Child(Parent): + def foo(self) -> None: ... # error: [override-of-final-method] + + # The declaration on `Parent` is not reachable, + # so this is fine + def bar(self) -> None: ... + + if sys.version_info >= (3, 10): + def foooo(self) -> None: ... # error: [override-of-final-method] + def baz(self) -> None: ... + else: + # Fine because this doesn't override any reachable definitions + def foooo(self) -> None: ... + # There are `@final` definitions being overridden here, + # but the definitions that override them are unreachable + def spam(self) -> None: ... + def baaaaar(self) -> None: ... +``` + +## Overloads in statically-known branches in stub files + + + +```toml +[environment] +python-version = "3.10" +``` + +```pyi +import sys +from typing_extensions import overload, final + +class Foo: + if sys.version_info >= (3, 10): + @overload + @final + def method(self, x: int) -> int: ... + else: + @overload + def method(self, x: int) -> int: ... + @overload + def method(self, x: str) -> str: ... + + if sys.version_info >= (3, 10): + @overload + def method2(self, x: int) -> int: ... + else: + @overload + @final + def method2(self, x: int) -> int: ... + @overload + def method2(self, x: str) -> str: ... + +class Bar(Foo): + @overload + def method(self, x: int) -> int: ... + @overload + def method(self, x: str) -> str: ... # error: [override-of-final-method] + + # This is fine: the only overload that is marked `@final` + # is in a statically-unreachable branch + @overload + def method2(self, x: int) -> int: ... + @overload + def method2(self, x: str) -> str: ... ``` diff --git a/crates/ty_python_semantic/resources/mdtest/generics/specialize_constrained.md b/crates/ty_python_semantic/resources/mdtest/generics/specialize_constrained.md index bd4ee67aaf..4e04b91944 100644 --- a/crates/ty_python_semantic/resources/mdtest/generics/specialize_constrained.md +++ b/crates/ty_python_semantic/resources/mdtest/generics/specialize_constrained.md @@ -41,7 +41,8 @@ def unbounded[T](): # revealed: None reveal_type(generic_context(unbounded).specialize_constrained(ConstraintSet.range(bool, T, bool) & ConstraintSet.range(Never, T, str))) - # revealed: ty_extensions.Specialization[T@unbounded = int] + # TODO: revealed: ty_extensions.Specialization[T@unbounded = int] + # revealed: ty_extensions.Specialization[T@unbounded = bool] reveal_type(generic_context(unbounded).specialize_constrained(ConstraintSet.range(Never, T, int) | ConstraintSet.range(Never, T, bool))) # revealed: ty_extensions.Specialization[T@unbounded = Never] reveal_type(generic_context(unbounded).specialize_constrained(ConstraintSet.range(Never, T, int) | ConstraintSet.range(Never, T, str))) @@ -175,7 +176,7 @@ def constrained_by_gradual[T: (Base, Any)](): # revealed: ty_extensions.Specialization[T@constrained_by_gradual = Base] reveal_type(generic_context(constrained_by_gradual).specialize_constrained(ConstraintSet.always())) # TODO: revealed: ty_extensions.Specialization[T@constrained_by_gradual = Any] - # revealed: ty_extensions.Specialization[T@constrained_by_gradual = object] + # revealed: ty_extensions.Specialization[T@constrained_by_gradual = Base] reveal_type(generic_context(constrained_by_gradual).specialize_constrained(ConstraintSet.range(Never, T, object))) # revealed: None reveal_type(generic_context(constrained_by_gradual).specialize_constrained(ConstraintSet.never())) @@ -251,6 +252,30 @@ def constrained_by_gradual_list[T: (list[Base], list[Any])](): # revealed: ty_extensions.Specialization[T@constrained_by_gradual_list = list[Sub]] reveal_type(generic_context(constrained_by_gradual_list).specialize_constrained(ConstraintSet.range(list[Sub], T, list[Sub]))) +# Same tests as above, but with the typevar constraints in a different order, to make sure the +# results do not depend on our BDD variable ordering. +def constrained_by_gradual_list_reverse[T: (list[Any], list[Base])](): + # revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = list[Base]] + reveal_type(generic_context(constrained_by_gradual_list_reverse).specialize_constrained(ConstraintSet.always())) + # revealed: None + reveal_type(generic_context(constrained_by_gradual_list_reverse).specialize_constrained(ConstraintSet.never())) + + # revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = list[Base]] + reveal_type(generic_context(constrained_by_gradual_list_reverse).specialize_constrained(ConstraintSet.range(Never, T, list[Base]))) + # TODO: revealed: ty_extensions.Specialization[T@constrained_by_gradual = list[Any]] + # revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = list[Unrelated]] + reveal_type(generic_context(constrained_by_gradual_list_reverse).specialize_constrained(ConstraintSet.range(Never, T, list[Unrelated]))) + + # TODO: revealed: ty_extensions.Specialization[T@constrained_by_gradual = list[Any]] + # revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = list[Super]] + reveal_type(generic_context(constrained_by_gradual_list_reverse).specialize_constrained(ConstraintSet.range(Never, T, list[Super]))) + # TODO: revealed: ty_extensions.Specialization[T@constrained_by_gradual = list[Any]] + # revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = list[Super]] + reveal_type(generic_context(constrained_by_gradual_list_reverse).specialize_constrained(ConstraintSet.range(list[Super], T, list[Super]))) + # TODO: revealed: ty_extensions.Specialization[T@constrained_by_gradual = list[Any]] + # revealed: ty_extensions.Specialization[T@constrained_by_gradual_list_reverse = list[Sub]] + reveal_type(generic_context(constrained_by_gradual_list_reverse).specialize_constrained(ConstraintSet.range(list[Sub], T, list[Sub]))) + def constrained_by_two_gradual_lists[T: (list[Any], list[Any])](): # TODO: revealed: ty_extensions.Specialization[T@constrained_by_gradual = list[Any]] # revealed: ty_extensions.Specialization[T@constrained_by_two_gradual_lists = Top[list[Any]]] diff --git a/crates/ty_python_semantic/resources/mdtest/implicit_type_aliases.md b/crates/ty_python_semantic/resources/mdtest/implicit_type_aliases.md index 8207d55bcc..ed73c9323b 100644 --- a/crates/ty_python_semantic/resources/mdtest/implicit_type_aliases.md +++ b/crates/ty_python_semantic/resources/mdtest/implicit_type_aliases.md @@ -190,14 +190,10 @@ def _( reveal_type(type_of_str_or_int) # revealed: type[str] | int reveal_type(int_or_callable) # revealed: int | ((str, /) -> bytes) reveal_type(callable_or_int) # revealed: ((str, /) -> bytes) | int - # TODO should be Unknown | int - reveal_type(type_var_or_int) # revealed: T@TypeVarOrInt | int - # TODO should be int | Unknown - reveal_type(int_or_type_var) # revealed: int | T@IntOrTypeVar - # TODO should be Unknown | None - reveal_type(type_var_or_none) # revealed: T@TypeVarOrNone | None - # TODO should be None | Unknown - reveal_type(none_or_type_var) # revealed: None | T@NoneOrTypeVar + reveal_type(type_var_or_int) # revealed: Unknown | int + reveal_type(int_or_type_var) # revealed: int | Unknown + reveal_type(type_var_or_none) # revealed: Unknown | None + reveal_type(none_or_type_var) # revealed: None | Unknown ``` If a type is unioned with itself in a value expression, the result is just that type. No @@ -529,28 +525,18 @@ def _( annotated_unknown: AnnotatedType, optional_unknown: MyOptional, ): - # TODO: This should be `list[Unknown]` - reveal_type(list_unknown) # revealed: list[T@MyList] - # TODO: This should be `dict[Unknown, Unknown]` - reveal_type(dict_unknown) # revealed: dict[T@MyDict, U@MyDict] - # TODO: Should be `type[Unknown]` - reveal_type(subclass_of_unknown) # revealed: type[T@MyType] - # TODO: Should be `tuple[int, Unknown]` - reveal_type(int_and_unknown) # revealed: tuple[int, T@IntAndType] - # TODO: Should be `tuple[Unknown, Unknown]` - reveal_type(pair_of_unknown) # revealed: tuple[T@Pair, T@Pair] - # TODO: Should be `tuple[Unknown, Unknown]` - reveal_type(unknown_and_unknown) # revealed: tuple[T@Sum, U@Sum] - # TODO: Should be `list[Unknown] | tuple[Unknown, ...]` - reveal_type(list_or_tuple) # revealed: list[T@ListOrTuple] | tuple[T@ListOrTuple, ...] - # TODO: Should be `list[Unknown] | tuple[Unknown, ...]` - reveal_type(list_or_tuple_legacy) # revealed: list[T@ListOrTupleLegacy] | tuple[T@ListOrTupleLegacy, ...] - # TODO: Should be `(...) -> Unknown` + reveal_type(list_unknown) # revealed: list[Unknown] + reveal_type(dict_unknown) # revealed: dict[Unknown, Unknown] + reveal_type(subclass_of_unknown) # revealed: type[Unknown] + reveal_type(int_and_unknown) # revealed: tuple[int, Unknown] + reveal_type(pair_of_unknown) # revealed: tuple[Unknown, Unknown] + reveal_type(unknown_and_unknown) # revealed: tuple[Unknown, Unknown] + reveal_type(list_or_tuple) # revealed: list[Unknown] | tuple[Unknown, ...] + reveal_type(list_or_tuple_legacy) # revealed: list[Unknown] | tuple[Unknown, ...] + # TODO: should be (...) -> Unknown reveal_type(my_callable) # revealed: @Todo(Callable[..] specialized with ParamSpec) - # TODO: Should be `Unknown` - reveal_type(annotated_unknown) # revealed: T@AnnotatedType - # TODO: Should be `Unknown | None` - reveal_type(optional_unknown) # revealed: T@MyOptional | None + reveal_type(annotated_unknown) # revealed: Unknown + reveal_type(optional_unknown) # revealed: Unknown | None ``` For a type variable with a default, we use the default type: @@ -563,10 +549,13 @@ MyListWithDefault = list[T_default] def _( list_of_str: MyListWithDefault[str], list_of_int: MyListWithDefault, + list_of_str_or_none: MyListWithDefault[str] | None, + list_of_int_or_none: MyListWithDefault | None, ): reveal_type(list_of_str) # revealed: list[str] - # TODO: this should be `list[int]` - reveal_type(list_of_int) # revealed: list[T_default@MyListWithDefault] + reveal_type(list_of_int) # revealed: list[int] + reveal_type(list_of_str_or_none) # revealed: list[str] | None + reveal_type(list_of_int_or_none) # revealed: list[int] | None ``` (Generic) implicit type aliases can be used as base classes: @@ -601,7 +590,7 @@ Generic implicit type aliases can be imported from other modules and specialized ```py from typing_extensions import TypeVar -T = TypeVar("T") +T = TypeVar("T", default=str) MyList = list[T] ``` @@ -615,9 +604,13 @@ import my_types as mt def _( list_of_ints1: MyList[int], list_of_ints2: mt.MyList[int], + list_of_str: mt.MyList, + list_of_str_or_none: mt.MyList | None, ): reveal_type(list_of_ints1) # revealed: list[int] reveal_type(list_of_ints2) # revealed: list[int] + reveal_type(list_of_str) # revealed: list[str] + reveal_type(list_of_str_or_none) # revealed: list[str] | None ``` ### In stringified annotations diff --git a/crates/ty_python_semantic/resources/mdtest/import/star.md b/crates/ty_python_semantic/resources/mdtest/import/star.md index adef7f91d3..54d2050259 100644 --- a/crates/ty_python_semantic/resources/mdtest/import/star.md +++ b/crates/ty_python_semantic/resources/mdtest/import/star.md @@ -715,7 +715,7 @@ reveal_type(Y) # revealed: Unknown # The `*` import is not considered a redefinition # of the global variable `Z` in this module, as the symbol in -# the `a` module is in a branch that is statically known +# the `exporter` module is in a branch that is statically known # to be dead code given the `python-version` configuration. # Thus this still reveals `Literal[True]`. reveal_type(Z) # revealed: Literal[True] diff --git a/crates/ty_python_semantic/resources/mdtest/liskov.md b/crates/ty_python_semantic/resources/mdtest/liskov.md index 24a1c3e2c2..ad16b99f08 100644 --- a/crates/ty_python_semantic/resources/mdtest/liskov.md +++ b/crates/ty_python_semantic/resources/mdtest/liskov.md @@ -583,3 +583,17 @@ class GoodChild2(Parent): @staticmethod def static_method(x: object) -> bool: ... ``` + +## Definitely bound members with no reachable definitions(!) + +We don't emit a Liskov-violation diagnostic here, but if you're writing code like this, you probably +have bigger problems: + +```py +from __future__ import annotations + +class MaybeEqWhile: + while ...: + def __eq__(self, other: MaybeEqWhile) -> bool: + return True +``` diff --git a/crates/ty_python_semantic/resources/mdtest/named_tuple.md b/crates/ty_python_semantic/resources/mdtest/named_tuple.md index 3d1a0ec544..ab1dafa187 100644 --- a/crates/ty_python_semantic/resources/mdtest/named_tuple.md +++ b/crates/ty_python_semantic/resources/mdtest/named_tuple.md @@ -610,3 +610,24 @@ class Child(Base): # This is fine - Child is not directly a NamedTuple _asdict = 42 ``` + +## Edge case: multiple reachable definitions with distinct issues + + + +```py +from typing import NamedTuple + +def coinflip() -> bool: + return True + +class Foo(NamedTuple): + if coinflip(): + _asdict: bool # error: [invalid-named-tuple] "NamedTuple field `_asdict` cannot start with an underscore" + else: + # TODO: there should only be one diagnostic here... + # + # error: [invalid-named-tuple] "Cannot overwrite NamedTuple attribute `_asdict`" + # error: [invalid-named-tuple] "Cannot overwrite NamedTuple attribute `_asdict`" + _asdict = True +``` diff --git a/crates/ty_python_semantic/resources/mdtest/override.md b/crates/ty_python_semantic/resources/mdtest/override.md index 386db9b340..9a963c07d7 100644 --- a/crates/ty_python_semantic/resources/mdtest/override.md +++ b/crates/ty_python_semantic/resources/mdtest/override.md @@ -19,54 +19,74 @@ class A: class Parent: def foo(self): ... + @property def my_property1(self) -> int: ... + @property def my_property2(self) -> int: ... + baz = None + @classmethod def class_method1(cls) -> int: ... + @staticmethod def static_method1() -> int: ... + @classmethod def class_method2(cls) -> int: ... + @staticmethod def static_method2() -> int: ... + @lossy_decorator def decorated_1(self): ... + @lossy_decorator def decorated_2(self): ... + @lossy_decorator def decorated_3(self): ... class Child(Parent): @override def foo(self): ... # fine: overrides `Parent.foo` + @property @override def my_property1(self) -> int: ... # fine: overrides `Parent.my_property1` + @override @property def my_property2(self) -> int: ... # fine: overrides `Parent.my_property2` + @override def baz(self): ... # fine: overrides `Parent.baz` + @classmethod @override def class_method1(cls) -> int: ... # fine: overrides `Parent.class_method1` + @staticmethod @override def static_method1() -> int: ... # fine: overrides `Parent.static_method1` + @override @classmethod def class_method2(cls) -> int: ... # fine: overrides `Parent.class_method2` + @override @staticmethod def static_method2() -> int: ... # fine: overrides `Parent.static_method2` + @override def decorated_1(self): ... # fine: overrides `Parent.decorated_1` + @override @lossy_decorator def decorated_2(self): ... # fine: overrides `Parent.decorated_2` + @lossy_decorator @override def decorated_3(self): ... # fine: overrides `Parent.decorated_3` @@ -76,28 +96,37 @@ class OtherChild(Parent): ... class Grandchild(OtherChild): @override def foo(self): ... # fine: overrides `Parent.foo` + @override @property - def bar(self) -> int: ... # fine: overrides `Parent.bar` + def my_property1(self) -> int: ... # fine: overrides `Parent.my_property1` + @override def baz(self): ... # fine: overrides `Parent.baz` + @classmethod @override def class_method1(cls) -> int: ... # fine: overrides `Parent.class_method1` + @staticmethod @override def static_method1() -> int: ... # fine: overrides `Parent.static_method1` + @override @classmethod def class_method2(cls) -> int: ... # fine: overrides `Parent.class_method2` + @override @staticmethod def static_method2() -> int: ... # fine: overrides `Parent.static_method2` + @override def decorated_1(self): ... # fine: overrides `Parent.decorated_1` + @override @lossy_decorator def decorated_2(self): ... # fine: overrides `Parent.decorated_2` + @lossy_decorator @override def decorated_3(self): ... # fine: overrides `Parent.decorated_3` @@ -105,27 +134,41 @@ class Grandchild(OtherChild): class Invalid: @override def ___reprrr__(self): ... # error: [invalid-explicit-override] + @override @classmethod def foo(self): ... # error: [invalid-explicit-override] + @classmethod @override def bar(self): ... # error: [invalid-explicit-override] + @staticmethod @override def baz(): ... # error: [invalid-explicit-override] + @override @staticmethod def eggs(): ... # error: [invalid-explicit-override] + @property @override - def bad_property1(self) -> int: ... # TODO: should emit `invalid-explicit-override` here + def bad_property1(self) -> int: ... # error: [invalid-explicit-override] + @override @property - def bad_property2(self) -> int: ... # TODO: should emit `invalid-explicit-override` here + def bad_property2(self) -> int: ... # error: [invalid-explicit-override] + + @property + @override + def bad_settable_property(self) -> int: ... # error: [invalid-explicit-override] + @bad_settable_property.setter + def bad_settable_property(self, x: int) -> None: ... + @lossy_decorator @override def lossy(self): ... # TODO: should emit `invalid-explicit-override` here + @override @lossy_decorator def lossy2(self): ... # TODO: should emit `invalid-explicit-override` here @@ -136,11 +179,14 @@ class LiskovViolatingButNotOverrideViolating(Parent): @override @property def foo(self) -> int: ... + @override def my_property1(self) -> int: ... + @staticmethod @override def class_method1() -> int: ... + @classmethod @override def static_method1(cls) -> int: ... @@ -174,6 +220,178 @@ class Foo: def bar(self): ... # error: [invalid-explicit-override] ``` +## Possibly-unbound definitions + +```py +from typing_extensions import override + +def coinflip() -> bool: + return False + +class Parent: + if coinflip(): + def method1(self) -> None: ... + def method2(self) -> None: ... + + if coinflip(): + def method3(self) -> None: ... + def method4(self) -> None: ... + else: + def method3(self) -> None: ... + def method4(self) -> None: ... + + def method5(self) -> None: ... + def method6(self) -> None: ... + +class Child(Parent): + @override + def method1(self) -> None: ... + @override + def method2(self) -> None: ... + + if coinflip(): + @override + def method3(self) -> None: ... + + if coinflip(): + @override + def method4(self) -> None: ... + else: + @override + def method4(self) -> None: ... + + if coinflip(): + @override + def method5(self) -> None: ... + + if coinflip(): + @override + def method6(self) -> None: ... + else: + @override + def method6(self) -> None: ... + + if coinflip(): + @override + def method7(self) -> None: ... # error: [invalid-explicit-override] + + if coinflip(): + @override + def method8(self) -> None: ... # error: [invalid-explicit-override] + else: + @override + def method8(self) -> None: ... +``` + +## Multiple reachable definitions, only one of which is decorated with `@override` + +The diagnostic should point to the first definition decorated with `@override`, which may not +necessarily be the first definition of the symbol overall: + +`runtime.py`: + +```py +from typing_extensions import override, overload + +def coinflip() -> bool: + return True + +class Foo: + if coinflip(): + def method(self, x): ... + elif coinflip(): + @overload + def method(self, x: str) -> str: ... + @overload + def method(self, x: int) -> int: ... + @override + def method(self, x: str | int) -> str | int: # error: [invalid-explicit-override] + return x + elif coinflip(): + @override + def method(self, x): ... +``` + +stub.pyi\`: + +```pyi +from typing_extensions import override, overload + +def coinflip() -> bool: + return True + +class Foo: + if coinflip(): + def method(self, x): ... + elif coinflip(): + @overload + @override + def method(self, x: str) -> str: ... # error: [invalid-explicit-override] + @overload + def method(self, x: int) -> int: ... + + if coinflip(): + def method2(self, x): ... + elif coinflip(): + @overload + @override + def method2(self, x: str) -> str: ... + @overload + def method2(self, x: int) -> int: ... + else: + # TODO: not sure why this is being emitted on this line rather than on + # the first overload in the `elif` block? Ideally it would be emitted + # on the first reachable definition, but perhaps this is due to the way + # name lookups are deferred in stub files...? -- AW + @override + def method2(self, x): ... # error: [invalid-explicit-override] +``` + +## Definitions in statically known branches + +```toml +[environment] +python-version = "3.10" +``` + +```py +import sys +from typing_extensions import override, overload + +class Parent: + if sys.version_info >= (3, 10): + def foo(self) -> None: ... + def foooo(self) -> None: ... + else: + def bar(self) -> None: ... + def baz(self) -> None: ... + def spam(self) -> None: ... + +class Child(Parent): + @override + def foo(self) -> None: ... + + # The declaration on `Parent` is not reachable, + # so this is an error + @override + def bar(self) -> None: ... # error: [invalid-explicit-override] + + if sys.version_info >= (3, 10): + @override + def foooo(self) -> None: ... + @override + def baz(self) -> None: ... # error: [invalid-explicit-override] + else: + # This doesn't override any reachable definitions, + # but the subclass definition also isn't a reachable definition + # from the end of the scope with the given configuration, + # so it's not flagged + @override + def foooo(self) -> None: ... + @override + def spam(self) -> None: ... +``` + ## Overloads The typing spec states that for an overloaded method, `@override` should only be applied to the @@ -247,6 +465,39 @@ class Spam: def baz(self, x: int) -> int: ... ``` +## Overloads in statically-known branches in stub files + +```toml +[environment] +python-version = "3.10" +``` + +```pyi +import sys +from typing_extensions import overload, override + +class Foo: + if sys.version_info >= (3, 10): + @overload + @override + def method(self, x: int) -> int: ... # error: [invalid-explicit-override] + else: + @overload + def method(self, x: int) -> int: ... + @overload + def method(self, x: str) -> str: ... + + if sys.version_info >= (3, 10): + @overload + def method2(self, x: int) -> int: ... + else: + @overload + @override + def method2(self, x: int) -> int: ... + @overload + def method2(self, x: str) -> str: ... +``` + ## Classes inheriting from `Any` ```py diff --git a/crates/ty_python_semantic/resources/mdtest/pep613_type_aliases.md b/crates/ty_python_semantic/resources/mdtest/pep613_type_aliases.md index c3e63c7b8e..c41b88b3b1 100644 --- a/crates/ty_python_semantic/resources/mdtest/pep613_type_aliases.md +++ b/crates/ty_python_semantic/resources/mdtest/pep613_type_aliases.md @@ -149,6 +149,101 @@ def _(x: MyAlias): reveal_type(x) # revealed: int | list[str] | set[str] ``` +## Typevar-specialized dynamic types + +We still recognize type aliases as being generic if a symbol of a dynamic type is explicitly +specialized with a type variable: + +```py +from typing import TypeVar, TypeAlias + +from unknown_module import UnknownClass # type: ignore + +T = TypeVar("T") + +MyAlias1: TypeAlias = UnknownClass[T] | None + +def _(a: MyAlias1[int]): + reveal_type(a) # revealed: Unknown | None +``` + +This also works with multiple type arguments: + +```py +U = TypeVar("U") +V = TypeVar("V") + +MyAlias2: TypeAlias = UnknownClass[T, U, V] | int + +def _(a: MyAlias2[int, str, bytes]): + reveal_type(a) # revealed: Unknown | int +``` + +If we specialize with fewer or more type arguments than expected, we emit an error: + +```py +def _( + # error: [invalid-type-arguments] "No type argument provided for required type variable `V`" + too_few: MyAlias2[int, str], + # error: [invalid-type-arguments] "Too many type arguments: expected 3, got 4" + too_many: MyAlias2[int, str, bytes, float], +): ... +``` + +We can also reference these type aliases from other type aliases: + +```py +MyAlias3: TypeAlias = MyAlias1[str] | MyAlias2[int, str, bytes] + +def _(c: MyAlias3): + reveal_type(c) # revealed: Unknown | None | int +``` + +Here, we test some other cases that might involve `@Todo` types, which also need special handling: + +```py +from typing_extensions import Callable, Concatenate, TypeAliasType + +MyAlias4: TypeAlias = Callable[Concatenate[dict[str, T], ...], list[U]] + +def _(c: MyAlias4[int, str]): + # TODO: should be (int, / ...) -> str + reveal_type(c) # revealed: Unknown + +T = TypeVar("T") + +MyList = TypeAliasType("MyList", list[T], type_params=(T,)) + +MyAlias5 = Callable[[MyList[T]], int] + +def _(c: MyAlias5[int]): + # TODO: should be (list[int], /) -> int + reveal_type(c) # revealed: (Unknown, /) -> int + +K = TypeVar("K") +V = TypeVar("V") + +MyDict = TypeAliasType("MyDict", dict[K, V], type_params=(K, V)) + +MyAlias6 = Callable[[MyDict[K, V]], int] + +def _(c: MyAlias6[str, bytes]): + # TODO: should be (dict[str, bytes], /) -> int + reveal_type(c) # revealed: (Unknown, /) -> int + +ListOrDict: TypeAlias = MyList[T] | dict[str, T] + +def _(x: ListOrDict[int]): + # TODO: should be list[int] | dict[str, int] + reveal_type(x) # revealed: Unknown | dict[str, int] + +MyAlias7: TypeAlias = Callable[Concatenate[T, ...], None] + +def _(c: MyAlias7[int]): + # TODO: should be (int, / ...) -> None + reveal_type(c) # revealed: Unknown +``` + ## Imported `alias.py`: diff --git a/crates/ty_python_semantic/resources/mdtest/pep695_type_aliases.md b/crates/ty_python_semantic/resources/mdtest/pep695_type_aliases.md index d4e4fafc73..799d1fc36f 100644 --- a/crates/ty_python_semantic/resources/mdtest/pep695_type_aliases.md +++ b/crates/ty_python_semantic/resources/mdtest/pep695_type_aliases.md @@ -223,7 +223,8 @@ T = TypeVar("T") IntAndT = TypeAliasType("IntAndT", tuple[int, T], type_params=(T,)) def f(x: IntAndT[str]) -> None: - reveal_type(x) # revealed: @Todo(Generic manual PEP-695 type alias) + # TODO: This should be `tuple[int, str]` + reveal_type(x) # revealed: Unknown ``` ### Error cases diff --git a/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi…_-_A_possibly-undefined…_(fc7b496fd1986deb).snap b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi…_-_A_possibly-undefined…_(fc7b496fd1986deb).snap index a137c44c51..68bd72f865 100644 --- a/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi…_-_A_possibly-undefined…_(fc7b496fd1986deb).snap +++ b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi…_-_A_possibly-undefined…_(fc7b496fd1986deb).snap @@ -65,13 +65,18 @@ mdtest path: crates/ty_python_semantic/resources/mdtest/final.md 51 | pass 52 | 53 | if coinflip(): -54 | def method2(self) -> None: ... # TODO: should emit [override-of-final-method] +54 | def method2(self) -> None: ... # error: [override-of-final-method] 55 | else: -56 | def method2(self) -> None: ... # TODO: should emit [override-of-final-method] +56 | def method2(self) -> None: ... 57 | 58 | if coinflip(): 59 | def method3(self) -> None: ... # error: [override-of-final-method] -60 | def method4(self) -> None: ... # error: [override-of-final-method] +60 | +61 | # TODO: we should emit Liskov violations here too: +62 | if coinflip(): +63 | method4 = 42 # error: [override-of-final-method] +64 | else: +65 | method4 = 56 ``` # Diagnostics @@ -240,6 +245,33 @@ info: rule `override-of-final-method` is enabled by default ``` +``` +error[override-of-final-method]: Cannot override `A.method2` + --> src/mdtest_snippet.py:54:13 + | +53 | if coinflip(): +54 | def method2(self) -> None: ... # error: [override-of-final-method] + | ^^^^^^^ Overrides a definition from superclass `A` +55 | else: +56 | def method2(self) -> None: ... + | +info: `A.method2` is decorated with `@final`, forbidding overrides + --> src/mdtest_snippet.py:16:9 + | +14 | def method2(self) -> None: ... +15 | else: +16 | @final + | ------ +17 | def method2(self) -> None: ... + | ------- `A.method2` defined here +18 | +19 | if coinflip(): + | +help: Remove the override of `method2` +info: rule `override-of-final-method` is enabled by default + +``` + ``` error[override-of-final-method]: Cannot override `A.method3` --> src/mdtest_snippet.py:59:13 @@ -247,7 +279,8 @@ error[override-of-final-method]: Cannot override `A.method3` 58 | if coinflip(): 59 | def method3(self) -> None: ... # error: [override-of-final-method] | ^^^^^^^ Overrides a definition from superclass `A` -60 | def method4(self) -> None: ... # error: [override-of-final-method] +60 | +61 | # TODO: we should emit Liskov violations here too: | info: `A.method3` is decorated with `@final`, forbidding overrides --> src/mdtest_snippet.py:20:9 @@ -267,12 +300,14 @@ info: rule `override-of-final-method` is enabled by default ``` error[override-of-final-method]: Cannot override `A.method4` - --> src/mdtest_snippet.py:60:13 + --> src/mdtest_snippet.py:63:9 | -58 | if coinflip(): -59 | def method3(self) -> None: ... # error: [override-of-final-method] -60 | def method4(self) -> None: ... # error: [override-of-final-method] - | ^^^^^^^ Overrides a definition from superclass `A` +61 | # TODO: we should emit Liskov violations here too: +62 | if coinflip(): +63 | method4 = 42 # error: [override-of-final-method] + | ^^^^^^^ Overrides a definition from superclass `A` +64 | else: +65 | method4 = 56 | info: `A.method4` is decorated with `@final`, forbidding overrides --> src/mdtest_snippet.py:29:9 diff --git a/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi…_-_Overloads_in_statica…_(29a698d9deaf7318).snap b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi…_-_Overloads_in_statica…_(29a698d9deaf7318).snap new file mode 100644 index 0000000000..fabea719d2 --- /dev/null +++ b/crates/ty_python_semantic/resources/mdtest/snapshots/final.md_-_Tests_for_the_`@typi…_-_Overloads_in_statica…_(29a698d9deaf7318).snap @@ -0,0 +1,94 @@ +--- +source: crates/ty_test/src/lib.rs +expression: snapshot +--- +--- +mdtest name: final.md - Tests for the `@typing(_extensions).final` decorator - Overloads in statically-known branches in stub files +mdtest path: crates/ty_python_semantic/resources/mdtest/final.md +--- + +# Python source files + +## mdtest_snippet.pyi + +``` + 1 | import sys + 2 | from typing_extensions import overload, final + 3 | + 4 | class Foo: + 5 | if sys.version_info >= (3, 10): + 6 | @overload + 7 | @final + 8 | def method(self, x: int) -> int: ... + 9 | else: +10 | @overload +11 | def method(self, x: int) -> int: ... +12 | @overload +13 | def method(self, x: str) -> str: ... +14 | +15 | if sys.version_info >= (3, 10): +16 | @overload +17 | def method2(self, x: int) -> int: ... +18 | else: +19 | @overload +20 | @final +21 | def method2(self, x: int) -> int: ... +22 | @overload +23 | def method2(self, x: str) -> str: ... +24 | +25 | class Bar(Foo): +26 | @overload +27 | def method(self, x: int) -> int: ... +28 | @overload +29 | def method(self, x: str) -> str: ... # error: [override-of-final-method] +30 | +31 | # This is fine: the only overload that is marked `@final` +32 | # is in a statically-unreachable branch +33 | @overload +34 | def method2(self, x: int) -> int: ... +35 | @overload +36 | def method2(self, x: str) -> str: ... +``` + +# Diagnostics + +``` +error[override-of-final-method]: Cannot override `Foo.method` + --> src/mdtest_snippet.pyi:29:9 + | +27 | def method(self, x: int) -> int: ... +28 | @overload +29 | def method(self, x: str) -> str: ... # error: [override-of-final-method] + | ^^^^^^ Overrides a definition from superclass `Foo` +30 | +31 | # This is fine: the only overload that is marked `@final` + | +info: `Foo.method` is decorated with `@final`, forbidding overrides + --> src/mdtest_snippet.pyi:7:9 + | + 5 | if sys.version_info >= (3, 10): + 6 | @overload + 7 | @final + | ------ + 8 | def method(self, x: int) -> int: ... + | ------ `Foo.method` defined here + 9 | else: +10 | @overload + | +help: Remove all overloads for `method` +info: rule `override-of-final-method` is enabled by default +23 | def method2(self, x: str) -> str: ... +24 | +25 | class Bar(Foo): + - @overload + - def method(self, x: int) -> int: ... + - @overload + - def method(self, x: str) -> str: ... # error: [override-of-final-method] +26 + +27 + # error: [override-of-final-method] +28 | +29 | # This is fine: the only overload that is marked `@final` +30 | # is in a statically-unreachable branch +note: This is an unsafe fix and may change runtime behavior + +``` diff --git a/crates/ty_python_semantic/resources/mdtest/snapshots/named_tuple.md_-_`NamedTuple`_-_Edge_case___multiple_…_(f30babd05c89dce9).snap b/crates/ty_python_semantic/resources/mdtest/snapshots/named_tuple.md_-_`NamedTuple`_-_Edge_case___multiple_…_(f30babd05c89dce9).snap new file mode 100644 index 0000000000..c7cd93b886 --- /dev/null +++ b/crates/ty_python_semantic/resources/mdtest/snapshots/named_tuple.md_-_`NamedTuple`_-_Edge_case___multiple_…_(f30babd05c89dce9).snap @@ -0,0 +1,74 @@ +--- +source: crates/ty_test/src/lib.rs +expression: snapshot +--- +--- +mdtest name: named_tuple.md - `NamedTuple` - Edge case: multiple reachable definitions with distinct issues +mdtest path: crates/ty_python_semantic/resources/mdtest/named_tuple.md +--- + +# Python source files + +## mdtest_snippet.py + +``` + 1 | from typing import NamedTuple + 2 | + 3 | def coinflip() -> bool: + 4 | return True + 5 | + 6 | class Foo(NamedTuple): + 7 | if coinflip(): + 8 | _asdict: bool # error: [invalid-named-tuple] "NamedTuple field `_asdict` cannot start with an underscore" + 9 | else: +10 | # TODO: there should only be one diagnostic here... +11 | # +12 | # error: [invalid-named-tuple] "Cannot overwrite NamedTuple attribute `_asdict`" +13 | # error: [invalid-named-tuple] "Cannot overwrite NamedTuple attribute `_asdict`" +14 | _asdict = True +``` + +# Diagnostics + +``` +error[invalid-named-tuple]: NamedTuple field name cannot start with an underscore + --> src/mdtest_snippet.py:8:9 + | + 6 | class Foo(NamedTuple): + 7 | if coinflip(): + 8 | _asdict: bool # error: [invalid-named-tuple] "NamedTuple field `_asdict` cannot start with an underscore" + | ^^^^^^^^^^^^^ Class definition will raise `TypeError` at runtime due to this field + 9 | else: +10 | # TODO: there should only be one diagnostic here... + | +info: rule `invalid-named-tuple` is enabled by default + +``` + +``` +error[invalid-named-tuple]: Cannot overwrite NamedTuple attribute `_asdict` + --> src/mdtest_snippet.py:14:9 + | +12 | # error: [invalid-named-tuple] "Cannot overwrite NamedTuple attribute `_asdict`" +13 | # error: [invalid-named-tuple] "Cannot overwrite NamedTuple attribute `_asdict`" +14 | _asdict = True + | ^^^^^^^ + | +info: This will cause the class creation to fail at runtime +info: rule `invalid-named-tuple` is enabled by default + +``` + +``` +error[invalid-named-tuple]: Cannot overwrite NamedTuple attribute `_asdict` + --> src/mdtest_snippet.py:14:9 + | +12 | # error: [invalid-named-tuple] "Cannot overwrite NamedTuple attribute `_asdict`" +13 | # error: [invalid-named-tuple] "Cannot overwrite NamedTuple attribute `_asdict`" +14 | _asdict = True + | ^^^^^^^ + | +info: This will cause the class creation to fail at runtime +info: rule `invalid-named-tuple` is enabled by default + +``` diff --git a/crates/ty_python_semantic/resources/mdtest/snapshots/override.md_-_`typing.override`_-_Basics_(b7c220f8171f11f0).snap b/crates/ty_python_semantic/resources/mdtest/snapshots/override.md_-_`typing.override`_-_Basics_(b7c220f8171f11f0).snap index 6e0cf0c01e..6c6c55d590 100644 --- a/crates/ty_python_semantic/resources/mdtest/snapshots/override.md_-_`typing.override`_-_Basics_(b7c220f8171f11f0).snap +++ b/crates/ty_python_semantic/resources/mdtest/snapshots/override.md_-_`typing.override`_-_Basics_(b7c220f8171f11f0).snap @@ -22,175 +22,221 @@ mdtest path: crates/ty_python_semantic/resources/mdtest/override.md 8 | 9 | class Parent: 10 | def foo(self): ... - 11 | @property - 12 | def my_property1(self) -> int: ... - 13 | @property - 14 | def my_property2(self) -> int: ... - 15 | baz = None - 16 | @classmethod - 17 | def class_method1(cls) -> int: ... - 18 | @staticmethod - 19 | def static_method1() -> int: ... + 11 | + 12 | @property + 13 | def my_property1(self) -> int: ... + 14 | + 15 | @property + 16 | def my_property2(self) -> int: ... + 17 | + 18 | baz = None + 19 | 20 | @classmethod - 21 | def class_method2(cls) -> int: ... - 22 | @staticmethod - 23 | def static_method2() -> int: ... - 24 | @lossy_decorator - 25 | def decorated_1(self): ... - 26 | @lossy_decorator - 27 | def decorated_2(self): ... - 28 | @lossy_decorator - 29 | def decorated_3(self): ... - 30 | - 31 | class Child(Parent): - 32 | @override - 33 | def foo(self): ... # fine: overrides `Parent.foo` - 34 | @property - 35 | @override - 36 | def my_property1(self) -> int: ... # fine: overrides `Parent.my_property1` - 37 | @override - 38 | @property - 39 | def my_property2(self) -> int: ... # fine: overrides `Parent.my_property2` - 40 | @override - 41 | def baz(self): ... # fine: overrides `Parent.baz` - 42 | @classmethod - 43 | @override - 44 | def class_method1(cls) -> int: ... # fine: overrides `Parent.class_method1` - 45 | @staticmethod + 21 | def class_method1(cls) -> int: ... + 22 | + 23 | @staticmethod + 24 | def static_method1() -> int: ... + 25 | + 26 | @classmethod + 27 | def class_method2(cls) -> int: ... + 28 | + 29 | @staticmethod + 30 | def static_method2() -> int: ... + 31 | + 32 | @lossy_decorator + 33 | def decorated_1(self): ... + 34 | + 35 | @lossy_decorator + 36 | def decorated_2(self): ... + 37 | + 38 | @lossy_decorator + 39 | def decorated_3(self): ... + 40 | + 41 | class Child(Parent): + 42 | @override + 43 | def foo(self): ... # fine: overrides `Parent.foo` + 44 | + 45 | @property 46 | @override - 47 | def static_method1() -> int: ... # fine: overrides `Parent.static_method1` - 48 | @override - 49 | @classmethod - 50 | def class_method2(cls) -> int: ... # fine: overrides `Parent.class_method2` - 51 | @override - 52 | @staticmethod - 53 | def static_method2() -> int: ... # fine: overrides `Parent.static_method2` - 54 | @override - 55 | def decorated_1(self): ... # fine: overrides `Parent.decorated_1` - 56 | @override - 57 | @lossy_decorator - 58 | def decorated_2(self): ... # fine: overrides `Parent.decorated_2` - 59 | @lossy_decorator - 60 | @override - 61 | def decorated_3(self): ... # fine: overrides `Parent.decorated_3` - 62 | - 63 | class OtherChild(Parent): ... - 64 | - 65 | class Grandchild(OtherChild): - 66 | @override - 67 | def foo(self): ... # fine: overrides `Parent.foo` + 47 | def my_property1(self) -> int: ... # fine: overrides `Parent.my_property1` + 48 | + 49 | @override + 50 | @property + 51 | def my_property2(self) -> int: ... # fine: overrides `Parent.my_property2` + 52 | + 53 | @override + 54 | def baz(self): ... # fine: overrides `Parent.baz` + 55 | + 56 | @classmethod + 57 | @override + 58 | def class_method1(cls) -> int: ... # fine: overrides `Parent.class_method1` + 59 | + 60 | @staticmethod + 61 | @override + 62 | def static_method1() -> int: ... # fine: overrides `Parent.static_method1` + 63 | + 64 | @override + 65 | @classmethod + 66 | def class_method2(cls) -> int: ... # fine: overrides `Parent.class_method2` + 67 | 68 | @override - 69 | @property - 70 | def bar(self) -> int: ... # fine: overrides `Parent.bar` - 71 | @override - 72 | def baz(self): ... # fine: overrides `Parent.baz` - 73 | @classmethod - 74 | @override - 75 | def class_method1(cls) -> int: ... # fine: overrides `Parent.class_method1` - 76 | @staticmethod - 77 | @override - 78 | def static_method1() -> int: ... # fine: overrides `Parent.static_method1` - 79 | @override - 80 | @classmethod - 81 | def class_method2(cls) -> int: ... # fine: overrides `Parent.class_method2` - 82 | @override - 83 | @staticmethod - 84 | def static_method2() -> int: ... # fine: overrides `Parent.static_method2` - 85 | @override - 86 | def decorated_1(self): ... # fine: overrides `Parent.decorated_1` - 87 | @override - 88 | @lossy_decorator - 89 | def decorated_2(self): ... # fine: overrides `Parent.decorated_2` - 90 | @lossy_decorator - 91 | @override - 92 | def decorated_3(self): ... # fine: overrides `Parent.decorated_3` - 93 | - 94 | class Invalid: - 95 | @override - 96 | def ___reprrr__(self): ... # error: [invalid-explicit-override] + 69 | @staticmethod + 70 | def static_method2() -> int: ... # fine: overrides `Parent.static_method2` + 71 | + 72 | @override + 73 | def decorated_1(self): ... # fine: overrides `Parent.decorated_1` + 74 | + 75 | @override + 76 | @lossy_decorator + 77 | def decorated_2(self): ... # fine: overrides `Parent.decorated_2` + 78 | + 79 | @lossy_decorator + 80 | @override + 81 | def decorated_3(self): ... # fine: overrides `Parent.decorated_3` + 82 | + 83 | class OtherChild(Parent): ... + 84 | + 85 | class Grandchild(OtherChild): + 86 | @override + 87 | def foo(self): ... # fine: overrides `Parent.foo` + 88 | + 89 | @override + 90 | @property + 91 | def my_property1(self) -> int: ... # fine: overrides `Parent.my_property1` + 92 | + 93 | @override + 94 | def baz(self): ... # fine: overrides `Parent.baz` + 95 | + 96 | @classmethod 97 | @override - 98 | @classmethod - 99 | def foo(self): ... # error: [invalid-explicit-override] -100 | @classmethod + 98 | def class_method1(cls) -> int: ... # fine: overrides `Parent.class_method1` + 99 | +100 | @staticmethod 101 | @override -102 | def bar(self): ... # error: [invalid-explicit-override] -103 | @staticmethod +102 | def static_method1() -> int: ... # fine: overrides `Parent.static_method1` +103 | 104 | @override -105 | def baz(): ... # error: [invalid-explicit-override] -106 | @override -107 | @staticmethod -108 | def eggs(): ... # error: [invalid-explicit-override] -109 | @property -110 | @override -111 | def bad_property1(self) -> int: ... # TODO: should emit `invalid-explicit-override` here +105 | @classmethod +106 | def class_method2(cls) -> int: ... # fine: overrides `Parent.class_method2` +107 | +108 | @override +109 | @staticmethod +110 | def static_method2() -> int: ... # fine: overrides `Parent.static_method2` +111 | 112 | @override -113 | @property -114 | def bad_property2(self) -> int: ... # TODO: should emit `invalid-explicit-override` here -115 | @lossy_decorator -116 | @override -117 | def lossy(self): ... # TODO: should emit `invalid-explicit-override` here -118 | @override +113 | def decorated_1(self): ... # fine: overrides `Parent.decorated_1` +114 | +115 | @override +116 | @lossy_decorator +117 | def decorated_2(self): ... # fine: overrides `Parent.decorated_2` +118 | 119 | @lossy_decorator -120 | def lossy2(self): ... # TODO: should emit `invalid-explicit-override` here -121 | -122 | # TODO: all overrides in this class should cause us to emit *Liskov* violations, -123 | # but not `@override` violations -124 | class LiskovViolatingButNotOverrideViolating(Parent): -125 | @override -126 | @property -127 | def foo(self) -> int: ... -128 | @override -129 | def my_property1(self) -> int: ... -130 | @staticmethod -131 | @override -132 | def class_method1() -> int: ... -133 | @classmethod -134 | @override -135 | def static_method1(cls) -> int: ... -136 | -137 | # Diagnostic edge case: `override` is very far away from the method definition in the source code: +120 | @override +121 | def decorated_3(self): ... # fine: overrides `Parent.decorated_3` +122 | +123 | class Invalid: +124 | @override +125 | def ___reprrr__(self): ... # error: [invalid-explicit-override] +126 | +127 | @override +128 | @classmethod +129 | def foo(self): ... # error: [invalid-explicit-override] +130 | +131 | @classmethod +132 | @override +133 | def bar(self): ... # error: [invalid-explicit-override] +134 | +135 | @staticmethod +136 | @override +137 | def baz(): ... # error: [invalid-explicit-override] 138 | -139 | T = TypeVar("T") -140 | -141 | def identity(x: T) -> T: ... +139 | @override +140 | @staticmethod +141 | def eggs(): ... # error: [invalid-explicit-override] 142 | -143 | class Foo: +143 | @property 144 | @override -145 | @identity -146 | @identity -147 | @identity -148 | @identity -149 | @identity -150 | @identity -151 | @identity -152 | @identity -153 | @identity -154 | @identity -155 | @identity -156 | @identity -157 | @identity -158 | @identity -159 | @identity -160 | @identity -161 | @identity -162 | @identity -163 | def bar(self): ... # error: [invalid-explicit-override] +145 | def bad_property1(self) -> int: ... # error: [invalid-explicit-override] +146 | +147 | @override +148 | @property +149 | def bad_property2(self) -> int: ... # error: [invalid-explicit-override] +150 | +151 | @property +152 | @override +153 | def bad_settable_property(self) -> int: ... # error: [invalid-explicit-override] +154 | @bad_settable_property.setter +155 | def bad_settable_property(self, x: int) -> None: ... +156 | +157 | @lossy_decorator +158 | @override +159 | def lossy(self): ... # TODO: should emit `invalid-explicit-override` here +160 | +161 | @override +162 | @lossy_decorator +163 | def lossy2(self): ... # TODO: should emit `invalid-explicit-override` here +164 | +165 | # TODO: all overrides in this class should cause us to emit *Liskov* violations, +166 | # but not `@override` violations +167 | class LiskovViolatingButNotOverrideViolating(Parent): +168 | @override +169 | @property +170 | def foo(self) -> int: ... +171 | +172 | @override +173 | def my_property1(self) -> int: ... +174 | +175 | @staticmethod +176 | @override +177 | def class_method1() -> int: ... +178 | +179 | @classmethod +180 | @override +181 | def static_method1(cls) -> int: ... +182 | +183 | # Diagnostic edge case: `override` is very far away from the method definition in the source code: +184 | +185 | T = TypeVar("T") +186 | +187 | def identity(x: T) -> T: ... +188 | +189 | class Foo: +190 | @override +191 | @identity +192 | @identity +193 | @identity +194 | @identity +195 | @identity +196 | @identity +197 | @identity +198 | @identity +199 | @identity +200 | @identity +201 | @identity +202 | @identity +203 | @identity +204 | @identity +205 | @identity +206 | @identity +207 | @identity +208 | @identity +209 | def bar(self): ... # error: [invalid-explicit-override] ``` # Diagnostics ``` error[invalid-explicit-override]: Method `___reprrr__` is decorated with `@override` but does not override anything - --> src/mdtest_snippet.pyi:95:5 - | -94 | class Invalid: -95 | @override - | --------- -96 | def ___reprrr__(self): ... # error: [invalid-explicit-override] - | ^^^^^^^^^^^ -97 | @override -98 | @classmethod - | + --> src/mdtest_snippet.pyi:124:5 + | +123 | class Invalid: +124 | @override + | --------- +125 | def ___reprrr__(self): ... # error: [invalid-explicit-override] + | ^^^^^^^^^^^ +126 | +127 | @override + | info: No `___reprrr__` definitions were found on any superclasses of `Invalid` info: rule `invalid-explicit-override` is enabled by default @@ -198,17 +244,17 @@ info: rule `invalid-explicit-override` is enabled by default ``` error[invalid-explicit-override]: Method `foo` is decorated with `@override` but does not override anything - --> src/mdtest_snippet.pyi:97:5 + --> src/mdtest_snippet.pyi:127:5 | - 95 | @override - 96 | def ___reprrr__(self): ... # error: [invalid-explicit-override] - 97 | @override +125 | def ___reprrr__(self): ... # error: [invalid-explicit-override] +126 | +127 | @override | --------- - 98 | @classmethod - 99 | def foo(self): ... # error: [invalid-explicit-override] +128 | @classmethod +129 | def foo(self): ... # error: [invalid-explicit-override] | ^^^ -100 | @classmethod -101 | @override +130 | +131 | @classmethod | info: No `foo` definitions were found on any superclasses of `Invalid` info: rule `invalid-explicit-override` is enabled by default @@ -217,16 +263,15 @@ info: rule `invalid-explicit-override` is enabled by default ``` error[invalid-explicit-override]: Method `bar` is decorated with `@override` but does not override anything - --> src/mdtest_snippet.pyi:101:5 + --> src/mdtest_snippet.pyi:132:5 | - 99 | def foo(self): ... # error: [invalid-explicit-override] -100 | @classmethod -101 | @override +131 | @classmethod +132 | @override | --------- -102 | def bar(self): ... # error: [invalid-explicit-override] +133 | def bar(self): ... # error: [invalid-explicit-override] | ^^^ -103 | @staticmethod -104 | @override +134 | +135 | @staticmethod | info: No `bar` definitions were found on any superclasses of `Invalid` info: rule `invalid-explicit-override` is enabled by default @@ -235,16 +280,15 @@ info: rule `invalid-explicit-override` is enabled by default ``` error[invalid-explicit-override]: Method `baz` is decorated with `@override` but does not override anything - --> src/mdtest_snippet.pyi:104:5 + --> src/mdtest_snippet.pyi:136:5 | -102 | def bar(self): ... # error: [invalid-explicit-override] -103 | @staticmethod -104 | @override +135 | @staticmethod +136 | @override | --------- -105 | def baz(): ... # error: [invalid-explicit-override] +137 | def baz(): ... # error: [invalid-explicit-override] | ^^^ -106 | @override -107 | @staticmethod +138 | +139 | @override | info: No `baz` definitions were found on any superclasses of `Invalid` info: rule `invalid-explicit-override` is enabled by default @@ -253,17 +297,17 @@ info: rule `invalid-explicit-override` is enabled by default ``` error[invalid-explicit-override]: Method `eggs` is decorated with `@override` but does not override anything - --> src/mdtest_snippet.pyi:106:5 + --> src/mdtest_snippet.pyi:139:5 | -104 | @override -105 | def baz(): ... # error: [invalid-explicit-override] -106 | @override +137 | def baz(): ... # error: [invalid-explicit-override] +138 | +139 | @override | --------- -107 | @staticmethod -108 | def eggs(): ... # error: [invalid-explicit-override] +140 | @staticmethod +141 | def eggs(): ... # error: [invalid-explicit-override] | ^^^^ -109 | @property -110 | @override +142 | +143 | @property | info: No `eggs` definitions were found on any superclasses of `Invalid` info: rule `invalid-explicit-override` is enabled by default @@ -271,21 +315,74 @@ info: rule `invalid-explicit-override` is enabled by default ``` ``` -error[invalid-explicit-override]: Method `bar` is decorated with `@override` but does not override anything - --> src/mdtest_snippet.pyi:163:9 +error[invalid-explicit-override]: Method `bad_property1` is decorated with `@override` but does not override anything + --> src/mdtest_snippet.pyi:144:5 | -161 | @identity -162 | @identity -163 | def bar(self): ... # error: [invalid-explicit-override] - | ^^^ - | - ::: src/mdtest_snippet.pyi:144:5 - | -143 | class Foo: +143 | @property 144 | @override | --------- -145 | @identity -146 | @identity +145 | def bad_property1(self) -> int: ... # error: [invalid-explicit-override] + | ^^^^^^^^^^^^^ +146 | +147 | @override + | +info: No `bad_property1` definitions were found on any superclasses of `Invalid` +info: rule `invalid-explicit-override` is enabled by default + +``` + +``` +error[invalid-explicit-override]: Method `bad_property2` is decorated with `@override` but does not override anything + --> src/mdtest_snippet.pyi:147:5 + | +145 | def bad_property1(self) -> int: ... # error: [invalid-explicit-override] +146 | +147 | @override + | --------- +148 | @property +149 | def bad_property2(self) -> int: ... # error: [invalid-explicit-override] + | ^^^^^^^^^^^^^ +150 | +151 | @property + | +info: No `bad_property2` definitions were found on any superclasses of `Invalid` +info: rule `invalid-explicit-override` is enabled by default + +``` + +``` +error[invalid-explicit-override]: Method `bad_settable_property` is decorated with `@override` but does not override anything + --> src/mdtest_snippet.pyi:152:5 + | +151 | @property +152 | @override + | --------- +153 | def bad_settable_property(self) -> int: ... # error: [invalid-explicit-override] + | ^^^^^^^^^^^^^^^^^^^^^ +154 | @bad_settable_property.setter +155 | def bad_settable_property(self, x: int) -> None: ... + | +info: No `bad_settable_property` definitions were found on any superclasses of `Invalid` +info: rule `invalid-explicit-override` is enabled by default + +``` + +``` +error[invalid-explicit-override]: Method `bar` is decorated with `@override` but does not override anything + --> src/mdtest_snippet.pyi:209:9 + | +207 | @identity +208 | @identity +209 | def bar(self): ... # error: [invalid-explicit-override] + | ^^^ + | + ::: src/mdtest_snippet.pyi:190:5 + | +189 | class Foo: +190 | @override + | --------- +191 | @identity +192 | @identity | info: No `bar` definitions were found on any superclasses of `Foo` info: rule `invalid-explicit-override` is enabled by default diff --git a/crates/ty_python_semantic/src/ast_node_ref.rs b/crates/ty_python_semantic/src/ast_node_ref.rs index 14916ec807..a3d1fae49a 100644 --- a/crates/ty_python_semantic/src/ast_node_ref.rs +++ b/crates/ty_python_semantic/src/ast_node_ref.rs @@ -1,6 +1,8 @@ use std::fmt::Debug; use std::marker::PhantomData; +#[cfg(debug_assertions)] +use ruff_db::files::File; use ruff_db::parsed::ParsedModuleRef; use ruff_python_ast::{AnyNodeRef, NodeIndex}; use ruff_python_ast::{AnyRootNodeRef, HasNodeIndex}; @@ -44,7 +46,7 @@ pub struct AstNodeRef { // cannot implement `Eq`, as indices are only unique within a given instance of the // AST. #[cfg(debug_assertions)] - module_addr: usize, + file: File, _node: PhantomData, } @@ -72,7 +74,7 @@ where Self { index, #[cfg(debug_assertions)] - module_addr: module_ref.module().addr(), + file: module_ref.module().file(), #[cfg(debug_assertions)] kind: AnyNodeRef::from(node).kind(), #[cfg(debug_assertions)] @@ -88,8 +90,7 @@ where #[track_caller] pub fn node<'ast>(&self, module_ref: &'ast ParsedModuleRef) -> &'ast T { #[cfg(debug_assertions)] - assert_eq!(module_ref.module().addr(), self.module_addr); - + assert_eq!(module_ref.module().file(), self.file); // The user guarantees that the module is from the same file and Salsa // revision, so the file contents cannot have changed. module_ref diff --git a/crates/ty_python_semantic/src/module_resolver/module.rs b/crates/ty_python_semantic/src/module_resolver/module.rs index 6927c3b89f..118c2aff45 100644 --- a/crates/ty_python_semantic/src/module_resolver/module.rs +++ b/crates/ty_python_semantic/src/module_resolver/module.rs @@ -120,7 +120,7 @@ impl std::fmt::Debug for Module<'_> { } #[allow(clippy::ref_option)] -#[salsa::tracked(returns(ref))] +#[salsa::tracked(returns(ref), heap_size=ruff_memory_usage::heap_size)] fn all_submodule_names_for_package<'db>( db: &'db dyn Db, module: Module<'db>, diff --git a/crates/ty_python_semantic/src/place.rs b/crates/ty_python_semantic/src/place.rs index 6e46819b14..3cb66efe33 100644 --- a/crates/ty_python_semantic/src/place.rs +++ b/crates/ty_python_semantic/src/place.rs @@ -459,7 +459,7 @@ fn core_module_scope(db: &dyn Db, core_module: KnownModule) -> Option( db: &'db dyn Db, bindings_with_constraints: BindingWithConstraintsIterator<'_, 'db>, -) -> Place<'db> { +) -> PlaceWithDefinition<'db> { place_from_bindings_impl(db, bindings_with_constraints, RequiresExplicitReExport::No) } @@ -487,20 +487,21 @@ type DeclaredTypeAndConflictingTypes<'db> = ( pub(crate) struct PlaceFromDeclarationsResult<'db> { place_and_quals: PlaceAndQualifiers<'db>, conflicting_types: Option>>>, - /// Contains `Some(declaration)` if the declared type originates from exactly one declaration. + /// Contains the first reachable declaration for this place, if any. /// This field is used for backreferences in diagnostics. - pub(crate) single_declaration: Option>, + pub(crate) first_declaration: Option>, } impl<'db> PlaceFromDeclarationsResult<'db> { fn conflict( place_and_quals: PlaceAndQualifiers<'db>, conflicting_types: Box>>, + first_declaration: Option>, ) -> Self { PlaceFromDeclarationsResult { place_and_quals, conflicting_types: Some(conflicting_types), - single_declaration: None, + first_declaration, } } @@ -798,6 +799,7 @@ pub(crate) fn place_by_id<'db>( if let Some(qualifiers) = declared.is_bare_final() { let bindings = all_considered_bindings(); return place_from_bindings_impl(db, bindings, requires_explicit_reexport) + .place .with_qualifiers(qualifiers); } @@ -809,7 +811,7 @@ pub(crate) fn place_by_id<'db>( qualifiers, } if qualifiers.contains(TypeQualifiers::CLASS_VAR) => { let bindings = all_considered_bindings(); - match place_from_bindings_impl(db, bindings, requires_explicit_reexport) { + match place_from_bindings_impl(db, bindings, requires_explicit_reexport).place { Place::Defined(inferred, origin, boundness) => Place::Defined( UnionType::from_elements(db, [Type::unknown(), inferred]), origin, @@ -835,7 +837,7 @@ pub(crate) fn place_by_id<'db>( let boundness_analysis = bindings.boundness_analysis; let inferred = place_from_bindings_impl(db, bindings, requires_explicit_reexport); - let place = match inferred { + let place = match inferred.place { // Place is possibly undeclared and definitely unbound Place::Undefined => { // TODO: We probably don't want to report `AlwaysDefined` here. This requires a bit of @@ -864,7 +866,8 @@ pub(crate) fn place_by_id<'db>( } => { let bindings = all_considered_bindings(); let boundness_analysis = bindings.boundness_analysis; - let mut inferred = place_from_bindings_impl(db, bindings, requires_explicit_reexport); + let mut inferred = + place_from_bindings_impl(db, bindings, requires_explicit_reexport).place; if boundness_analysis == BoundnessAnalysis::AssumeBound { if let Place::Defined(ty, origin, Definedness::PossiblyUndefined) = inferred { @@ -1010,7 +1013,7 @@ fn place_from_bindings_impl<'db>( db: &'db dyn Db, bindings_with_constraints: BindingWithConstraintsIterator<'_, 'db>, requires_explicit_reexport: RequiresExplicitReExport, -) -> Place<'db> { +) -> PlaceWithDefinition<'db> { let predicates = bindings_with_constraints.predicates; let reachability_constraints = bindings_with_constraints.reachability_constraints; let boundness_analysis = bindings_with_constraints.boundness_analysis; @@ -1039,6 +1042,8 @@ fn place_from_bindings_impl<'db>( }) }; + let mut first_definition = None; + let mut types = bindings_with_constraints.filter_map( |BindingWithConstraints { binding, @@ -1119,12 +1124,13 @@ fn place_from_bindings_impl<'db>( return None; } + first_definition.get_or_insert(binding); let binding_ty = binding_type(db, binding); Some(narrowing_constraint.narrow(db, binding_ty, binding.place(db))) }, ); - if let Some(first) = types.next() { + let place = if let Some(first) = types.next() { let ty = if let Some(second) = types.next() { let mut builder = PublicTypeBuilder::new(db); builder.add(first); @@ -1161,9 +1167,19 @@ fn place_from_bindings_impl<'db>( } } else { Place::Undefined + }; + + PlaceWithDefinition { + place, + first_definition, } } +pub(super) struct PlaceWithDefinition<'db> { + pub(super) place: Place<'db>, + pub(super) first_definition: Option>, +} + /// Accumulates types from multiple bindings or declarations, and eventually builds a /// union type from them. /// @@ -1294,7 +1310,6 @@ fn place_from_declarations_impl<'db>( let boundness_analysis = declarations.boundness_analysis; let mut declarations = declarations.peekable(); let mut first_declaration = None; - let mut exactly_one_declaration = false; let is_non_exported = |declaration: Definition<'db>| { requires_explicit_reexport.is_yes() && !is_reexported(db, declaration) @@ -1325,12 +1340,7 @@ fn place_from_declarations_impl<'db>( return None; } - if first_declaration.is_none() { - first_declaration = Some(declaration); - exactly_one_declaration = true; - } else { - exactly_one_declaration = false; - } + first_declaration.get_or_insert(declaration); let static_reachability = reachability_constraints.evaluate(db, predicates, reachability_constraint); @@ -1387,19 +1397,19 @@ fn place_from_declarations_impl<'db>( .with_qualifiers(declared.qualifiers()); if let Some(conflicting) = conflicting { - PlaceFromDeclarationsResult::conflict(place_and_quals, conflicting) + PlaceFromDeclarationsResult::conflict(place_and_quals, conflicting, first_declaration) } else { PlaceFromDeclarationsResult { place_and_quals, conflicting_types: None, - single_declaration: first_declaration.filter(|_| exactly_one_declaration), + first_declaration, } } } else { PlaceFromDeclarationsResult { place_and_quals: Place::Undefined.into(), conflicting_types: None, - single_declaration: None, + first_declaration: None, } } } diff --git a/crates/ty_python_semantic/src/semantic_index/predicate.rs b/crates/ty_python_semantic/src/semantic_index/predicate.rs index 7f96c13630..abefcc34b4 100644 --- a/crates/ty_python_semantic/src/semantic_index/predicate.rs +++ b/crates/ty_python_semantic/src/semantic_index/predicate.rs @@ -166,10 +166,10 @@ impl<'db> PatternPredicate<'db> { } } -/// A "placeholder predicate" that is used to model the fact that the boundness of a -/// (possible) definition or declaration caused by a `*` import cannot be fully determined -/// until type-inference time. This is essentially the same as a standard reachability constraint, -/// so we reuse the [`Predicate`] infrastructure to model it. +/// A "placeholder predicate" that is used to model the fact that the boundness of a (possible) +/// definition or declaration caused by a `*` import cannot be fully determined until type- +/// inference time. This is essentially the same as a standard reachability constraint, so we reuse +/// the [`Predicate`] infrastructure to model it. /// /// To illustrate, say we have a module `exporter.py` like so: /// @@ -183,14 +183,14 @@ impl<'db> PatternPredicate<'db> { /// ```py /// A = 1 /// -/// from importer import * +/// from exporter import * /// ``` /// -/// Since we cannot know whether or not is true at semantic-index time, -/// we record a definition for `A` in `b.py` as a result of the `from a import *` -/// statement, but place a predicate on it to record the fact that we don't yet -/// know whether this definition will be visible from all control-flow paths or not. -/// Essentially, we model `b.py` as something similar to this: +/// Since we cannot know whether or not is true at semantic-index time, we record +/// a definition for `A` in `importer.py` as a result of the `from exporter import *` statement, +/// but place a predicate on it to record the fact that we don't yet know whether this definition +/// will be visible from all control-flow paths or not. Essentially, we model `importer.py` as +/// something similar to this: /// /// ```py /// A = 1 @@ -199,8 +199,8 @@ impl<'db> PatternPredicate<'db> { /// from a import A /// ``` /// -/// At type-check time, the placeholder predicate for the `A` definition is evaluated by -/// attempting to resolve the `A` symbol in `a.py`'s global namespace: +/// At type-check time, the placeholder predicate for the `A` definition is evaluated by attempting +/// to resolve the `A` symbol in `exporter.py`'s global namespace: /// - If it resolves to a definitely bound symbol, then the predicate resolves to [`Truthiness::AlwaysTrue`] /// - If it resolves to an unbound symbol, then the predicate resolves to [`Truthiness::AlwaysFalse`] /// - If it resolves to a possibly bound symbol, then the predicate resolves to [`Truthiness::Ambiguous`] diff --git a/crates/ty_python_semantic/src/semantic_index/reachability_constraints.rs b/crates/ty_python_semantic/src/semantic_index/reachability_constraints.rs index 76ec1a70f0..9da8bbe87a 100644 --- a/crates/ty_python_semantic/src/semantic_index/reachability_constraints.rs +++ b/crates/ty_python_semantic/src/semantic_index/reachability_constraints.rs @@ -3,7 +3,7 @@ //! During semantic index building, we record so-called reachability constraints that keep track //! of a set of conditions that need to apply in order for a certain statement or expression to //! be reachable from the start of the scope. As an example, consider the following situation where -//! we have just processed two `if`-statements: +//! we have just processed an `if`-statement: //! ```py //! if test: //! @@ -101,13 +101,13 @@ //! //! ``` //! If we would not record any constraints at the branching point, we would have an `always-true` -//! reachability for the no-loop branch, and a `always-false` reachability for the branch which enters -//! the loop. Merging those would lead to a reachability of `always-true OR always-false = always-true`, +//! reachability for the no-loop branch, and a `always-true` reachability for the branch which enters +//! the loop. Merging those would lead to a reachability of `always-true OR always-true = always-true`, //! i.e. we would consider the end of the scope to be unconditionally reachable, which is not correct. //! //! Recording an ambiguous constraint at the branching point modifies the constraints in both branches to -//! `always-true AND ambiguous = ambiguous` and `always-false AND ambiguous = always-false`, respectively. -//! Merging these two using OR correctly leads to `ambiguous` for the end-of-scope reachability. +//! `always-true AND ambiguous = ambiguous`. Merging these two using OR correctly leads to `ambiguous` for +//! the end-of-scope reachability. //! //! //! ## Reachability constraints and bindings diff --git a/crates/ty_python_semantic/src/semantic_model.rs b/crates/ty_python_semantic/src/semantic_model.rs index f9ac728c40..2057db47ab 100644 --- a/crates/ty_python_semantic/src/semantic_model.rs +++ b/crates/ty_python_semantic/src/semantic_model.rs @@ -82,7 +82,7 @@ impl<'db> SemanticModel<'db> { memberdef.member.name, MemberDefinition { ty: memberdef.member.ty, - definition: memberdef.definition, + first_reachable_definition: memberdef.first_reachable_definition, }, ); } @@ -328,11 +328,11 @@ impl<'db> SemanticModel<'db> { } } -/// The type and definition (if available) of a symbol. +/// The type and definition of a symbol. #[derive(Clone, Debug)] pub struct MemberDefinition<'db> { pub ty: Type<'db>, - pub definition: Option>, + pub first_reachable_definition: Definition<'db>, } /// A classification of symbol names. diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs index eb145b291f..210e6a2e4f 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -763,7 +763,7 @@ impl<'db> DataclassParams<'db> { #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, salsa::Update, get_size2::GetSize)] pub enum Type<'db> { /// The dynamic type: a statically unknown set of values - Dynamic(DynamicType), + Dynamic(DynamicType<'db>), /// The empty set of values Never, /// A specific function object @@ -889,7 +889,10 @@ impl<'db> Type<'db> { } pub const fn is_unknown(&self) -> bool { - matches!(self, Type::Dynamic(DynamicType::Unknown)) + matches!( + self, + Type::Dynamic(DynamicType::Unknown | DynamicType::UnknownGeneric(_)) + ) } pub(crate) const fn is_never(&self) -> bool { @@ -975,7 +978,10 @@ impl<'db> Type<'db> { pub(crate) fn is_todo(&self) -> bool { self.as_dynamic().is_some_and(|dynamic| match dynamic { - DynamicType::Any | DynamicType::Unknown | DynamicType::Divergent(_) => false, + DynamicType::Any + | DynamicType::Unknown + | DynamicType::UnknownGeneric(_) + | DynamicType::Divergent(_) => false, DynamicType::Todo(_) | DynamicType::TodoStarredExpression | DynamicType::TodoUnpack => { true } @@ -1162,7 +1168,7 @@ impl<'db> Type<'db> { } } - pub(crate) const fn as_dynamic(self) -> Option { + pub(crate) const fn as_dynamic(self) -> Option> { match self { Type::Dynamic(dynamic_type) => Some(dynamic_type), _ => None, @@ -1176,7 +1182,7 @@ impl<'db> Type<'db> { } } - pub(crate) const fn expect_dynamic(self) -> DynamicType { + pub(crate) const fn expect_dynamic(self) -> DynamicType<'db> { self.as_dynamic().expect("Expected a Type::Dynamic variant") } @@ -6274,11 +6280,25 @@ impl<'db> Type<'db> { ), Type::Intersection(_) => { - Binding::single(self, Signature::todo("Type::Intersection.call()")).into() + Binding::single(self, Signature::todo("Type::Intersection.call")).into() } - // TODO: this is actually callable - Type::DataclassDecorator(_) => CallableBinding::not_callable(self).into(), + Type::DataclassDecorator(_) => { + let typevar = BoundTypeVarInstance::synthetic(db, "T", TypeVarVariance::Invariant); + let typevar_meta = SubclassOfType::from(db, typevar); + let context = GenericContext::from_typevar_instances(db, [typevar]); + let parameters = [Parameter::positional_only(Some(Name::new_static("cls"))) + .with_annotated_type(typevar_meta)]; + // Intersect with `Any` for the return type to reflect the fact that the `dataclass()` + // decorator adds methods to the class + let returns = IntersectionType::from_elements(db, [typevar_meta, Type::any()]); + let signature = Signature::new_generic( + Some(context), + Parameters::new(db, parameters), + Some(returns), + ); + Binding::single(self, signature).into() + } // TODO: some `SpecialForm`s are callable (e.g. TypedDicts) Type::SpecialForm(_) => CallableBinding::not_callable(self).into(), @@ -7885,14 +7905,18 @@ impl<'db> Type<'db> { typevars: &mut FxOrderSet>, visitor: &FindLegacyTypeVarsVisitor<'db>, ) { + let is_matching_typevar = |bound_typevar: &BoundTypeVarInstance<'db>| { + matches!( + bound_typevar.typevar(db).kind(db), + TypeVarKind::Legacy | TypeVarKind::TypingSelf | TypeVarKind::ParamSpec + ) && binding_context.is_none_or(|binding_context| { + bound_typevar.binding_context(db) == BindingContext::Definition(binding_context) + }) + }; + match self { Type::TypeVar(bound_typevar) => { - if matches!( - bound_typevar.typevar(db).kind(db), - TypeVarKind::Legacy | TypeVarKind::TypingSelf | TypeVarKind::ParamSpec - ) && binding_context.is_none_or(|binding_context| { - bound_typevar.binding_context(db) == BindingContext::Definition(binding_context) - }) { + if is_matching_typevar(&bound_typevar) { typevars.insert(bound_typevar); } } @@ -8032,6 +8056,14 @@ impl<'db> Type<'db> { } }, + Type::Dynamic(DynamicType::UnknownGeneric(generic_context)) => { + for variable in generic_context.variables(db) { + if is_matching_typevar(&variable) { + typevars.insert(variable); + } + } + } + Type::Dynamic(_) | Type::Never | Type::AlwaysTruthy @@ -8063,6 +8095,26 @@ impl<'db> Type<'db> { } } + /// Bind all unbound legacy type variables to the given context and then + /// add all legacy typevars to the provided set. + pub(crate) fn bind_and_find_all_legacy_typevars( + self, + db: &'db dyn Db, + binding_context: Option>, + variables: &mut FxOrderSet>, + ) { + self.apply_type_mapping( + db, + &TypeMapping::BindLegacyTypevars( + binding_context + .map(BindingContext::Definition) + .unwrap_or(BindingContext::Synthetic), + ), + TypeContext::default(), + ) + .find_legacy_typevars(db, None, variables); + } + /// Replace default types in parameters of callables with `Unknown`. pub(crate) fn replace_parameter_defaults(self, db: &'db dyn Db) -> Type<'db> { self.apply_type_mapping( @@ -8211,7 +8263,7 @@ impl<'db> Type<'db> { Self::SpecialForm(special_form) => special_form.definition(db), Self::Never => Type::SpecialForm(SpecialFormType::Never).definition(db), Self::Dynamic(DynamicType::Any) => Type::SpecialForm(SpecialFormType::Any).definition(db), - Self::Dynamic(DynamicType::Unknown) => Type::SpecialForm(SpecialFormType::Unknown).definition(db), + Self::Dynamic(DynamicType::Unknown | DynamicType::UnknownGeneric(_)) => Type::SpecialForm(SpecialFormType::Unknown).definition(db), Self::AlwaysTruthy => Type::SpecialForm(SpecialFormType::AlwaysTruthy).definition(db), Self::AlwaysFalsy => Type::SpecialForm(SpecialFormType::AlwaysFalsy).definition(db), @@ -8297,6 +8349,16 @@ impl<'db> Type<'db> { _ => None, } } + + /// Default-specialize all legacy typevars in this type. + /// + /// This is used when an implicit type alias is referenced without explicitly specializing it. + pub(crate) fn default_specialize(self, db: &'db dyn Db) -> Type<'db> { + let mut variables = FxOrderSet::default(); + self.find_legacy_typevars(db, None, &mut variables); + let generic_context = GenericContext::from_typevar_instances(db, variables); + self.apply_specialization(db, generic_context.default_specialization(db, None)) + } } impl<'db> From<&Type<'db>> for Type<'db> { @@ -8863,11 +8925,18 @@ pub struct DivergentType { impl get_size2::GetSize for DivergentType {} #[derive(Copy, Clone, Debug, Eq, Hash, PartialEq, salsa::Update, get_size2::GetSize)] -pub enum DynamicType { +pub enum DynamicType<'db> { /// An explicitly annotated `typing.Any` Any, /// An unannotated value, or a dynamic type resulting from an error Unknown, + /// Similar to `Unknown`, this represents a dynamic type that has been explicitly specialized + /// with legacy typevars, e.g. `UnknownClass[T]`, where `T` is a legacy typevar. We keep track + /// of the type variables in the generic context in case this type is later specialized again. + /// + /// TODO: Once we implement , this variant might + /// not be needed anymore. + UnknownGeneric(GenericContext<'db>), /// Temporary type for symbols that can't be inferred yet because of missing implementations. /// /// This variant should eventually be removed once ty is spec-compliant. @@ -8886,7 +8955,7 @@ pub enum DynamicType { Divergent(DivergentType), } -impl DynamicType { +impl DynamicType<'_> { fn normalized(self) -> Self { if matches!(self, Self::Divergent(_)) { self @@ -8904,11 +8973,11 @@ impl DynamicType { } } -impl std::fmt::Display for DynamicType { +impl std::fmt::Display for DynamicType<'_> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { DynamicType::Any => f.write_str("Any"), - DynamicType::Unknown => f.write_str("Unknown"), + DynamicType::Unknown | DynamicType::UnknownGeneric(_) => f.write_str("Unknown"), // `DynamicType::Todo`'s display should be explicit that is not a valid display of // any other type DynamicType::Todo(todo) => write!(f, "@Todo{todo}"), diff --git a/crates/ty_python_semantic/src/types/bound_super.rs b/crates/ty_python_semantic/src/types/bound_super.rs index 04cd24e40e..c67aacb323 100644 --- a/crates/ty_python_semantic/src/types/bound_super.rs +++ b/crates/ty_python_semantic/src/types/bound_super.rs @@ -172,7 +172,7 @@ impl<'db> BoundSuperError<'db> { #[derive(Debug, Copy, Clone, Hash, PartialEq, Eq, get_size2::GetSize)] pub enum SuperOwnerKind<'db> { - Dynamic(DynamicType), + Dynamic(DynamicType<'db>), Class(ClassType<'db>), Instance(NominalInstanceType<'db>), } diff --git a/crates/ty_python_semantic/src/types/class.rs b/crates/ty_python_semantic/src/types/class.rs index 9d40622d94..f3635fd3fa 100644 --- a/crates/ty_python_semantic/src/types/class.rs +++ b/crates/ty_python_semantic/src/types/class.rs @@ -1377,9 +1377,9 @@ pub(crate) struct Field<'db> { pub(crate) declared_ty: Type<'db>, /// Kind-specific metadata for this field pub(crate) kind: FieldKind<'db>, - /// The original declaration of this field, if there is exactly one. + /// The first declaration of this field. /// This field is used for backreferences in diagnostics. - pub(crate) single_declaration: Option>, + pub(crate) first_declaration: Option>, } impl Field<'_> { @@ -3041,7 +3041,7 @@ impl<'db> ClassLiteral<'db> { let symbol = table.symbol(symbol_id); let result = place_from_declarations(db, declarations.clone()); - let single_declaration = result.single_declaration; + let first_declaration = result.first_declaration; let attr = result.ignore_conflicting_declarations(); if attr.is_class_var() { continue; @@ -3049,7 +3049,9 @@ impl<'db> ClassLiteral<'db> { if let Some(attr_ty) = attr.place.ignore_possibly_undefined() { let bindings = use_def.end_of_scope_symbol_bindings(symbol_id); - let mut default_ty = place_from_bindings(db, bindings).ignore_possibly_undefined(); + let mut default_ty = place_from_bindings(db, bindings) + .place + .ignore_possibly_undefined(); default_ty = default_ty.map(|ty| ty.apply_optional_specialization(db, specialization)); @@ -3107,7 +3109,7 @@ impl<'db> ClassLiteral<'db> { let mut field = Field { declared_ty: attr_ty.apply_optional_specialization(db, specialization), kind, - single_declaration, + first_declaration, }; // Check if this is a KW_ONLY sentinel and mark subsequent fields as keyword-only @@ -3590,7 +3592,7 @@ impl<'db> ClassLiteral<'db> { // The attribute is declared in the class body. let bindings = use_def.end_of_scope_symbol_bindings(symbol_id); - let inferred = place_from_bindings(db, bindings); + let inferred = place_from_bindings(db, bindings).place; let has_binding = !inferred.is_undefined(); if has_binding { @@ -3833,7 +3835,9 @@ impl<'db> VarianceInferable<'db> for ClassLiteral<'db> { (symbol_id, place_and_qual) }) .chain(use_def_map.all_end_of_scope_symbol_bindings().map( - |(symbol_id, bindings)| (symbol_id, place_from_bindings(db, bindings).into()), + |(symbol_id, bindings)| { + (symbol_id, place_from_bindings(db, bindings).place.into()) + }, )) .filter_map(|(symbol_id, place_and_qual)| { if let Some(name) = table.place(symbol_id).as_symbol().map(Symbol::name) { diff --git a/crates/ty_python_semantic/src/types/class_base.rs b/crates/ty_python_semantic/src/types/class_base.rs index c29f72d4f9..c5ce573d3b 100644 --- a/crates/ty_python_semantic/src/types/class_base.rs +++ b/crates/ty_python_semantic/src/types/class_base.rs @@ -18,7 +18,7 @@ use crate::types::{ /// automatically construct the default specialization for that class. #[derive(Debug, Copy, Clone, Hash, PartialEq, Eq, salsa::Update, get_size2::GetSize)] pub enum ClassBase<'db> { - Dynamic(DynamicType), + Dynamic(DynamicType<'db>), Class(ClassType<'db>), /// Although `Protocol` is not a class in typeshed's stubs, it is at runtime, /// and can appear in the MRO of a class. @@ -62,7 +62,7 @@ impl<'db> ClassBase<'db> { match self { ClassBase::Class(class) => class.name(db), ClassBase::Dynamic(DynamicType::Any) => "Any", - ClassBase::Dynamic(DynamicType::Unknown) => "Unknown", + ClassBase::Dynamic(DynamicType::Unknown | DynamicType::UnknownGeneric(_)) => "Unknown", ClassBase::Dynamic( DynamicType::Todo(_) | DynamicType::TodoUnpack | DynamicType::TodoStarredExpression, ) => "@Todo", diff --git a/crates/ty_python_semantic/src/types/constraints.rs b/crates/ty_python_semantic/src/types/constraints.rs index 161f26b95f..a019860563 100644 --- a/crates/ty_python_semantic/src/types/constraints.rs +++ b/crates/ty_python_semantic/src/types/constraints.rs @@ -51,6 +51,19 @@ //! the constraint says that the typevar must specialize to that _exact_ type, not to a subtype or //! supertype of it. //! +//! ### Tracing +//! +//! This module is instrumented with debug- and trace-level `tracing` messages. You can set the +//! `TY_LOG` environment variable to see this output when testing locally. `tracing` log messages +//! typically have a `target` field, which is the name of the module the message appears in — in +//! this case, `ty_python_semantic::types::constraints`. We add additional detail to these targets, +//! in case you only want to debug parts of the implementation. For instance, if you want to debug +//! how we construct sequent maps, you could use +//! +//! ```sh +//! env TY_LOG=ty_python_semantic::types::constraints::SequentMap=trace ty check ... +//! ``` +//! //! [bdd]: https://en.wikipedia.org/wiki/Binary_decision_diagram use std::cell::RefCell; @@ -754,9 +767,10 @@ impl<'db> ConstrainedTypeVar<'db> { /// Terminal nodes (`false` and `true`) have their own dedicated enum variants. The /// [`Interior`][InteriorNode] variant represents interior nodes. /// -/// BDD nodes are _reduced_, which means that there are no duplicate nodes (which we handle via -/// Salsa interning), and that there are no redundant nodes, with `if_true` and `if_false` edges -/// that point at the same node. +/// BDD nodes are _quasi-reduced_, which means that there are no duplicate nodes (which we handle +/// via Salsa interning). Unlike the typical BDD representation, which is (fully) reduced, we do +/// allow redundant nodes, with `if_true` and `if_false` edges that point at the same node. That +/// means that our BDDs "remember" all of the individual constraints that they were created with. /// /// BDD nodes are also _ordered_, meaning that every path from the root of a BDD to a terminal node /// visits variables in the same order. [`ConstrainedTypeVar::ordering`] defines the variable @@ -769,7 +783,7 @@ enum Node<'db> { } impl<'db> Node<'db> { - /// Creates a new BDD node, ensuring that it is fully reduced. + /// Creates a new BDD node, ensuring that it is quasi-reduced. fn new( db: &'db dyn Db, constraint: ConstrainedTypeVar<'db>, @@ -784,9 +798,6 @@ impl<'db> Node<'db> { root_constraint.ordering(db) > constraint.ordering(db) }) ); - if if_true == if_false { - return if_true; - } Self::Interior(InteriorNode::new(db, constraint, if_true, if_false)) } @@ -854,10 +865,10 @@ impl<'db> Node<'db> { Node::AlwaysFalse => {} Node::Interior(interior) => { let constraint = interior.constraint(db); - path.walk_edge(map, constraint.when_true(), |path, _| { + path.walk_edge(db, map, constraint.when_true(), |path, _| { interior.if_true(db).for_each_path_inner(db, f, map, path); }); - path.walk_edge(map, constraint.when_false(), |path, _| { + path.walk_edge(db, map, constraint.when_false(), |path, _| { interior.if_false(db).for_each_path_inner(db, f, map, path); }); } @@ -892,7 +903,7 @@ impl<'db> Node<'db> { // impossible paths, and so we treat them as passing the "always satisfied" check. let constraint = interior.constraint(db); let true_always_satisfied = path - .walk_edge(map, constraint.when_true(), |path, _| { + .walk_edge(db, map, constraint.when_true(), |path, _| { interior .if_true(db) .is_always_satisfied_inner(db, map, path) @@ -903,7 +914,7 @@ impl<'db> Node<'db> { } // Ditto for the if_false branch - path.walk_edge(map, constraint.when_false(), |path, _| { + path.walk_edge(db, map, constraint.when_false(), |path, _| { interior .if_false(db) .is_always_satisfied_inner(db, map, path) @@ -941,7 +952,7 @@ impl<'db> Node<'db> { // impossible paths, and so we treat them as passing the "never satisfied" check. let constraint = interior.constraint(db); let true_never_satisfied = path - .walk_edge(map, constraint.when_true(), |path, _| { + .walk_edge(db, map, constraint.when_true(), |path, _| { interior.if_true(db).is_never_satisfied_inner(db, map, path) }) .unwrap_or(true); @@ -950,7 +961,7 @@ impl<'db> Node<'db> { } // Ditto for the if_false branch - path.walk_edge(map, constraint.when_false(), |path, _| { + path.walk_edge(db, map, constraint.when_false(), |path, _| { interior .if_false(db) .is_never_satisfied_inner(db, map, path) @@ -972,8 +983,15 @@ impl<'db> Node<'db> { /// Returns the `or` or union of two BDDs. fn or(self, db: &'db dyn Db, other: Self) -> Self { match (self, other) { - (Node::AlwaysTrue, _) | (_, Node::AlwaysTrue) => Node::AlwaysTrue, + (Node::AlwaysTrue, Node::AlwaysTrue) => Node::AlwaysTrue, (Node::AlwaysFalse, other) | (other, Node::AlwaysFalse) => other, + (Node::AlwaysTrue, Node::Interior(interior)) + | (Node::Interior(interior), Node::AlwaysTrue) => Node::new( + db, + interior.constraint(db), + Node::AlwaysTrue, + Node::AlwaysTrue, + ), (Node::Interior(a), Node::Interior(b)) => { // OR is commutative, which lets us halve the cache requirements let (a, b) = if b.0 < a.0 { (b, a) } else { (a, b) }; @@ -985,8 +1003,15 @@ impl<'db> Node<'db> { /// Returns the `and` or intersection of two BDDs. fn and(self, db: &'db dyn Db, other: Self) -> Self { match (self, other) { - (Node::AlwaysFalse, _) | (_, Node::AlwaysFalse) => Node::AlwaysFalse, + (Node::AlwaysFalse, Node::AlwaysFalse) => Node::AlwaysFalse, (Node::AlwaysTrue, other) | (other, Node::AlwaysTrue) => other, + (Node::AlwaysFalse, Node::Interior(interior)) + | (Node::Interior(interior), Node::AlwaysFalse) => Node::new( + db, + interior.constraint(db), + Node::AlwaysFalse, + Node::AlwaysFalse, + ), (Node::Interior(a), Node::Interior(b)) => { // AND is commutative, which lets us halve the cache requirements let (a, b) = if b.0 < a.0 { (b, a) } else { (a, b) }; @@ -1765,7 +1790,7 @@ impl<'db> InteriorNode<'db> { // we're about to remove. If so, we need to "remember" them by AND-ing them in with the // corresponding branch. let if_true = path - .walk_edge(map, self_constraint.when_true(), |path, new_range| { + .walk_edge(db, map, self_constraint.when_true(), |path, new_range| { let branch = self .if_true(db) .abstract_one_inner(db, should_remove, map, path); @@ -1782,7 +1807,7 @@ impl<'db> InteriorNode<'db> { }) .unwrap_or(Node::AlwaysFalse); let if_false = path - .walk_edge(map, self_constraint.when_false(), |path, new_range| { + .walk_edge(db, map, self_constraint.when_false(), |path, new_range| { let branch = self .if_false(db) .abstract_one_inner(db, should_remove, map, path); @@ -1802,13 +1827,13 @@ impl<'db> InteriorNode<'db> { } else { // Otherwise, we abstract the if_false/if_true edges recursively. let if_true = path - .walk_edge(map, self_constraint.when_true(), |path, _| { + .walk_edge(db, map, self_constraint.when_true(), |path, _| { self.if_true(db) .abstract_one_inner(db, should_remove, map, path) }) .unwrap_or(Node::AlwaysFalse); let if_false = path - .walk_edge(map, self_constraint.when_false(), |path, _| { + .walk_edge(db, map, self_constraint.when_false(), |path, _| { self.if_false(db) .abstract_one_inner(db, should_remove, map, path) }) @@ -1858,6 +1883,11 @@ impl<'db> InteriorNode<'db> { heap_size=ruff_memory_usage::heap_size, )] fn sequent_map(self, db: &'db dyn Db) -> SequentMap<'db> { + tracing::debug!( + target: "ty_python_semantic::types::constraints::SequentMap", + constraints = %Node::Interior(self).display(db), + "create sequent map", + ); let mut map = SequentMap::default(); Node::Interior(self).for_each_constraint(db, &mut |constraint| { map.add(db, constraint); @@ -2263,10 +2293,7 @@ impl<'db> ConstraintAssignment<'db> { } } - // Keep this for future debugging needs, even though it's not currently used when rendering - // constraint sets. - #[expect(dead_code)] - pub(crate) fn display(self, db: &'db dyn Db) -> impl Display { + fn display(self, db: &'db dyn Db) -> impl Display { struct DisplayConstraintAssignment<'db> { constraint: ConstraintAssignment<'db>, db: &'db dyn Db, @@ -2342,18 +2369,29 @@ impl<'db> SequentMap<'db> { continue; } - // Otherwise, check this constraint against all of the other ones we've seen so far, seeing + // First see if we can create any sequents from the constraint on its own. + tracing::trace!( + target: "ty_python_semantic::types::constraints::SequentMap", + constraint = %constraint.display(db), + "add sequents for constraint", + ); + self.add_sequents_for_single(db, constraint); + + // Then check this constraint against all of the other ones we've seen so far, seeing // if they're related to each other. let processed = std::mem::take(&mut self.processed); for other in &processed { if constraint != *other { + tracing::trace!( + target: "ty_python_semantic::types::constraints::SequentMap", + left = %constraint.display(db), + right = %other.display(db), + "add sequents for constraint pair", + ); self.add_sequents_for_pair(db, constraint, *other); } } self.processed = processed; - - // And see if we can create any sequents from the constraint on its own. - self.add_sequents_for_single(db, constraint); } } @@ -2377,8 +2415,14 @@ impl<'db> SequentMap<'db> { } } - fn add_single_tautology(&mut self, ante: ConstrainedTypeVar<'db>) { - self.single_tautologies.insert(ante); + fn add_single_tautology(&mut self, db: &'db dyn Db, ante: ConstrainedTypeVar<'db>) { + if self.single_tautologies.insert(ante) { + tracing::debug!( + target: "ty_python_semantic::types::constraints::SequentMap", + sequent = %format_args!("¬{} → false", ante.display(db)), + "add sequent", + ); + } } fn add_pair_impossibility( @@ -2387,8 +2431,16 @@ impl<'db> SequentMap<'db> { ante1: ConstrainedTypeVar<'db>, ante2: ConstrainedTypeVar<'db>, ) { - self.pair_impossibilities - .insert(Self::pair_key(db, ante1, ante2)); + if self + .pair_impossibilities + .insert(Self::pair_key(db, ante1, ante2)) + { + tracing::debug!( + target: "ty_python_semantic::types::constraints::SequentMap", + sequent = %format_args!("{} ∧ {} → false", ante1.display(db), ante2.display(db)), + "add sequent", + ); + } } fn add_pair_implication( @@ -2402,24 +2454,50 @@ impl<'db> SequentMap<'db> { if ante1.implies(db, post) || ante2.implies(db, post) { return; } - self.pair_implications + if self + .pair_implications .entry(Self::pair_key(db, ante1, ante2)) .or_default() - .insert(post); + .insert(post) + { + tracing::debug!( + target: "ty_python_semantic::types::constraints::SequentMap", + sequent = %format_args!( + "{} ∧ {} → {}", + ante1.display(db), + ante2.display(db), + post.display(db), + ), + "add sequent", + ); + } } fn add_single_implication( &mut self, + db: &'db dyn Db, ante: ConstrainedTypeVar<'db>, post: ConstrainedTypeVar<'db>, ) { if ante == post { return; } - self.single_implications + if self + .single_implications .entry(ante) .or_default() - .insert(post); + .insert(post) + { + tracing::debug!( + target: "ty_python_semantic::types::constraints::SequentMap", + sequent = %format_args!( + "{} → {}", + ante.display(db), + post.display(db), + ), + "add sequent", + ); + } } fn add_sequents_for_single(&mut self, db: &'db dyn Db, constraint: ConstrainedTypeVar<'db>) { @@ -2428,7 +2506,7 @@ impl<'db> SequentMap<'db> { let lower = constraint.lower(db); let upper = constraint.upper(db); if lower.is_never() && upper.is_object() { - self.add_single_tautology(constraint); + self.add_single_tautology(db, constraint); return; } @@ -2465,7 +2543,7 @@ impl<'db> SequentMap<'db> { _ => return, }; - self.add_single_implication(constraint, post_constraint); + self.add_single_implication(db, constraint, post_constraint); self.enqueue_constraint(post_constraint); } @@ -2660,25 +2738,50 @@ impl<'db> SequentMap<'db> { // elements. (For instance, when processing `T ≤ τ₁ & τ₂` and `T ≤ τ₂ & τ₁`, these clauses // would add sequents for `(T ≤ τ₁ & τ₂) → (T ≤ τ₂ & τ₁)` and vice versa.) if left_constraint.implies(db, right_constraint) { - self.add_single_implication(left_constraint, right_constraint); + tracing::debug!( + target: "ty_python_semantic::types::constraints::SequentMap", + left = %left_constraint.display(db), + right = %right_constraint.display(db), + "left implies right", + ); + self.add_single_implication(db, left_constraint, right_constraint); } if right_constraint.implies(db, left_constraint) { - self.add_single_implication(right_constraint, left_constraint); + tracing::debug!( + target: "ty_python_semantic::types::constraints::SequentMap", + left = %left_constraint.display(db), + right = %right_constraint.display(db), + "right implies left", + ); + self.add_single_implication(db, right_constraint, left_constraint); } match left_constraint.intersect(db, right_constraint) { Some(intersection_constraint) => { + tracing::debug!( + target: "ty_python_semantic::types::constraints::SequentMap", + left = %left_constraint.display(db), + right = %right_constraint.display(db), + intersection = %intersection_constraint.display(db), + "left and right overlap", + ); self.add_pair_implication( db, left_constraint, right_constraint, intersection_constraint, ); - self.add_single_implication(intersection_constraint, left_constraint); - self.add_single_implication(intersection_constraint, right_constraint); + self.add_single_implication(db, intersection_constraint, left_constraint); + self.add_single_implication(db, intersection_constraint, right_constraint); self.enqueue_constraint(intersection_constraint); } None => { + tracing::debug!( + target: "ty_python_semantic::types::constraints::SequentMap", + left = %left_constraint.display(db), + right = %right_constraint.display(db), + "left and right are disjoint", + ); self.add_pair_impossibility(db, left_constraint, right_constraint); } } @@ -2781,6 +2884,7 @@ impl<'db> PathAssignments<'db> { /// the path we're on. fn walk_edge( &mut self, + db: &'db dyn Db, map: &SequentMap<'db>, assignment: ConstraintAssignment<'db>, f: impl FnOnce(&mut Self, Range) -> R, @@ -2791,7 +2895,17 @@ impl<'db> PathAssignments<'db> { let start = self.assignments.len(); // Add the new assignment and anything we can derive from it. - let result = if self.add_assignment(map, assignment).is_err() { + tracing::trace!( + target: "ty_python_semantic::types::constraints::PathAssignment", + before = %format_args!( + "[{}]", + self.assignments[..start].iter().map(|assignment| assignment.display(db)).format(", "), + ), + edge = %assignment.display(db), + "walk edge", + ); + let found_conflict = self.add_assignment(db, map, assignment); + let result = if found_conflict.is_err() { // If that results in the path now being impossible due to a contradiction, return // without invoking the callback. None @@ -2801,6 +2915,14 @@ impl<'db> PathAssignments<'db> { // if that happens, `start..end` will mark the assignments that were added by the // `add_assignment` call above — that is, the new assignment for this edge along with // the derived information we inferred from it. + tracing::trace!( + target: "ty_python_semantic::types::constraints::PathAssignment", + new = %format_args!( + "[{}]", + self.assignments[start..].iter().map(|assignment| assignment.display(db)).format(", "), + ), + "new assignments", + ); let end = self.assignments.len(); Some(f(self, start..end)) }; @@ -2831,12 +2953,22 @@ impl<'db> PathAssignments<'db> { /// to become invalid, due to a contradiction, returns a [`PathAssignmentConflict`] error. fn add_assignment( &mut self, + db: &'db dyn Db, map: &SequentMap<'db>, assignment: ConstraintAssignment<'db>, ) -> Result<(), PathAssignmentConflict> { // First add this assignment. If it causes a conflict, return that as an error. If we've // already know this assignment holds, just return. if self.assignments.contains(&assignment.negated()) { + tracing::trace!( + target: "ty_python_semantic::types::constraints::PathAssignment", + assignment = %assignment.display(db), + facts = %format_args!( + "[{}]", + self.assignments.iter().map(|assignment| assignment.display(db)).format(", "), + ), + "found contradiction", + ); return Err(PathAssignmentConflict); } if !self.assignments.insert(assignment) { @@ -2852,6 +2984,15 @@ impl<'db> PathAssignments<'db> { if self.assignment_holds(ante.when_false()) { // The sequent map says (ante1) is always true, and the current path asserts that // it's false. + tracing::trace!( + target: "ty_python_semantic::types::constraints::PathAssignment", + ante = %ante.display(db), + facts = %format_args!( + "[{}]", + self.assignments.iter().map(|assignment| assignment.display(db)).format(", "), + ), + "found contradiction", + ); return Err(PathAssignmentConflict); } } @@ -2861,6 +3002,16 @@ impl<'db> PathAssignments<'db> { { // The sequent map says (ante1 ∧ ante2) is an impossible combination, and the // current path asserts that both are true. + tracing::trace!( + target: "ty_python_semantic::types::constraints::PathAssignment", + ante1 = %ante1.display(db), + ante2 = %ante2.display(db), + facts = %format_args!( + "[{}]", + self.assignments.iter().map(|assignment| assignment.display(db)).format(", "), + ), + "found contradiction", + ); return Err(PathAssignmentConflict); } } @@ -2870,7 +3021,7 @@ impl<'db> PathAssignments<'db> { if self.assignment_holds(ante1.when_true()) && self.assignment_holds(ante2.when_true()) { - self.add_assignment(map, post.when_true())?; + self.add_assignment(db, map, post.when_true())?; } } } @@ -2878,7 +3029,7 @@ impl<'db> PathAssignments<'db> { for (ante, posts) in &map.single_implications { for post in posts { if self.assignment_holds(ante.when_true()) { - self.add_assignment(map, post.when_true())?; + self.add_assignment(db, map, post.when_true())?; } } } @@ -2967,10 +3118,13 @@ impl<'db> SatisfiedClause<'db> { } fn display(&self, db: &'db dyn Db) -> String { + if self.constraints.is_empty() { + return String::from("always"); + } + // This is a bit heavy-handed, but we need to output the constraints in a consistent order // even though Salsa IDs are assigned non-deterministically. This Display output is only // used in test cases, so we don't need to over-optimize it. - let mut constraints: Vec<_> = self .constraints .iter() @@ -3097,7 +3251,7 @@ impl<'db> SatisfiedClauses<'db> { // used in test cases, so we don't need to over-optimize it. if self.clauses.is_empty() { - return String::from("always"); + return String::from("never"); } let mut clauses: Vec<_> = self .clauses @@ -3202,8 +3356,20 @@ impl<'db> GenericContext<'db> { db: &'db dyn Db, constraints: ConstraintSet<'db>, ) -> Result, ()> { + tracing::debug!( + target: "ty_python_semantic::types::constraints::specialize_constrained", + generic_context = %self.display_full(db), + constraints = %constraints.node.display(db), + "create specialization for constraint set", + ); + // If the constraint set is cyclic, don't even try to construct a specialization. if constraints.is_cyclic(db) { + tracing::error!( + target: "ty_python_semantic::types::constraints::specialize_constrained", + constraints = %constraints.node.display(db), + "constraint set is cyclic", + ); // TODO: Better error return Err(()); } @@ -3216,6 +3382,11 @@ impl<'db> GenericContext<'db> { .fold(constraints.node, |constraints, bound_typevar| { constraints.and(db, bound_typevar.valid_specializations(db)) }); + tracing::debug!( + target: "ty_python_semantic::types::constraints::specialize_constrained", + valid = %abstracted.display(db), + "limited to valid specializations", + ); // Then we find all of the "representative types" for each typevar in the constraint set. let mut error_occurred = false; @@ -3234,10 +3405,24 @@ impl<'db> GenericContext<'db> { let mut unconstrained = false; let mut greatest_lower_bound = Type::Never; let mut least_upper_bound = Type::object(); - abstracted.find_representative_types(db, bound_typevar.identity(db), |bounds| { + let identity = bound_typevar.identity(db); + tracing::trace!( + target: "ty_python_semantic::types::constraints::specialize_constrained", + bound_typevar = %identity.display(db), + abstracted = %abstracted.retain_one(db, identity).display(db), + "find specialization for typevar", + ); + abstracted.find_representative_types(db, identity, |bounds| { satisfied = true; match bounds { Some((lower_bound, upper_bound)) => { + tracing::trace!( + target: "ty_python_semantic::types::constraints::specialize_constrained", + bound_typevar = %identity.display(db), + lower_bound = %lower_bound.display(db), + upper_bound = %upper_bound.display(db), + "found representative type", + ); greatest_lower_bound = UnionType::from_elements(db, [greatest_lower_bound, lower_bound]); least_upper_bound = @@ -3253,6 +3438,11 @@ impl<'db> GenericContext<'db> { // for this constraint set. if !satisfied { // TODO: Construct a useful error here + tracing::debug!( + target: "ty_python_semantic::types::constraints::specialize_constrained", + bound_typevar = %identity.display(db), + "typevar cannot be satisfied", + ); error_occurred = true; return None; } @@ -3260,18 +3450,36 @@ impl<'db> GenericContext<'db> { // The BDD is satisfiable, but the typevar is unconstrained, then we use `None` to tell // specialize_recursive to fall back on the typevar's default. if unconstrained { + tracing::debug!( + target: "ty_python_semantic::types::constraints::specialize_constrained", + bound_typevar = %identity.display(db), + "typevar is unconstrained", + ); return None; } // If `lower ≰ upper`, then there is no type that satisfies all of the paths in the // BDD. That's an ambiguous specialization, as described above. if !greatest_lower_bound.is_subtype_of(db, least_upper_bound) { + tracing::debug!( + target: "ty_python_semantic::types::constraints::specialize_constrained", + bound_typevar = %identity.display(db), + greatest_lower_bound = %greatest_lower_bound.display(db), + least_upper_bound = %least_upper_bound.display(db), + "typevar bounds are incompatible", + ); error_occurred = true; return None; } // Of all of the types that satisfy all of the paths in the BDD, we choose the // "largest" one (i.e., "closest to `object`") as the specialization. + tracing::debug!( + target: "ty_python_semantic::types::constraints::specialize_constrained", + bound_typevar = %identity.display(db), + specialization = %least_upper_bound.display(db), + "found specialization for typevar", + ); Some(least_upper_bound) }); @@ -3297,18 +3505,32 @@ mod tests { fn test_display_graph_output() { let expected = indoc! {r#" (T = str) - ┡━₁ (U = str) - │ ┡━₁ always - │ └─₀ (U = bool) - │ ┡━₁ always - │ └─₀ never + ┡━₁ (T = bool) + │ ┡━₁ (U = str) + │ │ ┡━₁ (U = bool) + │ │ │ ┡━₁ always + │ │ │ └─₀ always + │ │ └─₀ (U = bool) + │ │ ┡━₁ always + │ │ └─₀ never + │ └─₀ (U = str) + │ ┡━₁ (U = bool) + │ │ ┡━₁ always + │ │ └─₀ always + │ └─₀ (U = bool) + │ ┡━₁ always + │ └─₀ never └─₀ (T = bool) ┡━₁ (U = str) - │ ┡━₁ always + │ ┡━₁ (U = bool) + │ │ ┡━₁ always + │ │ └─₀ always │ └─₀ (U = bool) │ ┡━₁ always │ └─₀ never - └─₀ never + └─₀ (U = str) + ┡━₁ never + └─₀ never "#} .trim_end(); diff --git a/crates/ty_python_semantic/src/types/diagnostic.rs b/crates/ty_python_semantic/src/types/diagnostic.rs index 8b20466b51..d72cbf8dbc 100644 --- a/crates/ty_python_semantic/src/types/diagnostic.rs +++ b/crates/ty_python_semantic/src/types/diagnostic.rs @@ -37,13 +37,11 @@ use itertools::Itertools; use ruff_db::{ diagnostic::{Annotation, Diagnostic, Span, SubDiagnostic, SubDiagnosticSeverity}, parsed::parsed_module, - source::source_text, }; use ruff_diagnostics::{Edit, Fix}; use ruff_python_ast::name::Name; -use ruff_python_ast::parenthesize::parentheses_iterator; +use ruff_python_ast::token::parentheses_iterator; use ruff_python_ast::{self as ast, AnyNodeRef, StringFlags}; -use ruff_python_trivia::CommentRanges; use ruff_text_size::{Ranged, TextRange}; use rustc_hash::FxHashSet; use std::fmt::{self, Formatter}; @@ -2423,9 +2421,7 @@ pub(super) fn report_invalid_assignment<'db>( // ) # ty: ignore <- or here // ``` - let comment_ranges = CommentRanges::from(context.module().tokens()); - let source = source_text(context.db(), context.file()); - parentheses_iterator(value_node.into(), None, &comment_ranges, &source) + parentheses_iterator(value_node.into(), None, context.module().tokens()) .last() .unwrap_or(value_node.range()) } else { diff --git a/crates/ty_python_semantic/src/types/enums.rs b/crates/ty_python_semantic/src/types/enums.rs index dbde339221..9a91d1da3d 100644 --- a/crates/ty_python_semantic/src/types/enums.rs +++ b/crates/ty_python_semantic/src/types/enums.rs @@ -77,7 +77,7 @@ pub(crate) fn enum_metadata<'db>( let ignored_names: Option> = if let Some(ignore) = table.symbol_id("_ignore_") { let ignore_bindings = use_def_map.all_reachable_symbol_bindings(ignore); - let ignore_place = place_from_bindings(db, ignore_bindings); + let ignore_place = place_from_bindings(db, ignore_bindings).place; match ignore_place { Place::Defined(Type::StringLiteral(ignored_names), _, _) => { @@ -111,7 +111,7 @@ pub(crate) fn enum_metadata<'db>( return None; } - let inferred = place_from_bindings(db, bindings); + let inferred = place_from_bindings(db, bindings).place; let value_ty = match inferred { Place::Undefined => { diff --git a/crates/ty_python_semantic/src/types/function.rs b/crates/ty_python_semantic/src/types/function.rs index 339689169c..4437e09698 100644 --- a/crates/ty_python_semantic/src/types/function.rs +++ b/crates/ty_python_semantic/src/types/function.rs @@ -373,7 +373,7 @@ impl<'db> OverloadLiteral<'db> { .scoped_use_id(db, scope); let Place::Defined(Type::FunctionLiteral(previous_type), _, Definedness::AlwaysDefined) = - place_from_bindings(db, use_def.bindings_at_use(use_id)) + place_from_bindings(db, use_def.bindings_at_use(use_id)).place else { return None; }; diff --git a/crates/ty_python_semantic/src/types/infer/builder.rs b/crates/ty_python_semantic/src/types/infer/builder.rs index d3a65e88b2..4d05719ebf 100644 --- a/crates/ty_python_semantic/src/types/infer/builder.rs +++ b/crates/ty_python_semantic/src/types/infer/builder.rs @@ -634,7 +634,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { &self.context, class, &field_name, - field.single_declaration, + field.first_declaration, ); } @@ -645,13 +645,13 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { } ) { field_with_default_encountered = - Some((field_name, field.single_declaration)); + Some((field_name, field.first_declaration)); } else if let Some(field_with_default) = field_with_default_encountered.as_ref() { report_namedtuple_field_without_default_after_field_with_default( &self.context, class, - (&field_name, field.single_declaration), + (&field_name, field.first_declaration), field_with_default, ); } @@ -1034,6 +1034,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { self.db(), use_def.end_of_scope_symbol_bindings(place.as_symbol().unwrap()), ) + .place { if function.file(self.db()) != self.file() { // If the function is not in this file, we don't need to check it. @@ -1727,6 +1728,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { let prior_bindings = use_def.bindings_at_definition(declaration); // unbound_ty is Never because for this check we don't care about unbound let inferred_ty = place_from_bindings(self.db(), prior_bindings) + .place .with_qualifiers(TypeQualifiers::empty()) .or_fall_back_to(self.db(), || { // Fallback to bindings declared on `types.ModuleType` if it's a global symbol @@ -8673,7 +8675,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { // If we're inferring types of deferred expressions, look them up from end-of-scope. if self.is_deferred() { let place = if let Some(place_id) = place_table.place_id(expr) { - place_from_bindings(db, use_def.all_reachable_bindings(place_id)) + place_from_bindings(db, use_def.all_reachable_bindings(place_id)).place } else { assert!( self.deferred_state.in_string_annotation(), @@ -8691,7 +8693,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { } let use_id = expr_ref.scoped_use_id(db, scope); - let place = place_from_bindings(db, use_def.bindings_at_use(use_id)); + let place = place_from_bindings(db, use_def.bindings_at_use(use_id)).place; (place, Some(use_id)) } @@ -8832,7 +8834,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { } } EnclosingSnapshotResult::FoundBindings(bindings) => { - let place = place_from_bindings(db, bindings).map_type(|ty| { + let place = place_from_bindings(db, bindings).place.map_type(|ty| { self.narrow_place_with_applicable_constraints( place_expr, ty, @@ -8952,13 +8954,14 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { return Place::Undefined.into(); } EnclosingSnapshotResult::FoundBindings(bindings) => { - let place = place_from_bindings(db, bindings).map_type(|ty| { - self.narrow_place_with_applicable_constraints( - place_expr, - ty, - &constraint_keys, - ) - }); + let place = + place_from_bindings(db, bindings).place.map_type(|ty| { + self.narrow_place_with_applicable_constraints( + place_expr, + ty, + &constraint_keys, + ) + }); constraint_keys.push(( FileScopeId::global(), ConstraintKey::NestedScope(file_scope_id), @@ -9571,6 +9574,9 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { (unknown @ Type::Dynamic(DynamicType::Unknown), _, _) | (_, unknown @ Type::Dynamic(DynamicType::Unknown), _) => Some(unknown), + (unknown @ Type::Dynamic(DynamicType::UnknownGeneric(_)), _, _) + | (_, unknown @ Type::Dynamic(DynamicType::UnknownGeneric(_)), _) => Some(unknown), + ( todo @ Type::Dynamic( DynamicType::Todo(_) @@ -10969,6 +10975,19 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { ); } } + Type::KnownInstance(KnownInstanceType::TypeAliasType(TypeAliasType::ManualPEP695( + _, + ))) => { + let slice_ty = self.infer_expression(slice, TypeContext::default()); + let mut variables = FxOrderSet::default(); + slice_ty.bind_and_find_all_legacy_typevars( + self.db(), + self.typevar_binding_context, + &mut variables, + ); + let generic_context = GenericContext::from_typevar_instances(self.db(), variables); + return Type::Dynamic(DynamicType::UnknownGeneric(generic_context)); + } Type::KnownInstance(KnownInstanceType::TypeAliasType(type_alias)) => { if let Some(generic_context) = type_alias.generic_context(self.db()) { return self.infer_explicit_type_alias_type_specialization( @@ -11107,33 +11126,74 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { )); } Type::SpecialForm(SpecialFormType::Callable) => { - // TODO: Remove this once we support ParamSpec properly. This is necessary to avoid - // a lot of false positives downstream, because we can't represent the specialized - // `Callable[P, _]` type yet. - if let Some(first_arg) = subscript - .slice - .as_ref() - .as_tuple_expr() - .and_then(|args| args.elts.first()) - && first_arg.is_name_expr() - { - let first_arg_ty = self.infer_expression(first_arg, TypeContext::default()); + let arguments = if let ast::Expr::Tuple(tuple) = &*subscript.slice { + &*tuple.elts + } else { + std::slice::from_ref(&*subscript.slice) + }; - if let Type::KnownInstance(KnownInstanceType::TypeVar(typevar)) = first_arg_ty - && typevar.kind(self.db()).is_paramspec() - { - return todo_type!("Callable[..] specialized with ParamSpec"); - } + // TODO: Remove this once we support ParamSpec and Concatenate properly. This is necessary + // to avoid a lot of false positives downstream, because we can't represent the typevar- + // specialized `Callable` types yet. + let num_arguments = arguments.len(); + if num_arguments == 2 { + let first_arg = &arguments[0]; + let second_arg = &arguments[1]; - if let Some(builder) = self.context.report_lint(&INVALID_TYPE_FORM, subscript) { - builder.into_diagnostic(format_args!( - "The first argument to `Callable` must be either a list of types, \ - ParamSpec, Concatenate, or `...`", + if first_arg.is_name_expr() { + let first_arg_ty = self.infer_expression(first_arg, TypeContext::default()); + if let Type::KnownInstance(KnownInstanceType::TypeVar(typevar)) = + first_arg_ty + && typevar.kind(self.db()).is_paramspec() + { + return todo_type!("Callable[..] specialized with ParamSpec"); + } + + if let Some(builder) = + self.context.report_lint(&INVALID_TYPE_FORM, subscript) + { + builder.into_diagnostic(format_args!( + "The first argument to `Callable` must be either a list of types, \ + ParamSpec, Concatenate, or `...`", + )); + } + return Type::KnownInstance(KnownInstanceType::Callable( + CallableType::unknown(self.db()), + )); + } else if first_arg.is_subscript_expr() { + let first_arg_ty = self.infer_expression(first_arg, TypeContext::default()); + if let Type::Dynamic(DynamicType::UnknownGeneric(generic_context)) = + first_arg_ty + { + let mut variables = generic_context + .variables(self.db()) + .collect::>(); + + let return_ty = + self.infer_expression(second_arg, TypeContext::default()); + return_ty.bind_and_find_all_legacy_typevars( + self.db(), + self.typevar_binding_context, + &mut variables, + ); + + let generic_context = + GenericContext::from_typevar_instances(self.db(), variables); + return Type::Dynamic(DynamicType::UnknownGeneric(generic_context)); + } + + if let Some(builder) = + self.context.report_lint(&INVALID_TYPE_FORM, subscript) + { + builder.into_diagnostic(format_args!( + "The first argument to `Callable` must be either a list of types, \ + ParamSpec, Concatenate, or `...`", + )); + } + return Type::KnownInstance(KnownInstanceType::Callable( + CallableType::unknown(self.db()), )); } - return Type::KnownInstance(KnownInstanceType::Callable( - CallableType::unknown(self.db()), - )); } let callable = self @@ -11240,6 +11300,17 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { ) => { return self.infer_explicit_type_alias_specialization(subscript, value_ty, false); } + Type::Dynamic(DynamicType::Unknown) => { + let slice_ty = self.infer_expression(slice, TypeContext::default()); + let mut variables = FxOrderSet::default(); + slice_ty.bind_and_find_all_legacy_typevars( + self.db(), + self.typevar_binding_context, + &mut variables, + ); + let generic_context = GenericContext::from_typevar_instances(self.db(), variables); + return Type::Dynamic(DynamicType::UnknownGeneric(generic_context)); + } _ => {} } @@ -11696,6 +11767,18 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { Some(Type::Dynamic(DynamicType::TodoUnpack)) } + (Type::SpecialForm(SpecialFormType::Concatenate), _) => { + // TODO: Add proper support for `Concatenate` + let mut variables = FxOrderSet::default(); + slice_ty.bind_and_find_all_legacy_typevars( + db, + self.typevar_binding_context, + &mut variables, + ); + let generic_context = GenericContext::from_typevar_instances(self.db(), variables); + Some(Type::Dynamic(DynamicType::UnknownGeneric(generic_context))) + } + (Type::SpecialForm(special_form), _) if special_form.class().is_special_form() => { Some(todo_type!("Inference of subscript on special form")) } diff --git a/crates/ty_python_semantic/src/types/infer/builder/annotation_expression.rs b/crates/ty_python_semantic/src/types/infer/builder/annotation_expression.rs index 8c211735cd..e143b5c7fa 100644 --- a/crates/ty_python_semantic/src/types/infer/builder/annotation_expression.rs +++ b/crates/ty_python_semantic/src/types/infer/builder/annotation_expression.rs @@ -144,18 +144,19 @@ impl<'db> TypeInferenceBuilder<'db, '_> { ) } _ => TypeAndQualifiers::declared( - ty.in_type_expression( - builder.db(), - builder.scope(), - builder.typevar_binding_context, - ) - .unwrap_or_else(|error| { - error.into_fallback_type( - &builder.context, - annotation, - builder.is_reachable(annotation), + ty.default_specialize(builder.db()) + .in_type_expression( + builder.db(), + builder.scope(), + builder.typevar_binding_context, ) - }), + .unwrap_or_else(|error| { + error.into_fallback_type( + &builder.context, + annotation, + builder.is_reachable(annotation), + ) + }), ), } } diff --git a/crates/ty_python_semantic/src/types/infer/builder/type_expression.rs b/crates/ty_python_semantic/src/types/infer/builder/type_expression.rs index 0e3a326139..56b0db5a09 100644 --- a/crates/ty_python_semantic/src/types/infer/builder/type_expression.rs +++ b/crates/ty_python_semantic/src/types/infer/builder/type_expression.rs @@ -91,6 +91,7 @@ impl<'db> TypeInferenceBuilder<'db, '_> { ast::Expr::Name(name) => match name.ctx { ast::ExprContext::Load => self .infer_name_expression(name) + .default_specialize(self.db()) .in_type_expression(self.db(), self.scope(), self.typevar_binding_context) .unwrap_or_else(|error| { error.into_fallback_type( @@ -108,6 +109,7 @@ impl<'db> TypeInferenceBuilder<'db, '_> { ast::Expr::Attribute(attribute_expression) => match attribute_expression.ctx { ast::ExprContext::Load => self .infer_attribute_expression(attribute_expression) + .default_specialize(self.db()) .in_type_expression(self.db(), self.scope(), self.typevar_binding_context) .unwrap_or_else(|error| { error.into_fallback_type( @@ -944,8 +946,17 @@ impl<'db> TypeInferenceBuilder<'db, '_> { } } KnownInstanceType::TypeAliasType(TypeAliasType::ManualPEP695(_)) => { - self.infer_type_expression(slice); - todo_type!("Generic manual PEP-695 type alias") + // TODO: support generic "manual" PEP 695 type aliases + let slice_ty = self.infer_expression(slice, TypeContext::default()); + let mut variables = FxOrderSet::default(); + slice_ty.bind_and_find_all_legacy_typevars( + self.db(), + self.typevar_binding_context, + &mut variables, + ); + let generic_context = + GenericContext::from_typevar_instances(self.db(), variables); + Type::Dynamic(DynamicType::UnknownGeneric(generic_context)) } KnownInstanceType::LiteralStringAlias(_) => { self.infer_type_expression(slice); @@ -982,6 +993,9 @@ impl<'db> TypeInferenceBuilder<'db, '_> { Type::unknown() } }, + Type::Dynamic(DynamicType::UnknownGeneric(_)) => { + self.infer_explicit_type_alias_specialization(subscript, value_ty, true) + } Type::Dynamic(_) => { // Infer slice as a value expression to avoid false-positive // `invalid-type-form` diagnostics, when we have e.g. diff --git a/crates/ty_python_semantic/src/types/list_members.rs b/crates/ty_python_semantic/src/types/list_members.rs index 66b92d69bc..eb47745b74 100644 --- a/crates/ty_python_semantic/src/types/list_members.rs +++ b/crates/ty_python_semantic/src/types/list_members.rs @@ -12,7 +12,9 @@ use rustc_hash::FxHashSet; use crate::{ Db, NameKind, - place::{Place, imported_symbol, place_from_bindings, place_from_declarations}, + place::{ + Place, PlaceWithDefinition, imported_symbol, place_from_bindings, place_from_declarations, + }, semantic_index::{ attribute_scopes, definition::Definition, global_scope, place_table, scope::ScopeId, semantic_index, use_def_map, @@ -35,48 +37,40 @@ pub(crate) fn all_members_of_scope<'db>( .all_end_of_scope_symbol_declarations() .filter_map(move |(symbol_id, declarations)| { let place_result = place_from_declarations(db, declarations); - let definition = place_result.single_declaration; - place_result + let first_reachable_definition = place_result.first_declaration?; + let ty = place_result .ignore_conflicting_declarations() .place - .ignore_possibly_undefined() - .map(|ty| { - let symbol = table.symbol(symbol_id); - let member = Member { - name: symbol.name().clone(), - ty, - }; - MemberWithDefinition { member, definition } - }) + .ignore_possibly_undefined()?; + let symbol = table.symbol(symbol_id); + let member = Member { + name: symbol.name().clone(), + ty, + }; + Some(MemberWithDefinition { + member, + first_reachable_definition, + }) }) .chain(use_def_map.all_end_of_scope_symbol_bindings().filter_map( move |(symbol_id, bindings)| { - // It's not clear to AG how to using a bindings - // iterator here to get the correct definition for - // this binding. Below, we look through all bindings - // with a definition and only take one if there is - // exactly one. I don't think this can be wrong, but - // it's probably omitting definitions in some cases. - let mut definition = None; - for binding in bindings.clone() { - if let Some(def) = binding.binding.definition() { - if definition.is_some() { - definition = None; - break; - } - definition = Some(def); - } - } - place_from_bindings(db, bindings) - .ignore_possibly_undefined() - .map(|ty| { - let symbol = table.symbol(symbol_id); - let member = Member { - name: symbol.name().clone(), - ty, - }; - MemberWithDefinition { member, definition } - }) + let PlaceWithDefinition { + place, + first_definition, + } = place_from_bindings(db, bindings); + + let first_reachable_definition = first_definition?; + let ty = place.ignore_possibly_undefined()?; + + let symbol = table.symbol(symbol_id); + let member = Member { + name: symbol.name().clone(), + ty, + }; + Some(MemberWithDefinition { + member, + first_reachable_definition, + }) }, )) } @@ -457,11 +451,11 @@ impl<'db> AllMembers<'db> { } } -/// A member of a type or scope, with an optional definition. +/// A member of a type or scope, with the first reachable definition of that member. #[derive(Clone, Debug, Eq, PartialEq, Hash)] pub struct MemberWithDefinition<'db> { pub member: Member<'db>, - pub definition: Option>, + pub first_reachable_definition: Definition<'db>, } /// A member of a type or scope. diff --git a/crates/ty_python_semantic/src/types/member.rs b/crates/ty_python_semantic/src/types/member.rs index 11a8648f50..dfb6cd47fa 100644 --- a/crates/ty_python_semantic/src/types/member.rs +++ b/crates/ty_python_semantic/src/types/member.rs @@ -75,7 +75,7 @@ pub(super) fn class_member<'db>(db: &'db dyn Db, scope: ScopeId<'db>, name: &str // Otherwise, we need to check if the symbol has bindings let use_def = use_def_map(db, scope); let bindings = use_def.end_of_scope_symbol_bindings(symbol_id); - let inferred = place_from_bindings(db, bindings); + let inferred = place_from_bindings(db, bindings).place; // TODO: we should not need to calculate inferred type second time. This is a temporary // solution until the notion of Boundness and Declaredness is split. See #16036, #16264 diff --git a/crates/ty_python_semantic/src/types/overrides.rs b/crates/ty_python_semantic/src/types/overrides.rs index 683f5549d1..5d6328a606 100644 --- a/crates/ty_python_semantic/src/types/overrides.rs +++ b/crates/ty_python_semantic/src/types/overrides.rs @@ -12,8 +12,8 @@ use crate::{ lint::LintId, place::Place, semantic_index::{ - definition::DefinitionKind, place_table, scope::ScopeId, symbol::ScopedSymbolId, - use_def_map, + definition::DefinitionKind, place::ScopedPlaceId, place_table, scope::ScopeId, + symbol::ScopedSymbolId, use_def_map, }, types::{ ClassBase, ClassLiteral, ClassType, KnownClass, Type, @@ -25,7 +25,7 @@ use crate::{ report_overridden_final_method, }, function::{FunctionDecorators, FunctionType, KnownFunction}, - list_members::{MemberWithDefinition, all_members_of_scope}, + list_members::{Member, MemberWithDefinition, all_members_of_scope}, }, }; @@ -53,10 +53,11 @@ pub(super) fn check_class<'db>(context: &InferContext<'db, '_>, class: ClassLite } let class_specialized = class.identity_specialization(db); - let own_class_members: FxHashSet<_> = all_members_of_scope(db, class.body_scope(db)).collect(); + let scope = class.body_scope(db); + let own_class_members: FxHashSet<_> = all_members_of_scope(db, scope).collect(); for member in own_class_members { - check_class_declaration(context, configuration, class_specialized, &member); + check_class_declaration(context, configuration, class_specialized, scope, &member); } } @@ -64,6 +65,7 @@ fn check_class_declaration<'db>( context: &InferContext<'db, '_>, configuration: OverrideRulesConfig, class: ClassType<'db>, + class_scope: ScopeId<'db>, member: &MemberWithDefinition<'db>, ) { /// Salsa-tracked query to check whether any of the definitions of a symbol @@ -101,40 +103,12 @@ fn check_class_declaration<'db>( .any(|definition| definition.kind(db).is_function_def()) } - fn extract_underlying_functions<'db>( - db: &'db dyn Db, - ty: Type<'db>, - ) -> Option; 1]>> { - match ty { - Type::FunctionLiteral(function) => Some(smallvec::smallvec_inline![function]), - Type::BoundMethod(method) => Some(smallvec::smallvec_inline![method.function(db)]), - Type::PropertyInstance(property) => { - extract_underlying_functions(db, property.getter(db)?) - } - Type::Union(union) => { - let mut functions = smallvec::smallvec![]; - for member in union.elements(db) { - if let Some(mut member_functions) = extract_underlying_functions(db, *member) { - functions.append(&mut member_functions); - } - } - if functions.is_empty() { - None - } else { - Some(functions) - } - } - _ => None, - } - } - let db = context.db(); - let MemberWithDefinition { member, definition } = member; - - let Some(definition) = definition else { - return; - }; + let MemberWithDefinition { + member, + first_reachable_definition, + } = member; let Place::Defined(type_on_subclass_instance, _, _) = Type::instance(db, class).member(db, &member.name).place @@ -153,10 +127,14 @@ fn check_class_declaration<'db>( if class_kind == Some(CodeGeneratorKind::NamedTuple) && configuration.check_prohibited_named_tuple_attrs() && PROHIBITED_NAMEDTUPLE_ATTRS.contains(&member.name.as_str()) - && !matches!(definition.kind(db), DefinitionKind::AnnotatedAssignment(_)) + && let Some(symbol_id) = place_table(db, class_scope).symbol_id(&member.name) + && let Some(bad_definition) = use_def_map(db, class_scope) + .all_reachable_bindings(ScopedPlaceId::Symbol(symbol_id)) + .filter_map(|binding| binding.binding.definition()) + .find(|def| !matches!(def.kind(db), DefinitionKind::AnnotatedAssignment(_))) && let Some(builder) = context.report_lint( &INVALID_NAMED_TUPLE, - definition.focus_range(db, context.module()), + bad_definition.focus_range(db, context.module()), ) { let mut diagnostic = builder.into_diagnostic(format_args!( @@ -212,8 +190,6 @@ fn check_class_declaration<'db>( .unwrap_or_default(); } - subclass_overrides_superclass_declaration = true; - let Place::Defined(superclass_type, _, _) = Type::instance(db, superclass) .member(db, &member.name) .place @@ -222,6 +198,8 @@ fn check_class_declaration<'db>( break; }; + subclass_overrides_superclass_declaration = true; + if configuration.check_final_method_overridden() { overridden_final_method = overridden_final_method.or_else(|| { let superclass_symbol_id = superclass_symbol_id?; @@ -297,7 +275,7 @@ fn check_class_declaration<'db>( context, &member.name, class, - *definition, + *first_reachable_definition, subclass_function, superclass, superclass_type, @@ -331,42 +309,18 @@ fn check_class_declaration<'db>( if !subclass_overrides_superclass_declaration && !has_dynamic_superclass - && definition.kind(db).is_function_def() - && let Type::FunctionLiteral(function) = member.ty - && function.has_known_decorator(db, FunctionDecorators::OVERRIDE) + // accessing `.kind()` here is fine as `definition` + // will always be a definition in the file currently being checked + && first_reachable_definition.kind(db).is_function_def() { - let function_literal = if context.in_stub() { - function.first_overload_or_implementation(db) - } else { - function.literal(db).last_definition(db) - }; - - if let Some(builder) = context.report_lint( - &INVALID_EXPLICIT_OVERRIDE, - function_literal.focus_range(db, context.module()), - ) { - let mut diagnostic = builder.into_diagnostic(format_args!( - "Method `{}` is decorated with `@override` but does not override anything", - member.name - )); - if let Some(decorator_span) = - function_literal.find_known_decorator_span(db, KnownFunction::Override) - { - diagnostic.annotate(Annotation::secondary(decorator_span)); - } - diagnostic.info(format_args!( - "No `{member}` definitions were found on any superclasses of `{class}`", - member = &member.name, - class = class.name(db) - )); - } + check_explicit_overrides(context, member, class); } if let Some((superclass, superclass_method)) = overridden_final_method { report_overridden_final_method( context, &member.name, - *definition, + *first_reachable_definition, member.ty, superclass, class, @@ -434,3 +388,72 @@ impl OverrideRulesConfig { self.contains(OverrideRulesConfig::PROHIBITED_NAMED_TUPLE_ATTR) } } + +fn check_explicit_overrides<'db>( + context: &InferContext<'db, '_>, + member: &Member<'db>, + class: ClassType<'db>, +) { + let db = context.db(); + let underlying_functions = extract_underlying_functions(db, member.ty); + let Some(functions) = underlying_functions else { + return; + }; + let Some(decorated_function) = functions + .iter() + .find(|function| function.has_known_decorator(db, FunctionDecorators::OVERRIDE)) + else { + return; + }; + let function_literal = if context.in_stub() { + decorated_function.first_overload_or_implementation(db) + } else { + decorated_function.literal(db).last_definition(db) + }; + + let Some(builder) = context.report_lint( + &INVALID_EXPLICIT_OVERRIDE, + function_literal.focus_range(db, context.module()), + ) else { + return; + }; + let mut diagnostic = builder.into_diagnostic(format_args!( + "Method `{}` is decorated with `@override` but does not override anything", + member.name + )); + if let Some(decorator_span) = + function_literal.find_known_decorator_span(db, KnownFunction::Override) + { + diagnostic.annotate(Annotation::secondary(decorator_span)); + } + diagnostic.info(format_args!( + "No `{member}` definitions were found on any superclasses of `{class}`", + member = &member.name, + class = class.name(db) + )); +} + +fn extract_underlying_functions<'db>( + db: &'db dyn Db, + ty: Type<'db>, +) -> Option; 1]>> { + match ty { + Type::FunctionLiteral(function) => Some(smallvec::smallvec_inline![function]), + Type::BoundMethod(method) => Some(smallvec::smallvec_inline![method.function(db)]), + Type::PropertyInstance(property) => extract_underlying_functions(db, property.getter(db)?), + Type::Union(union) => { + let mut functions = smallvec::smallvec![]; + for member in union.elements(db) { + if let Some(mut member_functions) = extract_underlying_functions(db, *member) { + functions.append(&mut member_functions); + } + } + if functions.is_empty() { + None + } else { + Some(functions) + } + } + _ => None, + } +} diff --git a/crates/ty_python_semantic/src/types/protocol_class.rs b/crates/ty_python_semantic/src/types/protocol_class.rs index 862349ea40..80ce2af47c 100644 --- a/crates/ty_python_semantic/src/types/protocol_class.rs +++ b/crates/ty_python_semantic/src/types/protocol_class.rs @@ -896,7 +896,10 @@ fn cached_protocol_interface<'db>( // type narrowing that uses `isinstance()` or `issubclass()` with // runtime-checkable protocols. for (symbol_id, bindings) in use_def_map.all_end_of_scope_symbol_bindings() { - let Some(ty) = place_from_bindings(db, bindings).ignore_possibly_undefined() else { + let Some(ty) = place_from_bindings(db, bindings) + .place + .ignore_possibly_undefined() + else { continue; }; direct_members.insert( diff --git a/crates/ty_python_semantic/src/types/subclass_of.rs b/crates/ty_python_semantic/src/types/subclass_of.rs index 0e3deed233..d906a472c3 100644 --- a/crates/ty_python_semantic/src/types/subclass_of.rs +++ b/crates/ty_python_semantic/src/types/subclass_of.rs @@ -322,7 +322,7 @@ impl<'db> VarianceInferable<'db> for SubclassOfType<'db> { #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, salsa::Update, get_size2::GetSize)] pub(crate) enum SubclassOfInner<'db> { Class(ClassType<'db>), - Dynamic(DynamicType), + Dynamic(DynamicType<'db>), TypeVar(BoundTypeVarInstance<'db>), } @@ -362,7 +362,7 @@ impl<'db> SubclassOfInner<'db> { } } - pub(crate) const fn into_dynamic(self) -> Option { + pub(crate) const fn into_dynamic(self) -> Option> { match self { Self::Class(_) | Self::TypeVar(_) => None, Self::Dynamic(dynamic) => Some(dynamic), @@ -465,8 +465,8 @@ impl<'db> From> for SubclassOfInner<'db> { } } -impl From for SubclassOfInner<'_> { - fn from(value: DynamicType) -> Self { +impl<'db> From> for SubclassOfInner<'db> { + fn from(value: DynamicType<'db>) -> Self { SubclassOfInner::Dynamic(value) } } diff --git a/crates/ty_python_semantic/src/types/type_ordering.rs b/crates/ty_python_semantic/src/types/type_ordering.rs index f57855fcb4..c47720011c 100644 --- a/crates/ty_python_semantic/src/types/type_ordering.rs +++ b/crates/ty_python_semantic/src/types/type_ordering.rs @@ -265,6 +265,9 @@ fn dynamic_elements_ordering(left: DynamicType, right: DynamicType) -> Ordering (DynamicType::Unknown, _) => Ordering::Less, (_, DynamicType::Unknown) => Ordering::Greater, + (DynamicType::UnknownGeneric(_), _) => Ordering::Less, + (_, DynamicType::UnknownGeneric(_)) => Ordering::Greater, + #[cfg(debug_assertions)] (DynamicType::Todo(TodoType(left)), DynamicType::Todo(TodoType(right))) => left.cmp(right), diff --git a/crates/ty_python_semantic/src/types/typed_dict.rs b/crates/ty_python_semantic/src/types/typed_dict.rs index 7a22f62507..14ee100d2d 100644 --- a/crates/ty_python_semantic/src/types/typed_dict.rs +++ b/crates/ty_python_semantic/src/types/typed_dict.rs @@ -365,7 +365,7 @@ pub(super) fn validate_typed_dict_key_assignment<'db, 'ast>( }; let add_item_definition_subdiagnostic = |diagnostic: &mut Diagnostic, message| { - if let Some(declaration) = item.single_declaration { + if let Some(declaration) = item.first_declaration { let file = declaration.file(db); let module = parsed_module(db, file).load(db);