Merge pull request #4504 from takluyver/inputtransformer-syntaxerror

Allow input transformers to raise SyntaxError, if they consider a line of input code invalid.  

The main motivating use case for this was Sage, this is a continuation of @vbraun's gh-4089.  For background, see http://python.6.x6.nabble.com/Raising-a-SyntaxError-in-InputTransformer-td5027773.html.

Also, took advantage of the opportunity to refactor and simplify `run_cell` a bit.

docs/source/whatsnew/pr/incompat-inputsplitter-source-raw-reset.rst

@@ -0,0 +1,6 b''
+* :class:`IPython.core.inputsplitter.IPythonInputSplitter` no longer has a method
+  ``source_raw_reset()``, but gains :meth:`~IPython.core.inputsplitter.IPythonInputSplitter.raw_reset`
+  instead. Use of ``source_raw_reset`` can be replaced with::
+  
+      raw = isp.source_raw
+      transformed = isp.source_reset()

docs/source/whatsnew/pr/inputtransformer-syntaxerrors.rst

@@ -0,0 +1,4 b''
+* Input transformers (see :doc:`/config/inputtransforms`) may now raise
+  :exc:`SyntaxError` if they determine that input is invalid. The input
+  transformation machinery in IPython will handle displaying the exception to
+  the user and resetting state.

IPython/core/inputsplitter.py

@@ -501,8 +501,14 b' class IPythonInputSplitter(InputSplitter):'
         self.source_raw = ''
         self.transformer_accumulating = False
         self.within_python_line = False
+
         for t in self.transforms:
-            t.reset()
+            try:
+                t.reset()
+            except SyntaxError:
+                # Nothing that calls reset() expects to handle transformer
+                # errors
+                pass
     
     def flush_transformers(self):
         def _flush(transform, out):
@@ -519,18 +525,19 b' class IPythonInputSplitter(InputSplitter):'
         if out is not None:
             self._store(out)
 
-    def source_raw_reset(self):
-        """Return input and raw source and perform a full reset.
+    def raw_reset(self):
+        """Return raw input only and perform a full reset.
         """
-        self.flush_transformers()
-        out = self.source
-        out_r = self.source_raw
+        out = self.source_raw
         self.reset()
-        return out, out_r
+        return out
     
     def source_reset(self):
-        self.flush_transformers()
-        return super(IPythonInputSplitter, self).source_reset()
+        try:
+            self.flush_transformers()
+            return self.source
+        finally:
+            self.reset()
 
     def push_accepts_more(self):
         if self.transformer_accumulating:
@@ -542,8 +549,12 b' class IPythonInputSplitter(InputSplitter):'
         """Process and translate a cell of input.
         """
         self.reset()
-        self.push(cell)
-        return self.source_reset()
+        try:
+            self.push(cell)
+            self.flush_transformers()
+            return self.source
+        finally:
+            self.reset()
 
     def push(self, lines):
         """Push one or more lines of IPython input.

IPython/core/inputtransformer.py

@@ -52,6 +52,9 b' class InputTransformer(with_metaclass(abc.ABCMeta, object)):'
         input or None if the transformer is waiting for more input.
         
         Must be overridden by subclasses.
