Skip to content

Commit a19ade7

Browse files
authored
Use strict optional checking in req_install.py (#11379)
* Use strict optional checking in req_install.py Suggested by pradyunsg in #11374 Since half of the API of this class depends on self.req not being None, it seems like we should just prevent users from passing None here. However, I wasn't able to make that change. Rather than sprinkle asserts everywhere, I added "checked" properties. I find this less ad hoc and easier to adapt if e.g. we're able to make self.req never None in the future. There are now some code paths where we have asserts that we didn't before. I relied on other type hints in pip's code base to be accurate. If that is not the case and we actually relied on some function being able to accept None when not typed as such, we may hit these asserts. But hopefully tests would catch such a thing. * news * black * inline asserts * code review * fix up merge issue * fix specifier bug
1 parent 3c0147a commit a19ade7

File tree

2 files changed

+20
-13
lines changed

2 files changed

+20
-13
lines changed

news/85F7E260-68FF-4C1E-A2CB-CF8634829D2D.trivial.rst

Whitespace-only changes.

src/pip/_internal/req/req_install.py

+20-13
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,3 @@
1-
# The following comment should be removed at some point in the future.
2-
# mypy: strict-optional=False
3-
41
import functools
52
import logging
63
import os
@@ -244,6 +241,7 @@ def supports_pyproject_editable(self) -> bool:
244241

245242
@property
246243
def specifier(self) -> SpecifierSet:
244+
assert self.req is not None
247245
return self.req.specifier
248246

249247
@property
@@ -257,7 +255,8 @@ def is_pinned(self) -> bool:
257255
258256
For example, some-package==1.2 is pinned; some-package>1.2 is not.
259257
"""
260-
specifiers = self.specifier
258+
assert self.req is not None
259+
specifiers = self.req.specifier
261260
return len(specifiers) == 1 and next(iter(specifiers)).operator in {"==", "==="}
262261

263262
def match_markers(self, extras_requested: Optional[Iterable[str]] = None) -> bool:
@@ -305,6 +304,7 @@ def hashes(self, trust_internet: bool = True) -> Hashes:
305304
else:
306305
link = None
307306
if link and link.hash:
307+
assert link.hash_name is not None
308308
good_hashes.setdefault(link.hash_name, []).append(link.hash)
309309
return Hashes(good_hashes)
310310

@@ -314,6 +314,7 @@ def from_path(self) -> Optional[str]:
314314
return None
315315
s = str(self.req)
316316
if self.comes_from:
317+
comes_from: Optional[str]
317318
if isinstance(self.comes_from, str):
318319
comes_from = self.comes_from
319320
else:
@@ -345,7 +346,7 @@ def ensure_build_location(
345346

346347
# When parallel builds are enabled, add a UUID to the build directory
347348
# name so multiple builds do not interfere with each other.
348-
dir_name: str = canonicalize_name(self.name)
349+
dir_name: str = canonicalize_name(self.req.name)
349350
if parallel_builds:
350351
dir_name = f"{dir_name}_{uuid.uuid4().hex}"
351352

@@ -388,6 +389,7 @@ def _set_requirement(self) -> None:
388389
)
389390

390391
def warn_on_mismatching_name(self) -> None:
392+
assert self.req is not None
391393
metadata_name = canonicalize_name(self.metadata["Name"])
392394
if canonicalize_name(self.req.name) == metadata_name:
393395
# Everything is fine.
@@ -457,6 +459,7 @@ def is_wheel_from_cache(self) -> bool:
457459
# Things valid for sdists
458460
@property
459461
def unpacked_source_directory(self) -> str:
462+
assert self.source_dir, f"No source dir for {self}"
460463
return os.path.join(
461464
self.source_dir, self.link and self.link.subdirectory_fragment or ""
462465
)
@@ -543,7 +546,7 @@ def prepare_metadata(self) -> None:
543546
Under PEP 517 and PEP 660, call the backend hook to prepare the metadata.
544547
Under legacy processing, call setup.py egg-info.
545548
"""
546-
assert self.source_dir
549+
assert self.source_dir, f"No source dir for {self}"
547550
details = self.name or f"from {self.link}"
548551

549552
if self.use_pep517:
@@ -592,18 +595,20 @@ def get_dist(self) -> BaseDistribution:
592595
if self.metadata_directory:
593596
return get_directory_distribution(self.metadata_directory)
594597
elif self.local_file_path and self.is_wheel:
598+
assert self.req is not None
595599
return get_wheel_distribution(
596-
FilesystemWheel(self.local_file_path), canonicalize_name(self.name)
600+
FilesystemWheel(self.local_file_path),
601+
canonicalize_name(self.req.name),
597602
)
598603
raise AssertionError(
599604
f"InstallRequirement {self} has no metadata directory and no wheel: "
600605
f"can't make a distribution."
601606
)
602607

603608
def assert_source_matches_version(self) -> None:
604-
assert self.source_dir
609+
assert self.source_dir, f"No source dir for {self}"
605610
version = self.metadata["version"]
606-
if self.req.specifier and version not in self.req.specifier:
611+
if self.req and self.req.specifier and version not in self.req.specifier:
607612
logger.warning(
608613
"Requested %s, but installing version %s",
609614
self,
@@ -696,9 +701,10 @@ def _clean_zip_name(name: str, prefix: str) -> str:
696701
name = name.replace(os.path.sep, "/")
697702
return name
698703

704+
assert self.req is not None
699705
path = os.path.join(parentdir, path)
700706
name = _clean_zip_name(path, rootdir)
701-
return self.name + "/" + name
707+
return self.req.name + "/" + name
702708

703709
def archive(self, build_dir: Optional[str]) -> None:
704710
"""Saves archive to provided build_dir.
@@ -777,8 +783,9 @@ def install(
777783
use_user_site: bool = False,
778784
pycompile: bool = True,
779785
) -> None:
786+
assert self.req is not None
780787
scheme = get_scheme(
781-
self.name,
788+
self.req.name,
782789
user=use_user_site,
783790
home=home,
784791
root=root,
@@ -792,7 +799,7 @@ def install(
792799
prefix=prefix,
793800
home=home,
794801
use_user_site=use_user_site,
795-
name=self.name,
802+
name=self.req.name,
796803
setup_py_path=self.setup_py_path,
797804
isolated=self.isolated,
798805
build_env=self.build_env,
@@ -805,7 +812,7 @@ def install(
805812
assert self.local_file_path
806813

807814
install_wheel(
808-
self.name,
815+
self.req.name,
809816
self.local_file_path,
810817
scheme=scheme,
811818
req_description=str(self.req),

0 commit comments

Comments
 (0)