It is interesting that we're paranoid about the dataclass not having its module imported on only this branch and not the branch above. Not suggesting a change, but a comment could be useful if you know why we only care in the else branch and not the if not module_name branch.

I was following the checks that were already present in the function. In the else branch, there was a check for if module:

module = sys.modules.get(cls.__module__)
if module and module.__dict__.get(module_name) is a_module:
      ns = sys.modules.get(a_type.__module__).__dict__

I’d actually consider removing that check — it does seem unnecessary. At least, I can’t think of a case where cls.__module__ would be missing from sys.modules. And the tests are still passing.

Yes, I saw the original code. My suggestion would be to hoist module = sys.modules.get(cls.__module__) out of the if..else. If you want to preserve the rather paranoid sanity check alongside the hoisted assignment, you could. Something like:

module = sys.modules.get(cls.__module__)
if module:  # not sure that we need to be this paranoid 
    ns = module.__dict__
if module_name:
...

InitVar lives in dataclasses, ClassVar lives in typing. Using an example of dataclasses.ClassVar is likely to cause confusion.

There is an edge case that is not handled by this: foo.typing_filtered.ClassVar. The regex only accounts for one module level. While it is probably uncommon to import foo.typing_filtered without aliasing it, it is possible.

I don't want to change the world for a simple bug fix, but it seems like partitioning on . is more robust (and probably more performant) than using the current regex.

While I agree that splitting on a dot would be faster, there are existing tests that cover cases like dataclasses.InitVar.[int] and dataclasses.InitVar+. If we want to support multiple module levels, we should extend the current regex pattern rather than replace it.

I think it's worth creating a new issue to track more levels of module nesting. And I think it's a mistake to test for dataclasses.InitVar.[int] and other invalid strings.

This bug was actually caught by __.typx.ClassVar in the real world. I typically stuff all of my common imports, including typing_extensions as typx, into an internals subpackage (__) so that I can do from . import __. Reduces module namespace pollution and lets me avoid setting __all__ to define module interfaces. But, I understand that my practice may not be common.

That said, any fix to multi-level traversal here or another PR is going to affect the code that is actually being fixed. Imo, it would make more sense to fix both together holistically (to save developer effort), if there is any interest in actually fixing the multi-level traversal.

I think this could be replaced with:

return (
    isinstance(a_type_module, types.ModuleType)
    and is_type_predicate(a_type_module.__dict__.get(type_name), a_module))

It looks like we can indeed replace it. But if we go with this version, the a_type parameter will no longer be used in the function body.

Also, I was thinking — in the original version, this line:

sys.modules.get(a_type.__module__)

could probably be replaced with a_module? Or am I missing something? It seems like a_type might not have been necessary from the start, and we should probably remove it.

If you think the code can be simplified, please do so, then I'll review that version.

I'm just starting to look at this PR, and will have more to say later. For now: I'd prefer this to say something more specific, like <some specific description> of ClassVar not recognized as typing.ClassVar. I'd like this because I'm sure we won't be fixing all instances of using ClassVar as string annotations, and we should mention the exact one being fixed here. Also, I don't want to mention __init__, because that's not the only method affected by this bug.