Skip to content

Comments

[red-knot] Improve Symbol API for callable types#14137

Merged
sharkdp merged 11 commits intomainfrom
david/unbound-methods-follow-up
Nov 7, 2024
Merged

[red-knot] Improve Symbol API for callable types#14137
sharkdp merged 11 commits intomainfrom
david/unbound-methods-follow-up

Conversation

@sharkdp
Copy link
Contributor

@sharkdp sharkdp commented Nov 6, 2024

Summary

  • Get rid of Symbol::unwrap_or (unclear semantics, not needed anymore)
  • Introduce Type::call_dunder
  • Emit new diagnostic for possibly-unbound __iter__ methods
  • Better diagnostics for callables with possibly-unbound / possibly-non-callable __call__ methods

part of: #14022

closes #14016

Test Plan

  • Updated test for iterables with possibly-unbound __iter__ methods.
  • New tests for callables

@sharkdp sharkdp added the ty Multi-file analysis & type inference label Nov 6, 2024
@github-actions
Copy link
Contributor

github-actions bot commented Nov 6, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@sharkdp sharkdp force-pushed the david/unbound-methods-follow-up branch from bf0e9e2 to 54db8aa Compare November 7, 2024 09:20
@sharkdp sharkdp force-pushed the david/unbound-methods-follow-up branch from 54db8aa to 923d8b3 Compare November 7, 2024 16:45
Copy link
Contributor

@carljm carljm left a comment

Choose a reason for hiding this comment

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

This looks good to me, just a couple minor nits! Thanks for pushing through a few different versions of this :)

Comment on lines 1309 to 1311
let dunder_iter_method = iterable_meta_type.member(db, "__iter__");
if let Symbol::Type(dunder_iter_method, _) = dunder_iter_method {
if let Symbol::Type(dunder_iter_method, boundness) = dunder_iter_method {
let Some(iterator_ty) = dunder_iter_method.call(db, &[self]).return_ty(db) else {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should/can we use call_dunder here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's slightly awkward since we only use the boundness information inside the nested conditional, but it's only slightly more code — while being more consistent.

We can actually also use it below for __get_item__ where it improved the code a lot.

Copy link
Contributor

@carljm carljm left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@sharkdp sharkdp merged commit 4f74db5 into main Nov 7, 2024
@sharkdp sharkdp deleted the david/unbound-methods-follow-up branch November 7, 2024 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants