[pyupgrade] Add fix safety section to docs (UP030) (#17443)

## Summary

add fix safety section to format_literals, for #15584

.github/workflows/build-binaries.yml

@@ -377,7 +377,7 @@ jobs:
           args: --release --locked --out dist
       - name: "Test wheel"
         if: matrix.target == 'x86_64-unknown-linux-musl'
-        uses: addnab/docker-run-action@v3
+        uses: addnab/docker-run-action@4f65fabd2431ebc8d299f8e5a018d79a769ae185 # v3
         with:
           image: alpine:latest
           options: -v ${{ github.workspace }}:/io -w /io

.github/workflows/ci.yaml

@@ -237,13 +237,13 @@ jobs:
       - name: "Install Rust toolchain"
         run: rustup show
       - name: "Install mold"
-        uses: rui314/setup-mold@v1
+        uses: rui314/setup-mold@e16410e7f8d9e167b74ad5697a9089a35126eb50 # v1
       - name: "Install cargo nextest"
-        uses: taiki-e/install-action@be7c31b6745feec79dec5eb79178466c0670bb2d # v2
+        uses: taiki-e/install-action@09dc018eee06ae1c9e0409786563f534210ceb83 # v2
         with:
           tool: cargo-nextest
       - name: "Install cargo insta"
-        uses: taiki-e/install-action@be7c31b6745feec79dec5eb79178466c0670bb2d # v2
+        uses: taiki-e/install-action@09dc018eee06ae1c9e0409786563f534210ceb83 # v2
         with:
           tool: cargo-insta
       - name: Red-knot mdtests (GitHub annotations)
@@ -291,13 +291,13 @@ jobs:
       - name: "Install Rust toolchain"
         run: rustup show
       - name: "Install mold"
-        uses: rui314/setup-mold@v1
+        uses: rui314/setup-mold@e16410e7f8d9e167b74ad5697a9089a35126eb50 # v1
       - name: "Install cargo nextest"
-        uses: taiki-e/install-action@be7c31b6745feec79dec5eb79178466c0670bb2d # v2
+        uses: taiki-e/install-action@09dc018eee06ae1c9e0409786563f534210ceb83 # v2
         with:
           tool: cargo-nextest
       - name: "Install cargo insta"
-        uses: taiki-e/install-action@be7c31b6745feec79dec5eb79178466c0670bb2d # v2
+        uses: taiki-e/install-action@09dc018eee06ae1c9e0409786563f534210ceb83 # v2
         with:
           tool: cargo-insta
       - name: "Run tests"
@@ -320,7 +320,7 @@ jobs:
       - name: "Install Rust toolchain"
         run: rustup show
       - name: "Install cargo nextest"
-        uses: taiki-e/install-action@be7c31b6745feec79dec5eb79178466c0670bb2d # v2
+        uses: taiki-e/install-action@09dc018eee06ae1c9e0409786563f534210ceb83 # v2
         with:
           tool: cargo-nextest
       - name: "Run tests"
@@ -376,7 +376,7 @@ jobs:
       - name: "Install Rust toolchain"
         run: rustup show
       - name: "Install mold"
-        uses: rui314/setup-mold@v1
+        uses: rui314/setup-mold@e16410e7f8d9e167b74ad5697a9089a35126eb50 # v1
       - name: "Build"
         run: cargo build --release --locked
 
@@ -401,13 +401,13 @@ jobs:
           MSRV: ${{ steps.msrv.outputs.value }}
         run: rustup default "${MSRV}"
       - name: "Install mold"
-        uses: rui314/setup-mold@v1
+        uses: rui314/setup-mold@e16410e7f8d9e167b74ad5697a9089a35126eb50 # v1
       - name: "Install cargo nextest"
-        uses: taiki-e/install-action@be7c31b6745feec79dec5eb79178466c0670bb2d # v2
+        uses: taiki-e/install-action@09dc018eee06ae1c9e0409786563f534210ceb83 # v2
         with:
           tool: cargo-nextest
       - name: "Install cargo insta"
-        uses: taiki-e/install-action@be7c31b6745feec79dec5eb79178466c0670bb2d # v2
+        uses: taiki-e/install-action@09dc018eee06ae1c9e0409786563f534210ceb83 # v2
         with:
           tool: cargo-insta
       - name: "Run tests"
@@ -433,7 +433,7 @@ jobs:
       - name: "Install Rust toolchain"
         run: rustup show
       - name: "Install cargo-binstall"
-        uses: cargo-bins/cargo-binstall@main
+        uses: cargo-bins/cargo-binstall@63aaa5c1932cebabc34eceda9d92a70215dcead6 # v1.12.3
         with:
           tool: cargo-fuzz@0.11.2
       - name: "Install cargo-fuzz"
@@ -455,7 +455,7 @@ jobs:
       - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
         with:
           persist-credentials: false
-      - uses: astral-sh/setup-uv@0c5e2b8115b80b4c7c5ddf6ffdd634974642d182 # v5.4.1
+      - uses: astral-sh/setup-uv@d4b2f3b6ecc6e67c4457f6d3e41ec42d3d0fcb86 # v5.4.2
       - uses: actions/download-artifact@95815c38cf2ff2164869cbab79da8d1f422bc89e # v4.2.1
         name: Download Ruff binary to test
         id: download-cached-binary
@@ -641,7 +641,7 @@ jobs:
       - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
         with:
           persist-credentials: false
-      - uses: cargo-bins/cargo-binstall@main
+      - uses: cargo-bins/cargo-binstall@63aaa5c1932cebabc34eceda9d92a70215dcead6 # v1.12.3
       - run: cargo binstall --no-confirm cargo-shear
       - run: cargo shear
 
@@ -681,7 +681,7 @@ jobs:
       - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
         with:
           persist-credentials: false
-      - uses: astral-sh/setup-uv@0c5e2b8115b80b4c7c5ddf6ffdd634974642d182 # v5.4.1
+      - uses: astral-sh/setup-uv@d4b2f3b6ecc6e67c4457f6d3e41ec42d3d0fcb86 # v5.4.2
       - name: "Cache pre-commit"
         uses: actions/cache@5a3ec84eff668545956fd18022155c47e93e2684 # v4.2.3
         with:
@@ -720,7 +720,7 @@ jobs:
       - name: "Install Rust toolchain"
         run: rustup show
       - name: Install uv
-        uses: astral-sh/setup-uv@0c5e2b8115b80b4c7c5ddf6ffdd634974642d182 # v5.4.1
+        uses: astral-sh/setup-uv@d4b2f3b6ecc6e67c4457f6d3e41ec42d3d0fcb86 # v5.4.2
       - name: "Install Insiders dependencies"
         if: ${{ env.MKDOCS_INSIDERS_SSH_KEY_EXISTS == 'true' }}
         run: uv pip install -r docs/requirements-insiders.txt --system
@@ -857,7 +857,7 @@ jobs:
         run: rustup show
 
       - name: "Install codspeed"
-        uses: taiki-e/install-action@be7c31b6745feec79dec5eb79178466c0670bb2d # v2
+        uses: taiki-e/install-action@09dc018eee06ae1c9e0409786563f534210ceb83 # v2
         with:
           tool: cargo-codspeed
 

.github/workflows/daily_fuzz.yaml

@@ -34,11 +34,11 @@ jobs:
       - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
         with:
           persist-credentials: false
-      - uses: astral-sh/setup-uv@0c5e2b8115b80b4c7c5ddf6ffdd634974642d182 # v5.4.1
+      - uses: astral-sh/setup-uv@d4b2f3b6ecc6e67c4457f6d3e41ec42d3d0fcb86 # v5.4.2
       - name: "Install Rust toolchain"
         run: rustup show
       - name: "Install mold"
-        uses: rui314/setup-mold@v1
+        uses: rui314/setup-mold@e16410e7f8d9e167b74ad5697a9089a35126eb50 # v1
       - uses: Swatinem/rust-cache@9d47c6ad4b02e050fd481d890b2ea34778fd09d6 # v2.7.8
       - name: Build ruff
         # A debug build means the script runs slower once it gets started,

.github/workflows/daily_property_tests.yaml

@@ -36,7 +36,7 @@ jobs:
       - name: "Install Rust toolchain"
         run: rustup show
       - name: "Install mold"
-        uses: rui314/setup-mold@v1
+        uses: rui314/setup-mold@e16410e7f8d9e167b74ad5697a9089a35126eb50 # v1
       - uses: Swatinem/rust-cache@9d47c6ad4b02e050fd481d890b2ea34778fd09d6 # v2.7.8
       - name: Build Red Knot
         # A release build takes longer (2 min vs 1 min), but the property tests run much faster in release

.github/workflows/mypy_primer.yaml

@@ -35,7 +35,7 @@ jobs:
           persist-credentials: false
 
       - name: Install the latest version of uv
-        uses: astral-sh/setup-uv@0c5e2b8115b80b4c7c5ddf6ffdd634974642d182 # v5.4.1
+        uses: astral-sh/setup-uv@d4b2f3b6ecc6e67c4457f6d3e41ec42d3d0fcb86 # v5.4.2
 
       - uses: Swatinem/rust-cache@9d47c6ad4b02e050fd481d890b2ea34778fd09d6 # v2.7.8
         with:

.github/workflows/publish-pypi.yml

@@ -22,7 +22,7 @@ jobs:
       id-token: write
     steps:
       - name: "Install uv"
-        uses: astral-sh/setup-uv@0c5e2b8115b80b4c7c5ddf6ffdd634974642d182 # v5.4.1
+        uses: astral-sh/setup-uv@d4b2f3b6ecc6e67c4457f6d3e41ec42d3d0fcb86 # v5.4.2
       - uses: actions/download-artifact@95815c38cf2ff2164869cbab79da8d1f422bc89e # v4.2.1
         with:
           pattern: wheels-*

.pre-commit-config.yaml

@@ -79,7 +79,7 @@ repos:
         pass_filenames: false # This makes it a lot faster
 
   - repo: https://github.com/astral-sh/ruff-pre-commit
-    rev: v0.11.5
+    rev: v0.11.6
     hooks:
       - id: ruff-format
       - id: ruff
@@ -97,7 +97,7 @@ repos:
   # zizmor detects security vulnerabilities in GitHub Actions workflows.
   # Additional configuration for the tool is found in `.github/zizmor.yml`
   - repo: https://github.com/woodruffw/zizmor-pre-commit
-    rev: v1.5.2
+    rev: v1.6.0
     hooks:
       - id: zizmor
 

Cargo.lock

@@ -334,9 +334,9 @@ dependencies = [
 
 [[package]]
 name = "clap"
-version = "4.5.36"
+version = "4.5.37"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "2df961d8c8a0d08aa9945718ccf584145eee3f3aa06cddbeac12933781102e04"
+checksum = "eccb054f56cbd38340b380d4a8e69ef1f02f1af43db2f0cc817a4774d80ae071"
 dependencies = [
  "clap_builder",
  "clap_derive",
@@ -344,9 +344,9 @@ dependencies = [
 
 [[package]]
 name = "clap_builder"
-version = "4.5.36"
+version = "4.5.37"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "132dbda40fb6753878316a489d5a1242a8ef2f0d9e47ba01c951ea8aa7d013a5"
+checksum = "efd9466fac8543255d3b1fcad4762c5e116ffe808c8a3043d4263cd4fd4862a2"
 dependencies = [
  "anstream",
  "anstyle",
@@ -478,7 +478,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "117725a109d387c937a1533ce01b450cbde6b88abceea8473c4d7a85853cda3c"
 dependencies = [
  "lazy_static",
- "windows-sys 0.48.0",
+ "windows-sys 0.52.0",
 ]
 
 [[package]]
@@ -487,7 +487,7 @@ version = "3.0.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "fde0e0ec90c9dfb3b4b1a0891a7dcd0e2bffde2f7efed5fe7c9bb00e5bfb915e"
 dependencies = [
- "windows-sys 0.48.0",
+ "windows-sys 0.52.0",
 ]
 
 [[package]]
@@ -918,7 +918,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "33d852cb9b869c2a9b3df2f71a3074817f01e1844f839a144f5fcef059a4eb5d"
 dependencies = [
  "libc",
- "windows-sys 0.59.0",
+ "windows-sys 0.52.0",
 ]
 
 [[package]]
@@ -1499,7 +1499,7 @@ checksum = "e04d7f318608d35d4b61ddd75cbdaee86b023ebe2bd5a66ee0915f0bf93095a9"
 dependencies = [
  "hermit-abi 0.5.0",
  "libc",
- "windows-sys 0.59.0",
+ "windows-sys 0.52.0",
 ]
 
 [[package]]
@@ -1553,9 +1553,9 @@ checksum = "4a5f13b858c8d314ee3e8f639011f7ccefe71f97f96e50151fb991f267928e2c"
 
 [[package]]
 name = "jiff"
-version = "0.2.4"
+version = "0.2.9"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "d699bc6dfc879fb1bf9bdff0d4c56f0884fc6f0d0eb0fba397a6d00cd9a6b85e"
+checksum = "59ec30f7142be6fe14e1b021f50b85db8df2d4324ea6e91ec3e5dcde092021d0"
 dependencies = [
  "jiff-static",
  "jiff-tzdb-platform",
@@ -1563,14 +1563,14 @@ dependencies = [
  "portable-atomic",
  "portable-atomic-util",
  "serde",
- "windows-sys 0.52.0",
+ "windows-sys 0.59.0",
 ]
 
 [[package]]
 name = "jiff-static"
-version = "0.2.4"
+version = "0.2.9"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "8d16e75759ee0aa64c57a56acbf43916987b20c77373cb7e808979e02b93c9f9"
+checksum = "526b834d727fd59d37b076b0c3236d9adde1b1729a4361e20b2026f738cc1dbe"
 dependencies = [
  "proc-macro2",
  "quote",
@@ -1645,9 +1645,9 @@ checksum = "bbd2bcb4c963f2ddae06a2efc7e9f3591312473c50c6685e1f298068316e66fe"
 
 [[package]]
 name = "libc"
-version = "0.2.171"
+version = "0.2.172"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "c19937216e9d3aa9956d9bb8dfc0b0c8beb6058fc4f7a4dc4d850edf86a237d6"
+checksum = "d750af042f7ef4f724306de029d18836c26c1765a54a6a3f094cbd23a7267ffa"
 
 [[package]]
 name = "libcst"
@@ -2327,9 +2327,9 @@ dependencies = [
 
 [[package]]
 name = "proc-macro2"
-version = "1.0.94"
+version = "1.0.95"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "a31971752e70b8b2686d7e46ec17fb38dad4051d94024c88df49b667caea9c84"
+checksum = "02b3e5e68a3a1a02aad3ec490a98007cbc13c37cbe84a3cd7b8e406d76e7f778"
 dependencies = [
  "unicode-ident",
 ]
@@ -2420,13 +2420,12 @@ dependencies = [
 
 [[package]]
 name = "rand"
-version = "0.9.0"
+version = "0.9.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "3779b94aeb87e8bd4e834cee3650289ee9e0d5677f976ecdb6d219e5f4f6cd94"
+checksum = "9fbfd9d094a40bf3ae768db9361049ace4c0e04a4fd6b359518bd7b73a73dd97"
 dependencies = [
  "rand_chacha 0.9.0",
  "rand_core 0.9.3",
- "zerocopy",
 ]
 
 [[package]]
@@ -3084,7 +3083,7 @@ version = "0.0.0"
 dependencies = [
  "anyhow",
  "itertools 0.14.0",
- "rand 0.9.0",
+ "rand 0.9.1",
  "ruff_diagnostics",
  "ruff_source_file",
  "ruff_text_size",
@@ -3428,7 +3427,7 @@ dependencies = [
  "errno",
  "libc",
  "linux-raw-sys 0.4.15",
- "windows-sys 0.59.0",
+ "windows-sys 0.52.0",
 ]
 
 [[package]]
@@ -3441,7 +3440,7 @@ dependencies = [
  "errno",
  "libc",
  "linux-raw-sys 0.9.3",
- "windows-sys 0.59.0",
+ "windows-sys 0.52.0",
 ]
 
 [[package]]
@@ -3675,9 +3674,9 @@ dependencies = [
 
 [[package]]
 name = "shellexpand"
-version = "3.1.0"
+version = "3.1.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "da03fa3b94cc19e3ebfc88c4229c49d8f08cdbd1228870a45f0ffdf84988e14b"
+checksum = "8b1fdf65dd6331831494dd616b30351c38e96e45921a27745cf98490458b90bb"
 dependencies = [
  "dirs",
 ]
@@ -3827,7 +3826,7 @@ dependencies = [
  "getrandom 0.3.2",
  "once_cell",
  "rustix 1.0.2",
- "windows-sys 0.59.0",
+ "windows-sys 0.52.0",
 ]
 
 [[package]]
@@ -4328,7 +4327,7 @@ checksum = "458f7a779bf54acc9f347480ac654f68407d3aab21269a6e3c9f922acd9e2da9"
 dependencies = [
  "getrandom 0.3.2",
  "js-sys",
- "rand 0.9.0",
+ "rand 0.9.1",
  "uuid-macro-internal",
  "wasm-bindgen",
 ]
@@ -4599,7 +4598,7 @@ version = "0.1.9"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "cf221c93e13a30d793f7645a0e7762c55d169dbb0a49671918a2319d289b10bb"
 dependencies = [
- "windows-sys 0.48.0",
+ "windows-sys 0.52.0",
 ]
 
 [[package]]

crates/red_knot_project/tests/check.rs

@@ -6,7 +6,9 @@ use ruff_db::parsed::parsed_module;
 use ruff_db::system::{SystemPath, SystemPathBuf, TestSystem};
 use ruff_python_ast::visitor::source_order;
 use ruff_python_ast::visitor::source_order::SourceOrderVisitor;
-use ruff_python_ast::{self as ast, Alias, Expr, Parameter, ParameterWithDefault, Stmt};
+use ruff_python_ast::{
+    self as ast, Alias, Comprehension, Expr, Parameter, ParameterWithDefault, Stmt,
+};
 
 fn setup_db(project_root: &SystemPath, system: TestSystem) -> anyhow::Result<ProjectDatabase> {
     let project = ProjectMetadata::discover(project_root, &system)?;
@@ -258,6 +260,14 @@ impl SourceOrderVisitor<'_> for PullTypesVisitor<'_> {
         source_order::walk_expr(self, expr);
     }
 
+    fn visit_comprehension(&mut self, comprehension: &Comprehension) {
+        self.visit_expr(&comprehension.iter);
+        self.visit_target(&comprehension.target);
+        for if_expr in &comprehension.ifs {
+            self.visit_expr(if_expr);
+        }
+    }
+
     fn visit_parameter(&mut self, parameter: &Parameter) {
         let _ty = parameter.inferred_type(&self.model);
 

crates/red_knot_python_semantic/resources/mdtest/attributes.md

@@ -397,15 +397,27 @@ class IntIterable:
     def __iter__(self) -> IntIterator:
         return IntIterator()
 
+class TupleIterator:
+    def __next__(self) -> tuple[int, str]:
+        return (1, "a")
+
+class TupleIterable:
+    def __iter__(self) -> TupleIterator:
+        return TupleIterator()
+
 class C:
     def __init__(self) -> None:
         [... for self.a in IntIterable()]
+        [... for (self.b, self.c) in TupleIterable()]
+        [... for self.d in IntIterable() for self.e in IntIterable()]
 
 c_instance = C()
 
-# TODO: Should be `Unknown | int`
-# error: [unresolved-attribute]
-reveal_type(c_instance.a)  # revealed: Unknown
+reveal_type(c_instance.a)  # revealed: Unknown | int
+reveal_type(c_instance.b)  # revealed: Unknown | int
+reveal_type(c_instance.c)  # revealed: Unknown | str
+reveal_type(c_instance.d)  # revealed: Unknown | int
+reveal_type(c_instance.e)  # revealed: Unknown | int
 ```
 
 #### Conditionally declared / bound attributes

crates/red_knot_python_semantic/resources/mdtest/function/return_type.md

@@ -74,8 +74,6 @@ class Baz(Bar):
 T = TypeVar("T")
 
 class Qux(Protocol[T]):
-    # TODO: no error
-    # error: [invalid-return-type]
     def f(self) -> int: ...
 
 class Foo(Protocol):

crates/red_knot_python_semantic/resources/mdtest/protocols.md

@@ -40,27 +40,63 @@ class Foo(Protocol, Protocol): ...  # error: [inconsistent-mro]
 reveal_type(Foo.__mro__)  # revealed: tuple[Literal[Foo], Unknown, Literal[object]]
 ```
 
+Protocols can also be generic, either by including `Generic[]` in the bases list, subscripting
+`Protocol` directly in the bases list, using PEP-695 type parameters, or some combination of the
+above:
+
+```py
+from typing import TypeVar, Generic
+
+T = TypeVar("T")
+
+class Bar0(Protocol[T]):
+    x: T
+
+class Bar1(Protocol[T], Generic[T]):
+    x: T
+
+class Bar2[T](Protocol):
+    x: T
+
+class Bar3[T](Protocol[T]):
+    x: T
+```
+
+It's an error to include both bare `Protocol` and subscripted `Protocol[]` in the bases list
+simultaneously:
+
+```py
+# TODO: should emit a `[duplicate-bases]` error here:
+class DuplicateBases(Protocol, Protocol[T]):
+    x: T
+
+# TODO: should not have `Generic` multiple times and `Protocol` multiple times
+# revealed: tuple[Literal[DuplicateBases], typing.Protocol, typing.Generic, @Todo(`Protocol[]` subscript), @Todo(`Generic[]` subscript), Literal[object]]
+reveal_type(DuplicateBases.__mro__)
+```
+
 The introspection helper `typing(_extensions).is_protocol` can be used to verify whether a class is
 a protocol class or not:
 
 ```py
 from typing_extensions import is_protocol
 
-# TODO: should be `Literal[True]`
-reveal_type(is_protocol(MyProtocol))  # revealed: bool
+reveal_type(is_protocol(MyProtocol))  # revealed: Literal[True]
+reveal_type(is_protocol(Bar0))  # revealed: Literal[True]
+reveal_type(is_protocol(Bar1))  # revealed: Literal[True]
+reveal_type(is_protocol(Bar2))  # revealed: Literal[True]
+reveal_type(is_protocol(Bar3))  # revealed: Literal[True]
 
 class NotAProtocol: ...
 
-# TODO: should be `Literal[False]`
-reveal_type(is_protocol(NotAProtocol))  # revealed: bool
+reveal_type(is_protocol(NotAProtocol))  # revealed: Literal[False]
 ```
 
 A type checker should follow the typeshed stubs if a non-class is passed in, and typeshed's stubs
-indicate that the argument passed in must be an instance of `type`. `Literal[False]` should be
-inferred as the return type, however.
+indicate that the argument passed in must be an instance of `type`.
 
 ```py
-# TODO: the diagnostic is correct, but should infer `Literal[False]`
+# We could also reasonably infer `Literal[False]` here, but it probably doesn't matter that much:
 # error: [invalid-argument-type]
 reveal_type(is_protocol("not a class"))  # revealed: bool
 ```
@@ -74,8 +110,7 @@ class SubclassOfMyProtocol(MyProtocol): ...
 # revealed: tuple[Literal[SubclassOfMyProtocol], Literal[MyProtocol], typing.Protocol, typing.Generic, Literal[object]]
 reveal_type(SubclassOfMyProtocol.__mro__)
 
-# TODO: should be `Literal[False]`
-reveal_type(is_protocol(SubclassOfMyProtocol))  # revealed: bool
+reveal_type(is_protocol(SubclassOfMyProtocol))  # revealed: Literal[False]
 ```
 
 A protocol class may inherit from other protocols, however, as long as it re-inherits from
@@ -84,8 +119,7 @@ A protocol class may inherit from other protocols, however, as long as it re-inh
 ```py
 class SubProtocol(MyProtocol, Protocol): ...
 
-# TODO: should be `Literal[True]`
-reveal_type(is_protocol(SubProtocol))  # revealed: bool
+reveal_type(is_protocol(SubProtocol))  # revealed: Literal[True]
 
 class OtherProtocol(Protocol):
     some_attribute: str
@@ -95,21 +129,20 @@ class ComplexInheritance(SubProtocol, OtherProtocol, Protocol): ...
 # revealed: tuple[Literal[ComplexInheritance], Literal[SubProtocol], Literal[MyProtocol], Literal[OtherProtocol], typing.Protocol, typing.Generic, Literal[object]]
 reveal_type(ComplexInheritance.__mro__)
 
-# TODO: should be `Literal[True]`
-reveal_type(is_protocol(ComplexInheritance))  # revealed: bool
+reveal_type(is_protocol(ComplexInheritance))  # revealed: Literal[True]
 ```
 
 If `Protocol` is present in the bases tuple, all other bases in the tuple must be protocol classes,
 or `TypeError` is raised at runtime when the class is created.
 
 ```py
-# TODO: should emit `[invalid-protocol]`
+# error: [invalid-protocol] "Protocol class `Invalid` cannot inherit from non-protocol class `NotAProtocol`"
 class Invalid(NotAProtocol, Protocol): ...
 
 # revealed: tuple[Literal[Invalid], Literal[NotAProtocol], typing.Protocol, typing.Generic, Literal[object]]
 reveal_type(Invalid.__mro__)
 
-# TODO: should emit an `[invalid-protocol`] error
+# error: [invalid-protocol] "Protocol class `AlsoInvalid` cannot inherit from non-protocol class `NotAProtocol`"
 class AlsoInvalid(MyProtocol, OtherProtocol, NotAProtocol, Protocol): ...
 
 # revealed: tuple[Literal[AlsoInvalid], Literal[MyProtocol], Literal[OtherProtocol], Literal[NotAProtocol], typing.Protocol, typing.Generic, Literal[object]]
@@ -134,6 +167,8 @@ reveal_type(Fine.__mro__)  # revealed: tuple[Literal[Fine], typing.Protocol, typ
 
 class StillFine(Protocol, Generic[T], object): ...
 class EvenThis[T](Protocol, object): ...
+class OrThis(Protocol[T], Generic[T]): ...
+class AndThis(Protocol[T], Generic[T], object): ...
 ```
 
 And multiple inheritance from a mix of protocol and non-protocol classes is fine as long as
@@ -150,8 +185,7 @@ But if `Protocol` is not present in the bases list, the resulting class doesn't
 class anymore:
 
 ```py
-# TODO: should reveal `Literal[False]`
-reveal_type(is_protocol(FineAndDandy))  # revealed: bool
+reveal_type(is_protocol(FineAndDandy))  # revealed: Literal[False]
 ```
 
 A class does not *have* to inherit from a protocol class in order for it to be considered a subtype
@@ -230,9 +264,10 @@ class Foo(typing.Protocol):
 class Bar(typing_extensions.Protocol):
     x: int
 
-# TODO: these should pass
-static_assert(typing_extensions.is_protocol(Foo))  # error: [static-assert-error]
-static_assert(typing_extensions.is_protocol(Bar))  # error: [static-assert-error]
+static_assert(typing_extensions.is_protocol(Foo))
+static_assert(typing_extensions.is_protocol(Bar))
+
+# TODO: should pass
 static_assert(is_equivalent_to(Foo, Bar))  # error: [static-assert-error]
 ```
 
@@ -247,9 +282,10 @@ class RuntimeCheckableFoo(typing.Protocol):
 class RuntimeCheckableBar(typing_extensions.Protocol):
     x: int
 
-# TODO: these should pass
-static_assert(typing_extensions.is_protocol(RuntimeCheckableFoo))  # error: [static-assert-error]
-static_assert(typing_extensions.is_protocol(RuntimeCheckableBar))  # error: [static-assert-error]
+static_assert(typing_extensions.is_protocol(RuntimeCheckableFoo))
+static_assert(typing_extensions.is_protocol(RuntimeCheckableBar))
+
+# TODO: should pass
 static_assert(is_equivalent_to(RuntimeCheckableFoo, RuntimeCheckableBar))  # error: [static-assert-error]
 
 # These should not error because the protocols are decorated with `@runtime_checkable`

crates/red_knot_python_semantic/resources/mdtest/unpacking.md

@@ -708,3 +708,95 @@ with ContextManager() as (a, b, c):
     reveal_type(b)  # revealed: Unknown
     reveal_type(c)  # revealed: Unknown
 ```
+
+## Comprehension
+
+Unpacking in a comprehension.
+
+### Same types
+
+```py
+def _(arg: tuple[tuple[int, int], tuple[int, int]]):
+    # revealed: tuple[int, int]
+    [reveal_type((a, b)) for a, b in arg]
+```
+
+### Mixed types (1)
+
+```py
+def _(arg: tuple[tuple[int, int], tuple[int, str]]):
+    # revealed: tuple[int, int | str]
+    [reveal_type((a, b)) for a, b in arg]
+```
+
+### Mixed types (2)
+
+```py
+def _(arg: tuple[tuple[int, str], tuple[str, int]]):
+    # revealed: tuple[int | str, str | int]
+    [reveal_type((a, b)) for a, b in arg]
+```
+
+### Mixed types (3)
+
+```py
+def _(arg: tuple[tuple[int, int, int], tuple[int, str, bytes], tuple[int, int, str]]):
+    # revealed: tuple[int, int | str, int | bytes | str]
+    [reveal_type((a, b, c)) for a, b, c in arg]
+```
+
+### Same literal values
+
+```py
+# revealed: tuple[Literal[1, 3], Literal[2, 4]]
+[reveal_type((a, b)) for a, b in ((1, 2), (3, 4))]
+```
+
+### Mixed literal values (1)
+
+```py
+# revealed: tuple[Literal[1, "a"], Literal[2, "b"]]
+[reveal_type((a, b)) for a, b in ((1, 2), ("a", "b"))]
+```
+
+### Mixed literals values (2)
+
+```py
+# error: "Object of type `Literal[1]` is not iterable"
+# error: "Object of type `Literal[2]` is not iterable"
+# error: "Object of type `Literal[4]` is not iterable"
+# error: [invalid-assignment] "Not enough values to unpack (expected 2, got 1)"
+# revealed: tuple[Unknown | Literal[3, 5], Unknown | Literal["a", "b"]]
+[reveal_type((a, b)) for a, b in (1, 2, (3, "a"), 4, (5, "b"), "c")]
+```
+
+### Custom iterator (1)
+
+```py
+class Iterator:
+    def __next__(self) -> tuple[int, int]:
+        return (1, 2)
+
+class Iterable:
+    def __iter__(self) -> Iterator:
+        return Iterator()
+
+# revealed: tuple[int, int]
+[reveal_type((a, b)) for a, b in Iterable()]
+```
+
+### Custom iterator (2)
+
+```py
+class Iterator:
+    def __next__(self) -> bytes:
+        return b""
+
+class Iterable:
+    def __iter__(self) -> Iterator:
+        return Iterator()
+
+def _(arg: tuple[tuple[int, str], Iterable]):
+    # revealed: tuple[int | bytes, str | bytes]
+    [reveal_type((a, b)) for a, b in arg]
+```

crates/red_knot_python_semantic/src/semantic_index.rs

@@ -940,7 +940,7 @@ def f(a: str, /, b: str, c: int = 1, *args, d: int = 2, **kwargs):
             panic!("expected generator definition")
         };
         let target = comprehension.target();
-        let name = target.id().as_str();
+        let name = target.as_name_expr().unwrap().id().as_str();
 
         assert_eq!(name, "x");
         assert_eq!(target.range(), TextRange::new(23.into(), 24.into()));

crates/red_knot_python_semantic/src/semantic_index/builder.rs

@@ -18,11 +18,12 @@ use crate::semantic_index::ast_ids::node_key::ExpressionNodeKey;
 use crate::semantic_index::ast_ids::AstIdsBuilder;
 use crate::semantic_index::definition::{
     AnnotatedAssignmentDefinitionKind, AnnotatedAssignmentDefinitionNodeRef,
-    AssignmentDefinitionKind, AssignmentDefinitionNodeRef, ComprehensionDefinitionNodeRef,
-    Definition, DefinitionCategory, DefinitionKind, DefinitionNodeKey, DefinitionNodeRef,
-    Definitions, ExceptHandlerDefinitionNodeRef, ForStmtDefinitionKind, ForStmtDefinitionNodeRef,
-    ImportDefinitionNodeRef, ImportFromDefinitionNodeRef, MatchPatternDefinitionNodeRef,
-    StarImportDefinitionNodeRef, TargetKind, WithItemDefinitionKind, WithItemDefinitionNodeRef,
+    AssignmentDefinitionKind, AssignmentDefinitionNodeRef, ComprehensionDefinitionKind,
+    ComprehensionDefinitionNodeRef, Definition, DefinitionCategory, DefinitionKind,
+    DefinitionNodeKey, DefinitionNodeRef, Definitions, ExceptHandlerDefinitionNodeRef,
+    ForStmtDefinitionKind, ForStmtDefinitionNodeRef, ImportDefinitionNodeRef,
+    ImportFromDefinitionNodeRef, MatchPatternDefinitionNodeRef, StarImportDefinitionNodeRef,
+    TargetKind, WithItemDefinitionKind, WithItemDefinitionNodeRef,
 };
 use crate::semantic_index::expression::{Expression, ExpressionKind};
 use crate::semantic_index::predicate::{
@@ -354,15 +355,14 @@ impl<'db> SemanticIndexBuilder<'db> {
         self.current_use_def_map_mut().merge(state);
     }
 
-    /// Return a 2-element tuple, where the first element is the [`ScopedSymbolId`] of the
-    /// symbol added, and the second element is a boolean indicating whether the symbol was *newly*
-    /// added or not
-    fn add_symbol(&mut self, name: Name) -> (ScopedSymbolId, bool) {
+    /// Add a symbol to the symbol table and the use-def map.
+    /// Return the [`ScopedSymbolId`] that uniquely identifies the symbol in both.
+    fn add_symbol(&mut self, name: Name) -> ScopedSymbolId {
         let (symbol_id, added) = self.current_symbol_table().add_symbol(name);
         if added {
             self.current_use_def_map_mut().add_symbol(symbol_id);
         }
-        (symbol_id, added)
+        symbol_id
     }
 
     fn add_attribute(&mut self, name: Name) -> ScopedSymbolId {
@@ -796,7 +796,7 @@ impl<'db> SemanticIndexBuilder<'db> {
                         ..
                     }) => (name, &None, default),
                 };
-                let (symbol, _) = self.add_symbol(name.id.clone());
+                let symbol = self.add_symbol(name.id.clone());
                 // TODO create Definition for PEP 695 typevars
                 // note that the "bound" on the typevar is a totally different thing than whether
                 // or not a name is "bound" by a typevar declaration; the latter is always true.
@@ -850,31 +850,35 @@ impl<'db> SemanticIndexBuilder<'db> {
 
         // The `iter` of the first generator is evaluated in the outer scope, while all subsequent
         // nodes are evaluated in the inner scope.
-        self.add_standalone_expression(&generator.iter);
+        let value = self.add_standalone_expression(&generator.iter);
         self.visit_expr(&generator.iter);
         self.push_scope(scope);
 
-        self.push_assignment(CurrentAssignment::Comprehension {
-            node: generator,
-            first: true,
-        });
-        self.visit_expr(&generator.target);
-        self.pop_assignment();
+        self.add_unpackable_assignment(
+            &Unpackable::Comprehension {
+                node: generator,
+                first: true,
+            },
+            &generator.target,
+            value,
+        );
 
         for expr in &generator.ifs {
             self.visit_expr(expr);
         }
 
         for generator in generators_iter {
-            self.add_standalone_expression(&generator.iter);
+            let value = self.add_standalone_expression(&generator.iter);
             self.visit_expr(&generator.iter);
 
-            self.push_assignment(CurrentAssignment::Comprehension {
-                node: generator,
-                first: false,
-            });
-            self.visit_expr(&generator.target);
-            self.pop_assignment();
+            self.add_unpackable_assignment(
+                &Unpackable::Comprehension {
+                    node: generator,
+                    first: false,
+                },
+                &generator.target,
+                value,
+            );
 
             for expr in &generator.ifs {
                 self.visit_expr(expr);
@@ -890,20 +894,20 @@ impl<'db> SemanticIndexBuilder<'db> {
             self.declare_parameter(parameter);
         }
         if let Some(vararg) = parameters.vararg.as_ref() {
-            let (symbol, _) = self.add_symbol(vararg.name.id().clone());
+            let symbol = self.add_symbol(vararg.name.id().clone());
             self.add_definition(
                 symbol,
                 DefinitionNodeRef::VariadicPositionalParameter(vararg),
             );
         }
         if let Some(kwarg) = parameters.kwarg.as_ref() {
-            let (symbol, _) = self.add_symbol(kwarg.name.id().clone());
+            let symbol = self.add_symbol(kwarg.name.id().clone());
             self.add_definition(symbol, DefinitionNodeRef::VariadicKeywordParameter(kwarg));
         }
     }
 
     fn declare_parameter(&mut self, parameter: &'db ast::ParameterWithDefault) {
-        let (symbol, _) = self.add_symbol(parameter.name().id().clone());
+        let symbol = self.add_symbol(parameter.name().id().clone());
 
         let definition = self.add_definition(symbol, parameter);
 
@@ -933,9 +937,30 @@ impl<'db> SemanticIndexBuilder<'db> {
 
         let current_assignment = match target {
             ast::Expr::List(_) | ast::Expr::Tuple(_) => {
+                if matches!(unpackable, Unpackable::Comprehension { .. }) {
+                    debug_assert_eq!(
+                        self.scopes[self.current_scope()].node().scope_kind(),
+                        ScopeKind::Comprehension
+                    );
+                }
+                // The first iterator of the comprehension is evaluated in the outer scope, while all subsequent
+                // nodes are evaluated in the inner scope.
+                // SAFETY: The current scope is the comprehension, and the comprehension scope must have a parent scope.
+                let value_file_scope =
+                    if let Unpackable::Comprehension { first: true, .. } = unpackable {
+                        self.scope_stack
+                            .iter()
+                            .rev()
+                            .nth(1)
+                            .expect("The comprehension scope must have a parent scope")
+                            .file_scope_id
+                    } else {
+                        self.current_scope()
+                    };
                 let unpack = Some(Unpack::new(
                     self.db,
                     self.file,
+                    value_file_scope,
                     self.current_scope(),
                     // SAFETY: `target` belongs to the `self.module` tree
                     #[allow(unsafe_code)]
@@ -1113,7 +1138,7 @@ where
                 // The symbol for the function name itself has to be evaluated
                 // at the end to match the runtime evaluation of parameter defaults
                 // and return-type annotations.
-                let (symbol, _) = self.add_symbol(name.id.clone());
+                let symbol = self.add_symbol(name.id.clone());
 
                 // Record a use of the function name in the scope that it is defined in, so that it
                 // can be used to find previously defined functions with the same name. This is
@@ -1148,11 +1173,11 @@ where
                 );
 
                 // In Python runtime semantics, a class is registered after its scope is evaluated.
-                let (symbol, _) = self.add_symbol(class.name.id.clone());
+                let symbol = self.add_symbol(class.name.id.clone());
                 self.add_definition(symbol, class);
             }
             ast::Stmt::TypeAlias(type_alias) => {
-                let (symbol, _) = self.add_symbol(
+                let symbol = self.add_symbol(
                     type_alias
                         .name
                         .as_name_expr()
@@ -1189,7 +1214,7 @@ where
                         (Name::new(alias.name.id.split('.').next().unwrap()), false)
                     };
 
-                    let (symbol, _) = self.add_symbol(symbol_name);
+                    let symbol = self.add_symbol(symbol_name);
                     self.add_definition(
                         symbol,
                         ImportDefinitionNodeRef {
@@ -1260,7 +1285,7 @@ where
                         //
                         // For more details, see the doc-comment on `StarImportPlaceholderPredicate`.
                         for export in exported_names(self.db, referenced_module) {
-                            let (symbol_id, newly_added) = self.add_symbol(export.clone());
+                            let symbol_id = self.add_symbol(export.clone());
                             let node_ref = StarImportDefinitionNodeRef { node, symbol_id };
                             let star_import = StarImportPlaceholderPredicate::new(
                                 self.db,
@@ -1269,28 +1294,15 @@ where
                                 referenced_module,
                             );
 
-                            // Fast path for if there were no previous definitions
-                            // of the symbol defined through the `*` import:
-                            // we can apply the visibility constraint to *only* the added definition,
-                            // rather than all definitions
-                            if newly_added {
-                                self.push_additional_definition(symbol_id, node_ref);
-                                self.current_use_def_map_mut()
-                                    .record_and_negate_star_import_visibility_constraint(
-                                        star_import,
-                                        symbol_id,
-                                    );
-                            } else {
-                                let pre_definition = self.flow_snapshot();
-                                self.push_additional_definition(symbol_id, node_ref);
-                                let constraint_id =
-                                    self.record_visibility_constraint(star_import.into());
-                                let post_definition = self.flow_snapshot();
-                                self.flow_restore(pre_definition.clone());
-                                self.record_negated_visibility_constraint(constraint_id);
-                                self.flow_merge(post_definition);
-                                self.simplify_visibility_constraints(pre_definition);
-                            }
+                            let pre_definition =
+                                self.current_use_def_map().single_symbol_snapshot(symbol_id);
+                            self.push_additional_definition(symbol_id, node_ref);
+                            self.current_use_def_map_mut()
+                                .record_and_negate_star_import_visibility_constraint(
+                                    star_import,
+                                    symbol_id,
+                                    pre_definition,
+                                );
                         }
 
                         continue;
@@ -1310,7 +1322,7 @@ where
                     self.has_future_annotations |= alias.name.id == "annotations"
                         && node.module.as_deref() == Some("__future__");
 
-                    let (symbol, _) = self.add_symbol(symbol_name.clone());
+                    let symbol = self.add_symbol(symbol_name.clone());
 
                     self.add_definition(
                         symbol,
@@ -1728,7 +1740,7 @@ where
                         // which is invalid syntax. However, it's still pretty obvious here that the user
                         // *wanted* `e` to be bound, so we should still create a definition here nonetheless.
                         if let Some(symbol_name) = symbol_name {
-                            let (symbol, _) = self.add_symbol(symbol_name.id.clone());
+                            let symbol = self.add_symbol(symbol_name.id.clone());
 
                             self.add_definition(
                                 symbol,
@@ -1804,7 +1816,7 @@ where
         let node_key = NodeKey::from_node(expr);
 
         match expr {
-            ast::Expr::Name(name_node @ ast::ExprName { id, ctx, .. }) => {
+            ast::Expr::Name(ast::ExprName { id, ctx, .. }) => {
                 let (is_use, is_definition) = match (ctx, self.current_assignment()) {
                     (ast::ExprContext::Store, Some(CurrentAssignment::AugAssign(_))) => {
                         // For augmented assignment, the target expression is also used.
@@ -1815,7 +1827,7 @@ where
                     (ast::ExprContext::Del, _) => (false, true),
                     (ast::ExprContext::Invalid, _) => (false, false),
                 };
-                let (symbol, _) = self.add_symbol(id.clone());
+                let symbol = self.add_symbol(id.clone());
 
                 if is_use {
                     self.mark_symbol_used(symbol);
@@ -1867,12 +1879,17 @@ where
                             // implemented.
                             self.add_definition(symbol, named);
                         }
-                        Some(CurrentAssignment::Comprehension { node, first }) => {
+                        Some(CurrentAssignment::Comprehension {
+                            unpack,
+                            node,
+                            first,
+                        }) => {
                             self.add_definition(
                                 symbol,
                                 ComprehensionDefinitionNodeRef {
+                                    unpack,
                                     iterable: &node.iter,
-                                    target: name_node,
+                                    target: expr,
                                     first,
                                     is_async: node.is_async,
                                 },
@@ -2143,14 +2160,37 @@ where
                                 DefinitionKind::WithItem(assignment),
                             );
                         }
-                        Some(CurrentAssignment::Comprehension { .. }) => {
-                            // TODO:
+                        Some(CurrentAssignment::Comprehension {
+                            unpack,
+                            node,
+                            first,
+                        }) => {
+                            // SAFETY: `iter` and `expr` belong to the `self.module` tree
+                            #[allow(unsafe_code)]
+                            let assignment = ComprehensionDefinitionKind {
+                                target_kind: TargetKind::from(unpack),
+                                iterable: unsafe {
+                                    AstNodeRef::new(self.module.clone(), &node.iter)
+                                },
+                                target: unsafe { AstNodeRef::new(self.module.clone(), expr) },
+                                first,
+                                is_async: node.is_async,
+                            };
+                            // Temporarily move to the scope of the method to which the instance attribute is defined.
+                            // SAFETY: `self.scope_stack` is not empty because the targets in comprehensions should always introduce a new scope.
+                            let scope = self.scope_stack.pop().expect("The popped scope must be a comprehension, which must have a parent scope");
+                            self.register_attribute_assignment(
+                                object,
+                                attr,
+                                DefinitionKind::Comprehension(assignment),
+                            );
+                            self.scope_stack.push(scope);
                         }
                         Some(CurrentAssignment::AugAssign(_)) => {
                             // TODO:
                         }
                         Some(CurrentAssignment::Named(_)) => {
-                            // TODO:
+                            // A named expression whose target is an attribute is syntactically prohibited
                         }
                         None => {}
                     }
@@ -2191,7 +2231,7 @@ where
             range: _,
         }) = pattern
         {
-            let (symbol, _) = self.add_symbol(name.id().clone());
+            let symbol = self.add_symbol(name.id().clone());
             let state = self.current_match_case.as_ref().unwrap();
             self.add_definition(
                 symbol,
@@ -2212,7 +2252,7 @@ where
             rest: Some(name), ..
         }) = pattern
         {
-            let (symbol, _) = self.add_symbol(name.id().clone());
+            let symbol = self.add_symbol(name.id().clone());
             let state = self.current_match_case.as_ref().unwrap();
             self.add_definition(
                 symbol,
@@ -2244,6 +2284,7 @@ enum CurrentAssignment<'a> {
     Comprehension {
         node: &'a ast::Comprehension,
         first: bool,
+        unpack: Option<(UnpackPosition, Unpack<'a>)>,
     },
     WithItem {
         item: &'a ast::WithItem,
@@ -2257,11 +2298,9 @@ impl CurrentAssignment<'_> {
         match self {
             Self::Assign { unpack, .. }
             | Self::For { unpack, .. }
-            | Self::WithItem { unpack, .. } => unpack.as_mut().map(|(position, _)| position),
-            Self::AnnAssign(_)
-            | Self::AugAssign(_)
-            | Self::Named(_)
-            | Self::Comprehension { .. } => None,
+            | Self::WithItem { unpack, .. }
+            | Self::Comprehension { unpack, .. } => unpack.as_mut().map(|(position, _)| position),
+            Self::AnnAssign(_) | Self::AugAssign(_) | Self::Named(_) => None,
         }
     }
 }
@@ -2316,13 +2355,17 @@ enum Unpackable<'a> {
         item: &'a ast::WithItem,
         is_async: bool,
     },
+    Comprehension {
+        first: bool,
+        node: &'a ast::Comprehension,
+    },
 }
 
 impl<'a> Unpackable<'a> {
     const fn kind(&self) -> UnpackKind {
         match self {
             Unpackable::Assign(_) => UnpackKind::Assign,
-            Unpackable::For(_) => UnpackKind::Iterable,
+            Unpackable::For(_) | Unpackable::Comprehension { .. } => UnpackKind::Iterable,
             Unpackable::WithItem { .. } => UnpackKind::ContextManager,
         }
     }
@@ -2337,6 +2380,11 @@ impl<'a> Unpackable<'a> {
                 is_async: *is_async,
                 unpack,
             },
+            Unpackable::Comprehension { node, first } => CurrentAssignment::Comprehension {
+                node,
+                first: *first,
+                unpack,
+            },
         }
     }
 }

crates/red_knot_python_semantic/src/semantic_index/definition.rs

@@ -281,8 +281,9 @@ pub(crate) struct ExceptHandlerDefinitionNodeRef<'a> {
 
 #[derive(Copy, Clone, Debug)]
 pub(crate) struct ComprehensionDefinitionNodeRef<'a> {
+    pub(crate) unpack: Option<(UnpackPosition, Unpack<'a>)>,
     pub(crate) iterable: &'a ast::Expr,
-    pub(crate) target: &'a ast::ExprName,
+    pub(crate) target: &'a ast::Expr,
     pub(crate) first: bool,
     pub(crate) is_async: bool,
 }
@@ -374,11 +375,13 @@ impl<'db> DefinitionNodeRef<'db> {
                 is_async,
             }),
             DefinitionNodeRef::Comprehension(ComprehensionDefinitionNodeRef {
+                unpack,
                 iterable,
                 target,
                 first,
                 is_async,
             }) => DefinitionKind::Comprehension(ComprehensionDefinitionKind {
+                target_kind: TargetKind::from(unpack),
                 iterable: AstNodeRef::new(parsed.clone(), iterable),
                 target: AstNodeRef::new(parsed, target),
                 first,
@@ -474,7 +477,9 @@ impl<'db> DefinitionNodeRef<'db> {
                 unpack: _,
                 is_async: _,
             }) => DefinitionNodeKey(NodeKey::from_node(target)),
-            Self::Comprehension(ComprehensionDefinitionNodeRef { target, .. }) => target.into(),
+            Self::Comprehension(ComprehensionDefinitionNodeRef { target, .. }) => {
+                DefinitionNodeKey(NodeKey::from_node(target))
+            }
             Self::VariadicPositionalParameter(node) => node.into(),
             Self::VariadicKeywordParameter(node) => node.into(),
             Self::Parameter(node) => node.into(),
@@ -550,7 +555,7 @@ pub enum DefinitionKind<'db> {
     AnnotatedAssignment(AnnotatedAssignmentDefinitionKind),
     AugmentedAssignment(AstNodeRef<ast::StmtAugAssign>),
     For(ForStmtDefinitionKind<'db>),
-    Comprehension(ComprehensionDefinitionKind),
+    Comprehension(ComprehensionDefinitionKind<'db>),
     VariadicPositionalParameter(AstNodeRef<ast::Parameter>),
     VariadicKeywordParameter(AstNodeRef<ast::Parameter>),
     Parameter(AstNodeRef<ast::ParameterWithDefault>),
@@ -749,19 +754,24 @@ impl MatchPatternDefinitionKind {
 }
 
 #[derive(Clone, Debug)]
-pub struct ComprehensionDefinitionKind {
-    iterable: AstNodeRef<ast::Expr>,
-    target: AstNodeRef<ast::ExprName>,
-    first: bool,
-    is_async: bool,
+pub struct ComprehensionDefinitionKind<'db> {
+    pub(super) target_kind: TargetKind<'db>,
+    pub(super) iterable: AstNodeRef<ast::Expr>,
+    pub(super) target: AstNodeRef<ast::Expr>,
+    pub(super) first: bool,
+    pub(super) is_async: bool,
 }
 
-impl ComprehensionDefinitionKind {
+impl<'db> ComprehensionDefinitionKind<'db> {
     pub(crate) fn iterable(&self) -> &ast::Expr {
         self.iterable.node()
     }
 
-    pub(crate) fn target(&self) -> &ast::ExprName {
+    pub(crate) fn target_kind(&self) -> TargetKind<'db> {
+        self.target_kind
+    }
+
+    pub(crate) fn target(&self) -> &ast::Expr {
         self.target.node()
     }
 

crates/red_knot_python_semantic/src/semantic_index/use_def.rs

@@ -775,7 +775,16 @@ impl<'db> UseDefMapBuilder<'db> {
             .add_and_constraint(self.scope_start_visibility, constraint);
     }
 
-    /// This method exists solely as a fast path for handling `*`-import visibility constraints.
+    /// Snapshot the state of a single symbol at the current point in control flow.
+    ///
+    /// This is only used for `*`-import visibility constraints, which are handled differently
+    /// to most other visibility constraints. See the doc-comment for
+    /// [`Self::record_and_negate_star_import_visibility_constraint`] for more details.
+    pub(super) fn single_symbol_snapshot(&self, symbol: ScopedSymbolId) -> SymbolState {
+        self.symbol_states[symbol].clone()
+    }
+
+    /// This method exists solely for handling `*`-import visibility constraints.
     ///
     /// The reason why we add visibility constraints for [`Definition`]s created by `*` imports
     /// is laid out in the doc-comment for [`StarImportPlaceholderPredicate`]. But treating these
@@ -784,12 +793,11 @@ impl<'db> UseDefMapBuilder<'db> {
     /// dominates. (Although `*` imports are not common generally, they are used in several
     /// important places by typeshed.)
     ///
-    /// To solve these regressions, it was observed that we could add a fast path for `*`-import
-    /// definitions which added a new symbol to the global scope (as opposed to `*`-import definitions
-    /// that provided redefinitions for *pre-existing* global-scope symbols). The fast path does a
-    /// number of things differently to our normal handling of visibility constraints:
+    /// To solve these regressions, it was observed that we could do significantly less work for
+    /// `*`-import definitions. We do a number of things differently here to our normal handling of
+    /// visibility constraints:
     ///
-    /// - It only applies and negates the visibility constraints to a single symbol, rather than to
+    /// - We only apply and negate the visibility constraints to a single symbol, rather than to
     ///   all symbols. This is possible here because, unlike most definitions, we know in advance that
     ///   exactly one definition occurs inside the "if-true" predicate branch, and we know exactly
     ///   which definition it is.
@@ -800,9 +808,9 @@ impl<'db> UseDefMapBuilder<'db> {
     ///   the visibility constraints is only important for symbols that did not have any new
     ///   definitions inside either the "if-predicate-true" branch or the "if-predicate-false" branch.
     ///
-    /// - It avoids multiple expensive calls to [`Self::snapshot`]. This is possible because we know
-    ///   the symbol is newly added, so we know the prior state of the symbol was
-    ///   [`SymbolState::undefined`].
+    /// - We only snapshot the state for a single symbol prior to the definition, rather than doing
+    ///   expensive calls to [`Self::snapshot`]. Again, this is possible because we know
+    ///   that only a single definition occurs inside the "if-predicate-true" predicate branch.
     ///
     /// - Normally we take care to check whether an "if-predicate-true" branch or an
     ///   "if-predicate-false" branch contains a terminal statement: these can affect the visibility
@@ -815,6 +823,7 @@ impl<'db> UseDefMapBuilder<'db> {
         &mut self,
         star_import: StarImportPlaceholderPredicate<'db>,
         symbol: ScopedSymbolId,
+        pre_definition_state: SymbolState,
     ) {
         let predicate_id = self.add_predicate(star_import.into());
         let visibility_id = self.visibility_constraints.add_atom(predicate_id);
@@ -822,10 +831,9 @@ impl<'db> UseDefMapBuilder<'db> {
             .visibility_constraints
             .add_not_constraint(visibility_id);
 
-        let mut post_definition_state = std::mem::replace(
-            &mut self.symbol_states[symbol],
-            SymbolState::undefined(self.scope_start_visibility),
-        );
+        let mut post_definition_state =
+            std::mem::replace(&mut self.symbol_states[symbol], pre_definition_state);
+
         post_definition_state
             .record_visibility_constraint(&mut self.visibility_constraints, visibility_id);
 

crates/red_knot_python_semantic/src/semantic_index/use_def/symbol_state.rs

@@ -314,7 +314,7 @@ impl SymbolBindings {
 }
 
 #[derive(Clone, Debug, PartialEq, Eq)]
-pub(super) struct SymbolState {
+pub(in crate::semantic_index) struct SymbolState {
     declarations: SymbolDeclarations,
     bindings: SymbolBindings,
 }

crates/red_knot_python_semantic/src/types/call/bind.rs

@@ -535,6 +535,15 @@ impl<'db> Bindings<'db> {
                         }
                     }
 
+                    Some(KnownFunction::IsProtocol) => {
+                        if let [Some(ty)] = overload.parameter_types() {
+                            overload.set_return_type(Type::BooleanLiteral(
+                                ty.into_class_literal()
+                                    .is_some_and(|class| class.is_protocol(db)),
+                            ));
+                        }
+                    }
+
                     Some(KnownFunction::Overload) => {
                         // TODO: This can be removed once we understand legacy generics because the
                         // typeshed definition for `typing.overload` is an identity function.

crates/red_knot_python_semantic/src/types/class.rs

@@ -582,6 +582,17 @@ impl<'db> ClassLiteralType<'db> {
             .collect()
     }
 
+    /// Determine if this class is a protocol.
+    pub(super) fn is_protocol(self, db: &'db dyn Db) -> bool {
+        self.explicit_bases(db).iter().any(|base| {
+            matches!(
+                base,
+                Type::KnownInstance(KnownInstanceType::Protocol)
+                    | Type::Dynamic(DynamicType::SubscriptedProtocol)
+            )
+        })
+    }
+
     /// Return the types of the decorators on this class
     #[salsa::tracked(return_ref)]
     fn decorators(self, db: &'db dyn Db) -> Box<[Type<'db>]> {
@@ -1416,14 +1427,42 @@ impl<'db> ClassLiteralType<'db> {
                             }
                         }
                     }
-                    DefinitionKind::Comprehension(_) => {
-                        // TODO:
+                    DefinitionKind::Comprehension(comprehension) => {
+                        match comprehension.target_kind() {
+                            TargetKind::Sequence(_, unpack) => {
+                                // We found an unpacking assignment like:
+                                //
+                                //     [... for .., self.name, .. in <iterable>]
+
+                                let unpacked = infer_unpack_types(db, unpack);
+                                let target_ast_id = comprehension
+                                    .target()
+                                    .scoped_expression_id(db, unpack.target_scope(db));
+                                let inferred_ty = unpacked.expression_type(target_ast_id);
+
+                                union_of_inferred_types = union_of_inferred_types.add(inferred_ty);
+                            }
+                            TargetKind::NameOrAttribute => {
+                                // We found an attribute assignment like:
+                                //
+                                //     [... for self.name in <iterable>]
+
+                                let iterable_ty = infer_expression_type(
+                                    db,
+                                    index.expression(comprehension.iterable()),
+                                );
+                                // TODO: Potential diagnostics resulting from the iterable are currently not reported.
+                                let inferred_ty = iterable_ty.iterate(db);
+
+                                union_of_inferred_types = union_of_inferred_types.add(inferred_ty);
+                            }
+                        }
                     }
                     DefinitionKind::AugmentedAssignment(_) => {
                         // TODO:
                     }
                     DefinitionKind::NamedExpression(_) => {
-                        // TODO:
+                        // A named expression whose target is an attribute is syntactically prohibited
                     }
                     _ => {}
                 }

crates/red_knot_python_semantic/src/types/diagnostic.rs

@@ -36,6 +36,7 @@ pub(crate) fn register_lints(registry: &mut LintRegistryBuilder) {
     registry.register_lint(&INVALID_EXCEPTION_CAUGHT);
     registry.register_lint(&INVALID_METACLASS);
     registry.register_lint(&INVALID_PARAMETER_DEFAULT);
+    registry.register_lint(&INVALID_PROTOCOL);
     registry.register_lint(&INVALID_RAISE);
     registry.register_lint(&INVALID_SUPER_ARGUMENT);
     registry.register_lint(&INVALID_TYPE_CHECKING_CONSTANT);
@@ -230,6 +231,34 @@ declare_lint! {
     }
 }
 
+declare_lint! {
+    /// ## What it does
+    /// Checks for invalidly defined protocol classes.
+    ///
+    /// ## Why is this bad?
+    /// An invalidly defined protocol class may lead to the type checker inferring
+    /// unexpected things. It may also lead to `TypeError`s at runtime.
+    ///
+    /// ## Examples
+    /// A `Protocol` class cannot inherit from a non-`Protocol` class;
+    /// this raises a `TypeError` at runtime:
+    ///
+    /// ```pycon
+    /// >>> from typing import Protocol
+    /// >>> class Foo(int, Protocol): ...
+    /// ...
+    /// Traceback (most recent call last):
+    ///   File "<python-input-1>", line 1, in <module>
+    ///     class Foo(int, Protocol): ...
+    /// TypeError: Protocols can only inherit from other protocols, got <class 'int'>
+    /// ```
+    pub(crate) static INVALID_PROTOCOL = {
+        summary: "detects invalid protocol class definitions",
+        status: LintStatus::preview("1.0.0"),
+        default_level: Level::Error,
+    }
+}
+
 declare_lint! {
     /// TODO #14889
     pub(crate) static INCONSISTENT_MRO = {

crates/red_knot_python_semantic/src/types/infer.rs

@@ -49,9 +49,9 @@ use crate::module_resolver::resolve_module;
 use crate::node_key::NodeKey;
 use crate::semantic_index::ast_ids::{HasScopedExpressionId, HasScopedUseId, ScopedExpressionId};
 use crate::semantic_index::definition::{
-    AnnotatedAssignmentDefinitionKind, AssignmentDefinitionKind, Definition, DefinitionKind,
-    DefinitionNodeKey, ExceptHandlerDefinitionKind, ForStmtDefinitionKind, TargetKind,
-    WithItemDefinitionKind,
+    AnnotatedAssignmentDefinitionKind, AssignmentDefinitionKind, ComprehensionDefinitionKind,
+    Definition, DefinitionKind, DefinitionNodeKey, ExceptHandlerDefinitionKind,
+    ForStmtDefinitionKind, TargetKind, WithItemDefinitionKind,
 };
 use crate::semantic_index::expression::{Expression, ExpressionKind};
 use crate::semantic_index::symbol::{
@@ -81,9 +81,9 @@ use crate::types::generics::GenericContext;
 use crate::types::mro::MroErrorKind;
 use crate::types::unpacker::{UnpackResult, Unpacker};
 use crate::types::{
-    todo_type, CallDunderError, CallableSignature, CallableType, Class, ClassLiteralType,
-    ClassType, DataclassMetadata, DynamicType, FunctionDecorators, FunctionType, GenericAlias,
-    GenericClass, IntersectionBuilder, IntersectionType, KnownClass, KnownFunction,
+    binding_type, todo_type, CallDunderError, CallableSignature, CallableType, Class,
+    ClassLiteralType, ClassType, DataclassMetadata, DynamicType, FunctionDecorators, FunctionType,
+    GenericAlias, GenericClass, IntersectionBuilder, IntersectionType, KnownClass, KnownFunction,
     KnownInstanceType, MemberLookupPolicy, MetaclassCandidate, NonGenericClass, Parameter,
     ParameterForm, Parameters, Signature, Signatures, SliceLiteralType, StringLiteralType,
     SubclassOfType, Symbol, SymbolAndQualifiers, Truthiness, TupleType, Type, TypeAliasType,
@@ -99,8 +99,8 @@ use super::diagnostic::{
     report_index_out_of_bounds, report_invalid_exception_caught, report_invalid_exception_cause,
     report_invalid_exception_raised, report_invalid_type_checking_constant,
     report_non_subscriptable, report_possibly_unresolved_reference, report_slice_step_size_zero,
-    report_unresolved_reference, INVALID_METACLASS, REDUNDANT_CAST, STATIC_ASSERT_ERROR,
-    SUBCLASS_OF_FINAL_CLASS, TYPE_ASSERTION_FAILURE,
+    report_unresolved_reference, INVALID_METACLASS, INVALID_PROTOCOL, REDUNDANT_CAST,
+    STATIC_ASSERT_ERROR, SUBCLASS_OF_FINAL_CLASS, TYPE_ASSERTION_FAILURE,
 };
 use super::slots::check_class_slots;
 use super::string_annotation::{
@@ -306,7 +306,7 @@ pub(super) fn infer_unpack_types<'db>(db: &'db dyn Db, unpack: Unpack<'db>) -> U
     let _span =
         tracing::trace_span!("infer_unpack_types", range=?unpack.range(db), ?file).entered();
 
-    let mut unpacker = Unpacker::new(db, unpack.scope(db));
+    let mut unpacker = Unpacker::new(db, unpack.target_scope(db), unpack.value_scope(db));
     unpacker.unpack(unpack.target(db), unpack.value(db));
     unpacker.finish()
 }
@@ -763,17 +763,21 @@ impl<'db> TypeInferenceBuilder<'db> {
                 continue;
             }
 
-            // (2) Check for inheritance from plain `Generic`,
-            //     and from classes that inherit from `@final` classes
+            let is_protocol = class.is_protocol(self.db());
+
+            // (2) Iterate through the class's explicit bases to check for various possible errors:
+            //     - Check for inheritance from plain `Generic`,
+            //     - Check for inheritance from a `@final` classes
+            //     - If the class is a protocol class: check for inheritance from a non-protocol class
             for (i, base_class) in class.explicit_bases(self.db()).iter().enumerate() {
                 let base_class = match base_class {
                     Type::KnownInstance(KnownInstanceType::Generic) => {
-                        // `Generic` can appear in the MRO of many classes,
+                        // Unsubscripted `Generic` can appear in the MRO of many classes,
                         // but it is never valid as an explicit base class in user code.
                         self.context.report_lint_old(
                             &INVALID_BASE,
                             &class_node.bases()[i],
-                            format_args!("Cannot inherit from plain `Generic`",),
+                            format_args!("Cannot inherit from plain `Generic`"),
                         );
                         continue;
                     }
@@ -782,18 +786,32 @@ impl<'db> TypeInferenceBuilder<'db> {
                     _ => continue,
                 };
 
-                if !base_class.is_final(self.db()) {
-                    continue;
+                if is_protocol
+                    && !(base_class.is_protocol(self.db())
+                        || base_class.is_known(self.db(), KnownClass::Object))
+                {
+                    self.context.report_lint_old(
+                        &INVALID_PROTOCOL,
+                        &class_node.bases()[i],
+                        format_args!(
+                            "Protocol class `{}` cannot inherit from non-protocol class `{}`",
+                            class.name(self.db()),
+                            base_class.name(self.db()),
+                        ),
+                    );
+                }
+
+                if base_class.is_final(self.db()) {
+                    self.context.report_lint_old(
+                        &SUBCLASS_OF_FINAL_CLASS,
+                        &class_node.bases()[i],
+                        format_args!(
+                            "Class `{}` cannot inherit from final class `{}`",
+                            class.name(self.db()),
+                            base_class.name(self.db()),
+                        ),
+                    );
                 }
-                self.context.report_lint_old(
-                    &SUBCLASS_OF_FINAL_CLASS,
-                    &class_node.bases()[i],
-                    format_args!(
-                        "Class `{}` cannot inherit from final class `{}`",
-                        class.name(self.db()),
-                        base_class.name(self.db()),
-                    ),
-                );
             }
 
             // (3) Check that the class's MRO is resolvable
@@ -946,13 +964,7 @@ impl<'db> TypeInferenceBuilder<'db> {
                 self.infer_named_expression_definition(named_expression.node(), definition);
             }
             DefinitionKind::Comprehension(comprehension) => {
-                self.infer_comprehension_definition(
-                    comprehension.iterable(),
-                    comprehension.target(),
-                    comprehension.is_first(),
-                    comprehension.is_async(),
-                    definition,
-                );
+                self.infer_comprehension_definition(comprehension, definition);
             }
             DefinitionKind::VariadicPositionalParameter(parameter) => {
                 self.infer_variadic_positional_parameter_definition(parameter, definition);
@@ -1230,7 +1242,7 @@ impl<'db> TypeInferenceBuilder<'db> {
 
     /// Returns `true` if the current scope is the function body scope of a method of a protocol
     /// (that is, a class which directly inherits `typing.Protocol`.)
-    fn in_class_that_inherits_protocol_directly(&self) -> bool {
+    fn in_protocol_class(&self) -> bool {
         let current_scope_id = self.scope().file_scope_id(self.db());
         let current_scope = self.index.scope(current_scope_id);
         let Some(parent_scope_id) = current_scope.parent() else {
@@ -1258,13 +1270,13 @@ impl<'db> TypeInferenceBuilder<'db> {
             return false;
         };
 
-        // TODO move this to `Class` once we add proper `Protocol` support
-        node_ref.bases().iter().any(|base| {
-            matches!(
-                self.file_expression_type(base),
-                Type::KnownInstance(KnownInstanceType::Protocol)
-            )
-        })
+        let class_definition = self.index.expect_single_definition(node_ref.node());
+
+        let Type::ClassLiteral(class) = binding_type(self.db(), class_definition) else {
+            return false;
+        };
+
+        class.is_protocol(self.db())
     }
 
     /// Returns `true` if the current scope is the function body scope of a function overload (that
@@ -1328,7 +1340,7 @@ impl<'db> TypeInferenceBuilder<'db> {
 
             if (self.in_stub()
                 || self.in_function_overload_or_abstractmethod()
-                || self.in_class_that_inherits_protocol_directly())
+                || self.in_protocol_class())
                 && self.return_types_and_ranges.is_empty()
                 && is_stub_suite(&function.body)
             {
@@ -1631,7 +1643,7 @@ impl<'db> TypeInferenceBuilder<'db> {
                     }
                 } else if (self.in_stub()
                     || self.in_function_overload_or_abstractmethod()
-                    || self.in_class_that_inherits_protocol_directly())
+                    || self.in_protocol_class())
                     && default
                         .as_ref()
                         .is_some_and(|d| d.is_ellipsis_literal_expr())
@@ -1937,11 +1949,13 @@ impl<'db> TypeInferenceBuilder<'db> {
         for item in items {
             let target = item.optional_vars.as_deref();
             if let Some(target) = target {
-                self.infer_target(target, &item.context_expr, |db, ctx_manager_ty| {
+                self.infer_target(target, &item.context_expr, |builder, context_expr| {
                     // TODO: `infer_with_statement_definition` reports a diagnostic if `ctx_manager_ty` isn't a context manager
                     //  but only if the target is a name. We should report a diagnostic here if the target isn't a name:
                     //  `with not_context_manager as a.x: ...
-                    ctx_manager_ty.enter(db)
+                    builder
+                        .infer_standalone_expression(context_expr)
+                        .enter(builder.db())
                 });
             } else {
                 // Call into the context expression inference to validate that it evaluates
@@ -2347,7 +2361,9 @@ impl<'db> TypeInferenceBuilder<'db> {
         } = assignment;
 
         for target in targets {
-            self.infer_target(target, value, |_, ty| ty);
+            self.infer_target(target, value, |builder, value_expr| {
+                builder.infer_standalone_expression(value_expr)
+            });
         }
     }
 
@@ -2357,23 +2373,16 @@ impl<'db> TypeInferenceBuilder<'db> {
     /// targets (unpacking). If `target` is an attribute expression, we check that the assignment
     /// is valid. For 'target's that are definitions, this check happens elsewhere.
     ///
-    /// The `to_assigned_ty` function is used to convert the inferred type of the `value` expression
-    /// to the type that is eventually assigned to the `target`.
-    ///
-    /// # Panics
-    ///
-    /// If the `value` is not a standalone expression.
-    fn infer_target<F>(&mut self, target: &ast::Expr, value: &ast::Expr, to_assigned_ty: F)
+    /// The `infer_value_expr` function is used to infer the type of the `value` expression which
+    /// are not `Name` expressions. The returned type is the one that is eventually assigned to the
+    /// `target`.
+    fn infer_target<F>(&mut self, target: &ast::Expr, value: &ast::Expr, infer_value_expr: F)
     where
-        F: Fn(&'db dyn Db, Type<'db>) -> Type<'db>,
+        F: Fn(&mut TypeInferenceBuilder<'db>, &ast::Expr) -> Type<'db>,
     {
         let assigned_ty = match target {
             ast::Expr::Name(_) => None,
-            _ => {
-                let value_ty = self.infer_standalone_expression(value);
-
-                Some(to_assigned_ty(self.db(), value_ty))
-            }
+            _ => Some(infer_value_expr(self, value)),
         };
         self.infer_target_impl(target, assigned_ty);
     }
@@ -3126,11 +3135,13 @@ impl<'db> TypeInferenceBuilder<'db> {
             is_async: _,
         } = for_statement;
 
-        self.infer_target(target, iter, |db, iter_ty| {
+        self.infer_target(target, iter, |builder, iter_expr| {
             // TODO: `infer_for_statement_definition` reports a diagnostic if `iter_ty` isn't iterable
             //  but only if the target is a name. We should report a diagnostic here if the target isn't a name:
             //  `for a.x in not_iterable: ...
-            iter_ty.iterate(db)
+            builder
+                .infer_standalone_expression(iter_expr)
+                .iterate(builder.db())
         });
 
         self.infer_body(body);
@@ -3959,15 +3970,17 @@ impl<'db> TypeInferenceBuilder<'db> {
             is_async: _,
         } = comprehension;
 
-        if !is_first {
-            self.infer_standalone_expression(iter);
-        }
-        // TODO more complex assignment targets
-        if let ast::Expr::Name(name) = target {
-            self.infer_definition(name);
-        } else {
-            self.infer_expression(target);
-        }
+        self.infer_target(target, iter, |builder, iter_expr| {
+            // TODO: `infer_comprehension_definition` reports a diagnostic if `iter_ty` isn't iterable
+            //  but only if the target is a name. We should report a diagnostic here if the target isn't a name:
+            //  `[... for a.x in not_iterable]
+            if is_first {
+                infer_same_file_expression_type(builder.db(), builder.index.expression(iter_expr))
+            } else {
+                builder.infer_standalone_expression(iter_expr)
+            }
+            .iterate(builder.db())
+        });
         for expr in ifs {
             self.infer_expression(expr);
         }
@@ -3975,12 +3988,12 @@ impl<'db> TypeInferenceBuilder<'db> {
 
     fn infer_comprehension_definition(
         &mut self,
-        iterable: &ast::Expr,
-        target: &ast::ExprName,
-        is_first: bool,
-        is_async: bool,
+        comprehension: &ComprehensionDefinitionKind<'db>,
         definition: Definition<'db>,
     ) {
+        let iterable = comprehension.iterable();
+        let target = comprehension.target();
+
         let expression = self.index.expression(iterable);
         let result = infer_expression_types(self.db(), expression);
 
@@ -3990,7 +4003,7 @@ impl<'db> TypeInferenceBuilder<'db> {
         // (2) We must *not* call `self.extend()` on the result of the type inference,
         //     because `ScopedExpressionId`s are only meaningful within their own scope, so
         //     we'd add types for random wrong expressions in the current scope
-        let iterable_type = if is_first {
+        let iterable_type = if comprehension.is_first() {
             let lookup_scope = self
                 .index
                 .parent_scope_id(self.scope().file_scope_id(self.db()))
@@ -4002,14 +4015,26 @@ impl<'db> TypeInferenceBuilder<'db> {
             result.expression_type(iterable.scoped_expression_id(self.db(), self.scope()))
         };
 
-        let target_type = if is_async {
+        let target_type = if comprehension.is_async() {
             // TODO: async iterables/iterators! -- Alex
             todo_type!("async iterables/iterators")
         } else {
-            iterable_type.try_iterate(self.db()).unwrap_or_else(|err| {
-                err.report_diagnostic(&self.context, iterable_type, iterable.into());
-                err.fallback_element_type(self.db())
-            })
+            match comprehension.target_kind() {
+                TargetKind::Sequence(unpack_position, unpack) => {
+                    let unpacked = infer_unpack_types(self.db(), unpack);
+                    if unpack_position == UnpackPosition::First {
+                        self.context.extend(unpacked.diagnostics());
+                    }
+                    let target_ast_id = target.scoped_expression_id(self.db(), self.scope());
+                    unpacked.expression_type(target_ast_id)
+                }
+                TargetKind::NameOrAttribute => {
+                    iterable_type.try_iterate(self.db()).unwrap_or_else(|err| {
+                        err.report_diagnostic(&self.context, iterable_type, iterable.into());
+                        err.fallback_element_type(self.db())
+                    })
+                }
+            }
         };
 
         self.types.expressions.insert(

crates/red_knot_python_semantic/src/types/unpacker.rs

@@ -18,16 +18,22 @@ use super::{TupleType, UnionType};
 /// Unpacks the value expression type to their respective targets.
 pub(crate) struct Unpacker<'db> {
     context: InferContext<'db>,
-    scope: ScopeId<'db>,
+    target_scope: ScopeId<'db>,
+    value_scope: ScopeId<'db>,
     targets: FxHashMap<ScopedExpressionId, Type<'db>>,
 }
 
 impl<'db> Unpacker<'db> {
-    pub(crate) fn new(db: &'db dyn Db, scope: ScopeId<'db>) -> Self {
+    pub(crate) fn new(
+        db: &'db dyn Db,
+        target_scope: ScopeId<'db>,
+        value_scope: ScopeId<'db>,
+    ) -> Self {
         Self {
-            context: InferContext::new(db, scope),
+            context: InferContext::new(db, target_scope),
             targets: FxHashMap::default(),
-            scope,
+            target_scope,
+            value_scope,
         }
     }
 
@@ -43,7 +49,7 @@ impl<'db> Unpacker<'db> {
         );
 
         let value_type = infer_expression_types(self.db(), value.expression())
-            .expression_type(value.scoped_expression_id(self.db(), self.scope));
+            .expression_type(value.scoped_expression_id(self.db(), self.value_scope));
 
         let value_type = match value.kind() {
             UnpackKind::Assign => {
@@ -79,8 +85,10 @@ impl<'db> Unpacker<'db> {
     ) {
         match target {
             ast::Expr::Name(_) | ast::Expr::Attribute(_) => {
-                self.targets
-                    .insert(target.scoped_expression_id(self.db(), self.scope), value_ty);
+                self.targets.insert(
+                    target.scoped_expression_id(self.db(), self.target_scope),
+                    value_ty,
+                );
             }
             ast::Expr::Starred(ast::ExprStarred { value, .. }) => {
                 self.unpack_inner(value, value_expr, value_ty);

crates/red_knot_python_semantic/src/unpack.rs

@@ -30,7 +30,9 @@ use crate::Db;
 pub(crate) struct Unpack<'db> {
     pub(crate) file: File,
 
-    pub(crate) file_scope: FileScopeId,
+    pub(crate) value_file_scope: FileScopeId,
+
+    pub(crate) target_file_scope: FileScopeId,
 
     /// The target expression that is being unpacked. For example, in `(a, b) = (1, 2)`, the target
     /// expression is `(a, b)`.
@@ -47,9 +49,19 @@ pub(crate) struct Unpack<'db> {
 }
 
 impl<'db> Unpack<'db> {
-    /// Returns the scope where the unpacking is happening.
-    pub(crate) fn scope(self, db: &'db dyn Db) -> ScopeId<'db> {
-        self.file_scope(db).to_scope_id(db, self.file(db))
+    /// Returns the scope in which the unpack value expression belongs.
+    ///
+    /// The scope in which the target and value expression belongs to are usually the same
+    /// except in generator expressions and comprehensions (list/dict/set), where the value
+    /// expression of the first generator is evaluated in the outer scope, while the ones in the subsequent
+    /// generators are evaluated in the comprehension scope.
+    pub(crate) fn value_scope(self, db: &'db dyn Db) -> ScopeId<'db> {
+        self.value_file_scope(db).to_scope_id(db, self.file(db))
+    }
+
+    /// Returns the scope where the unpack target expression belongs to.
+    pub(crate) fn target_scope(self, db: &'db dyn Db) -> ScopeId<'db> {
+        self.target_file_scope(db).to_scope_id(db, self.file(db))
     }
 
     /// Returns the range of the unpack target expression.

crates/ruff_linter/resources/test/fixtures/perflint/PERF401.py

@@ -260,3 +260,9 @@ def f():
     for i in range(5):
         if j := i:
             items.append(j)
+
+def f():
+    values = [1, 2, 3]
+    result = list()  # this should be replaced with a comprehension
+    for i in values:
+        result.append(i + 1)  # PERF401

crates/ruff_linter/src/rules/perflint/rules/manual_list_comprehension.rs

@@ -270,6 +270,15 @@ pub(crate) fn manual_list_comprehension(checker: &Checker, for_stmt: &ast::StmtF
         list_binding_value.is_some_and(|binding_value| match binding_value {
             // `value = []`
             Expr::List(list_expr) => list_expr.is_empty(),
+            // `value = list()`
+            // This might be linted against, but turning it into a list comprehension will also remove it
+            Expr::Call(call) => {
+                checker
+                    .semantic()
+                    .resolve_builtin_symbol(&call.func)
+                    .is_some_and(|name| name == "list")
+                    && call.arguments.is_empty()
+            }
             _ => false,
         });
 

crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__PERF401_PERF401.py.snap

@@ -208,5 +208,16 @@ PERF401.py:262:13: PERF401 Use a list comprehension to create a transformed list
 261 |         if j := i:
 262 |             items.append(j)
     |             ^^^^^^^^^^^^^^^ PERF401
+263 |
+264 | def f():
+    |
+    = help: Replace for loop with list comprehension
+
+PERF401.py:268:9: PERF401 Use a list comprehension to create a transformed list
+    |
+266 |     result = list()  # this should be replaced with a comprehension
+267 |     for i in values:
+268 |         result.append(i + 1)  # PERF401
+    |         ^^^^^^^^^^^^^^^^^^^^ PERF401
     |
     = help: Replace for loop with list comprehension

crates/ruff_linter/src/rules/perflint/snapshots/ruff_linter__rules__perflint__tests__preview__PERF401_PERF401.py.snap

@@ -492,6 +492,8 @@ PERF401.py:262:13: PERF401 [*] Use a list comprehension to create a transformed
 261 |         if j := i:
 262 |             items.append(j)
     |             ^^^^^^^^^^^^^^^ PERF401
+263 |
+264 | def f():
     |
     = help: Replace for loop with list comprehension
 
@@ -505,3 +507,25 @@ PERF401.py:262:13: PERF401 [*] Use a list comprehension to create a transformed
 261     |-        if j := i:
 262     |-            items.append(j)
     259 |+    items = [j for i in range(5) if (j := i)]
+263 260 | 
+264 261 | def f():
+265 262 |     values = [1, 2, 3]
+
+PERF401.py:268:9: PERF401 [*] Use a list comprehension to create a transformed list
+    |
+266 |     result = list()  # this should be replaced with a comprehension
+267 |     for i in values:
+268 |         result.append(i + 1)  # PERF401
+    |         ^^^^^^^^^^^^^^^^^^^^ PERF401
+    |
+    = help: Replace for loop with list comprehension
+
+ℹ Unsafe fix
+263 263 | 
+264 264 | def f():
+265 265 |     values = [1, 2, 3]
+266     |-    result = list()  # this should be replaced with a comprehension
+267     |-    for i in values:
+268     |-        result.append(i + 1)  # PERF401
+    266 |+    # this should be replaced with a comprehension
+    267 |+    result = [i + 1 for i in values]  # PERF401

crates/ruff_linter/src/rules/pyupgrade/rules/format_literals.rs

@@ -39,6 +39,10 @@ use crate::Locator;
 /// "{}, {}".format("Hello", "World")  # "Hello, World"
 /// ```
 ///
+/// This fix is marked as unsafe because:
+/// - Comments attached to arguments are not moved, which can cause comments to mismatch the actual arguments.
+/// - If arguments have side effects (e.g., print), reordering may change program behavior.
+///
 /// ## References
 /// - [Python documentation: Format String Syntax](https://docs.python.org/3/library/string.html#format-string-syntax)
 /// - [Python documentation: `str.format`](https://docs.python.org/3/library/stdtypes.html#str.format)

docs/requirements-insiders.txt

@@ -1,5 +1,5 @@
 PyYAML==6.0.2
-ruff==0.9.10
+ruff==0.11.6
 mkdocs==1.6.1
 mkdocs-material @ git+ssh://git@github.com/astral-sh/mkdocs-material-insiders.git@39da7a5e761410349e9a1b8abf593b0cdd5453ff
 mkdocs-redirects==1.2.2

docs/requirements.txt

@@ -1,5 +1,5 @@
 PyYAML==6.0.2
-ruff==0.9.10
+ruff==0.11.6
 mkdocs==1.6.1
 mkdocs-material==9.5.38
 mkdocs-redirects==1.2.2

knot.schema.json

@@ -460,6 +460,16 @@
             }
           ]
         },
+        "invalid-protocol": {
+          "title": "detects invalid protocol class definitions",
+          "description": "## What it does\nChecks for invalidly defined protocol classes.\n\n## Why is this bad?\nAn invalidly defined protocol class may lead to the type checker inferring\nunexpected things. It may also lead to `TypeError`s at runtime.\n\n## Examples\nA `Protocol` class cannot inherit from a non-`Protocol` class;\nthis raises a `TypeError` at runtime:\n\n```pycon\n>>> from typing import Protocol\n>>> class Foo(int, Protocol): ...\n...\nTraceback (most recent call last):\n  File \"<python-input-1>\", line 1, in <module>\n    class Foo(int, Protocol): ...\nTypeError: Protocols can only inherit from other protocols, got <class 'int'>\n```",
+          "default": "error",
+          "oneOf": [
+            {
+              "$ref": "#/definitions/Level"
+            }
+          ]
+        },
         "invalid-raise": {
           "title": "detects `raise` statements that raise invalid exceptions or use invalid causes",
           "description": "Checks for `raise` statements that raise non-exceptions or use invalid\ncauses for their raised exceptions.\n\n## Why is this bad?\nOnly subclasses or instances of `BaseException` can be raised.\nFor an exception's cause, the same rules apply, except that `None` is also\npermitted. Violating these rules results in a `TypeError` at runtime.\n\n## Examples\n```python\ndef f():\n    try:\n        something()\n    except NameError:\n        raise \"oops!\" from f\n\ndef g():\n    raise NotImplemented from 42\n```\n\nUse instead:\n```python\ndef f():\n    try:\n        something()\n    except NameError as e:\n        raise RuntimeError(\"oops!\") from e\n\ndef g():\n    raise NotImplementedError from None\n```\n\n## References\n- [Python documentation: The `raise` statement](https://docs.python.org/3/reference/simple_stmts.html#raise)\n- [Python documentation: Built-in Exceptions](https://docs.python.org/3/library/exceptions.html#built-in-exceptions)",