Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for pycodestyle not picking up modules in non-package dirs #199

Closed
wants to merge 12 commits into from
Closed
1 change: 1 addition & 0 deletions CONTRIBUTORS.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ Contributors
* Jeff Quast ([@jquast](https://github.com/jquast))
* Sam Spilsbury ([@smspillaz](https://github.com/smspillaz))
* Jose Antonio Perdiguero ([@PeRDy](https://github.com/PeRDy))
* Holger Joukl ([@hjoukl](https://github.com/hjoukl))
4 changes: 2 additions & 2 deletions prospector/finder.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,5 +198,5 @@ def find_python(ignores, paths, explicit_file_mode, workdir=None):
return SingleFiles(paths, workdir or os.getcwd())
else:
assert len(paths) == 1
files, modules, directories, packages = _find_paths(ignores, paths[0], paths[0])
return FoundFiles(paths[0], files, modules, directories, packages, ignores)
files, modules, packages, directories = _find_paths(ignores, paths[0], paths[0])
return FoundFiles(paths[0], files, modules, packages, directories, ignores)
15 changes: 12 additions & 3 deletions prospector/tools/pep8/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

from prospector.message import Location, Message
from prospector.tools.base import ToolBase
from prospector.finder import FoundFiles


__all__ = (
Expand Down Expand Up @@ -82,10 +83,11 @@ def excluded(self, filename, parent=None):

# If the file survived pep8's exclusion rules, check it against
# prospector's patterns.
if os.path.isdir(os.path.join(self._files.rootpath, filename)):
fullpath = os.path.join('' if parent is None else parent, filename)

if os.path.isdir(fullpath):
return False

fullpath = os.path.join(self._files.rootpath, parent, filename) if parent else filename
return fullpath not in self._module_paths


Expand Down Expand Up @@ -121,8 +123,15 @@ def configure(self, prospector_config, found_files):
break

# Instantiate our custom pep8 checker.
# TODO: Can this be implemented cleaner? Ultimately, pycodestyle needs
# TODO: to be driven differently for explicit paths vs root dir path
# TODO: to make both prospector and pycodestyle exclusion rules work.
if isinstance(found_files, FoundFiles):
checkpaths = [os.path.abspath(found_files.rootpath)]
else:
checkpaths = list(found_files.iter_module_paths())
self.checker = ProspectorStyleGuide(
paths=list(found_files.iter_package_paths()),
paths=checkpaths,
found_files=found_files,
config_file=external_config
)
Expand Down
231 changes: 230 additions & 1 deletion tests/finder/test_file_finder.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import os
from unittest import TestCase
from prospector.finder import find_python
from prospector.finder import find_python, FoundFiles, SingleFiles
from prospector.pathutils import is_virtualenv


Expand Down Expand Up @@ -48,3 +48,232 @@ def test_long_path_not_a_venv(self):
path.append('long_path_not_a_venv_long_path_not_a_v')
path = os.path.join(*path)
self.assertFalse(is_virtualenv(path))


class TestPathFinderDir(TestCase):

def _run_test(self, expected, method_under_test):
# method_under_test: unbound FoundFiles method
root = os.path.join(
os.path.dirname(__file__), 'testdata', 'dirs_mods_packages')
found = find_python(
ignores=[], paths=[root], explicit_file_mode=False)

expected = [os.path.normpath(os.path.join(root, e)) for e in expected]
expected.sort()
actual = list(method_under_test(found))
actual.sort()
self.assertEqual(actual, expected)

def test_file_paths(self):
expected = [
'package/__init__.py',
'package/subpackage/__init__.py',
'package/subpackage/subpackage_module1.py',
'package/subpackage/subpackage_module2.py',
'package/subpackage/subpackage_plain.txt',
'package/package_module1.py',
'package/package_module2.py',
'package/package_plain.txt',
'nonpackage/nonpackage_module1.py',
'nonpackage/nonpackage_module2.py',
'nonpackage/nonpackage_plain.txt',
'module1.py',
'module2.py',
'plain.txt',
]
self._run_test(expected, FoundFiles.iter_file_paths)

def test_module_paths(self):
expected = [
'package/__init__.py',
'package/subpackage/__init__.py',
'package/subpackage/subpackage_module1.py',
'package/subpackage/subpackage_module2.py',
'package/package_module1.py',
'package/package_module2.py',
'nonpackage/nonpackage_module1.py',
'nonpackage/nonpackage_module2.py',
'module1.py',
'module2.py',
]
self._run_test(expected, FoundFiles.iter_module_paths)

def test_directory_paths(self):
expected = [
'package',
'package/subpackage',
'nonpackage',
]
self._run_test(expected, FoundFiles.iter_directory_paths)

def test_package_paths(self):
expected = [
'package',
'package/subpackage',
]
self._run_test(expected, FoundFiles.iter_package_paths)


class TestPathFinderSingleFiles(TestCase):

def _run_test(self, paths, expected, method_under_test):
# method_under_test: unbound SingleFiles method
root = os.path.join(
os.path.dirname(__file__), 'testdata', 'dirs_mods_packages')
paths = [os.path.normpath(os.path.join(root, p)) for p in paths]
found = find_python(
ignores=[], paths=paths, explicit_file_mode=True)
expected = [os.path.normpath(os.path.join(root, e)) for e in expected]
expected.sort()
actual = list(method_under_test(found))
actual.sort()
self.assertEqual(actual, expected)

def test_file_paths(self):
paths = [
'package/__init__.py',
'package/subpackage/__init__.py',
'package/subpackage/subpackage_module1.py',
'package/subpackage/subpackage_module2.py',
'package/subpackage/subpackage_plain.txt',
'package/package_module1.py',
'package/package_module2.py',
'package/package_plain.txt',
'nonpackage/nonpackage_module1.py',
'nonpackage/nonpackage_module2.py',
'nonpackage/nonpackage_plain.txt',
'module1.py',
'module2.py',
'plain.txt',
]
expected = [
'package/__init__.py',
'package/subpackage/__init__.py',
'package/subpackage/subpackage_module1.py',
'package/subpackage/subpackage_module2.py',
'package/subpackage/subpackage_plain.txt',
'package/package_module1.py',
'package/package_module2.py',
'package/package_plain.txt',
'nonpackage/nonpackage_module1.py',
'nonpackage/nonpackage_module2.py',
'nonpackage/nonpackage_plain.txt',
'module1.py',
'module2.py',
'plain.txt',
]
self._run_test(paths, expected, SingleFiles.iter_file_paths)

def test_module_paths(self):
paths = [
'package/__init__.py',
'package/subpackage/__init__.py',
'package/subpackage/subpackage_module1.py',
'package/subpackage/subpackage_module2.py',
'package/subpackage/subpackage_plain.txt',
'package/package_module1.py',
'package/package_module2.py',
'package/package_plain.txt',
'nonpackage/nonpackage_module1.py',
'nonpackage/nonpackage_module2.py',
'nonpackage/nonpackage_plain.txt',
'module1.py',
'module2.py',
'plain.txt',
]
# SingleFiles.iter_module_paths yields all file paths, not just Python
# modules
expected = [
'package/__init__.py',
'package/subpackage/__init__.py',
'package/subpackage/subpackage_module1.py',
'package/subpackage/subpackage_module2.py',
'package/subpackage/subpackage_plain.txt',
'package/package_module1.py',
'package/package_module2.py',
'package/package_plain.txt',
'nonpackage/nonpackage_module1.py',
'nonpackage/nonpackage_module2.py',
'nonpackage/nonpackage_plain.txt',
'module1.py',
'module2.py',
'plain.txt',
]
self._run_test(paths, expected, SingleFiles.iter_module_paths)

def test_directory_paths(self):
paths = [
'package/__init__.py',
'package/subpackage/__init__.py',
'package/subpackage/subpackage_module1.py',
'package/subpackage/subpackage_module2.py',
'package/subpackage/subpackage_plain.txt',
'package/package_module1.py',
'package/package_module2.py',
'package/package_plain.txt',
'nonpackage/nonpackage_module1.py',
'nonpackage/nonpackage_module2.py',
'nonpackage/nonpackage_plain.txt',
'module1.py',
'module2.py',
'plain.txt',
]
# SingleFiles.iter_directory_paths yields the directories of all file
# paths (duplicates are not filtered)
expected = [
'package',
'package/subpackage',
'package/subpackage',
'package/subpackage',
'package/subpackage',
'package',
'package',
'package',
'nonpackage',
'nonpackage',
'nonpackage',
'',
'',
'',
]
self._run_test(paths, expected, SingleFiles.iter_directory_paths)

def test_package_paths(self):
paths = [
'package/__init__.py',
'package/subpackage/__init__.py',
'package/subpackage/subpackage_module1.py',
'package/subpackage/subpackage_module2.py',
'package/subpackage/subpackage_plain.txt',
'package/package_module1.py',
'package/package_module2.py',
'package/package_plain.txt',
'nonpackage/nonpackage_module1.py',
'nonpackage/nonpackage_module2.py',
'nonpackage/nonpackage_plain.txt',
'module1.py',
'module2.py',
'plain.txt',
]
# SingleFiles.iter_package_paths yields all file paths, not just Python
# packages
expected = [
'package/__init__.py',
'package/subpackage/__init__.py',
'package/subpackage/subpackage_module1.py',
'package/subpackage/subpackage_module2.py',
'package/subpackage/subpackage_plain.txt',
'package/package_module1.py',
'package/package_module2.py',
'package/package_plain.txt',
'nonpackage/nonpackage_module1.py',
'nonpackage/nonpackage_module2.py',
'nonpackage/nonpackage_plain.txt',
'module1.py',
'module2.py',
'plain.txt',
]
self._run_test(paths, expected, SingleFiles.iter_package_paths)


1 change: 1 addition & 0 deletions tests/finder/testdata/dirs_mods_packages/module1.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# Python module
1 change: 1 addition & 0 deletions tests/finder/testdata/dirs_mods_packages/module2.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# Python module
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# Python module
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# Python module
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# plain text file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# Python package
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# Python module
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# Python module
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# plain text file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# Python package
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# Python module
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# Python module
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# plain text file
1 change: 1 addition & 0 deletions tests/finder/testdata/dirs_mods_packages/plain.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# plain text file
Empty file added tests/tools/__init__.py
Empty file.
Loading