Fix up merge request 78

This simplifies the changes, reduces the scope of refactors apparently
for refactoring's sake and ensures that the internals are reasonable.

It also airs on the side of preserving information rather than
discarding or overwriting it.
This commit is contained in:
Ian Cordasco 2016-07-20 19:28:13 -05:00
parent 7934f8dce2
commit a1fdb5a2b5
No known key found for this signature in database
GPG key ID: 656D3395E4A9791A
5 changed files with 54 additions and 45 deletions

View file

@ -234,15 +234,14 @@ class Manager(object):
:rtype: :rtype:
bool bool
""" """
if path == '-':
if self.options.stdin_display_name == 'stdin':
return False
path = self.options.stdin_display_name
exclude = self.options.exclude exclude = self.options.exclude
if not exclude: if not exclude:
return False return False
if path == '-':
# stdin, use display name to check exclusion, if present
path = self.options.stdin_display_name
if path is None:
LOG.debug("unnamed stdin has not been excluded")
return False
basename = os.path.basename(path) basename = os.path.basename(path)
if utils.fnmatch(basename, exclude): if utils.fnmatch(basename, exclude):
LOG.debug('"%s" has been excluded', basename) LOG.debug('"%s" has been excluded', basename)
@ -269,14 +268,15 @@ class Manager(object):
# best solution right now. # best solution right now.
def should_create_file_checker(filename): def should_create_file_checker(filename):
"""Determine if we should create a file checker.""" """Determine if we should create a file checker."""
return ( matches_filename_patterns = utils.fnmatch(
filename == '-' or # stdin filename, filename_patterns
utils.fnmatch(filename, filename_patterns) and
os.path.exists(filename)
) )
is_stdin = filename == '-'
file_exists = os.path.exists(filename)
return (file_exists and matches_filename_patterns) or is_stdin
self.checkers = [ self.checkers = [
FileChecker(filename, self.checks, self.style_guide) FileChecker(filename, self.checks, self.options)
for argument in paths for argument in paths
for filename in utils.filenames_from(argument, for filename in utils.filenames_from(argument,
self.is_path_excluded) self.is_path_excluded)
@ -299,7 +299,7 @@ class Manager(object):
results_reported = results_found = 0 results_reported = results_found = 0
for checker in self.checkers: for checker in self.checkers:
results = sorted(checker.results, key=lambda tup: (tup[2], tup[3])) results = sorted(checker.results, key=lambda tup: (tup[2], tup[3]))
results_reported += self._handle_results(checker.filename, results_reported += self._handle_results(checker.display_name,
results) results)
results_found += len(results) results_found += len(results)
return (results_found, results_reported) return (results_found, results_reported)
@ -320,9 +320,9 @@ class Manager(object):
final_results[filename] = results final_results[filename] = results
for checker in self.checkers: for checker in self.checkers:
filename = checker.filename filename = checker.display_name
checker.results = sorted(final_results.get(filename, []), checker.results = sorted(final_results.get(filename, []),
key=lambda tup: (tup[1], tup[2])) key=lambda tup: (tup[2], tup[2]))
def run_serial(self): def run_serial(self):
"""Run the checkers in serial.""" """Run the checkers in serial."""
@ -384,7 +384,7 @@ class Manager(object):
class FileChecker(object): class FileChecker(object):
"""Manage running checks for a file and aggregate the results.""" """Manage running checks for a file and aggregate the results."""
def __init__(self, filename, checks, style_guide): def __init__(self, filename, checks, options):
"""Initialize our file checker. """Initialize our file checker.
:param str filename: :param str filename:
@ -393,26 +393,26 @@ class FileChecker(object):
The plugins registered to check the file. The plugins registered to check the file.
:type checks: :type checks:
flake8.plugins.manager.Checkers flake8.plugins.manager.Checkers
:param style_guide: :param options:
The initialized StyleGuide for this particular run. Parsed option values from config and command-line.
:type style_guide: :type options:
flake8.style_guide.StyleGuide optparse.Values
""" """
self.options = options
self.filename = filename
self.checks = checks self.checks = checks
self.style_guide = style_guide
self.results = [] self.results = []
self.processor = self._make_processor(filename) self.processor = self._make_processor()
self.filename = self.processor.filename self.display_name = self.processor.filename
self.statistics = { self.statistics = {
'tokens': 0, 'tokens': 0,
'logical lines': 0, 'logical lines': 0,
'physical lines': len(self.processor.lines), 'physical lines': len(self.processor.lines),
} }
def _make_processor(self, filename): def _make_processor(self):
try: try:
return processor.FileProcessor(filename, return processor.FileProcessor(self.filename, self.options)
self.style_guide.options)
except IOError: except IOError:
# If we can not read the file due to an IOError (e.g., the file # If we can not read the file due to an IOError (e.g., the file
# does not exist or we do not have the permissions to open it) # does not exist or we do not have the permissions to open it)

View file

@ -74,7 +74,7 @@ def register_default_options(option_manager):
) )
add_option( add_option(
'--stdin-display-name', '--stdin-display-name', default='stdin',
help='The name used when reporting errors from code passed via stdin.' help='The name used when reporting errors from code passed via stdin.'
' This is useful for editors piping the file contents to flake8.' ' This is useful for editors piping the file contents to flake8.'
' (Default: %default)', ' (Default: %default)',

View file

@ -59,8 +59,7 @@ class FileProcessor(object):
self.filename = filename self.filename = filename
self.lines = lines self.lines = lines
if lines is None: if lines is None:
# allow for stdin filename substitution self.lines = self.read_lines()
self.filename, self.lines = self.read_lines(filename)
self.strip_utf_bom() self.strip_utf_bom()
# Defaults for public attributes # Defaults for public attributes
@ -269,15 +268,15 @@ class FileProcessor(object):
self.indent_char = line[0] self.indent_char = line[0]
return line return line
def read_lines(self, filename): def read_lines(self):
# type: () -> List[str] # type: () -> List[str]
"""Read the lines for this file checker.""" """Read the lines for this file checker."""
if filename is None or filename == '-': if self.filename is None or self.filename == '-':
filename = self.options.stdin_display_name or 'stdin' self.filename = self.options.stdin_display_name
lines = self.read_lines_from_stdin() lines = self.read_lines_from_stdin()
else: else:
lines = self.read_lines_from_filename() lines = self.read_lines_from_filename()
return (filename, lines) return lines
def _readlines_py2(self): def _readlines_py2(self):
# type: () -> List[str] # type: () -> List[str]

View file

@ -212,22 +212,24 @@ def filenames_from(arg, predicate=None):
if predicate(arg): if predicate(arg):
return return
if arg == "-": if os.path.isdir(arg):
# stdin, don't call isdir()
yield arg
elif os.path.isdir(arg):
for root, sub_directories, files in os.walk(arg): for root, sub_directories, files in os.walk(arg):
if predicate(root):
sub_directories[:] = []
continue
# NOTE(sigmavirus24): os.walk() will skip a directory if you # NOTE(sigmavirus24): os.walk() will skip a directory if you
# remove it from the list of sub-directories. # remove it from the list of sub-directories.
sub_directories[:] = [ for directory in sub_directories:
directory for directory in sub_directories joined = os.path.join(root, directory)
if not predicate(os.path.join(root, directory)) if predicate(directory) or predicate(joined):
] sub_directories.remove(directory)
for filename in files: for filename in files:
joined = os.path.join(root, filename) joined = os.path.join(root, filename)
if not predicate(joined): if predicate(joined) or predicate(filename):
yield joined continue
yield joined
else: else:
yield arg yield arg

View file

@ -14,7 +14,7 @@ def options_from(**kwargs):
kwargs.setdefault('hang_closing', True) kwargs.setdefault('hang_closing', True)
kwargs.setdefault('max_line_length', 79) kwargs.setdefault('max_line_length', 79)
kwargs.setdefault('verbose', False) kwargs.setdefault('verbose', False)
kwargs.setdefault('stdin_display_name', None) kwargs.setdefault('stdin_display_name', 'stdin')
return optparse.Values(kwargs) return optparse.Values(kwargs)
@ -71,10 +71,18 @@ def test_stdin_filename_attribute(stdin_get_value):
stdin_get_value.return_value = stdin_value stdin_get_value.return_value = stdin_value
file_processor = processor.FileProcessor('-', options_from()) file_processor = processor.FileProcessor('-', options_from())
assert file_processor.filename == 'stdin' assert file_processor.filename == 'stdin'
@mock.patch('flake8.utils.stdin_get_value')
def test_read_lines_uses_display_name(stdin_get_value):
"""Verify that when processing stdin we use a display name if present."""
stdin_value = mock.Mock()
stdin_value.splitlines.return_value = []
stdin_get_value.return_value = stdin_value
file_processor = processor.FileProcessor('-', options_from( file_processor = processor.FileProcessor('-', options_from(
stdin_display_name="foo.py" stdin_display_name='display_name.py'
)) ))
assert file_processor.filename == 'foo.py' assert file_processor.filename == 'display_name.py'
def test_line_for(): def test_line_for():