+
+        Implementations may raise ``SyntaxError`` if the input is invalid. No
+        other exceptions may be raised.
         """
         pass
     

IPython/core/interactiveshell.py

@@ -2649,8 +2649,42 b' class InteractiveShell(SingletonConfigurable):'
         if silent:
             store_history = False
 
-        self.input_transformer_manager.push(raw_cell)
-        cell = self.input_transformer_manager.source_reset()
+        # If any of our input transformation (input_transformer_manager or
+        # prefilter_manager) raises an exception, we store it in this variable
+        # so that we can display the error after logging the input and storing
+        # it in the history.
+        preprocessing_exc_tuple = None
+        try:
+            # Static input transformations
+            cell = self.input_transformer_manager.transform_cell(raw_cell)
+        except SyntaxError:
+            preprocessing_exc_tuple = sys.exc_info()
+            cell = raw_cell  # cell has to exist so it can be stored/logged
+        else:
+            if len(cell.splitlines()) == 1:
+                # Dynamic transformations - only applied for single line commands
+                with self.builtin_trap:
+                    try:
+                        # use prefilter_lines to handle trailing newlines
+                        # restore trailing newline for ast.parse
+                        cell = self.prefilter_manager.prefilter_lines(cell) + '\n'
+                    except Exception:
+                        # don't allow prefilter errors to crash IPython
+                        preprocessing_exc_tuple = sys.exc_info()
+
+        # Store raw and processed history
+        if store_history:
+            self.history_manager.store_inputs(self.execution_count,
+                                              cell, raw_cell)
+        if not silent:
+            self.logger.log(cell, raw_cell)
+
+        # Display the exception if input processing failed.
+        if preprocessing_exc_tuple is not None:
+            self.showtraceback(preprocessing_exc_tuple)
+            if store_history:
+                self.execution_count += 1
+            return
 
         # Our own compiler remembers the __future__ environment. If we want to
         # run code with a separate __future__ environment, use the default
@@ -2658,73 +2692,53 b' class InteractiveShell(SingletonConfigurable):'
         compiler = self.compile if shell_futures else CachingCompiler()
 
         with self.builtin_trap:
-            prefilter_failed = False
-            if len(cell.splitlines()) == 1:
-                try:
-                    # use prefilter_lines to handle trailing newlines
-                    # restore trailing newline for ast.parse
-                    cell = self.prefilter_manager.prefilter_lines(cell) + '\n'
-                except AliasError as e:
-                    error(e)
-                    prefilter_failed = True
-                except Exception:
-                    # don't allow prefilter errors to crash IPython
-                    self.showtraceback()
-                    prefilter_failed = True
-
-            # Store raw and processed history
-            if store_history:
-                self.history_manager.store_inputs(self.execution_count,
-                                                  cell, raw_cell)
-            if not silent:
-                self.logger.log(cell, raw_cell)
+            cell_name = self.compile.cache(cell, self.execution_count)
 
-            if not prefilter_failed:
-                # don't run if prefilter failed
-                cell_name = self.compile.cache(cell, self.execution_count)
-
-                with self.display_trap:
+            with self.display_trap:
+                # Compile to bytecode
+                try:
+                    code_ast = compiler.ast_parse(cell, filename=cell_name)
+                except IndentationError:
+                    self.showindentationerror()
+                    if store_history:
+                        self.execution_count += 1
+                    return None
+                except (OverflowError, SyntaxError, ValueError, TypeError,
+                        MemoryError):
+                    self.showsyntaxerror()
+                    if store_history:
+                        self.execution_count += 1
+                    return None
+
+                # Apply AST transformations
+                code_ast = self.transform_ast(code_ast)
+
+                # Execute the user code
+                interactivity = "none" if silent else self.ast_node_interactivity
+                self.run_ast_nodes(code_ast.body, cell_name,
+                                   interactivity=interactivity, compiler=compiler)
+
+                # Execute any registered post-execution functions.
+                # unless we are silent
+                post_exec = [] if silent else iteritems(self._post_execute)
+
+                for func, status in post_exec:
+                    if self.disable_failing_post_execute and not status:
+                        continue
                     try:
-                        code_ast = compiler.ast_parse(cell, filename=cell_name)
-                    except IndentationError:
-                        self.showindentationerror()
-                        if store_history:
-                            self.execution_count += 1
-                        return None
-                    except (OverflowError, SyntaxError, ValueError, TypeError,
-                            MemoryError):
-                        self.showsyntaxerror()
-                        if store_history:
-                            self.execution_count += 1
-                        return None
-                    
-                    code_ast = self.transform_ast(code_ast)
-                    
-                    interactivity = "none" if silent else self.ast_node_interactivity
-                    self.run_ast_nodes(code_ast.body, cell_name,
-                                       interactivity=interactivity, compiler=compiler)
-                    
-                    # Execute any registered post-execution functions.
-                    # unless we are silent
-                    post_exec = [] if silent else iteritems(self._post_execute)
-                    
-                    for func, status in post_exec:
-                        if self.disable_failing_post_execute and not status:
-                            continue
-                        try:
-                            func()
-                        except KeyboardInterrupt:
-                            print("\nKeyboardInterrupt", file=io.stderr)
-                        except Exception:
-                            # register as failing:
-                            self._post_execute[func] = False
-                            self.showtraceback()
-                            print('\n'.join([
-                                "post-execution function %r produced an error." % func,
-                                "If this problem persists, you can disable failing post-exec functions with:",
-                                "",
-                                "    get_ipython().disable_failing_post_execute = True"
-                            ]), file=io.stderr)
+                        func()
+                    except KeyboardInterrupt:
+                        print("\nKeyboardInterrupt", file=io.stderr)
+                    except Exception:
+                        # register as failing:
+                        self._post_execute[func] = False
+                        self.showtraceback()
+                        print('\n'.join([
+                            "post-execution function %r produced an error." % func,
+                            "If this problem persists, you can disable failing post-exec functions with:",
+                            "",
+                            "    get_ipython().disable_failing_post_execute = True"
+                        ]), file=io.stderr)
 
         if store_history:
             # Write output to the database. Does nothing unless

IPython/core/tests/test_inputsplitter.py

@@ -412,7 +412,8 b' class IPythonInputTestCase(InputSplitterTestCase):'
                     continue
 
                 isp.push(raw+'\n')
-                out, out_raw = isp.source_raw_reset()
+                out_raw = isp.source_raw
+                out = isp.source_reset()
                 self.assertEqual(out.rstrip(), out_t,
                         tt.pair_fail_msg.format("inputsplitter",raw, out_t, out))
                 self.assertEqual(out_raw.rstrip(), raw.rstrip())
@@ -431,7 +432,8 b' class IPythonInputTestCase(InputSplitterTestCase):'
                         isp.push(lraw)
                         raw_parts.append(lraw)
 
-                out, out_raw = isp.source_raw_reset()
+                out_raw = isp.source_raw
+                out = isp.source_reset()
                 out_t = '\n'.join(out_t_parts).rstrip()
                 raw = '\n'.join(raw_parts).rstrip()
                 self.assertEqual(out.rstrip(), out_t)
@@ -498,7 +500,8 b" if __name__ == '__main__':"
             # Here we just return input so we can use it in a test suite, but a
             # real interpreter would instead send it for execution somewhere.
             #src = isp.source; raise EOFError # dbg
-            src, raw = isp.source_raw_reset()
+            raw = isp.source_raw
+            src = isp.source_reset()
             print('Input source was:\n', src)
             print('Raw source was:\n', raw)
     except EOFError:
@@ -545,9 +548,7 b' class CellMagicsCommon(object):'
 
     def test_whole_cell(self):
         src = "%%cellm line\nbody\n"
-        sp = self.sp
-        sp.push(src)
-        out = sp.source_reset()
+        out = self.sp.transform_cell(src)
         ref = u"get_ipython().run_cell_magic({u}'cellm', {u}'line', {u}'body')\n"
         nt.assert_equal(out, py3compat.u_format(ref))
     

IPython/core/tests/test_interactiveshell.py

@@ -33,6 +33,7 b' from os.path import join'
 import nose.tools as nt
 
 # Our own
+from IPython.core.inputtransformer import InputTransformer
 from IPython.testing.decorators import skipif, skip_win32, onlyif_unicode_paths
 from IPython.testing import tools as tt
 from IPython.utils import io
@@ -674,4 +675,41 b' def test_user_expression():'
 
 
 
+class TestSyntaxErrorTransformer(unittest.TestCase):
+    """Check that SyntaxError raised by an input transformer is handled by run_cell()"""
+
+    class SyntaxErrorTransformer(InputTransformer):
+
+        def push(self, line):
+            pos = line.find('syntaxerror')
+            if pos >= 0:
+                e = SyntaxError('input contains "syntaxerror"')
+                e.text = line
+                e.offset = pos + 1
+                raise e
+            return line
+
+        def reset(self):
+            pass
+
+    def setUp(self):
+        self.transformer = TestSyntaxErrorTransformer.SyntaxErrorTransformer()
+        ip.input_splitter.python_line_transforms.append(self.transformer)
+        ip.input_transformer_manager.python_line_transforms.append(self.transformer)
+
+    def tearDown(self):
+        ip.input_splitter.python_line_transforms.remove(self.transformer)
+        ip.input_transformer_manager.python_line_transforms.remove(self.transformer)
+
+    def test_syntaxerror_input_transformer(self):
+        with tt.AssertPrints('1234'):
+            ip.run_cell('1234')
+        with tt.AssertPrints('SyntaxError: invalid syntax'):
+            ip.run_cell('1 2 3')   # plain python syntax error
+        with tt.AssertPrints('SyntaxError: input contains "syntaxerror"'):
+            ip.run_cell('2345  # syntaxerror')  # input transformer syntax error
+        with tt.AssertPrints('3456'):
+            ip.run_cell('3456')
+
+
 

IPython/qt/console/frontend_widget.py

@@ -224,7 +224,10 b' class FrontendWidget(HistoryConsoleWidget, BaseFrontendMixin):'
             'interactive' is True; otherwise, it is False.
         """
         self._input_splitter.reset()
