Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[redknot] add module type and attribute lookup for some types #11416

Merged
merged 27 commits into from
May 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
8a09f9f
infer_definition_type for ImportDefinition
plredmond May 14, 2024
746b651
stub of red-knot unspecified-encoding lint rule (like #11288, specifi…
plredmond May 14, 2024
0b949c3
debug info so that panic shows what ctor was not matched
plredmond May 14, 2024
6030d2f
add ModuleTypeId and return one from infer_definition_type
plredmond May 17, 2024
3470a15
add Module (id) to ModuleTypeId
plredmond May 17, 2024
b7563f0
implement member lookup for ModuleTypeId
plredmond May 17, 2024
b88e9ea
better todo messages
plredmond May 17, 2024
ba234ff
import struct
plredmond May 17, 2024
59a9e46
clarify about fmtDisplay
plredmond May 17, 2024
fae67f6
start attribute-lookup implementation
plredmond May 18, 2024
280c009
start Call case in infer_expr_type; infer definition types for assign…
plredmond May 23, 2024
723e283
symbol we want to match against
plredmond May 24, 2024
baf8cb3
test of infer_expr_type Attribute case
plredmond May 24, 2024
8d967e1
remove lint stubs
plredmond May 24, 2024
d7268f7
a comment
plredmond May 24, 2024
6a94690
remove infer_expr_type for call
plredmond May 24, 2024
25395b6
remove NOTE from ModuleTypeId methods
plredmond May 28, 2024
71dd471
ClassTypeId method get_member
plredmond May 28, 2024
e028ac5
remove unwrap in Type::get_member and simplify cases for module and c…
plredmond May 28, 2024
feac185
correct typo in comment
plredmond May 28, 2024
51594b6
explain TODO for union of get_member
plredmond May 28, 2024
becb724
clarification about get_member on intersections
plredmond May 28, 2024
95b46c1
rename ClassTypeId::get_member to get_class_member
plredmond May 28, 2024
1bde5d6
infer_definition_type: move case for Import above case for ImportFrom
plredmond May 28, 2024
affbc3c
complete matching over Definition variants in infer_definition_type b…
plredmond May 28, 2024
8b2e202
clippy -- fix unused variable
plredmond May 28, 2024
a200b06
appease clippy
plredmond May 28, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 71 additions & 2 deletions crates/red_knot/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@
use crate::ast_ids::NodeKey;
use crate::db::{QueryResult, SemanticDb, SemanticJar};
use crate::files::FileId;
use crate::symbols::{symbol_table, GlobalSymbolId, ScopeId, ScopeKind, SymbolId};
use crate::module::{Module, ModuleName};
use crate::symbols::{
resolve_global_symbol, symbol_table, GlobalSymbolId, ScopeId, ScopeKind, SymbolId,
};
use crate::{FxDashMap, FxIndexSet, Name};
use ruff_index::{newtype_index, IndexVec};
use rustc_hash::FxHashMap;
Expand All @@ -25,6 +28,8 @@ pub enum Type {
Unbound,
/// a specific function object
Function(FunctionTypeId),
/// a specific module object
Module(ModuleTypeId),
/// a specific class object
Class(ClassTypeId),
/// the set of Python objects with the given class in their __class__'s method resolution order
Expand All @@ -46,6 +51,35 @@ impl Type {
pub const fn is_unknown(&self) -> bool {
matches!(self, Type::Unknown)
}

pub fn get_member(&self, db: &dyn SemanticDb, name: &Name) -> QueryResult<Option<Type>> {
match self {
Type::Any => todo!("attribute lookup on Any type"),
Type::Never => todo!("attribute lookup on Never type"),
Type::Unknown => todo!("attribute lookup on Unknown type"),
Type::Unbound => todo!("attribute lookup on Unbound type"),
Type::Function(_) => todo!("attribute lookup on Function type"),
Type::Module(module_id) => module_id.get_member(db, name),
Type::Class(class_id) => class_id.get_class_member(db, name),
Type::Instance(_) => {
// TODO MRO? get_own_instance_member, get_instance_member
todo!("attribute lookup on Instance type")
}
Type::Union(union_id) => {
let jar: &SemanticJar = db.jar()?;
let _todo_union_ref = jar.type_store.get_union(*union_id);
// TODO perform the get_member on each type in the union
// TODO return the union of those results
// TODO if any of those results is `None` then include Unknown in the result union
todo!("attribute lookup on Union type")
}
Type::Intersection(_) => {
// TODO perform the get_member on each type in the intersection
// TODO return the intersection of those results
todo!("attribute lookup on Intersection type")
}
}
}
}

impl From<FunctionTypeId> for Type {
Expand Down Expand Up @@ -336,6 +370,31 @@ impl FunctionTypeId {
}
}

#[derive(Copy, Clone, Debug, Hash, Eq, PartialEq)]
pub struct ModuleTypeId {
module: Module,
file_id: FileId,
plredmond marked this conversation as resolved.
Show resolved Hide resolved
}

impl ModuleTypeId {
fn module(self, db: &dyn SemanticDb) -> QueryResult<ModuleStoreRef> {
let jar: &SemanticJar = db.jar()?;
Ok(jar.type_store.add_or_get_module(self.file_id).downgrade())
}

pub(crate) fn name(self, db: &dyn SemanticDb) -> QueryResult<ModuleName> {
self.module.name(db)
}

fn get_member(self, db: &dyn SemanticDb, name: &Name) -> QueryResult<Option<Type>> {
if let Some(symbol_id) = resolve_global_symbol(db, self.name(db)?, name)? {
Ok(Some(infer_symbol_type(db, symbol_id)?))
} else {
Ok(None)
}
}
}

#[derive(Copy, Clone, Debug, Hash, Eq, PartialEq)]
pub struct ClassTypeId {
file_id: FileId,
Expand Down Expand Up @@ -389,7 +448,13 @@ impl ClassTypeId {
}
}

// TODO: get_own_instance_member, get_class_member, get_instance_member
/// Get own class member or fall back to super-class member.
fn get_class_member(self, db: &dyn SemanticDb, name: &Name) -> QueryResult<Option<Type>> {
self.get_own_class_member(db, name)
.or_else(|_| self.get_super_class_member(db, name))
}

// TODO: get_own_instance_member, get_instance_member
}

#[derive(Copy, Clone, Debug, Hash, Eq, PartialEq)]
Expand Down Expand Up @@ -529,6 +594,10 @@ impl std::fmt::Display for DisplayType<'_> {
Type::Never => f.write_str("Never"),
Type::Unknown => f.write_str("Unknown"),
Type::Unbound => f.write_str("Unbound"),
Type::Module(module_id) => {
// NOTE: something like this?: "<module 'module-name' from 'path-from-fileid'>"
todo!("{module_id:?}")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left this as a TODO for now, but could implement it if you tell me what you'd like. It's the type of a module, which doesn't have much info when python prints it out. Probably a bikeshed.

>>> import x
>>> x
<module 'x' from '/home/asteroid/code/redknot.attrchain/x.py'>
>>> type(x)
<class 'module'>

Copy link
Contributor

@carljm carljm May 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think copying the runtime output (for the module object, not the type, since this is not the general module type but the literal type for a particular module) is reasonable here; totally fine with you either doing it in this PR or leaving it as a todo for later, up to you.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, DisplayType only has access to TypeStore. We'd need a SemanticDb to call ModuleTypeId::name and/or a Files to call Files::path on FileId. I don't think I can access either of these in this context. I'll leave it for now.

}
// TODO functions and classes should display using a fully qualified name
Type::Class(class_id) => {
f.write_str("Literal[")?;
Expand Down
71 changes: 65 additions & 6 deletions crates/red_knot/src/types/infer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@ use ruff_python_ast::AstNode;

use crate::db::{QueryResult, SemanticDb, SemanticJar};

use crate::module::ModuleName;
use crate::module::{resolve_module, ModuleName};
use crate::parse::parse;
use crate::symbols::{
resolve_global_symbol, symbol_table, Definition, GlobalSymbolId, ImportFromDefinition,
resolve_global_symbol, symbol_table, Definition, GlobalSymbolId, ImportDefinition,
ImportFromDefinition,
};
use crate::types::Type;
use crate::FileId;
use crate::types::{ModuleTypeId, Type};
use crate::{FileId, Name};

// FIXME: Figure out proper dead-lock free synchronisation now that this takes `&db` instead of `&mut db`.
#[tracing::instrument(level = "trace", skip(db))]
Expand Down Expand Up @@ -46,6 +47,15 @@ pub fn infer_definition_type(
let file_id = symbol.file_id;

match definition {
Definition::Import(ImportDefinition {
module: module_name,
}) => {
if let Some(module) = resolve_module(db, module_name.clone())? {
Ok(Type::Module(ModuleTypeId { module, file_id }))
} else {
Ok(Type::Unknown)
}
}
Definition::ImportFrom(ImportFromDefinition {
module,
name,
Expand Down Expand Up @@ -114,10 +124,20 @@ pub fn infer_definition_type(
let parsed = parse(db.upcast(), file_id)?;
let ast = parsed.ast();
let node = node_key.resolve_unwrap(ast.as_any_node_ref());
// TODO handle unpacking assignment correctly
// TODO handle unpacking assignment correctly (here and for AnnotatedAssignment case, below)
infer_expr_type(db, file_id, &node.value)
}
_ => todo!("other kinds of definitions"),
Definition::AnnotatedAssignment(node_key) => {
let parsed = parse(db.upcast(), file_id)?;
let ast = parsed.ast();
let node = node_key.resolve_unwrap(ast.as_any_node_ref());
// TODO actually look at the annotation
let Some(value) = &node.value else {
return Ok(Type::Unknown);
};
// TODO handle unpacking assignment correctly (here and for Assignment case, above)
infer_expr_type(db, file_id, value)
}
}
}

Expand All @@ -133,6 +153,13 @@ fn infer_expr_type(db: &dyn SemanticDb, file_id: FileId, expr: &ast::Expr) -> Qu
Ok(Type::Unknown)
}
}
ast::Expr::Attribute(ast::ExprAttribute { value, attr, .. }) => {
let value_type = infer_expr_type(db, file_id, value)?;
let attr_name = &Name::new(&attr.id);
value_type
.get_member(db, attr_name)
.map(|ty| ty.unwrap_or(Type::Unknown))
}
_ => todo!("full expression type resolution"),
}
}
Expand Down Expand Up @@ -289,4 +316,36 @@ mod tests {

Ok(())
}

#[test]
fn resolve_module_member() -> anyhow::Result<()> {
let case = create_test()?;
let db = &case.db;

let a_path = case.src.path().join("a.py");
let b_path = case.src.path().join("b.py");
std::fs::write(a_path, "import b; D = b.C")?;
std::fs::write(b_path, "class C: pass")?;
let a_file = resolve_module(db, ModuleName::new("a"))?
.expect("module should be found")
.path(db)?
.file();
let a_syms = symbol_table(db, a_file)?;
let d_sym = a_syms
.root_symbol_id_by_name("D")
.expect("D symbol should be found");

let ty = infer_symbol_type(
db,
GlobalSymbolId {
file_id: a_file,
symbol_id: d_sym,
},
)?;

let jar = HasJar::<SemanticJar>::jar(db)?;
assert!(matches!(ty, Type::Class(_)));
assert_eq!(format!("{}", ty.display(&jar.type_store)), "Literal[C]");
Ok(())
}
}
Loading