Skip to content

Commit

Permalink
Linter: Avoid processing empty streams
Browse files Browse the repository at this point in the history
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 coala#5415
  • Loading branch information
jayvdb committed Oct 31, 2018
1 parent bc35d28 commit 05c173a
Show file tree
Hide file tree
Showing 2 changed files with 137 additions and 7 deletions.
38 changes: 33 additions & 5 deletions coalib/bearlib/abstractions/Linter.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -676,14 +676,42 @@ 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 stdout == '\n':
stdout = ''
if stderr == '\n':
stderr = ''

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.warning(
'{}: No output; skipping processing'.format(
self.__class__.__name__))
return []

if options['strip_ansi']:
output = tuple(map(strip_ansi, output))
if len(output) == 1:
Expand Down
106 changes: 104 additions & 2 deletions tests/bearlib/abstractions/LinterTest.py
Original file line number Diff line number Diff line change
Expand Up @@ -298,15 +298,27 @@ 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()

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()
Expand All @@ -320,6 +332,96 @@ 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, [
'WARNING: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',
'WARNING: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',
'WARNING:root:TestLinter: No output; skipping processing',
])

def test_strip_ansi(self):
process_output_mock = Mock()

Expand Down

0 comments on commit 05c173a

Please sign in to comment.