Allow types-foo to depend on foo

[Avasam]

This is relevant setuptools in python/typeshed#13369 (comment) and might be relevant to pywin32 later as well.

Relates to #90

[hauntsaninja]

I don't think type checkers will resolve this correctly.

[Avasam]

I don't think type checkers will resolve this correctly.

Why do you think that? From type-checkers' PoV, setuptools and pkg_resources are two different packages. And all tests in python/typeshed#13369 (where setuptools needs pkg_resources) pass.

Test file:

from setuptools import setup

import pkg_resources
from typing_extensions import reveal_type

reveal_type(pkg_resources.Requirement)

setup()

w/o types-setuptools:

> mypy --strict test.py
test.py:1: error: Library stubs not installed for "setuptools"  [import-untyped]
test.py:1: note: Hint: "python3 -m pip install types-setuptools"
test.py:1: note: (or run "mypy --install-types" to install all missing stub packages)
test.py:1: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
test.py:6: note: Revealed type is "def (requirement_string: builtins.str) -> pkg_resources.Requirement"
Found 1 error in 1 file (checked 1 source file)

with types-setuptools:

> mypy --strict test.py
test.py:6: note: Revealed type is "def (requirement_string: builtins.str) -> pkg_resources.Requirement"
Success: no issues found in 1 source file

with types-setuptools, but manually deleted site-packages/pkg_resources-stubs

> mypy --strict test.py
test.py:6: note: Revealed type is "def (requirement_string: builtins.str) -> pkg_resources.Requirement"
Success: no issues found in 1 source file

[Avasam]

Alternative syntax:

Suggested change

	if (
	runtime_req_name != upstream_distribution # Allow `types-foo` to require `foo`
	and not runtime_in_upstream_requires(req, data)
	and not runtime_in_upstream_sdist_requires(req, data)
	):
	if not (
	runtime_req_name == upstream_distribution # Allow `types-foo` to require `foo`
	or runtime_in_upstream_requires(req, data)
	or runtime_in_upstream_sdist_requires(req, data)
	):

[Avasam]

if you want me to hide this check behind a function for consistency, I'd call it runtime_is_upstream.

[srittau]

I think this is the bug that I alluded to when I wrote the refactoring PR. But wouldn't it be clearer to just check req.name == upstream_distribution at the beginning of the function and exit early? This would also skip fetching the data from PyPI.

[Avasam]

But wouldn't it be clearer to just check req.name == upstream_distribution at the beginning of the function and exit early? This would also skip fetching the data from PyPI.

If we don't mind not validating that upstream_distribution does exist on pypi in this case.
Of course for any existing stubs on typeshed we'd assume that its upstream does exist on pypi.

But let's say someone adds a new stubs, mistyped the distribution's name, and makes it reference on itself. It's quite the edge case, but to be considered.

Hmm, I guess stubtest and maybe other tools as well would fail. So probably fine.

[Avasam]

I think this is the bug that I alluded to when I wrote the refactoring PR. But wouldn't it be clearer to just check req.name == upstream_distribution

I'm not sure which side the the EXTERNAL_RUNTIME_REQ_MAP makes sense here. (and tbh I may be too tired rn to make sense of it ^^") But I was reusing what's in runtime_in_upstream_sdist_requires and runtime_in_upstream_requires

Edit: I think it doesn't really matter because we wouldn't self reference, let's say django-stubs, and this mapping specifically lists stubs outside typeshed. So yeah, simpler to use req.name directly.