From ba3e704168486baf60fe2ba3be67be3fb5e0b27f Mon Sep 17 00:00:00 2001
From: John Vandenberg <jayvdb@gmail.com>
Date: Tue, 30 Oct 2018 22:24:00 +0700
Subject: [PATCH] Linter: Avoid processing empty streams

If 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 https://github.com/coala/coala/issues/2711
Fixes https://github.com/coala/coala/issues/5415
---
 coalib/bearlib/abstractions/Linter.py    | 24 +++++++++++++-----
 tests/bearlib/abstractions/LinterTest.py | 31 ++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 6 deletions(-)

diff --git a/coalib/bearlib/abstractions/Linter.py b/coalib/bearlib/abstractions/Linter.py
index 4d509f4276..93446e691a 100644
--- a/coalib/bearlib/abstractions/Linter.py
+++ b/coalib/bearlib/abstractions/Linter.py
@@ -676,11 +676,12 @@ def run(self, filename=None, file=None, **kwargs):
                 self.debug("Running '{}'".format(
                     ' '.join(str(arg) for arg in arguments)))
 
-                stdout, stderr = run_shell_command(
+                result = run_shell_command(
                     arguments,
                     stdin=''.join(file) if options['use_stdin'] else None,
                     cwd=self.get_config_dir())
 
+                stdout, stderr = result
                 output = None
                 if options['use_stdout'] and options['use_stderr']:
                     output = stdout, stderr
@@ -688,16 +689,27 @@ def run(self, filename=None, file=None, **kwargs):
                     if stdout:
                         output = stdout,
                     if stderr:
-                        self.warn(f'discarded stderr: {stderr}')
+                        logging.warning(
+                            '{}: Discarded stderr: {}'.format(
+                                self.__class__.__name__, stderr))
 
                 elif options['use_stderr']:
                     if stderr:
                         output = stderr,
                     if stdout:
-                        self.info(f'discarded stdout: {stdout}')
-
-                if not stdout:
-                    self.warn(f'no output')
+                        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']:
diff --git a/tests/bearlib/abstractions/LinterTest.py b/tests/bearlib/abstractions/LinterTest.py
index d6d87319c7..d77328e520 100644
--- a/tests/bearlib/abstractions/LinterTest.py
+++ b/tests/bearlib/abstractions/LinterTest.py
@@ -320,6 +320,37 @@ 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()