Merge pull request ipython#4504 from takluyver/inputtransformer-synta…

…xerror

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 ipythongh-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.

IPython/core/inputsplitter.py

@@ -501,8 +501,14 @@ def reset(self):
         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 @@ def _flush(transform, out):
         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 @@ def transform_cell(self, cell):
         """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 @@ def push(self, line):
         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,82 +2649,96 @@ def run_cell(self, raw_cell, store_history=False, silent=False, shell_futures=Tr
         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
         # compiler
         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 @@ def test_syntax(self):
                     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 @@ def test_syntax_multiline(self):
                         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 @@ def test_cellmagic_preempt(self):
             # 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 @@ 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 @@
 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 @@ 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 @@ def _is_complete(self, source, interactive):
             '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 @@ def process_input_line(self, line, store_history=True):
             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 @@ def interact(self, display_banner=None):
                 #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 @@ def interact(self, display_banner=None):
                 # 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)