[WIP] pylint and related cleanups

src/pip/_internal/cli/autocompletion.py

@@ -110,7 +110,7 @@ def get_path_completion_type(cwords, cword, opts):
     :return: path completion type (``file``, ``dir``, ``path`` or None)
     """
     if cword < 2 or not cwords[cword - 2].startswith('-'):
-        return
+        return None
     for opt in opts:
         if opt.help == optparse.SUPPRESS_HELP:
             continue

src/pip/_internal/cli/parser.py

@@ -85,10 +85,9 @@ def format_description(self, description):
 
     def format_epilog(self, epilog):
         # leave full control over epilog to us
-        if epilog:
-            return epilog
-        else:
+        if not epilog:
             return ''
+        return epilog
 
     def indent_lines(self, text, indent):
         new_lines = [indent + line for line in text.split('\n')]

src/pip/_internal/commands/__init__.py

@@ -64,8 +64,7 @@ def get_similar_commands(name):
 
     if close_commands:
         return close_commands[0]
-    else:
-        return False
+    return False
 
 
 def _sort_commands(cmddict, order):

src/pip/_internal/commands/check.py

@@ -1,6 +1,7 @@
 import logging
 
 from pip._internal.cli.base_command import Command
+from pip._internal.cli.status_codes import ERROR, SUCCESS
 from pip._internal.operations.check import (
     check_package_set, create_package_set_from_installed,
 )
@@ -36,6 +37,7 @@ def run(self, options, args):
                 )
 
         if missing or conflicting or parsing_probs:
-            return 1
-        else:
-            logger.info("No broken requirements found.")
+            return ERROR
+
+        logger.info("No broken requirements found.")
+        return SUCCESS

src/pip/_internal/commands/configuration.py

@@ -133,15 +133,16 @@ def _determine_file(self, options, need_value):
             kinds.VENV: options.venv_file
         }
 
-        if sum(file_options.values()) == 0:
+        file_options_given = sum(file_options.values())
+        if not file_options_given:
             if not need_value:
                 return None
             # Default to user, unless there's a virtualenv file.
-            elif os.path.exists(venv_config_file):
+            if os.path.exists(venv_config_file):
                 return kinds.VENV
-            else:
-                return kinds.USER
-        elif sum(file_options.values()) == 1:
+            return kinds.USER
+
+        if file_options_given == 1:
             # There's probably a better expression for this.
             return [key for key in file_options if file_options[key]][0]
 
@@ -201,8 +202,7 @@ def _get_n_args(self, args, example, n):
 
         if n == 1:
             return args[0]
-        else:
-            return args
+        return args
 
     def _save_configuration(self):
         # We successfully ran a modifying command. Need to save the
@@ -219,9 +219,8 @@ def _save_configuration(self):
     def _determine_editor(self, options):
         if options.editor is not None:
             return options.editor
-        elif "VISUAL" in os.environ:
+        if "VISUAL" in os.environ:
             return os.environ["VISUAL"]
-        elif "EDITOR" in os.environ:
+        if "EDITOR" in os.environ:
             return os.environ["EDITOR"]
-        else:
-            raise PipError("Could not determine editor to use.")
+        raise PipError("Could not determine editor to use.")

src/pip/_internal/commands/hash.py

@@ -5,7 +5,7 @@
 import sys
 
 from pip._internal.cli.base_command import Command
-from pip._internal.cli.status_codes import ERROR
+from pip._internal.cli.status_codes import ERROR, SUCCESS
 from pip._internal.utils.hashes import FAVORITE_HASH, STRONG_HASHES
 from pip._internal.utils.misc import read_chunks
 
@@ -46,6 +46,7 @@ def run(self, options, args):
         for path in args:
             logger.info('%s:\n--hash=%s:%s',
                         path, algorithm, _hash_of_file(path, algorithm))
+        return SUCCESS
 
 
 def _hash_of_file(path, algorithm):

src/pip/_internal/download.py

@@ -285,7 +285,7 @@ def __init__(self, *args, **kwargs):
     def get(self, *args, **kwargs):
         # If we don't have a directory, then the cache should be a no-op.
         if self.directory is None:
-            return
+            return None
 
         try:
             return super(SafeFileCache, self).get(*args, **kwargs)
@@ -298,7 +298,7 @@ def get(self, *args, **kwargs):
     def set(self, *args, **kwargs):
         # If we don't have a directory, then the cache should be a no-op.
         if self.directory is None:
-            return
+            return None
 
         try:
             return super(SafeFileCache, self).set(*args, **kwargs)
@@ -311,7 +311,7 @@ def set(self, *args, **kwargs):
     def delete(self, *args, **kwargs):
         # If we don't have a directory, then the cache should be a no-op.
         if self.directory is None:
-            return
+            return None
 
         try:
             return super(SafeFileCache, self).delete(*args, **kwargs)
@@ -511,6 +511,8 @@ def _get_used_vcs_backend(link):
         if link.scheme in backend.schemes:
             vcs_backend = backend(link.url)
             return vcs_backend
+    # If we can't find anything, we explicitly return None
+    return None
 
 
 def is_vcs_url(link):

src/pip/_internal/exceptions.py

@@ -76,8 +76,7 @@ def __str__(self):
         for cls, errors_of_cls in groupby(self.errors, lambda e: e.__class__):
             lines.append(cls.head)
             lines.extend(e.body() for e in errors_of_cls)
-        if lines:
-            return '\n'.join(lines)
+        return '\n'.join(lines)
 
     def __nonzero__(self):
         return bool(self.errors)

src/pip/_internal/locations.py

@@ -50,7 +50,7 @@ def running_under_virtualenv():
     """
     if hasattr(sys, 'real_prefix'):
         return True
