From 68264ec2f4fba7f12ce57737ea533562adaf80c3 Mon Sep 17 00:00:00 2001 From: John Vandenberg Date: Tue, 30 Oct 2018 19:28:56 +0700 Subject: [PATCH] Linter: Avoid processing empty streams When a linter returns an empty stream, it should not be processed. In the case of process_output_corrected, processing it is extremely destructive, zero-ing the file. In the other modes, there is no output to be processed, so it is merely wasted processing. Also emit warnings for any messages found on a stream which is not being processed. Followup enhancements might add ways for linter bears to skip known messages where a linter is noisy. Fixes https://github.com/coala/coala/issues/5415 --- coalib/bearlib/abstractions/Linter.py | 34 ++++++-- tests/bearlib/abstractions/LinterTest.py | 105 ++++++++++++++++++++++- 2 files changed, 132 insertions(+), 7 deletions(-) diff --git a/coalib/bearlib/abstractions/Linter.py b/coalib/bearlib/abstractions/Linter.py index 7cfc5fddbe..9358d61425 100644 --- a/coalib/bearlib/abstractions/Linter.py +++ b/coalib/bearlib/abstractions/Linter.py @@ -2,7 +2,7 @@ from functools import partial, partialmethod import logging import inspect -from itertools import chain, compress +from itertools import chain import re import shutil from subprocess import check_call, CalledProcessError, DEVNULL @@ -676,14 +676,38 @@ def run(self, filename=None, file=None, **kwargs): self.debug("Running '{}'".format( ' '.join(str(arg) for arg in arguments))) - output = run_shell_command( + result = run_shell_command( arguments, stdin=''.join(file) if options['use_stdin'] else None, cwd=self.get_config_dir()) - output = tuple(compress( - output, - (options['use_stdout'], options['use_stderr']))) + stdout, stderr = result + + if not options['use_stdout'] and stdout: + logging.warning( + '{}: Discarded stdout: {}'.format( + self.__class__.__name__, stdout)) + stdout = '' + + if not options['use_stderr'] and stderr: + logging.warning( + '{}: Discarded stderr: {}'.format( + self.__class__.__name__, stderr)) + stderr = '' + + output = None + if stdout and stderr: + output = stdout, stderr + elif stdout: + output = stdout, + elif stderr: + output = stderr, + else: + logging.info( + '{}: No output; skipping processing'.format( + self.__class__.__name__)) + return [] + if options['strip_ansi']: output = tuple(map(strip_ansi, output)) if len(output) == 1: diff --git a/tests/bearlib/abstractions/LinterTest.py b/tests/bearlib/abstractions/LinterTest.py index d6d87319c7..2b6b600052 100644 --- a/tests/bearlib/abstractions/LinterTest.py +++ b/tests/bearlib/abstractions/LinterTest.py @@ -298,7 +298,13 @@ def create_arguments(filename, file, config_file): uut = (linter(sys.executable, use_stdout=True) (TestLinter) (self.section, None)) - uut.run('', []) + + with self.assertLogs('', level='DEBUG') as cm: + uut.run('', []) + + self.assertEqual(cm.output, [ + 'WARNING:root:TestLinter: Discarded stderr: hello stderr\n', + ]) process_output_mock.assert_called_once_with('hello stdout\n', '', []) process_output_mock.reset_mock() @@ -306,7 +312,13 @@ def create_arguments(filename, file, config_file): uut = (linter(sys.executable, use_stdout=False, use_stderr=True) (TestLinter) (self.section, None)) - uut.run('', []) + + with self.assertLogs('', level='DEBUG') as cm: + uut.run('', []) + + self.assertEqual(cm.output, [ + 'WARNING:root:TestLinter: Discarded stdout: hello stdout\n', + ]) process_output_mock.assert_called_once_with('hello stderr\n', '', []) process_output_mock.reset_mock() @@ -320,6 +332,95 @@ def create_arguments(filename, file, config_file): process_output_mock.assert_called_once_with(('hello stdout\n', 'hello stderr\n'), '', []) + def test_no_output(self): + process_output_mock = Mock() + logging.getLogger().setLevel(logging.DEBUG) + + class TestLinter: + + @staticmethod + def process_output(output, filename, file): + process_output_mock(output, filename, file) + + @staticmethod + def create_arguments(filename, file, config_file): + return '-c', '' + + uut = (linter(sys.executable, use_stdout=True, use_stderr=True) + (TestLinter) + (self.section, None)) + + with self.assertLogs('', level='DEBUG') as cm: + uut.run('', []) + + process_output_mock.assert_not_called() + + self.assertEqual(cm.output, [ + 'INFO:root:TestLinter: No output; skipping processing', + ]) + + def test_discarded_stderr(self): + process_output_mock = Mock() + logging.getLogger().setLevel(logging.DEBUG) + + class TestLinter: + + @staticmethod + def process_output(output, filename, file): + process_output_mock(output, filename, file) + + @staticmethod + def create_arguments(filename, file, config_file): + code = '\n'.join(['import sys', + "print('hello stderr', file=sys.stderr)", + ]) + return '-c', code + + uut = (linter(sys.executable, use_stdout=True, use_stderr=False) + (TestLinter) + (self.section, None)) + + with self.assertLogs('', level='DEBUG') as cm: + uut.run('', []) + + process_output_mock.assert_not_called() + + self.assertEqual(cm.output, [ + 'WARNING:root:TestLinter: Discarded stderr: hello stderr\n', + 'INFO:root:TestLinter: No output; skipping processing', + ]) + + def test_discarded_stdout(self): + process_output_mock = Mock() + logging.getLogger().setLevel(logging.DEBUG) + + class TestLinter: + + @staticmethod + def process_output(output, filename, file): + process_output_mock(output, filename, file) + + @staticmethod + def create_arguments(filename, file, config_file): + code = '\n'.join(['import sys', + "print('hello stdout', file=sys.stdout)", + ]) + return '-c', code + + uut = (linter(sys.executable, use_stdout=False, use_stderr=True) + (TestLinter) + (self.section, None)) + + with self.assertLogs('', level='DEBUG') as cm: + uut.run('', []) + + process_output_mock.assert_not_called() + + self.assertEqual(cm.output, [ + 'WARNING:root:TestLinter: Discarded stdout: hello stdout\n', + 'INFO:root:TestLinter: No output; skipping processing', + ]) + def test_strip_ansi(self): process_output_mock = Mock()