bug#10483: [PATCH] cperl-mode: Avoid endless loop

["Harald Jörg" <haj@posteo.de>]

[-- Attachment #1: Type: text/plain, Size: 857 bytes --]

[Re-sent to list: my first reply went to Eli Zaretskii only.  Sorry.]
On 8/31/20 8:49 PM, Eli Zaretskii wrote:
>> Cc: 10483@debbugs.gnu.org
>> From: Harald Jörg <haj@posteo.de>
>> Date: Mon, 31 Aug 2020 20:08:06 +0200
>>
>> I was hesitating in this particular case because _if_ this issue ever
>> happens again, then the test just hangs forever.
>>
>> Is there a safeguard against this?
> 
> You could run the test in a subordinate Emacs process, and have the
> parent process wait for it to complete with a timeout.

So be it: My attempt to do this is attached, together with tests
to verify that my loop termination isn't harmful to the function.

As always, a review is welcome. I still haven't much experience
in Elisp.  I haven't found examples to steal from, and also missed
the fork/waitpid stuff which I use for such tasks in Perl.
-- 
Cheers,
haj

[-- Attachment #2: 0001-2020-09-01-Harald-J-rg-haj-posteo.de.patch --]
[-- Type: text/x-patch, Size: 7082 bytes --]

From eb41f40648ee6696acc54964f3aac364336c06e3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Harald=20J=C3=B6rg?= <haj@posteo.de>
Date: Tue, 1 Sep 2020 15:50:54 +0200
Subject: [PATCH] =?UTF-8?q?2020-09-01=20=20Harald=20J=C3=B6rg=20=20<haj@po?=
 =?UTF-8?q?steo.de>?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

 * test/lisp/progmodes/cperl-mode-tests.el (cperl-mode-test-bug-10483):
 Add this test to verify that Emacs doesn't hang over a Perl statement
 which isn't terminated on the same line.

 * test/lisp/progmodes/cperl-mode-tests.el (cperl-mode-test-indent-exp):
 Verify that the fix avoiding the endless loop doesn't stop indenting
 from working properly.

 * test/lisp/progmodes/cperl-mode-resources/cperl-indent-exp.pl:
 Code and result snippets for testing `cperl-indent-exp'
---
 .../cperl-mode-resources/cperl-indent-exp.pl  | 52 +++++++++++
 test/lisp/progmodes/cperl-mode-tests.el       | 91 ++++++++++++++++++-
 2 files changed, 139 insertions(+), 4 deletions(-)
 create mode 100644 test/lisp/progmodes/cperl-mode-resources/cperl-indent-exp.pl

diff --git a/test/lisp/progmodes/cperl-mode-resources/cperl-indent-exp.pl b/test/lisp/progmodes/cperl-mode-resources/cperl-indent-exp.pl
new file mode 100644
index 0000000000..4a9842ffa5
--- /dev/null
+++ b/test/lisp/progmodes/cperl-mode-resources/cperl-indent-exp.pl
@@ -0,0 +1,52 @@
+#!/usr/bin/env perl
+use strict;
+use warnings;
+use 5.020;
+
+# This file contains test input and expected output for the tests in
+# cperl-mode-tests.el, cperl-mode-test-indent-exp.  The code is
+# syntactically valid, but doesn't make much sense.
+
+# -------- for loop: input --------
+for my $foo (@ARGV)
+{
+...;
+}
+# -------- for loop: expected output --------
+for my $foo (@ARGV) {
+  ...;
+}
+# -------- for loop: end --------
+
+# -------- while loop: input --------
+{
+while (1)
+{
+say "boring loop";
+}
+continue
+{
+last; # no endless loop, though
+}
+}
+# -------- while loop: expected output --------
+{
+  while (1) {
+    say "boring loop";
+  } continue {
+    last; # no endless loop, though
+  }
+}
+# -------- while loop: end --------
+
+# -------- if-then-else: input --------
+if (my $foo) { bar() } elsif (quux()) { baz() } else { quuux }
+# -------- if-then-else: expected output --------
+if (my $foo) {
+  bar();
+} elsif (quux()) {
+  baz();
+} else {
+  quuux;
+}
+# -------- if-then-else: end --------
diff --git a/test/lisp/progmodes/cperl-mode-tests.el b/test/lisp/progmodes/cperl-mode-tests.el
index be8b42d99a..61412025fc 100644
--- a/test/lisp/progmodes/cperl-mode-tests.el
+++ b/test/lisp/progmodes/cperl-mode-tests.el
@@ -9,15 +9,20 @@
 
 ;;; Commentary:
 
-;; This is a collection of tests for the fontification of CPerl-mode.
-
-;; Run these tests interactively:
-;; (ert-run-tests-interactively '(tag :fontification))
+;; This is a collection of tests for CPerl-mode.
 
 ;;; Code:
 
 (defvar cperl-test-mode #'cperl-mode)
 
+(defvar cperl-mode-tests-data-directory
+  (expand-file-name "lisp/progmodes/cperl-mode-resources"
+                    (or (getenv "EMACS_TEST_DIRECTORY")
+                        (expand-file-name "../../../"
+                                          (or load-file-name
+                                              buffer-file-name))))
+  "Directory containing cperl-mode test data.")
+
 (defun cperl-test-ppss (text regexp)
   "Return the `syntax-ppss' of the first character matched by REGEXP in TEXT."
   (interactive)
@@ -48,4 +53,82 @@ cperl-mode-test-bug-42168
   (let ((code "{ $a- / $b } # /"))
     (should (equal (nth 8 (cperl-test-ppss code "/")) 7))))
 
+(defun cperl-mode-test--run-bug-10483 ()
+  "Runs a short program, intended to be under timer scrutiny.
+This function is intended to be used by an Emacs subprocess in
+batch mode.  The message buffer is used to report the result of
+running `cperl-indent-exp' for a very simple input.  The result
+is expected to be different from the input, to verify that
+indentation actually takes place.."
+  (let ((code "poop ('foo', \n'bar')")) ; see the bug report
+    (message "Test Bug#10483 started")
+    (with-temp-buffer
+      (insert code)
+      (funcall cperl-test-mode)
+      (goto-char (point-min))
+      (search-forward "poop")
+      (cperl-indent-exp)
+      (message "%s" (buffer-string)))))
+
+(ert-deftest cperl-mode-test-bug-10483 ()
+  "Verifies that a piece of code which ends in a paren without a
+statement terminato ron tne same line does not loop forever.  The
+test starts an asynchronous Emacs batch process under timeout
+control."
+  (interactive)
+  (let* ((emacs (concat invocation-directory invocation-name))
+         (test-function 'cperl-mode-test--run-bug-10483)
+         (test-function-name (symbol-name test-function))
+         (test-file (symbol-file test-function 'defun))
+         (ran-out-of-time nil)
+         (process-connection-type nil)
+         runner)
+    (with-temp-buffer
+      (with-timeout (1
+                     (delete-process runner)
+                     (setq ran-out-of-time t))
+        (setq runner (start-process "speedy"
+                                    (current-buffer)
+                                    emacs
+                                    "-batch"
+                                    "--quick"
+                                    "--load" test-file
+                                    "--funcall" test-function-name))
+        (while (accept-process-output runner)))
+      (should (equal ran-out-of-time nil))
+      (goto-char (point-min))
+      ;; just a very simple test for indentation: This should
+      ;; be rather robust with regard to indentation defaults
+      (should (string-match
+               "poop ('foo', \n      'bar')" (buffer-string))))))
+
+(ert-deftest cperl-mode-test-indent-exp ()
+  "Run various tests for `cperl-indent-exp' edge cases.
+These exercise some standard blocks and also the special
+treatment for Perl expressions where a closing paren isn't the
+end of the statement."
+  (let ((file (expand-file-name "cperl-indent-exp.pl"
+                                cperl-mode-tests-data-directory)))
+    (with-temp-buffer
+      (insert-file-contents file)
+      (goto-char (point-min))
+      (while (re-search-forward
+              (concat "^# ?-+ \\_<\\(?1:.+?\\)\\_>: input ?-+\n"
+                      "\\(?2:\\(?:.*\n\\)+?\\)"
+                      "# ?-+ \\1: expected output ?-+\n"
+                      "\\(?3:\\(?:.*\n\\)+?\\)"
+                      "# ?-+ \\1: end ?-+")
+              nil t)
+        (let ((name (match-string 1))
+              (code (match-string 2))
+              (expected (match-string 3))
+              got)
+          (with-temp-buffer
+            (insert code)
+            (goto-char (point-min))
+            (cperl-indent-exp) ; here we go!
+            (setq expected (concat "test case " name ":\n" expected))
+            (setq got (concat "test case " name ":\n" (buffer-string)))
+            (should (equal got expected))))))))
+
 ;;; cperl-mode-tests.el ends here
-- 
2.20.1