Skip to content

Comments

[red-knot] Add failing tests for iterating over maybe-iterable unions#14016

Merged
AlexWaygood merged 1 commit intomainfrom
alex/failing-iter-tests
Oct 31, 2024
Merged

[red-knot] Add failing tests for iterating over maybe-iterable unions#14016
AlexWaygood merged 1 commit intomainfrom
alex/failing-iter-tests

Conversation

@AlexWaygood
Copy link
Member

Summary

This adds failing tests (with TODO comments) for the issues described in #14012, so that we can clearly see the limitations of our current inference. It fixes a couple of micro-bugs along the way, but nothing substantial, for the reasons described in #14012.

Test Plan

cargo test -p red_knot_python_semantic

@AlexWaygood AlexWaygood added the ty Multi-file analysis & type inference label Oct 31, 2024
}

if let Type::Unknown | Type::Any = self {
if matches!(self, Type::Unknown | Type::Any | Type::Todo) {
Copy link
Member Author

Choose a reason for hiding this comment

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

we could possibly add a Type::is_dynamic() method to reduce the chances of bugs like this?

Comment on lines 1189 to +1192
// TODO: `type[Any]`?
Type::Any => Type::Todo,
Type::Any => Type::Any,
// TODO: `type[Unknown]`?
Type::Unknown => Type::Todo,
Type::Unknown => Type::Unknown,
Copy link
Member Author

Choose a reason for hiding this comment

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

Without this change, x in some of the loops I'm adding tests for was inferred as T | Todo rather than T | Unknown, because we lookup __iter__ and __next__ on the meta-type of an instance, and we had the meta-type of Unknown as being Todo. There is a TODO here (type[Unknown] would be more precise than Unknown), but it's not a big enough todo for us to infer Type::Todo in this situation, in my opinion 😄 Type::Unknown and Type::Any are better for now, in my opinion.

@github-actions
Copy link
Contributor

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@AlexWaygood AlexWaygood merged commit d1189c2 into main Oct 31, 2024
@AlexWaygood AlexWaygood deleted the alex/failing-iter-tests branch October 31, 2024 18:20
sharkdp added a commit that referenced this pull request Nov 7, 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
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.

2 participants