From a1fdb5a2b5934ef5629df2335b2d495ba5252812 Mon Sep 17 00:00:00 2001 From: Ian Cordasco Date: Wed, 20 Jul 2016 19:28:13 -0500 Subject: [PATCH] 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. --- src/flake8/checker.py | 50 +++++++++++++++---------------- src/flake8/main/options.py | 2 +- src/flake8/processor.py | 11 ++++--- src/flake8/utils.py | 22 +++++++------- tests/unit/test_file_processor.py | 14 +++++++-- 5 files changed, 54 insertions(+), 45 deletions(-) diff --git a/src/flake8/checker.py b/src/flake8/checker.py index 7c70ace..ba412e3 100644 --- a/src/flake8/checker.py +++ b/src/flake8/checker.py @@ -234,15 +234,14 @@ class Manager(object): :rtype: bool """ + if path == '-': + if self.options.stdin_display_name == 'stdin': + return False + path = self.options.stdin_display_name + exclude = self.options.exclude if not exclude: 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) if utils.fnmatch(basename, exclude): LOG.debug('"%s" has been excluded', basename) @@ -269,14 +268,15 @@ class Manager(object): # best solution right now. def should_create_file_checker(filename): """Determine if we should create a file checker.""" - return ( - filename == '-' or # stdin - utils.fnmatch(filename, filename_patterns) and - os.path.exists(filename) + matches_filename_patterns = utils.fnmatch( + filename, filename_patterns ) + is_stdin = filename == '-' + file_exists = os.path.exists(filename) + return (file_exists and matches_filename_patterns) or is_stdin self.checkers = [ - FileChecker(filename, self.checks, self.style_guide) + FileChecker(filename, self.checks, self.options) for argument in paths for filename in utils.filenames_from(argument, self.is_path_excluded) @@ -299,7 +299,7 @@ class Manager(object): results_reported = results_found = 0 for checker in self.checkers: 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_found += len(results) return (results_found, results_reported) @@ -320,9 +320,9 @@ class Manager(object): final_results[filename] = results for checker in self.checkers: - filename = checker.filename + filename = checker.display_name 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): """Run the checkers in serial.""" @@ -384,7 +384,7 @@ class Manager(object): class FileChecker(object): """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. :param str filename: @@ -393,26 +393,26 @@ class FileChecker(object): The plugins registered to check the file. :type checks: flake8.plugins.manager.Checkers - :param style_guide: - The initialized StyleGuide for this particular run. - :type style_guide: - flake8.style_guide.StyleGuide + :param options: + Parsed option values from config and command-line. + :type options: + optparse.Values """ + self.options = options + self.filename = filename self.checks = checks - self.style_guide = style_guide self.results = [] - self.processor = self._make_processor(filename) - self.filename = self.processor.filename + self.processor = self._make_processor() + self.display_name = self.processor.filename self.statistics = { 'tokens': 0, 'logical lines': 0, 'physical lines': len(self.processor.lines), } - def _make_processor(self, filename): + def _make_processor(self): try: - return processor.FileProcessor(filename, - self.style_guide.options) + return processor.FileProcessor(self.filename, self.options) except IOError: # 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) diff --git a/src/flake8/main/options.py b/src/flake8/main/options.py index dbfdf9b..c725c38 100644 --- a/src/flake8/main/options.py +++ b/src/flake8/main/options.py @@ -74,7 +74,7 @@ def register_default_options(option_manager): ) add_option( - '--stdin-display-name', + '--stdin-display-name', default='stdin', help='The name used when reporting errors from code passed via stdin.' ' This is useful for editors piping the file contents to flake8.' ' (Default: %default)', diff --git a/src/flake8/processor.py b/src/flake8/processor.py index ae82b50..3e7dba5 100644 --- a/src/flake8/processor.py +++ b/src/flake8/processor.py @@ -59,8 +59,7 @@ class FileProcessor(object): self.filename = filename self.lines = lines if lines is None: - # allow for stdin filename substitution - self.filename, self.lines = self.read_lines(filename) + self.lines = self.read_lines() self.strip_utf_bom() # Defaults for public attributes @@ -269,15 +268,15 @@ class FileProcessor(object): self.indent_char = line[0] return line - def read_lines(self, filename): + def read_lines(self): # type: () -> List[str] """Read the lines for this file checker.""" - if filename is None or filename == '-': - filename = self.options.stdin_display_name or 'stdin' + if self.filename is None or self.filename == '-': + self.filename = self.options.stdin_display_name lines = self.read_lines_from_stdin() else: lines = self.read_lines_from_filename() - return (filename, lines) + return lines def _readlines_py2(self): # type: () -> List[str] diff --git a/src/flake8/utils.py b/src/flake8/utils.py index 52417fc..7f31f8f 100644 --- a/src/flake8/utils.py +++ b/src/flake8/utils.py @@ -212,22 +212,24 @@ def filenames_from(arg, predicate=None): if predicate(arg): return - if arg == "-": - # stdin, don't call isdir() - yield arg - elif os.path.isdir(arg): + if os.path.isdir(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 # remove it from the list of sub-directories. - sub_directories[:] = [ - directory for directory in sub_directories - if not predicate(os.path.join(root, directory)) - ] + for directory in sub_directories: + joined = os.path.join(root, directory) + if predicate(directory) or predicate(joined): + sub_directories.remove(directory) for filename in files: joined = os.path.join(root, filename) - if not predicate(joined): - yield joined + if predicate(joined) or predicate(filename): + continue + yield joined else: yield arg diff --git a/tests/unit/test_file_processor.py b/tests/unit/test_file_processor.py index 8bc0e2d..547d6a0 100644 --- a/tests/unit/test_file_processor.py +++ b/tests/unit/test_file_processor.py @@ -14,7 +14,7 @@ def options_from(**kwargs): kwargs.setdefault('hang_closing', True) kwargs.setdefault('max_line_length', 79) kwargs.setdefault('verbose', False) - kwargs.setdefault('stdin_display_name', None) + kwargs.setdefault('stdin_display_name', 'stdin') return optparse.Values(kwargs) @@ -71,10 +71,18 @@ def test_stdin_filename_attribute(stdin_get_value): stdin_get_value.return_value = stdin_value file_processor = processor.FileProcessor('-', options_from()) 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( - 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():