Merge pull request #32 from cfournie/lint

Lint

Makefile

@@ -1,22 +1,31 @@
-PYTHON_VERSION := `python -c 'import sys; print(sys.version_info.major)'`
+python_version := `python -c 'import sys; print(sys.version_info.major)'`
+python_files := find . -path '*/.*' -prune -o -name '*.py' -print0
 
 all: install test lint
 
+clean:
+		find . \( -name '*.pyc' -o -name '*.pyo' -o -name '*~' \) -print -delete >/dev/null
+		find . -name '__pycache__' -exec rm -rvf '{}' + >/dev/null
+		rm -fr *.egg-info
+
 install:
-	pip install -e .
-	@if [ "$(PYTHON_VERSION)" = "2" ]; then pip install -r python27_requirements.txt; fi
-	pip install -r dev_requirements.txt
-test: install
-	py.test -vv --cov=important tests/
-	important -v
+		pip install -e .
+		@if [ "$(python_version)" = "2" ]; then pip install -r python27_requirements.txt; fi
+		pip install -r dev_requirements.txt
+
+test: clean install
+		py.test -vv --cov=important tests/
+		important -v
 
 coverage:
-	py.test -vv --cov=important --cov-report html tests/
+		py.test -vv --cov=important --cov-report html tests/
 