-    elif sys.prefix != getattr(sys, "base_prefix", sys.prefix):
+    if sys.prefix != getattr(sys, "base_prefix", sys.prefix):
         return True
 
     return False
@@ -65,10 +65,9 @@ def virtualenv_no_global():
     # no-global-site-packages.txt file
     site_mod_dir = os.path.dirname(os.path.abspath(site.__file__))
     no_global_file = os.path.join(site_mod_dir, 'no-global-site-packages.txt')
-    if running_under_virtualenv() and os.path.isfile(no_global_file):
-        return True
-    else:
-        return False
+    return (
+        running_under_virtualenv() and os.path.isfile(no_global_file)
+    )
 
 
 if running_under_virtualenv():

src/pip/_internal/models/link.py

@@ -53,8 +53,7 @@ def __str__(self):
         if self.comes_from:
             return '%s (from %s)%s' % (redact_password_from_url(self.url),
                                        self.comes_from, rp)
-        else:
-            return redact_password_from_url(str(self.url))
+        return redact_password_from_url(str(self.url))
 
     def __repr__(self):
         return '<Link %s>' % self

src/pip/_internal/operations/prepare.py

@@ -40,12 +40,10 @@ def make_abstract_dist(req):
 
     :return: A concrete DistAbstraction.
     """
-    if req.editable:
-        return IsSDist(req)
-    elif req.link and req.link.is_wheel:
+    if not req.editable and req.link and req.link.is_wheel:
         return IsWheel(req)
-    else:
-        return IsSDist(req)
+
+    return IsSDist(req)
 
 
 class DistAbstraction(object):
@@ -227,11 +225,11 @@ def _download_should_save(self):
             self.download_dir = expanduser(self.download_dir)
             if os.path.exists(self.download_dir):
                 return True
-            else:
-                logger.critical('Could not find download directory')
-                raise InstallationError(
-                    "Could not find or access download directory '%s'"
-                    % display_path(self.download_dir))
+
+            logger.critical('Could not find download directory')
+            raise InstallationError(
+                "Could not find or access download directory '%s'"
+                % display_path(self.download_dir))
         return False
 
     def prepare_linked_requirement(

src/pip/_internal/pep425tags.py

@@ -68,8 +68,8 @@ def get_impl_version_info():
         return (sys.version_info[0],
                 sys.pypy_version_info.major,  # type: ignore
                 sys.pypy_version_info.minor)  # type: ignore
-    else:
-        return sys.version_info[0], sys.version_info[1]
+
+    return sys.version_info[0], sys.version_info[1]
 
 
 def get_impl_tag():

src/pip/_internal/req/constructors.py

@@ -92,8 +92,8 @@ def parse_editable(editable_req):
                 url_no_extras,
                 Requirement("placeholder" + extras.lower()).extras,
             )
-        else:
-            return package_name, url_no_extras, None
+
+        return package_name, url_no_extras, None
 
     for version_control in vcs:
         if url.lower().startswith('%s:' % version_control):

src/pip/_internal/req/req_install.py

@@ -238,8 +238,8 @@ def match_markers(self, extras_requested=None):
             return any(
                 self.markers.evaluate({'extra': extra})
                 for extra in extras_requested)
-        else:
-            return True
+
+        return True
 
     @property
     def has_hash_options(self):
@@ -626,30 +626,27 @@ def egg_info_path(self):
             if self.editable:
                 filenames = []
                 for root, dirs, files in os.walk(base):
-                    for dir in vcs.dirnames:
-                        if dir in dirs:
-                            dirs.remove(dir)
+                    for dir_ in vcs.dirnames:
+                        if dir_ in dirs:
+                            dirs.remove(dir_)
                     # Iterate over a copy of ``dirs``, since mutating
                     # a list while iterating over it can cause trouble.
                     # (See https://github.com/pypa/pip/pull/462.)
-                    for dir in list(dirs):
+                    for dir_ in list(dirs):
                         # Don't search in anything that looks like a virtualenv
                         # environment
-                        if (
-                                os.path.lexists(
-                                    os.path.join(root, dir, 'bin', 'python')
-                                ) or
-                                os.path.exists(
-                                    os.path.join(
-                                        root, dir, 'Scripts', 'Python.exe'
-                                    )
-                                )):
-                            dirs.remove(dir)
+                        if os.path.lexists(
+                            os.path.join(root, dir_, 'bin', 'python')
+                        ) or os.path.exists(
+                            os.path.join(root, dir_, 'Scripts', 'Python.exe')
+                        ):
+                            dirs.remove(dir_)
                         # Also don't search through tests
-                        elif dir == 'test' or dir == 'tests':
-                            dirs.remove(dir)
-                    filenames.extend([os.path.join(root, dir)
-                                      for dir in dirs])
+                        elif dir_ in ('test', 'tests'):
+                            dirs.remove(dir_)
+                    filenames.extend(
+                        [os.path.join(root, dir_) for dir_ in dirs]
+                    )
                 filenames = [f for f in filenames if f.endswith('.egg-info')]
 
             if not filenames:
@@ -959,8 +956,8 @@ def install(
             def prepend_root(path):
                 if root is None or not os.path.isabs(path):
                     return path
-                else:
-                    return change_root(root, path)
+
+                return change_root(root, path)
 
             with open(record_filename) as f:
                 for line in f:

src/pip/_internal/req/req_uninstall.py

@@ -301,20 +301,26 @@ def _display(msg, paths):
         return ask('Proceed (y/n)? ', ('y', 'n')) == 'y'
 
     def rollback(self):
-        """Rollback the changes previously made by remove()."""
+        """Rollback the changes previously made by remove().
+
+        Returns whether the rollback was successful.
+        """
         if not self._save_dirs:
             logger.error(
                 "Can't roll back %s; was not uninstalled",
                 self.dist.project_name,
             )
             return False
+
         logger.info('Rolling back uninstall of %s', self.dist.project_name)
         for path, tmp_path in self._moved_paths:
             logger.debug('Replacing %s', path)
             renames(tmp_path, path)
         for pth in self.pth.values():
             pth.rollback()
 
+        return True
+
     def commit(self):
         """Remove temporary save dir: rollback will no longer be possible."""
         for save_dir in self._save_dirs:

src/pip/_internal/resolve.py

@@ -141,11 +141,11 @@ def _is_upgrade_allowed(self, req):
         # type: (InstallRequirement) -> bool
         if self.upgrade_strategy == "to-satisfy-only":
             return False
-        elif self.upgrade_strategy == "eager":
+        if self.upgrade_strategy == "eager":
             return True
-        else:
-            assert self.upgrade_strategy == "only-if-needed"
-            return req.is_direct
+
+        assert self.upgrade_strategy == "only-if-needed"
+        return req.is_direct
 
     def _set_req_to_reinstall(self, req):
         # type: (InstallRequirement) -> None

src/pip/_internal/utils/compat.py

@@ -216,10 +216,10 @@ def samefile(file1, file2):
     """Provide an alternative for os.path.samefile on Windows/Python2"""
     if hasattr(os.path, 'samefile'):
         return os.path.samefile(file1, file2)
-    else:
-        path1 = os.path.normcase(os.path.abspath(file1))
-        path2 = os.path.normcase(os.path.abspath(file2))
-        return path1 == path2
+
+    path1 = os.path.normcase(os.path.abspath(file1))
+    path2 = os.path.normcase(os.path.abspath(file2))
+    return path1 == path2
 
 
 if hasattr(shutil, 'get_terminal_size'):