address feedback

Author: michaelm-openai

mypy/checker.py

@@ -5378,11 +5378,10 @@ def analyze_iterable_item_type_without_expression(
         echk = self.expr_checker
         iterable: Type
         iterable = get_proper_type(type)
+
         if self.options.disallow_str_iteration and self.is_str_iteration_type(iterable):
             self.msg.str_iteration_disallowed(context)
-            item_type = self.named_type("builtins.str")
-            iterator_type = self.named_generic_type("typing.Iterator", [item_type])
-            return iterator_type, item_type
+
         iterator = echk.check_method_call_by_name("__iter__", iterable, [], [], context)[0]
 
         if (

mypy/subtypes.py

@@ -484,7 +484,18 @@ def visit_instance(self, left: Instance) -> bool:
             and self.options.disallow_str_iteration
             and left.type.fullname == "builtins.str"
             and isinstance(right, Instance)
-            and right.type.fullname in ("typing.Sequence", "collections.abc.Sequence")
+            and right.type.fullname
+            in (
+                "collections.abc.Collection",
+                "collections.abc.Iterable",
+                "collections.abc.Reversible",
+                "collections.abc.Sequence",
+                "typing.Collection",
+                "typing.Iterable",
+                "typing.Reversible",
+                "typing.Sequence",
+                "_typeshed.SupportsLenAndGetItem",
+            )
         ):
             return False
         if isinstance(right, TupleType) and right.partial_fallback.type.is_enum:

test-data/unit/check-flags.test

@@ -2452,33 +2452,50 @@ f(memoryview(b"asdf"))  # E: Argument 1 to "f" has incompatible type "memoryview
 [builtins fixtures/primitives.pyi]
 
 [case testDisallowStrIteration]
-# flags: --disallow-str-iteration --python-version 3.12
-from typing import Iterable, Sequence
+# flags: --disallow-str-iteration
+from typing import Collection, Iterable, Reversible, Sequence, TypeVar
+
+def takes_str(x: str):
+    for ch in x:  # E: Iterating over "str" is disallowed  # N: This is because --disallow-str-iteration is enabled
+        reveal_type(ch)  # N: Revealed type is "builtins.str"
+    [ch for ch in x]  # E: Iterating over "str" is disallowed  # N: This is because --disallow-str-iteration is enabled
 
 s = "hello"
-for ch in s:  # E: Iterating over "str" is disallowed  # N: This is because --disallow-str-iteration is enabled
-    reveal_type(ch)  # N: Revealed type is "builtins.str"
-[x for x in s]  # E: Iterating over "str" is disallowed  # N: This is because --disallow-str-iteration is enabled
 
-def takes_seq(x: Sequence[int]) -> None: ...
 def takes_seq_str(x: Sequence[str]) -> None: ...
 takes_seq_str(s)  # E: Argument 1 to "takes_seq_str" has incompatible type "str"; expected "Sequence[str]"
 
 def takes_iter_str(x: Iterable[str]) -> None: ...
 takes_iter_str(s)  # E: Argument 1 to "takes_iter_str" has incompatible type "str"; expected "Iterable[str]"
 
+def takes_collection_str(x: Collection[str]) -> None: ...
+takes_collection_str(s)  # E: Argument 1 to "takes_collection_str" has incompatible type "str"; expected "Collection[str]"
+
+def takes_reversible_str(x: Reversible[str]) -> None: ...
+takes_reversible_str(s)  # E: Argument 1 to "takes_reversible_str" has incompatible type "str"; expected "Reversible[str]"
+
 seq: Sequence[str] = s  # E: Incompatible types in assignment (expression has type "str", variable has type "Sequence[str]")
 iterable: Iterable[str] = s  # E: Incompatible types in assignment (expression has type "str", variable has type "Iterable[str]")
+collection: Collection[str] = s  # E: Incompatible types in assignment (expression has type "str", variable has type "Collection[str]")
+reversible: Reversible[str] = s  # E: Incompatible types in assignment (expression has type "str", variable has type "Reversible[str]")
 
-def takes_maybe_seq(x: str | Sequence[str]) -> None:
+def takes_maybe_seq(x: "str | Sequence[int]") -> None:
     for ch in x:  # E: Iterating over "str" is disallowed  # N: This is because --disallow-str-iteration is enabled
-        reveal_type(ch)  # N: Revealed type is "builtins.str"
+        reveal_type(ch)  # N: Revealed type is "builtins.str | builtins.int"
+
+T = TypeVar('T', bound=str)
 
-def takes_bound[T: str](x: T) -> None:
+def takes_str_upper_bound(x: T) -> None:
     for ch in x:  # E: Iterating over "str" is disallowed  # N: This is because --disallow-str-iteration is enabled
         reveal_type(ch)  # N: Revealed type is "builtins.str"
 
+reveal_type(reversed(s))  # N: Revealed type is "builtins.reversed[builtins.str]"  # E: Argument 1 to "reversed" has incompatible type "str"; expected "Reversible[str]"
+
+[builtins fixtures/str-iter.pyi]
+[typing fixtures/typing-str-iter.pyi]
+
 [case testIterStrOverload]
+# flags: --disallow-str-iteration
 reveal_type(iter("foo"))  # N: Revealed type is "typing.Iterable[builtins.str]"
 [builtins fixtures/dict.pyi]
 

test-data/unit/fixtures/str-iter.pyi

@@ -0,0 +1,52 @@
+# Builtins stub used in disallow-str-iteration tests.
+
+
+from _typeshed import SupportsLenAndGetItem
+from typing import Generic, Iterator, Sequence, Reversible, TypeVar, overload
+
+_T = TypeVar("_T")
+
+class object:
+    def __init__(self) -> None: pass
+
+class type: pass
+class int: pass
+class bool(int): pass
+class ellipsis: pass
+class slice: pass
+
+class str:
+    def __iter__(self) -> Iterator[str]: pass
+    def __len__(self) -> int: pass
+    def __contains__(self, item: object) -> bool: pass
+    def __reversed__(self) -> Iterator[str]: pass
+    def __getitem__(self, i: int) -> str: pass
+
+class list(Sequence[_T], Generic[_T]):
+    def __iter__(self) -> Iterator[_T]: pass
+    def __len__(self) -> int: pass
+    def __contains__(self, item: object) -> bool: pass
+    def __reversed__(self) -> Iterator[_T]: pass
+    @overload
+    def __getitem__(self, i: int, /) -> _T: ...
+    @overload
+    def __getitem__(self, s: slice, /) -> list[_T]: ...
+
+class tuple(Sequence[_T], Generic[_T]):
+    def __iter__(self) -> Iterator[_T]: pass
+    def __len__(self) -> int: pass
+    def __contains__(self, item: object) -> bool: pass
+    def __reversed__(self) -> Iterator[_T]: pass
+    @overload
+    def __getitem__(self, i: int, /) -> _T: ...
+    @overload
+    def __getitem__(self, s: slice, /) -> list[_T]: ...
+
+class dict: pass
+
+class reversed(Iterator[_T]):
+    @overload
+    def __new__(cls, sequence: Reversible[_T], /) -> Iterator[_T]: ... # type: ignore[misc]
+    @overload
+    def __new__(cls, sequence: SupportsLenAndGetItem[_T], /) -> Iterator[_T]: ... # type: ignore[misc]
+    def __next__(self) -> _T: ...

test-data/unit/fixtures/typing-str-iter.pyi

@@ -0,0 +1,62 @@
+# Minimal typing fixture for disallow-str-iteration tests.
+
+from abc import ABCMeta, abstractmethod
+
+Any = object()
+TypeVar = 0
+Generic = 0
+Protocol = 0
+overload = 0
+
+_T = TypeVar("_T")
+_KT = TypeVar("_KT")
+_T_co = TypeVar("_T_co", covariant=True)
+_VT_co = TypeVar("_VT_co", covariant=True)  # Value type covariant containers.
+_TC = TypeVar("_TC", bound=type[object])
+
+@runtime_checkable
+class Iterable(Protocol[_T_co]):
+    @abstractmethod
+    def __iter__(self) -> Iterator[_T_co]: ...
+
+@runtime_checkable
+class Iterator(Iterable[_T_co], Protocol[_T_co]):
+    @abstractmethod
+    def __next__(self) -> _T_co: ...
+    def __iter__(self) -> Iterator[_T_co]: ...
+
+@runtime_checkable
+class Reversible(Iterable[_T_co], Protocol[_T_co]):
+    @abstractmethod
+    def __reversed__(self) -> Iterator[_T_co]: ...
+
+@runtime_checkable
+class Container(Protocol[_T_co]):
+    # This is generic more on vibes than anything else
+    @abstractmethod
+    def __contains__(self, x: object, /) -> bool: ...
+
+@runtime_checkable
+class Collection(Iterable[_T_co], Container[_T_co], Protocol[_T_co]):
+    # Implement Sized (but don't have it as a base class).
+    @abstractmethod
+    def __len__(self) -> int: ...
+
+class Sequence(Reversible[_T_co], Collection[_T_co]):
+    @overload
+    @abstractmethod
+    def __getitem__(self, index: int) -> _T_co: ...
+    @overload
+    @abstractmethod
+    def __getitem__(self, index: slice) -> Sequence[_T_co]: ...
+    def __contains__(self, value: object) -> bool: ...
+    def __iter__(self) -> Iterator[_T_co]: ...
+    def __reversed__(self) -> Iterator[_T_co]: ...
+
+class Mapping(Collection[_KT], Generic[_KT, _VT_co]):
+    @abstractmethod
+    def __getitem__(self, key: _KT, /) -> _VT_co: ...
+    def __contains__(self, key: object, /) -> bool: ...
+
+def runtime_checkable(cls: _TC) -> _TC:
+    return cls

test-data/unit/lib-stub/_typeshed.pyi

@@ -2,7 +2,12 @@ from typing import Protocol, TypeVar, Iterable
 
 _KT = TypeVar("_KT")
 _VT_co = TypeVar("_VT_co", covariant=True)
+_T_co = TypeVar("_T_co", covariant=True)
 
 class SupportsKeysAndGetItem(Protocol[_KT, _VT_co]):
     def keys(self) -> Iterable[_KT]: pass
     def __getitem__(self, __key: _KT) -> _VT_co: pass
+
+class SupportsLenAndGetItem(Protocol[_T_co]):
+    def __len__(self) -> int: ...
+    def __getitem__(self, k: int, /) -> _T_co: ...