-        complete = self._input_splitter.push(source)
+        try:
+            complete = self._input_splitter.push(source)
+        except SyntaxError:
+            return True
         if interactive:
             complete = not self._input_splitter.push_accepts_more()
         return complete

IPython/sphinxext/ipython_directive.py

@@ -343,7 +343,7 b' class EmbeddedSphinxShell(object):'
             splitter.push(line)
             more = splitter.push_accepts_more()
             if not more:
-                source_raw = splitter.source_raw_reset()[1]
+                source_raw = splitter.raw_reset()
                 self.IP.run_cell(source_raw, store_history=store_history)
         finally:
             sys.stdout = stdout

IPython/terminal/console/interactiveshell.py

@@ -464,7 +464,7 b' class ZMQTerminalInteractiveShell(TerminalInteractiveShell):'
                 #double-guard against keyboardinterrupts during kbdint handling
                 try:
                     self.write('\nKeyboardInterrupt\n')
-                    source_raw = self.input_splitter.source_raw_reset()[1]
+                    source_raw = self.input_splitter.raw_reset()
                     hlen_b4_cell = self._replace_rlhist_multiline(source_raw, hlen_b4_cell)
                     more = False
                 except KeyboardInterrupt:
@@ -486,13 +486,18 b' class ZMQTerminalInteractiveShell(TerminalInteractiveShell):'
                 # asynchronously by signal handlers, for example.
                 self.showtraceback()
             else:
