Merge pull request #8423 from McSinyx/logging-format

Nitpick logging calls

.pre-commit-config.yaml

@@ -21,7 +21,8 @@ repos:
   hooks:
   - id: flake8
     additional_dependencies: [
-        'flake8-bugbear==20.1.4'
+        'flake8-bugbear==20.1.4',
+        'flake8-logging-format==0.6.0',
     ]
     exclude: tests/data
 

setup.cfg

@@ -23,7 +23,14 @@ exclude =
     .scratch,
     _vendor,
     data
+enable-extensions = G
+ignore =
+    G200, G202,
+    # pycodestyle checks ignored in the default configuration
+    E121, E123, E126, E133, E226, E241, E242, E704, W503, W504, W505,
 per-file-ignores =
+    # G: The plugin logging-format treats every .log and .error as logging.
+    noxfile.py: G
     # B011: Do not call assert False since python -O removes these calls
     tests/*: B011
     # TODO: Remove IOError from except (OSError, IOError) blocks in
@@ -33,6 +40,7 @@ per-file-ignores =
     src/pip/_internal/utils/filesystem.py: B014
     src/pip/_internal/network/cache.py: B014
     src/pip/_internal/utils/misc.py: B014
+
 [mypy]
 follow_imports = silent
 ignore_missing_imports = True

src/pip/_internal/cache.py

@@ -228,10 +228,9 @@ class SimpleWheelCache(Cache):
                 continue
             if canonicalize_name(wheel.name) != canonical_package_name:
                 logger.debug(
-                    "Ignoring cached wheel {} for {} as it "
-                    "does not match the expected distribution name {}.".format(
-                        wheel_name, link, package_name
-                    )
+                    "Ignoring cached wheel %s for %s as it "
+                    "does not match the expected distribution name %s.",
+                    wheel_name, link, package_name,
                 )
                 continue
             if not wheel.supported(supported_tags):

src/pip/_internal/cli/cmdoptions.py

@@ -12,7 +12,6 @@ pass on state. To be consistent, all options will follow this design.
 
 from __future__ import absolute_import
 
-import logging
 import os
 import textwrap
 import warnings
@@ -35,8 +34,6 @@ if MYPY_CHECK_RUNNING:
     from optparse import OptionParser, Values
     from pip._internal.cli.parser import ConfigOptionParser
 
-logger = logging.getLogger(__name__)
-
 
 def raise_option_error(parser, option, msg):
     # type: (OptionParser, Option, str) -> None
@@ -834,11 +831,11 @@ def _handle_merge_hash(option, opt_str, value, parser):
     try:
         algo, digest = value.split(':', 1)
     except ValueError:
-        parser.error('Arguments to {} must be a hash name '
+        parser.error('Arguments to {} must be a hash name '  # noqa
                      'followed by a value, like --hash=sha256:'
                      'abcde...'.format(opt_str))
     if algo not in STRONG_HASHES:
-        parser.error('Allowed hash algorithms for {} are {}.'.format(
+        parser.error('Allowed hash algorithms for {} are {}.'.format(  # noqa
                      opt_str, ', '.join(STRONG_HASHES)))
     parser.values.hashes.setdefault(algo, []).append(digest)
 

src/pip/_internal/commands/cache.py

@@ -59,8 +59,9 @@ class CacheCommand(Command):
 
         # Determine action
         if not args or args[0] not in handlers:
-            logger.error("Need an action ({}) to perform.".format(
-                ", ".join(sorted(handlers)))
+            logger.error(
+                "Need an action (%s) to perform.",
+                ", ".join(sorted(handlers)),
             )
             return ERROR
 

src/pip/_internal/commands/configuration.py

@@ -105,8 +105,9 @@ class ConfigurationCommand(Command):
 
         # Determine action
         if not args or args[0] not in handlers:
-            logger.error("Need an action ({}) to perform.".format(
-                ", ".join(sorted(handlers)))
+            logger.error(
+                "Need an action (%s) to perform.",
+                ", ".join(sorted(handlers)),
             )
             return ERROR
 
@@ -266,9 +267,8 @@ class ConfigurationCommand(Command):
         try:
             self.configuration.save()
         except Exception:
-            logger.error(
-                "Unable to save configuration. Please report this as a bug.",
-                exc_info=True
+            logger.exception(
+                "Unable to save configuration. Please report this as a bug."
             )
             raise PipError("Internal Error.")
 

src/pip/_internal/commands/debug.py

@@ -29,7 +29,7 @@ logger = logging.getLogger(__name__)
 
 def show_value(name, value):
     # type: (str, Optional[str]) -> None
-    logger.info('{}: {}'.format(name, value))
+    logger.info('%s: %s', name, value)
 
 
 def show_sys_implementation():
@@ -102,9 +102,9 @@ def get_vendor_version_from_module(module_name):
 
 def show_actual_vendor_versions(vendor_txt_versions):
     # type: (Dict[str, str]) -> None
-    # Logs the actual version and print extra info
-    # if there is a conflict or if the actual version could not be imported.
-
+    """Log the actual version and print extra info if there is
+    a conflict or if the actual version could not be imported.
+    """
     for module_name, expected_version in vendor_txt_versions.items():
         extra_message = ''
         actual_version = get_vendor_version_from_module(module_name)
@@ -115,14 +115,7 @@ def show_actual_vendor_versions(vendor_txt_versions):
         elif actual_version != expected_version:
             extra_message = ' (CONFLICT: vendor.txt suggests version should'\
                             ' be {})'.format(expected_version)
-
-        logger.info(
-            '{name}=={actual}{extra}'.format(
-                name=module_name,
-                actual=actual_version,
-                extra=extra_message
-            )
-        )
+        logger.info('%s==%s%s', module_name, actual_version, extra_message)
 
 
 def show_vendor_versions():

src/pip/_internal/commands/install.py

@@ -240,7 +240,7 @@ class InstallCommand(RequirementCommand):
 
         install_options = options.install_options or []
 
-        logger.debug("Using {}".format(get_pip_version()))
+        logger.debug("Using %s", get_pip_version())
         options.use_user_site = decide_user_install(
             options.use_user_site,
             prefix_path=options.prefix_path,
@@ -436,7 +436,7 @@ class InstallCommand(RequirementCommand):
             message = create_env_error_message(
                 error, show_traceback, options.use_user_site,
             )
-            logger.error(message, exc_info=show_traceback)
+            logger.error(message, exc_info=show_traceback)  # noqa
 
             return ERROR
 
@@ -509,10 +509,9 @@ class InstallCommand(RequirementCommand):
         try:
             return check_install_conflicts(to_install)
         except Exception:
-            logger.error(
+            logger.exception(
                 "Error while checking for conflicts. Please file an issue on "
-                "pip's issue tracker: https://github.com/pypa/pip/issues/new",
-                exc_info=True
+                "pip's issue tracker: https://github.com/pypa/pip/issues/new"
             )
             return None
 

src/pip/_internal/index/collector.py

@@ -529,8 +529,7 @@ def group_locations(locations, expand_dir=False):
                     urls.append(url)
                 else:
                     logger.warning(
-                        "Path '{0}' is ignored: "
-                        "it is a directory.".format(path),
+                        "Path '%s' is ignored: it is a directory.", path,
                     )
             elif os.path.isfile(path):
                 sort_path(path)

src/pip/_internal/models/search_scope.py

@@ -95,8 +95,8 @@ class SearchScope(object):
                 # exceptions for malformed URLs
                 if not purl.scheme and not purl.netloc:
                     logger.warning(
-                        'The index url "{}" seems invalid, '
-                        'please provide a scheme.'.format(redacted_index_url))
+                        'The index url "%s" seems invalid, '
+                        'please provide a scheme.', redacted_index_url)
 
                 redacted_index_urls.append(redacted_index_url)
 

src/pip/_internal/operations/build/metadata.py

@@ -1,7 +1,6 @@
 """Metadata generation logic for source distributions.
 """
 
-import logging
 import os
 
 from pip._internal.utils.subprocess import runner_with_spinner_message
@@ -12,8 +11,6 @@ if MYPY_CHECK_RUNNING:
     from pip._internal.build_env import BuildEnvironment
     from pip._vendor.pep517.wrappers import Pep517HookCaller
 
-logger = logging.getLogger(__name__)
-
 
 def generate_metadata(build_env, backend):
     # type: (BuildEnvironment, Pep517HookCaller) -> str

src/pip/_internal/operations/build/wheel.py

@@ -27,7 +27,7 @@ def build_wheel_pep517(
     if build_options:
         # PEP 517 does not support --build-options
         logger.error('Cannot build wheel for %s using PEP 517 when '
-                     '--build-option is present' % (name,))
+                     '--build-option is present', name)
         return None
     try:
         logger.debug('Destination directory: %s', tempd)

src/pip/_internal/operations/install/wheel.py

@@ -308,9 +308,7 @@ def get_csv_rows_for_installed(
     installed_rows = []  # type: List[InstalledCSVRow]
     for row in old_csv_rows:
         if len(row) > 3:
-            logger.warning(
-                'RECORD line has more than three elements: {}'.format(row)
-            )
+            logger.warning('RECORD line has more than three elements: %s', row)
         old_record_path = _parse_record_path(row[0])
         new_record_path = installed.pop(old_record_path, old_record_path)
         if new_record_path in changed:

src/pip/_internal/req/constructors.py

@@ -173,8 +173,9 @@ def deduce_helpful_msg(req):
                     " the packages specified within it."
                 ).format(req)
         except RequirementParseError:
-            logger.debug("Cannot parse '{}' as requirements \
-            file".format(req), exc_info=True)
+            logger.debug(
+                "Cannot parse '%s' as requirements file", req, exc_info=True
+            )
     else:
         msg += " File '{}' does not exist.".format(req)
     return msg

src/pip/_internal/req/req_uninstall.py

@@ -612,8 +612,7 @@ class UninstallPthEntries(object):
         # If the file doesn't exist, log a warning and return
         if not os.path.isfile(self.file):
             logger.warning(
-                "Cannot remove entries from nonexistent file {}".format(
-                    self.file)
+                "Cannot remove entries from nonexistent file %s", self.file
             )
             return
         with open(self.file, 'rb') as fh:

src/pip/_internal/resolution/legacy/resolver.py

@@ -438,7 +438,7 @@ class Resolver(BaseResolver):
                 )
                 for missing in missing_requested:
                     logger.warning(
-                        '%s does not provide the extra \'%s\'',
+                        "%s does not provide the extra '%s'",
                         dist, missing
                     )
 

src/pip/_internal/resolution/resolvelib/factory.py

@@ -377,13 +377,13 @@ class Factory(object):
         # satisfied. We just report that case.
         if len(e.causes) == 1:
             req, parent = e.causes[0]
+            if parent is None:
+                req_disp = str(req)
+            else:
+                req_disp = '{} (from {})'.format(req, parent.name)
             logger.critical(
-                "Could not find a version that satisfies " +
-                "the requirement " +
-                str(req) +
-                ("" if parent is None else " (from {})".format(
-                    parent.name
-                ))
+                "Could not find a version that satisfies the requirement %s",
+                req_disp,
             )
             return DistributionNotFound(
                 'No matching distribution found for {}'.format(req)

src/pip/_internal/utils/compat.py

@@ -121,10 +121,11 @@ def str_to_display(data, desc=None):
     try:
         decoded_data = data.decode(encoding)
     except UnicodeDecodeError:
-        if desc is None:
-            desc = 'Bytes object'
-        msg_format = '{} does not appear to be encoded as %s'.format(desc)
-        logger.warning(msg_format, encoding)
+        logger.warning(
+            '%s does not appear to be encoded as %s',
+            desc or 'Bytes object',
+            encoding,
+        )
         decoded_data = data.decode(encoding, errors=backslashreplace_decode)
 
     # Make sure we can print the output, by encoding it to the output

src/pip/_internal/utils/compatibility_tags.py

@@ -3,7 +3,6 @@
 
 from __future__ import absolute_import
 
-import logging
 import re
 
 from pip._vendor.packaging.tags import (
@@ -23,8 +22,6 @@ if MYPY_CHECK_RUNNING:
 
     from pip._vendor.packaging.tags import PythonVersion
 
-logger = logging.getLogger(__name__)
-
 _osx_arch_pat = re.compile(r'(.+)_(\d+)_(\d+)_(.+)')
 
 

src/pip/_internal/utils/subprocess.py

@@ -242,8 +242,10 @@ def call_subprocess(
             raise InstallationError(exc_msg)
         elif on_returncode == 'warn':
             subprocess_logger.warning(
-                'Command "{}" had error code {} in {}'.format(
-                    command_desc, proc.returncode, cwd)
+                'Command "%s" had error code %s in %s',
+                command_desc,
+                proc.returncode,
+                cwd,
             )
         elif on_returncode == 'ignore':
             pass

src/pip/_internal/utils/temp_dir.py

@@ -185,7 +185,7 @@ class TempDirectory(object):
         path = os.path.realpath(
             tempfile.mkdtemp(prefix="pip-{}-".format(kind))
         )
-        logger.debug("Created temporary directory: {}".format(path))
+        logger.debug("Created temporary directory: %s", path)
         return path
 
     def cleanup(self):
@@ -270,5 +270,5 @@ class AdjacentTempDirectory(TempDirectory):
                 tempfile.mkdtemp(prefix="pip-{}-".format(kind))
             )
 
-        logger.debug("Created temporary directory: {}".format(path))
+        logger.debug("Created temporary directory: %s", path)
         return path

tests/unit/test_compat.py

@@ -125,8 +125,8 @@ def test_console_to_str_warning(monkeypatch):
     some_bytes = b"a\xE9b"
 
     def check_warning(msg, *args, **kwargs):
-        assert msg.startswith(
-            "Subprocess output does not appear to be encoded as")
+        assert 'does not appear to be encoded as' in msg
+        assert args[0] == 'Subprocess output'
 
     monkeypatch.setattr(locale, 'getpreferredencoding', lambda: 'utf-8')
     monkeypatch.setattr(pip_compat.logger, 'warning', check_warning)