Additional typechecking fixes (#4755)

* new FIXME: are nameless repositories desired/expected?

* new TODO: should the LegacyRepository really inherit from the PyPiRepository?

mypy.ini

@@ -13,7 +13,7 @@ warn_unused_configs = True
 # should be added to this whitelist.
 # see https://github.com/python-poetry/poetry/pull/4510.
 
-[mypy-poetry.config.*]
+[mypy-poetry.config.file_config_source]
 ignore_errors = True
 
 [mypy-poetry.console.*]
@@ -34,16 +34,13 @@ ignore_errors = True
 [mypy-poetry.mixology.*]
 ignore_errors = True
 
-[mypy-poetry.packages.*]
-ignore_errors = True
-
-[mypy-poetry.publishing.*]
+[mypy-poetry.packages.locker]
 ignore_errors = True
 
 [mypy-poetry.puzzle.*]
 ignore_errors = True
 
-[mypy-poetry.repositories.*]
+[mypy-poetry.repositories.installed_repository]
 ignore_errors = True
 
 [mypy-poetry.utils.*]

poetry/config/config.py

@@ -14,9 +14,6 @@ from .config_source import ConfigSource
 from .dict_config_source import DictConfigSource
 
 
-_NOT_SET = object()
-
-
 def boolean_validator(val: str) -> bool:
     return val in {"true", "false", "1", "0"}
 
@@ -27,7 +24,7 @@ def boolean_normalizer(val: str) -> bool:
 
 class Config:
 
-    default_config = {
+    default_config: Dict[str, Any] = {
         "cache-dir": str(CACHE_DIR),
         "virtualenvs": {
             "create": True,
@@ -45,12 +42,8 @@ class Config:
         self._config = deepcopy(self.default_config)
         self._use_environment = use_environment
         self._base_dir = base_dir
-        self._config_source = DictConfigSource()
-        self._auth_config_source = DictConfigSource()
-
-    @property
-    def name(self) -> str:
-        return str(self._file.path)
+        self._config_source: ConfigSource = DictConfigSource()
+        self._auth_config_source: ConfigSource = DictConfigSource()
 
     @property
     def config(self) -> Dict:
@@ -114,9 +107,9 @@ class Config:
             env = "POETRY_{}".format(
                 "_".join(k.upper().replace("-", "_") for k in keys)
             )
-            value = os.getenv(env, _NOT_SET)
-            if value is not _NOT_SET:
-                return self.process(self._get_normalizer(setting_name)(value))
+            env_value = os.getenv(env)
+            if env_value is not None:
+                return self.process(self._get_normalizer(setting_name)(env_value))
 
         value = self._config
         for key in keys:

poetry/config/dict_config_source.py

@@ -6,7 +6,7 @@ from .config_source import ConfigSource
 
 class DictConfigSource(ConfigSource):
     def __init__(self) -> None:
-        self._config = {}
+        self._config: Dict[str, Any] = {}
 
     @property
     def config(self) -> Dict[str, Any]:

poetry/inspection/info.py

@@ -59,7 +59,7 @@ class PackageInfo:
         platform: Optional[str] = None,
         requires_dist: Optional[List[str]] = None,
         requires_python: Optional[str] = None,
-        files: Optional[List[str]] = None,
+        files: Optional[List[Dict[str, str]]] = None,
         cache_version: Optional[str] = None,
     ):
         self.name = name

poetry/packages/project_package.py

@@ -21,5 +21,3 @@ class ProjectPackage(_ProjectPackage):
         else:
             self._version = version
             self._pretty_version = pretty_version or version.text
-
-        return self

poetry/publishing/publisher.py

@@ -44,7 +44,7 @@ class Publisher:
         password: Optional[str],
         cert: Optional[Path] = None,
         client_cert: Optional[Path] = None,
-        dry_run: Optional[bool] = False,
+        dry_run: bool = False,
     ) -> None:
         if not repository_name:
             url = "https://upload.pypi.org/legacy/"

poetry/publishing/uploader.py

@@ -57,8 +57,8 @@ class Uploader:
         self._poetry = poetry
         self._package = poetry.package
         self._io = io
-        self._username = None
-        self._password = None
+        self._username: Optional[str] = None
+        self._password: Optional[str] = None
 
     @property
     def user_agent(self) -> str:
@@ -89,14 +89,15 @@ class Uploader:
 
         return sorted(wheels + tars)
 
-    def auth(self, username: str, password: str) -> None:
+    def auth(self, username: Optional[str], password: Optional[str]) -> None:
         self._username = username
         self._password = password
 
     def make_session(self) -> requests.Session:
         session = requests.session()
-        if self.is_authenticated():
-            session.auth = (self._username, self._password)
+        auth = self.get_auth()
+        if auth is not None:
+            session.auth = auth
 
         session.headers["User-Agent"] = self.user_agent
         for scheme in ("http://", "https://"):
@@ -104,8 +105,11 @@ class Uploader:
 
         return session
 
-    def is_authenticated(self) -> bool:
-        return self._username is not None and self._password is not None
+    def get_auth(self) -> Optional[Tuple[str, str]]:
+        if self._username is None or self._password is None:
+            return None
+
+        return (self._username, self._password)
 
     def upload(
         self,
@@ -147,16 +151,15 @@ class Uploader:
 
         md5_digest = md5_hash.hexdigest()
         sha2_digest = sha256_hash.hexdigest()
+        blake2_256_digest: Optional[str] = None
         if _has_blake2:
             blake2_256_digest = blake2_256_hash.hexdigest()
-        else:
-            blake2_256_digest = None
 
+        py_version: Optional[str] = None
         if file_type == "bdist_wheel":
             wheel_info = wheel_file_re.match(file.name)
+            if wheel_info is not None:
                 py_version = wheel_info.group("pyver")
-        else:
-            py_version = None
 
         data = {
             # identify release
@@ -231,7 +234,7 @@ class Uploader:
             }
         )
 
-        data_to_send = self._prepare_data(data)
+        data_to_send: List[Tuple[str, Any]] = self._prepare_data(data)
 
         with file.open("rb") as fp:
             data_to_send.append(
@@ -256,7 +259,7 @@ class Uploader:
                         allow_redirects=False,
                         headers={"Content-Type": monitor.content_type},
                     )
-                if dry_run or 200 <= resp.status_code < 300:
+                if resp is None or 200 <= resp.status_code < 300:
                     bar.set_format(
                         f" - Uploading <c1>{file.name}</c1> <fg=green>%percent%%</>"
                     )

poetry/repositories/base_repository.py

@@ -10,22 +10,22 @@ if TYPE_CHECKING:
 
 class BaseRepository:
     def __init__(self) -> None:
-        self._packages = []
+        self._packages: List[Package] = []
 
     @property
     def packages(self) -> List["Package"]:
         return self._packages
 
-    def has_package(self, package: "Package") -> None:
+    def has_package(self, package: "Package") -> bool:
         raise NotImplementedError()
 
     def package(
         self, name: str, version: str, extras: Optional[List[str]] = None
-    ) -> None:
+    ) -> "Package":
         raise NotImplementedError()
 
-    def find_packages(self, dependency: "Dependency") -> None:
+    def find_packages(self, dependency: "Dependency") -> List["Package"]:
         raise NotImplementedError()
 
-    def search(self, query: str) -> None:
+    def search(self, query: str) -> List["Package"]:
         raise NotImplementedError()

poetry/repositories/legacy_repository.py

@@ -5,6 +5,7 @@ import urllib.parse
 import warnings
 
 from collections import defaultdict
+from html import unescape
 from pathlib import Path
 from typing import TYPE_CHECKING
 from typing import Any
@@ -12,6 +13,7 @@ from typing import Dict
 from typing import Iterator
 from typing import List
 from typing import Optional
+from urllib.parse import quote
 
 import requests
 import requests.auth
@@ -43,23 +45,6 @@ from .pypi_repository import PyPiRepository
 if TYPE_CHECKING:
     from poetry.core.packages.dependency import Dependency
 
-try:
-    from html import unescape
-except ImportError:
-    try:
-        from html.parser import HTMLParser
-    except ImportError:
-        from HTMLParser import HTMLParser
-
-    unescape = HTMLParser().unescape
-
-
-try:
-    from urllib.parse import quote
-except ImportError:
-    from urllib import quote
-
-
 with warnings.catch_warnings():
     warnings.simplefilter("ignore")
     import html5lib
@@ -164,6 +149,8 @@ class Page:
         return self._clean_re.sub(lambda match: "%%%2x" % ord(match.group(0)), url)
 
 
+# TODO: revisit whether the LegacyRepository should inherit from PyPiRepository.
+# <https://github.com/python-poetry/poetry/pull/4755#discussion_r748865374>.
 class LegacyRepository(PyPiRepository):
     def __init__(
         self,
@@ -269,7 +256,7 @@ class LegacyRepository(PyPiRepository):
         if self._cache.store("matches").has(key):
             versions = self._cache.store("matches").get(key)
         else:
-            page = self._get("/{}/".format(dependency.name.replace(".", "-")))
+            page = self._get_page("/{}/".format(dependency.name.replace(".", "-")))
             if page is None:
                 return []
 
@@ -338,14 +325,14 @@ class LegacyRepository(PyPiRepository):
             return package
 
     def find_links_for_package(self, package: Package) -> List[Link]:
-        page = self._get("/{}/".format(package.name.replace(".", "-")))
+        page = self._get_page("/{}/".format(package.name.replace(".", "-")))
         if page is None:
             return []
 
         return list(page.links_for_version(package.version))
 
     def _get_release_info(self, name: str, version: str) -> dict:
-        page = self._get("/{}/".format(canonicalize_name(name).replace(".", "-")))
+        page = self._get_page("/{}/".format(canonicalize_name(name).replace(".", "-")))
         if page is None:
             raise PackageNotFound(f'No package named "{name}"')
 
@@ -419,7 +406,7 @@ class LegacyRepository(PyPiRepository):
 
         return data.asdict()
 
-    def _get(self, endpoint: str) -> Optional[Page]:
+    def _get_page(self, endpoint: str) -> Optional[Page]:
         url = self._url + endpoint
         try:
             response = self.session.get(url)

poetry/repositories/pool.py

@@ -22,11 +22,11 @@ class Pool(BaseRepository):
         if repositories is None:
             repositories = []
 
-        self._lookup: Dict[str, int] = {}
+        self._lookup: Dict[Optional[str], int] = {}
         self._repositories: List[Repository] = []
         self._default = False
         self._has_primary_repositories = False
-        self._secondary_start_idx = None
+        self._secondary_start_idx: Optional[int] = None
 
         for repository in repositories:
             self.add_repository(repository)
@@ -65,6 +65,8 @@ class Pool(BaseRepository):
         """
         Adds a repository to the pool.
         """
+        # FIXME: surely it's a problem that the repository name can be None here?
+        # All nameless repositories will collide in self._lookup.
         repository_name = (
             repository.name.lower() if repository.name is not None else None
         )

poetry/repositories/repository.py

@@ -24,7 +24,7 @@ class Repository(BaseRepository):
             self.add_package(package)
 
     @property
-    def name(self) -> str:
+    def name(self) -> Optional[str]:
         return self._name
 
     def package(

tests/repositories/test_legacy_repository.py

@@ -28,7 +28,7 @@ class MockRepository(LegacyRepository):
             "legacy", url="http://legacy.foo.bar", disable_cache=True
         )
 
-    def _get(self, endpoint):
+    def _get_page(self, endpoint):
         parts = endpoint.split("/")
         name = parts[1]
 
@@ -49,7 +49,7 @@ class MockRepository(LegacyRepository):
 def test_page_relative_links_path_are_correct():
     repo = MockRepository()
 
-    page = repo._get("/relative")
+    page = repo._get_page("/relative")
 
     for link in page.links:
         assert link.netloc == "legacy.foo.bar"
@@ -59,7 +59,7 @@ def test_page_relative_links_path_are_correct():
 def test_page_absolute_links_path_are_correct():
     repo = MockRepository()
 
-    page = repo._get("/absolute")
+    page = repo._get_page("/absolute")
 
     for link in page.links:
         assert link.netloc == "files.pythonhosted.org"
@@ -68,7 +68,7 @@ def test_page_absolute_links_path_are_correct():
 
 def test_sdist_format_support():
     repo = MockRepository()
-    page = repo._get("/relative")
+    page = repo._get_page("/relative")
     bz2_links = list(filter(lambda link: link.ext == ".tar.bz2", page.links))
     assert len(bz2_links) == 1
     assert bz2_links[0].filename == "poetry-0.1.1.tar.bz2"
@@ -335,21 +335,21 @@ class MockHttpRepository(LegacyRepository):
 def test_get_200_returns_page(http):
     repo = MockHttpRepository({"/foo": 200}, http)
 
-    assert repo._get("/foo")
+    assert repo._get_page("/foo")
 
 
 @pytest.mark.parametrize("status_code", [401, 403, 404])
 def test_get_40x_and_returns_none(http, status_code):
     repo = MockHttpRepository({"/foo": status_code}, http)
 
-    assert repo._get("/foo") is None
+    assert repo._get_page("/foo") is None
 
 
 def test_get_5xx_raises(http):
     repo = MockHttpRepository({"/foo": 500}, http)
 
     with pytest.raises(RepositoryError):
-        repo._get("/foo")
+        repo._get_page("/foo")
 
 
 def test_get_redirected_response_url(http, monkeypatch):
@@ -363,4 +363,4 @@ def test_get_redirected_response_url(http, monkeypatch):
         return response
 
     monkeypatch.setattr(repo.session, "get", get_mock)
-    assert repo._get("/foo")._url == "http://legacy.redirect.bar/foo/"
+    assert repo._get_page("/foo")._url == "http://legacy.redirect.bar/foo/"