-                self.input_splitter.push(line)
-                more = self.input_splitter.push_accepts_more()
+                try:
+                    self.input_splitter.push(line)
+                    more = self.input_splitter.push_accepts_more()
+                except SyntaxError:
+                    # Run the code directly - run_cell takes care of displaying
+                    # the exception.
+                    more = False
                 if (self.SyntaxTB.last_syntax_error and
                     self.autoedit_syntax):
                     self.edit_syntax_error()
                 if not more:
-                    source_raw = self.input_splitter.source_raw_reset()[1]
+                    source_raw = self.input_splitter.raw_reset()
                     hlen_b4_cell = self._replace_rlhist_multiline(source_raw, hlen_b4_cell)
                     self.run_cell(source_raw)
                 

IPython/terminal/interactiveshell.py

@@ -529,7 +529,7 b' class TerminalInteractiveShell(InteractiveShell):'
                 #double-guard against keyboardinterrupts during kbdint handling
                 try:
                     self.write('\nKeyboardInterrupt\n')
-                    source_raw = self.input_splitter.source_raw_reset()[1]
+                    source_raw = self.input_splitter.raw_reset()
                     hlen_b4_cell = \
                         self._replace_rlhist_multiline(source_raw, hlen_b4_cell)
                     more = False
@@ -552,13 +552,18 b' class TerminalInteractiveShell(InteractiveShell):'
                 # asynchronously by signal handlers, for example.
                 self.showtraceback()
             else:
-                self.input_splitter.push(line)
-                more = self.input_splitter.push_accepts_more()
+                try:
+                    self.input_splitter.push(line)
+                    more = self.input_splitter.push_accepts_more()
+                except SyntaxError:
+                    # Run the code directly - run_cell takes care of displaying
+                    # the exception.
+                    more = False
                 if (self.SyntaxTB.last_syntax_error and
                     self.autoedit_syntax):
                     self.edit_syntax_error()
                 if not more:
-                    source_raw = self.input_splitter.source_raw_reset()[1]
+                    source_raw = self.input_splitter.raw_reset()
                     self.run_cell(source_raw, store_history=True)
                     hlen_b4_cell = \
                         self._replace_rlhist_multiline(source_raw, hlen_b4_cell)

IPython/terminal/tests/test_interactivshell.py

@@ -17,12 +17,68 b' Authors'
 #-----------------------------------------------------------------------------
 # stdlib
 import sys
+import types
 import unittest
 
+from IPython.core.inputtransformer import InputTransformer
 from IPython.testing.decorators import skipif
 from IPython.utils import py3compat
 from IPython.testing import tools as tt
 
