Conversation
|
| str(1) | ||
| str(object=1) | ||
|
|
||
| str(b"M\xc3\xbcsli", "utf-8") |
There was a problem hiding this comment.
I can tell what you had for breakfast this morning!
| Parameter::positional_or_keyword(Name::new_static("object")) | ||
| // TODO: Should be `ReadableBuffer` instead of this union type: | ||
| .with_annotated_type(UnionType::from_elements( | ||
| db, | ||
| [ | ||
| KnownClass::Bytes.to_instance(db), | ||
| KnownClass::Bytearray.to_instance(db), | ||
| ], | ||
| )) | ||
| .with_default_type(Type::bytes_literal(db, b"")), | ||
| Parameter::positional_or_keyword(Name::new_static("encoding")) | ||
| .with_annotated_type(KnownClass::Str.to_instance(db)) | ||
| .with_default_type(Type::string_literal(db, "utf-8")), | ||
| Parameter::positional_or_keyword(Name::new_static("errors")) | ||
| .with_annotated_type(KnownClass::Str.to_instance(db)) | ||
| .with_default_type(Type::string_literal(db, "strict")), |
There was a problem hiding this comment.
interesting: these overloads differ from typeshed's overloads in that str(encoding='utf8') is permitted by these overloads, but not by typeshed's. It's hard to say whether it should or shouldn't be permitted, really: it works at runtime, but arguably the type checker would be doing the user a favour by telling the user off for doing it, since there's no reason why it would ever be useful (it's almost certainly indicative of a user error)
There was a problem hiding this comment.
Oh, right. I didn't realize that object is required in the second overload in typeshed. Should we just ignore the discrepancy? Change it in Red Knot? Change it in typeshed?
I agree that it's useless to do str(encoding='utf-8'), but it is valid at runtime, clearly specified as such in the documentation, and seemingly harmless. I also doubt that we would catch actual errors by disallowing it. So I'm inclined to say that it should be changed in typeshed?
There was a problem hiding this comment.
hmm, that's a fair point about the documentation. I still weakly prefer the typeshed stubs as they are: I just can't think of any reason why a user would want to write str(encoding='utf8') when they could just as well write str() or "". I think the only reason why it would ever appear in a codebase would be as a result of a typo or a bad codemod, so I'd prefer for type checkers to complain about it.
I really don't have a strong opinion, though! If you feel like sending a PR to typeshed, feel free -- the other typeshed maintainers might feel differently 😄
There was a problem hiding this comment.
I don't feel strongly about it, and not really familiar with typeshed's policies when it comes to questions like this, so I guess it's fine to just leave it as a discrepancy.
There was a problem hiding this comment.
I think typeshed should be changed: this is a classic case of "what should a type checker report" vs "what should a linter report". Perfectly reasonable to write a lint rule saying "this call is silly", but IMO not reasonable for a type checker to claim the call is a type error, when it clearly is not.
There was a problem hiding this comment.
(But "thinking typeshed should be changed" is as far as I will take this, I don't care enough to submit a PR and follow through on debating it 😆 )
There was a problem hiding this comment.
Perfectly reasonable to write a lint rule saying "this call is silly", but IMO not reasonable for a type checker to claim the call is a type error, when it clearly is not.
hmm, sounds like there could be a gap in the market for a type-aware linter here 🤔
* origin/main: [red-knot] Fix more [redundant-cast] false positives (#17170) [red-knot] Three-argument type-calls take 'str' as the first argument (#17168) Control flow: `return` and `raise` (#17121) Bump 0.11.3 (#17173) [red-knot] Improve `Debug` implementation for `semantic_index::SymbolTable` (#17172) [red-knot] Fix `str(…)` calls (#17163) [red-knot] visibility_constraint analysis for match cases (#17077) [red-knot] Fix playground crashes when diagnostics are stale (#17165)
* origin/main: (82 commits) [red-knot] Fix more [redundant-cast] false positives (#17170) [red-knot] Three-argument type-calls take 'str' as the first argument (#17168) Control flow: `return` and `raise` (#17121) Bump 0.11.3 (#17173) [red-knot] Improve `Debug` implementation for `semantic_index::SymbolTable` (#17172) [red-knot] Fix `str(…)` calls (#17163) [red-knot] visibility_constraint analysis for match cases (#17077) [red-knot] Fix playground crashes when diagnostics are stale (#17165) [red-knot] Callable types are disjoint from literals (#17160) [red-knot] Fix inference for `pow` between two literal integers (#17161) [red-knot] Add GitHub PR annotations when mdtests fail in CI (#17150) [red-knot] Fix equivalence of differently ordered unions that contain `Callable` types (#17145) [red-knot] Add initial set of tests for unreachable code (#17159) [`airflow`] Move `AIR302` to `AIR301` and `AIR303` to `AIR302` (#17151) ruff_db: simplify lifetimes on `DiagnosticDisplay` [red-knot] Detect division-by-zero in unions and intersections (#17157) [`airflow`] Add autofix infrastructure to `AIR302` name checks (#16965) [`flake8-bandit`] Mark `str` and `list[str]` literals as trusted input (`S603`) (#17136) [`airflow`] Add autofix for `AIR302` attribute checks (#16977) [`airflow`] Extend `AIR302` with additional symbols (#17085) ...
## Summary The existing signature for `str` calls had various problems, one of which I noticed while looking at some ecosystem projects (`scrapy`, added as a project to mypy_primer in this PR). ## Test Plan - New tests for `str(…)` calls. - Observed reduction of false positives in ecosystem checks
…astral-sh#17168) ## Summary Similar to astral-sh#17163, a minor fix in the signature of `type(…)`. ## Test Plan New MD tests
3 participants
Summary
The existing signature for
strcalls had various problems, one of which I noticed while looking at some ecosystem projects (scrapy, added as a project to mypy_primer in this PR).Test Plan
str(…)calls.