Subject: bug#34852: 26.1; seq-intersection ignores nil as element

From: Nicolas Petton <[email-masked]>
Date: Wed, 20 Mar 2019 21:51:30 +0100

Hi,

Here's a patch for master.

From 985aa028f05a418dddfd6394b739f42c32b01345 Mon Sep 17 00:00:00 2001
From: Nicolas Petton <[email-masked]>
Date: Wed, 20 Mar 2019 21:44:01 +0100
Subject: [PATCH] New seq-contains-p predicate (Bug#34852)

* lisp/emacs-lisp/seq.el (seq-contains-p): New predicate function.  It
is a replacement for seq-contains which cannot be used as a predicate
when a sequence contains nil values as it returns the element found.
(seq-contains): Make obsolete.

* test/lisp/emacs-lisp/seq-tests.el (test-seq-contains-p):
(test-seq-intersection-with-nil, test-seq-set-equal-p-with-nil,
test-difference-with-nil): Add regression tests.

* doc/lispref/sequences.texi (Sequence Functions): Document
seq-contains-p.
---
 doc/lispref/sequences.texi        |  9 +++++----
 lisp/emacs-lisp/seq.el            | 20 +++++++++++++++-----
 test/lisp/emacs-lisp/seq-tests.el | 21 +++++++++++++++++++++
 3 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/doc/lispref/sequences.texi b/doc/lispref/sequences.texi
index 0c3c4e3b28..a7f270c068 100644
--- a/doc/lispref/sequences.texi
+++ b/doc/lispref/sequences.texi
@@ -782,10 +782,11 @@ Sequence Functions
 @end defun

-@defun seq-contains sequence elt &optional function
-  This function returns the first element in @var{sequence} that is equal to
-@var{elt}.  If the optional argument @var{function} is non-@code{nil},
-it is a function of two arguments to use instead of the default @code{equal}.
+@defun seq-contains-p sequence elt &optional function
+  This function returns non-@code{nil} if at least one element in
+@var{sequence} is equal to @var{elt}.  If the optional argument
+@var{function} is non-@code{nil}, it is a function of two arguments to
+use instead of the default @code{equal}.
 
 @example
 @group
diff --git a/lisp/emacs-lisp/seq.el b/lisp/emacs-lisp/seq.el
index 4a811d7895..39c93e25ed 100644
--- a/lisp/emacs-lisp/seq.el
+++ b/lisp/emacs-lisp/seq.el
@@ -356,6 +356,7 @@ seq-sort-by
     count))
 
 (cl-defgeneric seq-contains (sequence elt &optional testfn)
+  (declare (obsolete "Use `seq-contains-p' instead." "27.1"))
   "Return the first element in SEQUENCE that is equal to ELT.
 Equality is defined by TESTFN if non-nil or by `equal' if nil."
   (seq-some (lambda (e)
@@ -363,11 +364,20 @@ seq-sort-by
                 e))
             sequence))
 
+(cl-defgeneric seq-contains-p (sequence elt &optional testfn)
+  "Return non-nil if SEQUENCE contains an element equal to ELT.
+Equality is defined by TESTFN if non-nil or by `equal' if nil."
+    (catch 'seq--break
+      (seq-doseq (e sequence)
+        (when (funcall (or testfn #'equal) e elt)
+          (throw 'seq--break t)))
+      nil))
+
 (cl-defgeneric seq-set-equal-p (sequence1 sequence2 &optional testfn)
   "Return non-nil if SEQUENCE1 and SEQUENCE2 contain the same elements, regardless of order.
 Equality is defined by TESTFN if non-nil or by `equal' if nil."
-  (and (seq-every-p (lambda (item1) (seq-contains sequence2 item1 testfn)) sequence1)
-       (seq-every-p (lambda (item2) (seq-contains sequence1 item2 testfn)) sequence2)))
+  (and (seq-every-p (lambda (item1) (seq-contains-p sequence2 item1 testfn)) sequence1)
+       (seq-every-p (lambda (item2) (seq-contains-p sequence1 item2 testfn)) sequence2)))
 
 (cl-defgeneric seq-position (sequence elt &optional testfn)
   "Return the index of the first element in SEQUENCE that is equal to ELT.
@@ -385,7 +395,7 @@ seq-sort-by
 TESTFN is used to compare elements, or `equal' if TESTFN is nil."
   (let ((result '()))
     (seq-doseq (elt sequence)
-      (unless (seq-contains result elt testfn)
+      (unless (seq-contains-p result elt testfn)
         (setq result (cons elt result))))
     (nreverse result)))
 
@@ -410,7 +420,7 @@ seq-sort-by
   "Return a list of the elements that appear in both SEQUENCE1 and SEQUENCE2.
 Equality is defined by TESTFN if non-nil or by `equal' if nil."
   (seq-reduce (lambda (acc elt)
-                (if (seq-contains sequence2 elt testfn)
+                (if (seq-contains-p sequence2 elt testfn)
                     (cons elt acc)
                   acc))
               (seq-reverse sequence1)
@@ -420,7 +430,7 @@ seq-sort-by
   "Return a list of the elements that appear in SEQUENCE1 but not in SEQUENCE2.
 Equality is defined by TESTFN if non-nil or by `equal' if nil."
   (seq-reduce (lambda (acc elt)
-                (if (not (seq-contains sequence2 elt testfn))
+                (if (not (seq-contains-p sequence2 elt testfn))
                     (cons elt acc)
                   acc))
               (seq-reverse sequence1)
diff --git a/test/lisp/emacs-lisp/seq-tests.el b/test/lisp/emacs-lisp/seq-tests.el
index d8f00cfea4..6522def423 100644
--- a/test/lisp/emacs-lisp/seq-tests.el
+++ b/test/lisp/emacs-lisp/seq-tests.el
@@ -185,6 +185,14 @@ test-sequences-oddp
   (with-test-sequences (seq '(3 4 5 6))
     (should (= 5 (seq-contains seq 5)))))
 
+(ert-deftest test-seq-contains-p ()
+  (with-test-sequences (seq '(3 4 5 6))
+    (should (eq (seq-contains-p seq 3) t))
+    (should-not (seq-contains-p seq 7)))
+  (with-test-sequences (seq '())
+    (should-not (seq-contains-p seq 3))
+    (should-not (seq-contains-p seq nil))))
+
 (ert-deftest test-seq-every-p ()
   (with-test-sequences (seq '(43 54 22 1))
     (should (seq-every-p (lambda (elt) t) seq))
@@ -436,5 +444,18 @@ test-sequences-oddp
     (should (equal (seq-rest lst) '(2 3)))
     (should (equal (seq-rest vec) [2 3]))))
 
+;; Regression tests for bug#34852
+(progn
+  (ert-deftest test-seq-intersection-with-nil ()
+    (should (equal (seq-intersection '(1 2 nil) '(1 nil)) '(1 nil))))
+
+  (ert-deftest test-seq-set-equal-p-with-nil ()
+    (should (seq-set-equal-p '("a" "b" nil)
+                             '(nil "b" "a"))))
+
+  (ert-deftest test-difference-with-nil ()
+    (should (equal (seq-difference '(1 nil) '(2 nil))
+                   '(1)))))
+
 (provide 'seq-tests)
 ;;; seq-tests.el ends here
-- 
2.20.1

Nico