+# Decorator for interaction loop tests -----------------------------------------
+
+class mock_input_helper(object):
+    """Machinery for tests of the main interact loop.
+
+    Used by the mock_input decorator.
+    """
+    def __init__(self, testgen):
+        self.testgen = testgen
+        self.exception = None
+        self.ip = get_ipython()
+
+    def __enter__(self):
+        self.orig_raw_input = self.ip.raw_input
+        self.ip.raw_input = self.fake_input
+        return self
+
+    def __exit__(self, etype, value, tb):
+        self.ip.raw_input = self.orig_raw_input
+
+    def fake_input(self, prompt):
+        try:
+            return next(self.testgen)
+        except StopIteration:
+            self.ip.exit_now = True
+            return u''
+        except:
+            self.exception = sys.exc_info()
+            self.ip.exit_now = True
+            return u''
+
+def mock_input(testfunc):
+    """Decorator for tests of the main interact loop.
+
+    Write the test as a generator, yield-ing the input strings, which IPython
+    will see as if they were typed in at the prompt.
+    """
+    def test_method(self):
+        testgen = testfunc(self)
+        with mock_input_helper(testgen) as mih:
+            mih.ip.interact(display_banner=False)
+
+        if mih.exception is not None:
+            # Re-raise captured exception
+            etype, value, tb = mih.exception
+            import traceback
+            traceback.print_tb(tb, file=sys.stdout)
+            del tb  # Avoid reference loop
+            raise value
+
+    return test_method
+
+# Test classes -----------------------------------------------------------------
+
 class InteractiveShellTestCase(unittest.TestCase):
     def rl_hist_entries(self, rl, n):
         """Get last n readline history entries as a list"""
@@ -171,6 +227,42 b' class InteractiveShellTestCase(unittest.TestCase):'
         expected = [ py3compat.unicode_to_str(e, enc) for e in expected ]
         self.assertEqual(hist, expected)
     
+    @mock_input
+    def test_inputtransformer_syntaxerror(self):
+        ip = get_ipython()
+        transformer = SyntaxErrorTransformer()
+        ip.input_splitter.python_line_transforms.append(transformer)
+        ip.input_transformer_manager.python_line_transforms.append(transformer)
+
+        try:
+            #raise Exception
+            with tt.AssertPrints('4', suppress=False):
+                yield u'print(2*2)'
+
+            with tt.AssertPrints('SyntaxError: input contains', suppress=False):
+                yield u'print(2345) # syntaxerror'
+
+            with tt.AssertPrints('16', suppress=False):
+                yield u'print(4*4)'
+
+        finally:
+            ip.input_splitter.python_line_transforms.remove(transformer)
+            ip.input_transformer_manager.python_line_transforms.remove(transformer)
+
+
+class SyntaxErrorTransformer(InputTransformer):
+    def push(self, line):
+        pos = line.find('syntaxerror')
+        if pos >= 0:
+            e = SyntaxError('input contains "syntaxerror"')
+            e.text = line
+            e.offset = pos + 1
+            raise e
+        return line
+
+    def reset(self):
+        pass
+
 class TerminalMagicsTestCase(unittest.TestCase):
     def test_paste_magics_message(self):
         """Test that an IndentationError while using paste magics doesn't

IPython/utils/io.py

@@ -44,6 +44,11 b' class IOStream:'
         for meth in filter(clone, dir(stream)):
             setattr(self, meth, getattr(stream, meth))
 
+    def __repr__(self):
+        cls = self.__class__
+        tpl = '{mod}.{cls}({args})'
+        return tpl.format(mod=cls.__module__, cls=cls.__name__, args=self.stream)
+
     def write(self,data):
         try:
             self._swrite(data)

docs/source/config/inputtransforms.rst

@@ -46,6 +46,14 b' it gets added to both, e.g.::'
     ip.input_splitter.logical_line_transforms.append(my_transformer())
     ip.input_transformer_manager.logical_line_transforms.append(my_transformer())
 
+These transformers may raise :exc:`SyntaxError` if the input code is invalid, but
+in most cases it is clearer to pass unrecognised code through unmodified and let
+Python's own parser decide whether it is valid.
+
+.. versionchanged:: 2.0
+
+   Added the option to raise :exc:`SyntaxError`.
+
 Stateless transformations
 -------------------------