Merge pull request #8336 from deveshks/remove-isinstance-fixme

Remove isinstance FIXME from pip._internal.commands

5 files changed, 16 insertions, 9 deletions

diff --git a/news/F4F62E2A-7A1F-475C-95DE-004ED3B87DFB.trivial b/news/F4F62E2A-7A1F-475C-95DE-004ED3B87DFB.trivial
new file mode 100644
index 000000000..e69de29bb
--- /dev/null
+++ b/news/F4F62E2A-7A1F-475C-95DE-004ED3B87DFB.trivial
diff --git a/src/pip/_internal/cli/base_command.py b/src/pip/_internal/cli/base_command.py
index c52ffa2f2..f51682b60 100644
--- a/src/pip/_internal/cli/base_command.py
+++ b/src/pip/_internal/cli/base_command.py
@@ -19,7 +19,6 @@ from pip._internal.cli.parser import (
 from pip._internal.cli.status_codes import (
     ERROR,
     PREVIOUS_BUILD_DIR_ERROR,
-    SUCCESS,
     UNKNOWN_ERROR,
     VIRTUALENV_NOT_FOUND,
 )
@@ -106,7 +105,7 @@ class Command(CommandContextMixIn):
         assert not hasattr(options, 'no_index')
 
     def run(self, options, args):
-        # type: (Values, List[Any]) -> Any
+        # type: (Values, List[Any]) -> int
         raise NotImplementedError
 
     def parse_args(self, args):
@@ -193,10 +192,8 @@ class Command(CommandContextMixIn):
 
         try:
             status = self.run(options, args)
-            # FIXME: all commands should return an exit status
-            # and when it is done, isinstance is not needed anymore
-            if isinstance(status, int):
-                return status
+            assert isinstance(status, int)
+            return status
         except PreviousBuildDirError as exc:
             logger.critical(str(exc))
             logger.debug('Exception information:', exc_info=True)
@@ -232,5 +229,3 @@ class Command(CommandContextMixIn):
             return UNKNOWN_ERROR
         finally:
             self.handle_pip_version_check(options)
-
-        return SUCCESS
diff --git a/src/pip/_internal/commands/configuration.py b/src/pip/_internal/commands/configuration.py
index a8d3aaddb..e4db35cab 100644
--- a/src/pip/_internal/commands/configuration.py
+++ b/src/pip/_internal/commands/configuration.py
@@ -14,6 +14,11 @@ from pip._internal.configuration import (
 )
 from pip._internal.exceptions import PipError
 from pip._internal.utils.misc import get_prog, write_output
+from pip._internal.utils.typing import MYPY_CHECK_RUNNING
+
+if MYPY_CHECK_RUNNING:
+    from typing import List
+    from optparse import Values
 
 logger = logging.getLogger(__name__)
 
@@ -92,6 +97,7 @@ class ConfigurationCommand(Command):
         self.parser.insert_option_group(0, self.cmd_opts)
 
     def run(self, options, args):
+        # type: (Values, List[str]) -> int
         handlers = {
             "list": self.list_values,
             "edit": self.open_in_editor,
diff --git a/tests/functional/test_help.py b/tests/functional/test_help.py
index 1d1e439cf..00a395006 100644
--- a/tests/functional/test_help.py
+++ b/tests/functional/test_help.py
@@ -1,7 +1,7 @@
 import pytest
 from mock import Mock
 
-from pip._internal.cli.base_command import ERROR, SUCCESS
+from pip._internal.cli.status_codes import ERROR, SUCCESS
 from pip._internal.commands import commands_dict, create_command
 from pip._internal.exceptions import CommandError
 
diff --git a/tests/unit/test_base_command.py b/tests/unit/test_base_command.py
index a58f83b34..27147e769 100644
--- a/tests/unit/test_base_command.py
+++ b/tests/unit/test_base_command.py
@@ -30,8 +30,11 @@ class FakeCommand(Command):
 
     def run(self, options, args):
         logging.getLogger("pip.tests").info("fake")
+        # Return SUCCESS from run if run_func is not provided
         if self.run_func:
             return self.run_func()
+        else:
+            return SUCCESS
 
 
 class FakeCommandWithUnicode(FakeCommand):
@@ -159,6 +162,7 @@ def test_base_command_provides_tempdir_helpers():
     def assert_helpers_set(options, args):
         assert temp_dir._tempdir_manager is not None
         assert temp_dir._tempdir_registry is not None
+        return SUCCESS
 
     c = Command("fake", "fake")
     c.run = Mock(side_effect=assert_helpers_set)
@@ -183,6 +187,7 @@ def test_base_command_global_tempdir_cleanup(kind, exists):
     def create_temp_dirs(options, args):
         c.tempdir_registry.set_delete(not_deleted, False)
         Holder.value = TempDirectory(kind=kind, globally_managed=True).path
+        return SUCCESS
 
     c = Command("fake", "fake")
     c.run = Mock(side_effect=create_temp_dirs)
@@ -206,6 +211,7 @@ def test_base_command_local_tempdir_cleanup(kind, exists):
             path = d.path
             assert os.path.exists(path)
         assert os.path.exists(path) == exists
+        return SUCCESS
 
     c = Command("fake", "fake")
     c.run = Mock(side_effect=create_temp_dirs)