fix: handle StrawberryUnion in is_implemented_by for generic TypeVar resolution

[bellini666]

Summary

• When a generic type like Collection[A | B] is used in a union (Collection[A | B] | Error), resolving the return type raised UnallowedReturnTypeForUnion
• The is_implemented_by method now handles StrawberryUnion as an expected concrete type by checking if the runtime type is a member of the union
• Added test covering both success and error branches of a generic with union TypeVar in an outer union

Fixes #4288

Summary by Sourcery

Handle generic fields whose type variables are unions when used inside outer unions, ensuring correct runtime resolution for union return types.

Bug Fixes:

• Fix resolution of generic return types where a TypeVar is bound to a union and the generic itself is wrapped in an outer union, avoiding UnallowedReturnTypeForUnion errors.

Tests:

• Add coverage for generic fields with union type variables inside outer unions, verifying both successful and error resolution paths.

[sourcery-ai]

Reviewer's Guide

Extends is_implemented_by to treat StrawberryUnion as a valid concrete type when resolving generic TypeVars inside outer unions, and adds a regression test covering both success and error resolution paths for such schemas.

File-Level Changes

Change	Details	Files
Teach generic type resolution to accept StrawberryUnion as a concrete implementation when resolving TypeVars inside unions.	Import StrawberryUnion inside is_implemented_by to avoid circular imports while enabling union-specific checks. Add a branch that, when the expected concrete type is a StrawberryUnion, returns True if the runtime concrete type is one of the union member types. Keep the existing identity check as a fallback when the expected type is not a StrawberryUnion.	strawberry/types/base.py
Add regression test for a generic collection whose TypeVar is a union, used inside an outer union with an error type.	Define simple GraphQL types Animal, Plant, Error and a generic Collection[T] type. Add Query fields returning Collection[Animal	Plant]

Assessment against linked issues

Issue	Objective	Addressed	Explanation
#4288	Allow a generic type whose TypeVar is a union (e.g. Collection[A	B]) to be used inside an outer union (e.g. Collection[A	B]
#4288	Add automated test coverage for using a generic type with a union TypeVar inside an outer union, covering both the generic branch and the error branch.	✅

Possibly linked issues

• UnallowedReturnTypeForUnion on generic list union with another type #4288: PR updates is_implemented_by to recognize StrawberryUnion members, resolving the UnallowedReturnTypeForUnion reported for Collection[A|B]|Error.
• #0: PR updates is_implemented_by’s generic concrete-type resolution for StrawberryUnion, matching the issue’s request about generics with unions.

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've left some high level feedback:

• The runtime import of StrawberryUnion inside is_implemented_by may introduce hidden circular dependencies; consider either moving this import to the module level or adding a brief comment explaining why it must stay local.
• Instead of directly accessing expected_concrete_type.types and using the in operator, consider delegating union membership checks to a dedicated helper or method on StrawberryUnion (if available) so the implementation detail of how members are stored remains encapsulated.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The runtime import of `StrawberryUnion` inside `is_implemented_by` may introduce hidden circular dependencies; consider either moving this import to the module level or adding a brief comment explaining why it must stay local.
- Instead of directly accessing `expected_concrete_type.types` and using the `in` operator, consider delegating union membership checks to a dedicated helper or method on `StrawberryUnion` (if available) so the implementation detail of how members are stored remains encapsulated.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[botberry]

Thanks for adding the RELEASE.md file!

Here's a preview of the changelog:

---

Fix UnallowedReturnTypeForUnion when using a generic type with a union
TypeVar (e.g. Collection[A | B]) inside an outer union
(Collection[A | B] | Error).

Here's the tweet text:

🆕 Release (next) is out! Thanks to @_bellini666 for the PR 👏

Get it here 👉 https://strawberry.rocks/release/(next)

[greptile-apps]

Greptile Summary

This PR fixes a regression where returning a generic type parameterised with a union TypeVar (e.g. Collection[Animal | Plant]) inside an outer union raised UnallowedReturnTypeForUnion because is_implemented_by did not know how to compare a concrete Python class against a StrawberryUnion expected type.

