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, or when
there was a non-zero exit code.

Followup enhancements might add ways for linter bears
to skip known messages where a linter is noisy, or
ignore known non-zero codes.

Fixes coala#2711
Fixes coala#5415
  • Loading branch information
jayvdb committed Oct 31, 2018
1 parent 284dfed commit 4b193aa
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 5 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
output = None
if options['use_stdout'] and options['use_stderr']:
output = stdout, stderr
elif options['use_stdout']:
if stdout:
output = stdout,
if stderr:
logging.warning(
'{}: Discarded stderr: {}'.format(
self.__class__.__name__, stderr))

elif options['use_stderr']:
if stderr:
output = stderr,
if stdout:
logging.warning(
'{}: Discarded stdout: {}'.format(
self.__class__.__name__, stdout))

if result.code:
logging.warning(
'{}: Exit code {}'.format(
self.__class__.__name__, result.code))

if not output:
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
30 changes: 30 additions & 0 deletions tests/bearlib/abstractions/LinterTest.py
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,36 @@ def create_arguments(filename, file, config_file):
process_output_mock.assert_called_once_with(('hello stdout\n',
'hello stderr\n'), '', [])

def test_discarded_stream(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)
(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_strip_ansi(self):
process_output_mock = Mock()

Expand Down

0 comments on commit 4b193aa

Please sign in to comment.