bpo-32989: IDLE - fix bad editor call of pyparse method (GH-5968)

Fix comments and add tests for editor newline_and_indent_event method.
Remove unused None default for function parameter of pyparse find_good_parse_start method
and code triggered by that default.

Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>

5 files changed, 152 insertions, 39 deletions

diff --git a/Lib/idlelib/NEWS.txt b/Lib/idlelib/NEWS.txt
index cbf55d9..9f8894e 100644
--- a/Lib/idlelib/NEWS.txt
+++ b/Lib/idlelib/NEWS.txt
@@ -3,6 +3,9 @@ Released on 2020-10-05?
 ======================================
 
 
+bpo-32989: Add tests for editor newline_and_indent_event method.
+Remove dead code from pyparse find_good_parse_start method.
+
 bpo-38943: Fix autocomplete windows not always appearing on some
 systems.  Patch by Johnny Najera.
 
diff --git a/Lib/idlelib/editor.py b/Lib/idlelib/editor.py
index 92dcf57..c9f1a16 100644
--- a/Lib/idlelib/editor.py
+++ b/Lib/idlelib/editor.py
@@ -1342,38 +1342,51 @@ class EditorWindow(object):
             text.undo_block_stop()
 
     def newline_and_indent_event(self, event):
+        """Insert a newline and indentation after Enter keypress event.
+
+        Properly position the cursor on the new line based on information
+        from the current line.  This takes into account if the current line
+        is a shell prompt, is empty, has selected text, contains a block
+        opener, contains a block closer, is a continuation line, or
+        is inside a string.
+        """
         text = self.text
         first, last = self.get_selection_indices()
         text.undo_block_start()
-        try:
+        try:  # Close undo block and expose new line in finally clause.
             if first and last:
                 text.delete(first, last)
                 text.mark_set("insert", first)
             line = text.get("insert linestart", "insert")
+
+            # Count leading whitespace for indent size.
             i, n = 0, len(line)
             while i < n and line[i] in " \t":
-                i = i+1
+                i += 1
             if i == n:
-                # the cursor is in or at leading indentation in a continuation
-                # line; just inject an empty line at the start
+                # The cursor is in or at leading indentation in a continuation
+                # line; just inject an empty line at the start.
                 text.insert("insert linestart", '\n')
                 return "break"
             indent = line[:i]
-            # strip whitespace before insert point unless it's in the prompt
+
+            # Strip whitespace before insert point unless it's in the prompt.
             i = 0
             while line and line[-1] in " \t" and line != self.prompt_last_line:
                 line = line[:-1]
-                i = i+1
+                i += 1
             if i:
                 text.delete("insert - %d chars" % i, "insert")
-            # strip whitespace after insert point
+
+            # Strip whitespace after insert point.
             while text.get("insert") in " \t":
                 text.delete("insert")
-            # start new line
+
+            # Insert new line.
             text.insert("insert", '\n')
 
-            # adjust indentation for continuations and block
-            # open/close first need to find the last stmt
+            # Adjust indentation for continuations and block open/close.
+            # First need to find the last statement.
             lno = index2line(text.index('insert'))
             y = pyparse.Parser(self.indentwidth, self.tabwidth)
             if not self.prompt_last_line:
@@ -1383,7 +1396,7 @@ class EditorWindow(object):
                     rawtext = text.get(startatindex, "insert")
                     y.set_code(rawtext)
                     bod = y.find_good_parse_start(
-                              self._build_char_in_string_func(startatindex))
+                            self._build_char_in_string_func(startatindex))
                     if bod is not None or startat == 1:
                         break
                 y.set_lo(bod or 0)
@@ -1399,26 +1412,26 @@ class EditorWindow(object):
 
             c = y.get_continuation_type()
             if c != pyparse.C_NONE:
-                # The current stmt hasn't ended yet.
+                # The current statement hasn't ended yet.
                 if c == pyparse.C_STRING_FIRST_LINE:
-                    # after the first line of a string; do not indent at all
+                    # After the first line of a string do not indent at all.
                     pass
                 elif c == pyparse.C_STRING_NEXT_LINES:
-                    # inside a string which started before this line;
-                    # just mimic the current indent
+                    # Inside a string which started before this line;
+                    # just mimic the current indent.
                     text.insert("insert", indent)
                 elif c == pyparse.C_BRACKET:
