Skip to content

Python mypy errors when record member names match type names #2552

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 18 additions & 0 deletions fixtures/keywords/rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,4 +94,22 @@ pub fn get_else(e: r#else) -> r#else {
e
}

// python/mypy - not keywords, but type names in strange places.
#[derive(uniffi::Record, Default)]
pub struct RecordWithTypeNames {
bool: bool,
int: i32,
float: f32,
double: f64,
str: String,
bytes: Vec<u8>,
dict: HashMap<String, bool>,
list: Vec<bool>,
}

#[uniffi::export]
fn get_type_names() -> RecordWithTypeNames {
RecordWithTypeNames::default()
}

uniffi::include_scaffolding!("keywords");
6 changes: 6 additions & 0 deletions fixtures/keywords/rust/tests/bindings/test_keywords.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,9 @@
import keywords_rust
# but might as well call something.
keywords_rust._if(0)

# python/mypy tests - not keywords, but type names in strange places.
tn = keywords_rust.get_type_names()
assert(tn.bool == False)
assert(tn.str == "")
# etc - no need to check all the values, we are just checking mypy is happy.
1 change: 1 addition & 0 deletions uniffi_bindgen/src/bindings/python/pipeline/nodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,7 @@ pub struct TypeNode {
pub canonical_name: String,
pub is_used_as_error: bool,
pub type_name: String,
pub type_anno_name: String,
pub ffi_converter_name: String,
pub ffi_type: FfiTypeNode,
}
Expand Down
40 changes: 40 additions & 0 deletions uniffi_bindgen/src/bindings/python/pipeline/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ pub fn pass(module: &mut Module) -> Result<()> {
ty.ffi_converter_name = format!("_UniffiFfiConverter{}", ty.canonical_name);
}
ty.type_name = type_name(&ty.ty);
ty.type_anno_name = type_anno_name(&ty.ty);
});
module.visit_mut(|return_ty: &mut ReturnType| {
return_ty.type_name = match &return_ty.ty {
Expand Down Expand Up @@ -102,3 +103,42 @@ fn type_name(ty: &Type) -> String {
} => format!("dict[{}, {}]", type_name(key_type), type_name(value_type)),
}
}

// The name for the type we should use in type annotations.
// We prefer `builtins.name` for builtin types, otherwise things
// like:
// > class Foo:
// > bool: bool
// get upset.
// Note:
// * We actually use `__python_builtins`, which we arrange to import, to avoid name clashes
// should someone try to use `builtins` as a name)
// * In theory we could do this in `type_name` (and remove this), but some odd things fail in that case.
fn type_anno_name(ty: &Type) -> String {
match ty {
Type::Boolean => "__python_builtins.bool".to_string(),
Type::String => "__python_builtins.str".to_string(),
Type::Bytes => "__python_builtins.bytes".to_string(),
Type::Int8 => "__python_builtins.int".to_string(),
Type::Int16
| Type::Int32
| Type::Int64
| Type::UInt8
| Type::UInt16
| Type::UInt32
| Type::UInt64 => "__python_builtins.int".to_string(),
Type::Float32 | Type::Float64 => "__python_builtins.float".to_string(),
Type::Sequence { inner_type } => {
format!("typing.List[{}]", type_anno_name(inner_type))
}
Type::Map {
key_type,
value_type,
} => format!(
"__python_builtins.dict[{}, {}]",
type_anno_name(key_type),
type_anno_name(value_type)
),
_ => type_name(ty),
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
{{ arg.name }}
{%- match arg.default %}
{%- when Some(literal) %}: typing.Union[object, {{ arg.ty.type_name }}] = _DEFAULT
{%- else %}: {{ arg.ty.type_name }}
{%- else %}: {{ arg.ty.type_anno_name }}
{%- endmatch %}
{%- if !loop.last %},{% endif -%}
{%- endfor %}
2 changes: 2 additions & 0 deletions uniffi_bindgen/src/bindings/python/templates/Module.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@
{%- for import in imports %}
import {{ import }}
{%- endfor %}
{# avoid name-clashes for 'builtins' - maybe we should do this for the other modules? #}
import builtins as __python_builtins


# Used for default argument values
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
class {{ rec.self_type.type_name }}:
{{ rec.docstring|docstring(4) -}}
{% for field in rec.fields -%}
{{ field.name }}: {{ field.ty.type_name}}
{{ field.name }}: {{ field.ty.type_anno_name}}
{{ rec.docstring|docstring(4) -}}
{% endfor -%}

{%- if !rec.fields.is_empty() %}
def __init__(self, *, {% for field in rec.fields %}
{{- field.name }}: {{- field.ty.type_name}}
{{- field.name }}: {{- field.ty.type_anno_name}}
{%- if field.default.is_some() %} = _DEFAULT{% endif %}
{%- if !loop.last %}, {% endif %}
{%- endfor %}):
Expand Down