The fix adds a single, well-placed guard in StrawberryObjectDefinition.is_implemented_by (strawberry/types/base.py) that checks whether expected_concrete_type is a StrawberryUnion and, if so, tests membership of real_concrete_type in union.types instead of falling through to the strict identity check. The change is minimal, follows the existing in-function import pattern, and is covered by a new end-to-end test.

Key changes:

• strawberry/types/base.py: Three-line guard added between the issubclass check and the identity-equality check in is_implemented_by; uses a deferred from strawberry.types.union import StrawberryUnion to avoid circular imports, consistent with the adjacent StrawberryEnumDefinition import.
• tests/schema/test_generics.py: New test test_generic_with_union_typevar_in_outer_union validates both the Collection (success) and Error (fallthrough) resolution paths for the fixed scenario.

Confidence Score: 4/5

• PR is safe to merge; the fix is narrowly scoped to a previously broken code path and cannot regress existing behaviour for non-StrawberryUnion types.
• The change only affects the is_implemented_by path when expected_concrete_type is a StrawberryUnion, a case that previously always short-circuited to False, so existing tests cannot be broken by it. The new test covers both resolution outcomes. Minor gap: the membership check (real_concrete_type in expected_concrete_type.types) relies on object identity/equality of resolved annotation types; this works correctly for @strawberry.type classes and enums due to caching, but the interaction with LazyType members inside the union is not explicitly tested.
• No files require special attention; the two changed files are small and well-contained.

Important Files Changed

Filename	Overview
strawberry/types/base.py	Adds a targeted check in is_implemented_by to handle StrawberryUnion as an expected_concrete_type, correctly returning True when real_concrete_type is a member of the union; the in-function import follows the existing enum pattern and the placement before the identity check is correct.
tests/schema/test_generics.py	New test test_generic_with_union_typevar_in_outer_union covers both success (Collection returned) and error-path (Error returned) branches, providing good coverage for the fix; a minor gap is that only Animal appears as the first list element, so the Plant-first ordering is not explicitly exercised.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["is_implemented_by(root) called"] --> B{type_definition is self?}
    B -- Yes --> C[return True]
    B -- No --> D{type_definition is concrete_of?}
    D -- No --> E[return False]
    D -- Yes --> F[Iterate fields with TypeVar]
    F --> G[Get value from root, unwrap StrawberryList]
    G --> H[Get expected_concrete_type from type_var_map]
    H --> I{isinstance expected_concrete_type, type AND issubclass?}
    I -- Yes --> C
    I -- No --> J{isinstance expected_concrete_type, StrawberryUnion?}
    J -- Yes --> K{real_concrete_type in union.types?}
    K -- Yes --> C
    K -- No --> L{real_concrete_type is not expected_concrete_type?}
    J -- No --> L
    L -- True --> E
    L -- False --> F
    F -- All fields matched --> C

Loading

_{Last reviewed commit: b6a4e6d}

[Copilot]

Pull request overview

Fixes union type resolution for generics where a TypeVar resolves to a union (e.g. Collection[A | B]) and that generic is then used inside an outer union, avoiding UnallowedReturnTypeForUnion during schema execution.

Changes:

• Extend StrawberryObjectDefinition.is_implemented_by to treat StrawberryUnion as a valid expected concrete type by checking runtime member types.
• Add a regression test for Collection[Animal | Plant] | Error covering both success and error branches.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
strawberry/types/base.py	Updates generic type matching to accept StrawberryUnion as an expected concrete type during is_implemented_by checks.
tests/schema/test_generics.py	Adds regression test reproducing the reported error scenario with a union TypeVar inside an outer union.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[codspeed-hq]

Merging this PR will not alter performance

✅ 31 untouched benchmarks

---

_{Comparing worktree-fix-unallowed-return-type-for-union (fc62bf0) with main (35a6310)}