-                    # line up with the first (if any) element of the
+                    # Line up with the first (if any) element of the
                     # last open bracket structure; else indent one
                     # level beyond the indent of the line with the
-                    # last open bracket
+                    # last open bracket.
                     self.reindent_to(y.compute_bracket_indent())
                 elif c == pyparse.C_BACKSLASH:
-                    # if more than one line in this stmt already, just
+                    # If more than one line in this statement already, just
                     # mimic the current indent; else if initial line
                     # has a start on an assignment stmt, indent to
                     # beyond leftmost =; else to beyond first chunk of
-                    # non-whitespace on initial line
+                    # non-whitespace on initial line.
                     if y.get_num_lines_in_stmt() > 1:
                         text.insert("insert", indent)
                     else:
@@ -1427,9 +1440,9 @@ class EditorWindow(object):
                     assert 0, "bogus continuation type %r" % (c,)
                 return "break"
 
-            # This line starts a brand new stmt; indent relative to
+            # This line starts a brand new statement; indent relative to
             # indentation of initial line of closest preceding
-            # interesting stmt.
+            # interesting statement.
             indent = y.get_base_indent_string()
             text.insert("insert", indent)
             if y.is_block_opener():
diff --git a/Lib/idlelib/idle_test/test_editor.py b/Lib/idlelib/idle_test/test_editor.py
index 240db71..91e8ef8 100644
--- a/Lib/idlelib/idle_test/test_editor.py
+++ b/Lib/idlelib/idle_test/test_editor.py
@@ -2,6 +2,7 @@
 
 from idlelib import editor
 import unittest
+from collections import namedtuple
 from test.support import requires
 from tkinter import Tk
 
@@ -91,5 +92,103 @@ class TestGetLineIndent(unittest.TestCase):
                 )
 
 
+class IndentAndNewlineTest(unittest.TestCase):
+
+    @classmethod
+    def setUpClass(cls):
+        requires('gui')
+        cls.root = Tk()
+        cls.root.withdraw()
+        cls.window = Editor(root=cls.root)
+        cls.window.indentwidth = 2
+        cls.window.tabwidth = 2
+
+    @classmethod
+    def tearDownClass(cls):
+        cls.window._close()
+        del cls.window
+        cls.root.update_idletasks()
+        for id in cls.root.tk.call('after', 'info'):
+            cls.root.after_cancel(id)
+        cls.root.destroy()
+        del cls.root
+
+    def insert(self, text):
+        t = self.window.text
+        t.delete('1.0', 'end')
+        t.insert('end', text)
+        # Force update for colorizer to finish.
+        t.update()
+
+    def test_indent_and_newline_event(self):
+        eq = self.assertEqual
+        w = self.window
+        text = w.text
+        get = text.get
+        nl = w.newline_and_indent_event
+
+        TestInfo = namedtuple('Tests', ['label', 'text', 'expected', 'mark'])
+
+        tests = (TestInfo('Empty line inserts with no indent.',
+                          '  \n  def __init__(self):',
+                          '\n  \n  def __init__(self):\n',
+                          '1.end'),
+                 TestInfo('Inside bracket before space, deletes space.',
+                          '  def f1(self, a, b):',
+                          '  def f1(self,\n         a, b):\n',
+                          '1.14'),
+                 TestInfo('Inside bracket after space, deletes space.',
+                          '  def f1(self, a, b):',
+                          '  def f1(self,\n         a, b):\n',
+                          '1.15'),
+                 TestInfo('Inside string with one line - no indent.',
+                          '  """Docstring."""',
+                          '  """Docstring.\n"""\n',
+                          '1.15'),
+                 TestInfo('Inside string with more than one line.',
+                          '  """Docstring.\n  Docstring Line 2"""',
+                          '  """Docstring.\n  Docstring Line 2\n  """\n',
+                          '2.18'),
+                 TestInfo('Backslash with one line.',
+                          'a =\\',
+                          'a =\\\n  \n',
+                          '1.end'),
+                 TestInfo('Backslash with more than one line.',
+                          'a =\\\n          multiline\\',
+                          'a =\\\n          multiline\\\n          \n',
+                          '2.end'),
+                 TestInfo('Block opener - indents +1 level.',
+                          '  def f1(self):\n    pass',
+                          '  def f1(self):\n    \n    pass\n',
+                          '1.end'),
+                 TestInfo('Block closer - dedents -1 level.',
+                          '  def f1(self):\n    pass',
+                          '  def f1(self):\n    pass\n  \n',
+                          '2.end'),
+                 )
+
+        w.prompt_last_line = ''
+        for test in tests:
+            with self.subTest(label=test.label):
+                self.insert(test.text)
+                text.mark_set('insert', test.mark)
+                nl(event=None)
+                eq(get('1.0', 'end'), test.expected)
+
+        # Selected text.
+        self.insert('  def f1(self, a, b):\n    return a + b')
+        text.tag_add('sel', '1.17', '1.end')
+        nl(None)
+        # Deletes selected text before adding new line.
+        eq(get('1.0', 'end'), '  def f1(self, a,\n         \n    return a + b\n')
+
+        # Preserves the whitespace in shell prompt.
+        w.prompt_last_line = '>>> '
+        self.insert('>>> \t\ta =')
+        text.mark_set('insert', '1.5')
+        nl(None)
+        eq(get('1.0', 'end'), '>>> \na =\n')
+
+
 if __name__ == '__main__':
     unittest.main(verbosity=2)
