Isolate, reuse PackageFinder best candidate logic

Split out how PackageFinder finds the best candidate, and reuse it in the
self version check, to avoid the latter duplicating (and incorrectly
implementing) the same logic.

news/5175.bugfix

@@ -0,0 +1 @@
+Make pip's self version check avoid recommending upgrades to prereleases if the currently-installed version is stable.

src/pip/_internal/index.py

@@ -55,7 +55,8 @@
     BuildTag = Tuple[Any, ...]  # either empty tuple or Tuple[int, str]
     CandidateSortingKey = Tuple[int, _BaseVersion, BuildTag, Optional[int]]
 
-__all__ = ['FormatControl', 'PackageFinder']
+
+__all__ = ['FormatControl', 'FoundCandidates', 'PackageFinder']
 
 
 SECURE_ORIGINS = [
@@ -254,6 +255,63 @@ def _get_html_page(link, session=None):
     return None
 
 
+class FoundCandidates(object):
+    """A collection of candidates, returned by `PackageFinder.find_candidates`.
+
+    Arguments:
+
+    * `candidates`: A sequence of all available candidates found.
+    * `specifier`: Specifier to filter applicable versions.
+    * `prereleases`: Whether prereleases should be accounted. Pass None to
+        infer from the specifier.
+    * `sort_key`: A callable used as the key function when choosing the best
+        candidate.
+    """
+    def __init__(
+        self,
+        candidates,     # type: List[InstallationCandidate]
+        specifier,      # type: specifiers.BaseSpecifier
+        prereleases,    # type: Optional[bool]
+        sort_key,       # type: Callable[[InstallationCandidate], Any]
+    ):
+        # type: (...) -> None
+        self._candidates = candidates
+        self._specifier = specifier
+        self._prereleases = prereleases
+        self._sort_key = sort_key
+
+    @property
+    def all(self):
+        # type: () -> Iterable[InstallationCandidate]
+        return iter(self._candidates)
+
+    @property
+    def applicable(self):
+        # type: () -> Iterable[InstallationCandidate]
+        # Filter out anything which doesn't match our specifier.
+        versions = set(self._specifier.filter(
+            # We turn the version object into a str here because otherwise
+            # when we're debundled but setuptools isn't, Python will see
+            # packaging.version.Version and
+            # pkg_resources._vendor.packaging.version.Version as different
+            # types. This way we'll use a str as a common data interchange
+            # format. If we stop using the pkg_resources provided specifier
+            # and start using our own, we can drop the cast to str().
+            [str(c.version) for c in self._candidates],
+            prereleases=self._prereleases,
+        ))
+        # Again, converting to str to deal with debundling.
+        return (c for c in self._candidates if str(c.version) in versions)
+
+    @property
+    def best(self):
+        # type: () -> Optional[InstallationCandidate]
+        try:
+            return max(self.applicable, key=self._sort_key)
+        except ValueError:  # Raised by max() if self.applicable is empty.
+            return None
+
+
 class PackageFinder(object):
     """This finds packages.
 
@@ -628,6 +686,27 @@ def find_all_candidates(self, project_name):
         # This is an intentional priority ordering
         return file_versions + find_links_versions + page_versions
 
+    def find_candidates(self, project_name, specifier):
+        """Find matches for the given project and specifier.
+
+        `specifier` should implement `filter` to allow version filtering (e.g.
+        ``packaging.specifiers.SpecifierSet``).
+
+        Returns a FoundCandidates instance that contains the following
+        properties:
+
+        * `all`: All candidates matching `project_name` found on the index.
+        * `applicable`: Candidates matching `project_name` and `specifier`.
+        * `best`: The best candidate available. This may be None if no
+            applicable candidates are found.
+        """
+        return FoundCandidates(
+            self.find_all_candidates(project_name),
+            specifier=specifier,
+            prereleases=(self.allow_all_prereleases or None),
+            sort_key=self._candidate_sort_key,
+        )
+
     def find_requirement(self, req, upgrade):
         # type: (InstallRequirement, bool) -> Optional[Link]
         """Try to find a Link matching req
@@ -636,35 +715,8 @@ def find_requirement(self, req, upgrade):
         Returns a Link if found,
         Raises DistributionNotFound or BestVersionAlreadyInstalled otherwise
         """
-        all_candidates = self.find_all_candidates(req.name)
-
-        # Filter out anything which doesn't match our specifier
-        compatible_versions = set(
-            req.specifier.filter(
-                # We turn the version object into a str here because otherwise
-                # when we're debundled but setuptools isn't, Python will see
-                # packaging.version.Version and
-                # pkg_resources._vendor.packaging.version.Version as different
-                # types. This way we'll use a str as a common data interchange
-                # format. If we stop using the pkg_resources provided specifier
-                # and start using our own, we can drop the cast to str().
-                [str(c.version) for c in all_candidates],
-                prereleases=(
-                    self.allow_all_prereleases
-                    if self.allow_all_prereleases else None
-                ),
-            )
-        )
-        applicable_candidates = [
-            # Again, converting to str to deal with debundling.
-            c for c in all_candidates if str(c.version) in compatible_versions
-        ]
-
-        if applicable_candidates:
-            best_candidate = max(applicable_candidates,
-                                 key=self._candidate_sort_key)
-        else:
-            best_candidate = None
+        candidates = self.find_candidates(req.name, req.specifier)
+        best_candidate = candidates.best
 
         if req.satisfied_by is not None:
             installed_version = parse_version(req.satisfied_by.version)
@@ -678,7 +730,7 @@ def find_requirement(self, req, upgrade):
                 req,
                 ', '.join(
                     sorted(
-                        {str(c.version) for c in all_candidates},
+                        {str(c.version) for c in candidates.all},
                         key=parse_version,
                     )
                 )
@@ -710,21 +762,24 @@ def find_requirement(self, req, upgrade):
                 )
             return None
 
+        compatible_version_display = ", ".join(
+            str(v) for v in sorted({c.version for c in candidates.applicable})
+        ) or "none"
+
         if best_installed:
             # We have an existing version, and its the best version
             logger.debug(
                 'Installed version (%s) is most up-to-date (past versions: '
                 '%s)',
                 installed_version,
-                ', '.join(sorted(compatible_versions, key=parse_version)) or
-                "none",
+                compatible_version_display,
             )
             raise BestVersionAlreadyInstalled
 
         logger.debug(
             'Using version %s (newest of versions: %s)',
             best_candidate.version,
-            ', '.join(sorted(compatible_versions, key=parse_version))
+            compatible_version_display,
         )
         return best_candidate.location
 

src/pip/_internal/utils/outdated.py

@@ -8,6 +8,7 @@
 
 from pip._vendor import lockfile, pkg_resources
 from pip._vendor.packaging import version as packaging_version
+from pip._vendor.packaging.specifiers import SpecifierSet
 
 from pip._internal.index import PackageFinder
 from pip._internal.utils.compat import WINDOWS
@@ -129,12 +130,10 @@ def pip_version_check(session, options):
                 trusted_hosts=options.trusted_hosts,
                 session=session,
             )
-            all_candidates = finder.find_all_candidates("pip")
-            if not all_candidates:
+            candidate = finder.find_candidates("pip", SpecifierSet()).best
+            if candidate is None:
                 return
-            pypi_version = str(
-                max(all_candidates, key=lambda c: c.version).version
-            )
+            pypi_version = str(candidate.version)
 
             # save that we've performed a check
             state.save(pypi_version, current_time)

tests/unit/test_unit_outdated.py

@@ -1,3 +1,4 @@
+import collections
 import datetime
 import os
 import sys
@@ -28,8 +29,10 @@ class MockPackageFinder(object):
     def __init__(self, *args, **kwargs):
         pass
 
-    def find_all_candidates(self, project_name):
-        return self.INSTALLATION_CANDIDATES
+    def find_candidates(self, project_name, specifier):
+        return collections.namedtuple("FoundCandidates", "best")(
+            best=self.INSTALLATION_CANDIDATES[0],
+        )
 
 
 class MockDistribution(object):