Skip to content

Commit

Permalink
fix: qualifies builtins types in TypeInfo::builtin (#139)
Browse files Browse the repository at this point in the history
Builtin type identifiers like `int` or `string` can be shadowed with
local definitions. This PR avoids that by importing `builtins` module
and qualifies identifier with it. There is still the case where
`builtins` can accidentally redefined, but at least this change saves
corner cases.
  • Loading branch information
konn authored Jan 17, 2025
1 parent bee1ba7 commit 9660cd1
Show file tree
Hide file tree
Showing 13 changed files with 164 additions and 88 deletions.
34 changes: 34 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,4 @@ quote = "1.0.38"
serde = { version = "1.0.217", features = ["derive"] }
syn = "2.0.96"
toml = "0.8.19"
test-case = "3.3.1"
5 changes: 3 additions & 2 deletions examples/mixed/python/mixed/main_mod.pyi
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# This file is automatically generated by pyo3_stub_gen
# ruff: noqa: E501, F401

import builtins

class A:
def show_x(self) -> None:
Expand All @@ -12,9 +13,9 @@ class B:
...


def create_a(x:int) -> A:
def create_a(x:builtins.int) -> A:
...

def create_b(x:int) -> B:
def create_b(x:builtins.int) -> B:
...

6 changes: 4 additions & 2 deletions examples/mixed_sub/python/mixed_sub/main_mod/__init__.pyi
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# This file is automatically generated by pyo3_stub_gen
# ruff: noqa: E501, F401

import builtins
from . import int
from . import sub_mod

class A:
Expand All @@ -13,9 +15,9 @@ class B:
...


def create_a(x:int) -> A:
def create_a(x:builtins.int) -> A:
...

def create_b(x:int) -> B:
def create_b(x:builtins.int) -> B:
...

8 changes: 8 additions & 0 deletions examples/mixed_sub/python/mixed_sub/main_mod/int.pyi
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# This file is automatically generated by pyo3_stub_gen
# ruff: noqa: E501, F401

import builtins

def dummy_int_fun(x:builtins.int) -> builtins.int:
...

3 changes: 2 additions & 1 deletion examples/mixed_sub/python/mixed_sub/main_mod/sub_mod.pyi
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
# This file is automatically generated by pyo3_stub_gen
# ruff: noqa: E501, F401

import builtins

class C:
def show_x(self) -> None:
...


def create_c(x:int) -> C:
def create_c(x:builtins.int) -> C:
...

16 changes: 16 additions & 0 deletions examples/mixed_sub/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,20 @@ fn create_c(x: usize) -> C {
C { x }
}

#[gen_stub_pyfunction(module = "mixed_sub.main_mod.int")]
#[pyfunction]
fn dummy_int_fun(x: usize) -> usize {
x
}

#[pymodule]
fn main_mod(m: &Bound<PyModule>) -> PyResult<()> {
m.add_class::<A>()?;
m.add_class::<B>()?;
m.add_function(wrap_pyfunction!(create_a, m)?)?;
m.add_function(wrap_pyfunction!(create_b, m)?)?;
sub_mod(m)?;
int_mod(m)?;
Ok(())
}

Expand All @@ -87,6 +94,15 @@ fn sub_mod(parent: &Bound<PyModule>) -> PyResult<()> {
Ok(())
}

/// A dummy module to pollute namespace with unqualified `int`
fn int_mod(parent: &Bound<PyModule>) -> PyResult<()> {
let py = parent.py();
let sub = PyModule::new(py, "int")?;
sub.add_function(wrap_pyfunction!(dummy_int_fun, &sub)?)?;
parent.add_submodule(&sub)?;
Ok(())
}

define_stub_info_gatherer!(stub_info);

/// Test of unit test for testing link problem
Expand Down
21 changes: 11 additions & 10 deletions examples/pure/pure.pyi
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
# This file is automatically generated by pyo3_stub_gen
# ruff: noqa: E501, F401

import builtins
import os
import pathlib
import typing
from enum import Enum, auto

MY_CONSTANT: int
MY_CONSTANT: builtins.int
class A:
x: int
def __new__(cls,x:int): ...
x: builtins.int
def __new__(cls,x:builtins.int): ...
def show_x(self) -> None:
...

Expand All @@ -21,31 +22,31 @@ class Number(Enum):
FLOAT = auto()
INTEGER = auto()

def ahash_dict() -> dict[str, int]:
def ahash_dict() -> builtins.dict[builtins.str, builtins.int]:
...

def create_a(x:int=2) -> A:
def create_a(x:builtins.int=2) -> A:
...

def create_dict(n:int) -> dict[int, list[int]]:
def create_dict(n:builtins.int) -> builtins.dict[builtins.int, builtins.list[builtins.int]]:
...

def default_value(num:Number=...) -> Number:
...

def echo_path(path:str | os.PathLike | pathlib.Path) -> str:
def echo_path(path:builtins.str | os.PathLike | pathlib.Path) -> builtins.str:
...

def read_dict(dict:typing.Mapping[int, typing.Mapping[int, int]]) -> None:
def read_dict(dict:typing.Mapping[builtins.int, typing.Mapping[builtins.int, builtins.int]]) -> None:
...

def str_len(x:str) -> int:
def str_len(x:builtins.str) -> builtins.int:
r"""
Returns the length of the string.
"""
...

def sum(v:typing.Sequence[int]) -> int:
def sum(v:typing.Sequence[builtins.int]) -> builtins.int:
r"""
Returns the sum of two numbers as a string.
"""
Expand Down
3 changes: 3 additions & 0 deletions pyo3-stub-gen/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ toml.workspace = true
version = "0.7.0"
path = "../pyo3-stub-gen-derive"

[dev-dependencies]
test-case.workspace = true

[features]
default = ["numpy"]
numpy = ["dep:numpy"]
2 changes: 1 addition & 1 deletion pyo3-stub-gen/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@
//! assert_eq!(
//! method.to_string().trim(),
//! r#"
//! def foo(self, x:int) -> int:
//! def foo(self, x:builtins.int) -> builtins.int:
//! r"""
//! This is a foo method.
//! """
Expand Down
114 changes: 68 additions & 46 deletions pyo3-stub-gen/src/stub_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ impl fmt::Display for TypeInfo {
impl TypeInfo {
/// A `None` type annotation.
pub fn none() -> Self {
// NOTE: since 3.10, NoneType is provided from types module,
// but there is no corresponding definitions prior to 3.10.
Self {
name: "None".to_string(),
import: HashSet::new(),
Expand All @@ -75,11 +77,57 @@ impl TypeInfo {
}
}

/// A type annotation of a built-in type, such as `int`, `str`, or `float`. Generic builtin types are also possible, such as `dict[str, str]`.
/// A `list[Type]` type annotation.
pub fn list_of<T: PyStubType>() -> Self {
let TypeInfo { name, mut import } = T::type_output();
import.insert("builtins".into());
TypeInfo {
name: format!("builtins.list[{}]", name),
import,
}
}

/// A `set[Type]` type annotation.
pub fn set_of<T: PyStubType>() -> Self {
let TypeInfo { name, mut import } = T::type_output();
import.insert("builtins".into());
TypeInfo {
name: format!("builtins.set[{}]", name),
import,
}
}

/// A `dict[Type]` type annotation.
pub fn dict_of<K: PyStubType, V: PyStubType>() -> Self {
let TypeInfo {
name: name_k,
mut import,
} = K::type_output();
let TypeInfo {
name: name_v,
import: import_v,
} = V::type_output();
import.extend(import_v);
import.insert("builtins".into());
TypeInfo {
name: format!("builtins.set[{}, {}]", name_k, name_v),
import,
}
}

/// A type annotation of a built-in type provided from `builtins` module, such as `int`, `str`, or `float`. Generic builtin types are also possible, such as `dict[str, str]`.
pub fn builtin(name: &str) -> Self {
Self {
name: format!("builtins.{name}"),
import: hashset! { "builtins".into() },
}
}

/// Unqualified type.
pub fn unqualified(name: &str) -> Self {
Self {
name: name.to_string(),
import: HashSet::new(),
import: hashset! {},
}
}

Expand Down Expand Up @@ -182,49 +230,23 @@ mod test {
use super::*;
use maplit::hashset;
use std::collections::HashMap;

#[test]
fn test() {
assert_eq!(bool::type_input().name, "bool");
assert!(bool::type_input().import.is_empty());

assert_eq!(<&str>::type_input().name, "str");
assert!(<&str>::type_input().import.is_empty());

assert_eq!(Vec::<u32>::type_input().name, "typing.Sequence[int]");
assert_eq!(
Vec::<u32>::type_input().import,
hashset! { "typing".into() }
);

assert_eq!(Vec::<u32>::type_output().name, "list[int]");
assert!(Vec::<u32>::type_output().import.is_empty());

assert_eq!(
HashMap::<u32, String>::type_input().name,
"typing.Mapping[int, str]"
);
assert_eq!(
HashMap::<u32, String>::type_input().import,
hashset! { "typing".into() }
);

assert_eq!(HashMap::<u32, String>::type_output().name, "dict[int, str]");
assert!(HashMap::<u32, String>::type_output().import.is_empty());

assert_eq!(
HashMap::<u32, Vec<u32>>::type_input().name,
"typing.Mapping[int, typing.Sequence[int]]"
);
assert_eq!(
HashMap::<u32, Vec<u32>>::type_input().import,
hashset! { "typing".into() }
);

assert_eq!(
HashMap::<u32, Vec<u32>>::type_output().name,
"dict[int, list[int]]"
);
assert!(HashMap::<u32, Vec<u32>>::type_output().import.is_empty());
use test_case::test_case;

#[test_case(bool::type_input(), "builtins.bool", hashset! { "builtins".into() } ; "bool_input")]
#[test_case(<&str>::type_input(), "builtins.str", hashset! { "builtins".into() } ; "str_input")]
#[test_case(Vec::<u32>::type_input(), "typing.Sequence[builtins.int]", hashset! { "typing".into(), "builtins".into() } ; "Vec_u32_input")]
#[test_case(Vec::<u32>::type_output(), "builtins.list[builtins.int]", hashset! { "builtins".into() } ; "Vec_u32_output")]
#[test_case(HashMap::<u32, String>::type_input(), "typing.Mapping[builtins.int, builtins.str]", hashset! { "typing".into(), "builtins".into() } ; "HashMap_u32_String_input")]
#[test_case(HashMap::<u32, String>::type_output(), "builtins.dict[builtins.int, builtins.str]", hashset! { "builtins".into() } ; "HashMap_u32_String_output")]
#[test_case(HashMap::<u32, Vec<u32>>::type_input(), "typing.Mapping[builtins.int, typing.Sequence[builtins.int]]", hashset! { "builtins".into(), "typing".into() } ; "HashMap_u32_Vec_u32_input")]
#[test_case(HashMap::<u32, Vec<u32>>::type_output(), "builtins.dict[builtins.int, builtins.list[builtins.int]]", hashset! { "builtins".into() } ; "HashMap_u32_Vec_u32_output")]
#[test_case(HashSet::<u32>::type_input(), "builtins.set[builtins.int]", hashset! { "builtins".into() } ; "HashSet_u32_input")]
fn test(tinfo: TypeInfo, name: &str, import: HashSet<ModuleRef>) {
assert_eq!(tinfo.name, name);
if import.is_empty() {
assert!(tinfo.import.is_empty());
} else {
assert_eq!(tinfo.import, import);
}
}
}
12 changes: 7 additions & 5 deletions pyo3-stub-gen/src/stub_type/builtins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,7 @@ macro_rules! impl_builtin {
($ty:ty, $pytype:expr) => {
impl PyStubType for $ty {
fn type_output() -> TypeInfo {
TypeInfo {
name: $pytype.to_string(),
import: HashSet::new(),
}
TypeInfo::builtin($pytype)
}
}
};
Expand All @@ -33,7 +30,12 @@ macro_rules! impl_with_module {
};
}

impl_builtin!((), "None");
// NOTE:
impl PyStubType for () {
fn type_output() -> TypeInfo {
TypeInfo::none()
}
}
impl_builtin!(bool, "bool");
impl_builtin!(u8, "int");
impl_builtin!(u16, "int");
Expand Down
Loading

0 comments on commit 9660cd1

Please sign in to comment.