diff --git a/Lib/idlelib/idle_test/test_pyparse.py b/Lib/idlelib/idle_test/test_pyparse.py
index f7154e6..a2b13c3 100644
--- a/Lib/idlelib/idle_test/test_pyparse.py
+++ b/Lib/idlelib/idle_test/test_pyparse.py
@@ -18,7 +18,7 @@ class ParseMapTest(unittest.TestCase):
         # trans is the production instance of ParseMap, used in _study1
         parser = pyparse.Parser(4, 4)
         self.assertEqual('\t a([{b}])b"c\'d\n'.translate(pyparse.trans),
-                          'xxx(((x)))x"x\'x\n')
+                         'xxx(((x)))x"x\'x\n')
 
 
 class PyParseTest(unittest.TestCase):
@@ -61,14 +61,17 @@ class PyParseTest(unittest.TestCase):
 
         # Split def across lines.
         setcode('"""This is a module docstring"""\n'
-               'class C():\n'
-               '    def __init__(self, a,\n'
-               '                 b=True):\n'
-               '        pass\n'
-               )
+                'class C():\n'
+                '    def __init__(self, a,\n'
+                '                 b=True):\n'
+                '        pass\n'
+                )
 
-        # No value sent for is_char_in_string().
-        self.assertIsNone(start())
+        # Passing no value or non-callable should fail (issue 32989).
+        with self.assertRaises(TypeError):
+            start()
+        with self.assertRaises(TypeError):
+            start(False)
 
         # Make text look like a string.  This returns pos as the start
         # position, but it's set to None.
@@ -91,10 +94,10 @@ class PyParseTest(unittest.TestCase):
         # Code without extra line break in def line - mostly returns the same
         # values.
         setcode('"""This is a module docstring"""\n'
-               'class C():\n'
-               '    def __init__(self, a, b=True):\n'
-               '        pass\n'
-               )
+                'class C():\n'
+                '    def __init__(self, a, b=True):\n'
+                '        pass\n'
+                )
         eq(start(is_char_in_string=lambda index: False), 44)
         eq(start(is_char_in_string=lambda index: index > 44), 44)
         eq(start(is_char_in_string=lambda index: index >= 44), 33)
diff --git a/Lib/idlelib/pyparse.py b/Lib/idlelib/pyparse.py
index feb57cb..9fa2010 100644
--- a/Lib/idlelib/pyparse.py
+++ b/Lib/idlelib/pyparse.py
@@ -133,8 +133,7 @@ class Parser:
         self.code = s
         self.study_level = 0
 
-    def find_good_parse_start(self, is_char_in_string=None,
-                              _synchre=_synchre):
+    def find_good_parse_start(self, is_char_in_string, _synchre=_synchre):
         """
         Return index of a good place to begin parsing, as close to the
         end of the string as possible.  This will be the start of some
@@ -149,10 +148,6 @@ class Parser:
         """
         code, pos = self.code, None
 
-        if not is_char_in_string:
-            # no clue -- make the caller pass everything
-            return None
-
         # Peek back from the end for a good place to start,
         # but don't try too often; pos will be left None, or
         # bumped to a legitimate synch point.