-autolint:
-	pip install autopep8
-	autopep8 --in-place --aggressive --aggressive *.py
-	autopep8 --in-place --aggressive --aggressive **/*.py
+autopep8:
+		@echo 'Auto Formatting...'
+		@$(python_files) | xargs -0 autopep8 --max-line-length 120 --jobs 0 --in-place --aggressive
 
 lint:
-	flake8 --ignore D .
+		flake8 --ignore D .
+		@echo 'Linting...'
+		@pylint --rcfile=pylintrc important tests
+		@flake8

dev_requirements.txt

@@ -1,9 +1,13 @@
 -r requirements.txt
-pytest>=2.7
-pytest-cov
-pytest-mock
-flake8
-mock
+pytest>=2.7  # Test framework
+pytest-cov   # Obtain test coverage
+pytest-mock  # Provide mocking
+mock         # Provide mocking
+flake8       # Linting
+pylint       # Linting
+autopep8     # Autolinting
+ipdb         # Interactive debugger
+ipython      # Interactive shell
 
 # Modules tested
 dnspython          # Installed as directory `dns`

important/__main__.py

@@ -1,9 +1,11 @@
-import click
 import logging
 import os
 import sys
 
 from configparser import ConfigParser
+
+import click
+
 from important.parse import parse_dir_imports, parse_file_imports, \
     parse_requirements
 from important.check import check_unused_requirements, check_import_frequencies
@@ -19,8 +21,8 @@
 
 # If a setup file exists, override cli arguments with those values
 if os.path.exists('setup.cfg'):
-    config = ConfigParser()
-    config.read('setup.cfg')
+    CONFIG = ConfigParser()
+    CONFIG.read('setup.cfg')
 
     def split(key_value):
         if key_value[0] in ('sourcecode',):
@@ -29,7 +31,7 @@ def split(key_value):
             return key_value[0], key_value[1].split()
 
     CONTEXT_SETTINGS['default_map'] = \
-        dict(map(split, config.items('important')))
+        dict(map(split, CONFIG.items('important')))
 
 
 @click.command(help="Check imports within SOURCECODE (except those files "
@@ -102,10 +104,8 @@ def check(requirements, constraints, ignore, ignorefile, exclude, sourcecode,
             (r.name for r in parse_requirements(ignorefile_path))
         )
     if ignore:
-        parsed_requirements = filter(
-            lambda r: r.name not in ignore,
-            parsed_requirements
-        )
+        parsed_requirements = [r for r in parsed_requirements
+                               if r.name not in ignore]
 
     if verbose >= 2:
         click.echo('Read requirements:')
@@ -129,7 +129,7 @@ def check(requirements, constraints, ignore, ignorefile, exclude, sourcecode,
         raise click.BadParameter("could not parse SOURCECODE '%s'; path is "
                                  "either not a file or not a directory" %
                                  sourcecode)
-    filenames = set(map(lambda i: i.filename, imports))
+    filenames = set(i.filename for i in imports)
 
     output = []
 

important/check.py

@@ -1,7 +1,7 @@
 from __future__ import unicode_literals
 
 from collections import defaultdict
-from important.parse import translate_requirement_to_module_names
+from important.parse import translate_req_to_module_names
 
 
 def _base_module_name(import_statement):
@@ -16,7 +16,7 @@ def check_unused_requirements(imports, requirements):
     # Translate package names into module names that can be imported
     module_requirements = {}
     for requirement in requirements:
-        modules = translate_requirement_to_module_names(requirement)
+        modules = translate_req_to_module_names(requirement)
         for module in modules:
             module_requirements[module] = requirement
     # Translate imported modules into package names
@@ -46,11 +46,10 @@ def check_import_frequencies(imports, requirements):
     module_frequencies = frequency_count_imports(imports)
     violations = dict()
     for requirement, constraint in constraints.items():
-        modules = translate_requirement_to_module_names(requirement)
+        modules = translate_req_to_module_names(requirement)
         for module in modules:
-            if module in module_frequencies \
-                    and not constraint.contains(
-                        str(module_frequencies[module])):
+            if module in module_frequencies and not constraint.contains(
+                    str(module_frequencies[module])):
                 violations[requirement] = (constraint,
                                            module_frequencies[module])
     return violations

important/parse.py

@@ -1,16 +1,19 @@
 from __future__ import unicode_literals
 
 import ast
+import io
 import logging
 import os
-import pip
 import pkgutil
 import re
 import stat
 import sys
 
 from collections import namedtuple
-from io import open
+from itertools import chain
+
+import pip
+
 from pip.commands.show import search_packages_info
 from pip.req import parse_requirements as pip_parse_requirements
 
@@ -22,7 +25,7 @@
 
 Import = namedtuple('Import', ['module', 'filename', 'lineno', 'col_offset'])
 
-logger = logging.getLogger()
+LOGGER = logging.getLogger()
 
 
 def _imports(source, filepath):
@@ -43,14 +46,12 @@ def _ast_imports(root):
 
 def is_excluded(path, exclusions):
     if exclusions:
-        generators = (
-            filter(lambda e: os.path.samefile(path, e), exclusions),
-            filter(lambda e: os.path.samefile(os.path.dirname(path), e),
-                   exclusions)
-        )
-        for generator in generators:
-            for _ in generator:
-                return True
+        for _ in chain(
+                (e for e in exclusions if os.path.samefile(path, e)),
+                (e for e in exclusions if
+                 os.path.samefile(os.path.dirname(path), e)),
+        ):
+            return True
     return False
 
 
@@ -64,37 +65,34 @@ def parse_file_imports(filepath, exclusions=None, directory=None):
     display_filepath = os.path.relpath(filepath, directory)
     # Compile and parse abstract syntax tree and find import statements
     try:
-        with open(filepath) as fh:
-            source = fh.read()
+        with io.open(filepath) as handle:
+            source = handle.read()
         # Remove lines with only comments (e.g. PEP 263 encodings)
         source = '\n'.join(
-            map(
-                lambda l: '' if l.startswith('#') else l,
-                source.split('\n')
-            )
-        )
+            '' if l.startswith('#') else l for l in source.split('\n'))
         # Parse
         for statement in _imports(source, filepath):
             module, lineno, col_offset = statement
             yield Import(module, display_filepath, lineno, col_offset)
-    except SyntaxError as e:
-        logger.warning('Skipping {filename} due to syntax error: {error}'
-                       .format(filename=e.filename, error=str(e)))
-    except UnicodeDecodeError as e:
-        logger.warning('Skipping {filename} due to decoding error: {error}'
-                       .format(filename=filepath, error=str(e)))
+    except SyntaxError as exc:
+        LOGGER.warning('Skipping %(filename)s due to syntax error: %(error)s',
+                       filename=exc.filename, error=str(exc))
+    except UnicodeDecodeError as exc:
+        LOGGER.warning('Skipping %(filename)s due to decode error: %(error)s',
+                       filename=filepath, error=str(exc))
 
 
 def _is_script(filepath):
     if os.access(filepath, os.F_OK | os.R_OK | os.X_OK) and \
        not stat.S_ISSOCK(os.stat(filepath).st_mode):
         try:
-            with open(filepath, mode='r') as fh:
-                first_line = fh.readline()
+            with io.open(filepath, mode='r') as handle:
+                first_line = handle.readline()
             return bool(RE_SHEBANG.match(first_line))
-        except UnicodeDecodeError as e:
-            logger.warning('Skipping {filename} due to decoding error: {error}'
-                           .format(filename=filepath, error=str(e)))
+        except UnicodeDecodeError as exc:
+            LOGGER.warning(
+                'Skipping %(filename)s due to decode error: %(error)s',
+                filename=filepath, error=str(exc))
     return False
 
 
@@ -104,8 +102,7 @@ def parse_dir_imports(current_directory, exclusions=None):
         return
     # Iterate over all Python/script files
     for root, dirs, files in os.walk(current_directory, topdown=True):
-        dirs[:] = filter(lambda d: d not in exclusions,
-                         [os.path.join(root, d) for d in dirs])
+        dirs[:] = [os.path.join(root, d) for d in dirs if d not in exclusions]
         for filename in files:
             filename = filename.decode('utf-8') \
                 if hasattr(filename, 'decode') and isinstance(filename, str) \
@@ -132,7 +129,7 @@ def parse_requirements(filename):
             yield requirement
 
 
-def translate_requirement_to_module_names(requirement_name):
+def translate_req_to_module_names(requirement_name):
     provides = set()
 
     def is_module_folder(filepath):
@@ -152,8 +149,7 @@ def is_top_level_file(filepath):
             # Assume that only one module is installed in this case
             continue
         # Handle modules that are installed as folders in site-packages
-        folders = map(lambda filepath: os.path.dirname(filepath),
-                      result['files'])
+        folders = [os.path.dirname(filepath) for filepath in result['files']]
         folders = filter(is_module_folder, folders)
         provides |= set(folders)
         # Handle modules that are installed as .py files in site-packages
@@ -166,7 +162,6 @@ def is_top_level_file(filepath):
     else:
         module_name = requirement_name.split('.')[0]
         if module_name not in ALL_MODULES:
-            logger.warning("Cannot find install location of '{requirement}'; please \
-install this package for more accurate name resolution"
-                           .format(requirement=requirement_name))
+            LOGGER.warning("Cannot find install location of '%s'; please \
+install this package for more accurate name resolution", requirement_name)
         return provides if provides else set([requirement_name])