PyCQA / flake8-import-order

Flake8 plugin that checks import order against various Python Style Guides
GNU Lesser General Public License v3.0
278 stars 72 forks source link

Unresolveable I202 #132

Closed sigmavirus24 closed 6 years ago

sigmavirus24 commented 6 years ago

Hi there,

I upgraded flake8-import-order for flake8 and it started failing on L22 of our src/flake8/__init__.py file. I couldn't figure out how to resolve it so I simply had to silence it. Below are the contents of the file:

"""Top-level module for Flake8.

This module

- initializes logging for the command-line tool
- tracks the version of the package
- provides a way to configure logging for the command-line tool

.. autofunction:: flake8.configure_logging

"""
import logging
try:
    from logging import NullHandler
except ImportError:
    class NullHandler(logging.Handler):
        """Shim for version of Python < 2.7."""

        def emit(self, record):
            """Do nothing."""
            pass
import sys  # noqa: I202

LOG = logging.getLogger(__name__)
LOG.addHandler(NullHandler())

# Clean up after LOG config
del NullHandler

__version__ = '3.5.0'
__version_info__ = tuple(int(i) for i in __version__.split('.') if i.isdigit())

# There is nothing lower than logging.DEBUG (10) in the logging library,
# but we want an extra level to avoid being too verbose when using -vv.
_EXTRA_VERBOSE = 5
logging.addLevelName(_EXTRA_VERBOSE, 'VERBOSE')

_VERBOSITY_TO_LOG_LEVEL = {
    # output more than warnings but not debugging info
    1: logging.INFO,  # INFO is a numerical level of 20
    # output debugging information
    2: logging.DEBUG,  # DEBUG is a numerical level of 10
    # output extra verbose debugging information
    3: _EXTRA_VERBOSE,
}

LOG_FORMAT = ('%(name)-25s %(processName)-11s %(relativeCreated)6d '
              '%(levelname)-8s %(message)s')

def configure_logging(verbosity, filename=None, logformat=LOG_FORMAT):
    """Configure logging for flake8.

    :param int verbosity:
        How verbose to be in logging information.
    :param str filename:
        Name of the file to append log information to.
        If ``None`` this will log to ``sys.stderr``.
        If the name is "stdout" or "stderr" this will log to the appropriate
        stream.
    """
    if verbosity <= 0:
        return
    if verbosity > 3:
        verbosity = 3

    log_level = _VERBOSITY_TO_LOG_LEVEL[verbosity]

    if not filename or filename in ('stderr', 'stdout'):
        fileobj = getattr(sys, filename or 'stderr')
        handler_cls = logging.StreamHandler
    else:
        fileobj = filename
        handler_cls = logging.FileHandler

    handler = handler_cls(fileobj)
    handler.setFormatter(logging.Formatter(logformat))
    LOG.addHandler(handler)
    LOG.setLevel(log_level)
    LOG.debug('Added a %s logging handler to logger root at %s',
              filename, __name__)

And this is the output of flake8 --bug-report in case that helps:

{
  "dependencies": [
    {
      "dependency": "setuptools",
      "version": "38.1.0"
    }
  ],
  "platform": {
    "python_implementation": "CPython",
    "python_version": "3.6.3",
    "system": "Linux"
  },
  "plugins": [
    {
      "is_local": false,
      "plugin": "flake8-docstrings",
      "version": "1.1.0, pydocstyle: 2.1.1"
    },
    {
      "is_local": false,
      "plugin": "import-order",
      "version": "0.15"
    },
    {
      "is_local": false,
      "plugin": "mccabe",
      "version": "0.6.1"
    },
    {
      "is_local": false,
      "plugin": "pycodestyle",
      "version": "2.3.1"
    },
    {
      "is_local": false,
      "plugin": "pyflakes",
      "version": "1.6.0"
    }
  ],
  "version": "3.5.0"
}
pgjones commented 6 years ago

Thanks. I think this is the same as #123, which should be fixed in master if you could check? I'll release shortly if it does. Note that this is the commit that should fix this 87a611f0e24d2ccf0308944128ea47bd1eede2b8.

sigmavirus24 commented 6 years ago

That fixes it if and only if I remove the new line after NullHandler's docstring. If I keep the spacing as above, then it still reports I202.

pgjones commented 6 years ago

I see, this makes sense in that the checker at the moment only knows that newlines exist and nothing about the surrounding context.

I think this is best to note in the limitations (imports within try-excepts are already ignored) as knowing about the context surrounding a newline seems to add too much complexity. Unless you disagree or this is not an isolated type of error...

pgjones commented 6 years ago
diff --git a/README.rst b/README.rst
index b7ac22e..cca3e05 100644
--- a/README.rst
+++ b/README.rst
@@ -102,6 +102,26 @@ is also the same for all Python versions because otherwise it would
 be impossible to write programs that work under both Python 2 and 3
 *and* pass the import order check.

+The ``I202`` check will consider any blank line between imports to
+count, even if the line is not contextually related to the
+imports. For example,
+
+.. code-block:: python
+
+    import logging
+    try:
+        from logging import NullHandler
+    except ImportError:
+        class NullHandler(logging.Handler):
+            """Shim for version of Python < 2.7."""
+
+            def emit(self, record):
+                pass
+    import sys  # I202 due to the blank line before the 'def emit'
+
+will trigger a ``I202`` error despite the blank line not being
+contextually related.
+
 Extending styles
 ----------------

^ Proposed wording.

sigmavirus24 commented 6 years ago

👍 And what's funnier, is that I just remembered that Flake8 3.0+ doesn't support Python 2.6 so in reality that should never be reached.

pgjones commented 6 years ago

Thanks, it is in master, releasing now.