From 47212fc763788660ff9051ccee1f6fa8a0db7bdd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= <ludo@gnu.org>
Date: Thu, 16 Jan 2020 15:00:18 +0100
Subject: records: Improve reporting of "invalid field specifier" errors.

Previously users would just see:

  error: invalid field specifier

without source location or hints.

* guix/records.scm (expand): Add optional 'parent-form' parameter and
pass it to 'syntax-violation' when it is true.
(make-syntactic-constructor): Pass S as a third argument to
'report-invalid-field-specifier'.
* guix/ui.scm (report-load-error): For 'syntax-error', show SUBFORM or
FORM in the message.
* tests/records.scm ("define-record-type* & wrong field specifier"): Add
a 'subform' parameter and adjust test accordingly.
("define-record-type* & wrong field specifier, identifier"): New test.
* tests/guix-system.sh: Add test.
---
 tests/guix-system.sh | 22 +++++++++++++++++++++-
 tests/records.scm    | 34 +++++++++++++++++++++++++++++++---
 2 files changed, 52 insertions(+), 4 deletions(-)

(limited to 'tests')

diff --git a/tests/guix-system.sh b/tests/guix-system.sh
index 1b2c425725..271627c2a5 100644
--- a/tests/guix-system.sh
+++ b/tests/guix-system.sh
@@ -1,5 +1,5 @@
 # GNU Guix --- Functional package management for GNU
-# Copyright © 2014, 2015, 2016, 2017, 2018, 2019 Ludovic Courtès <ludo@gnu.org>
+# Copyright © 2014, 2015, 2016, 2017, 2018, 2019, 2020 Ludovic Courtès <ludo@gnu.org>
 # Copyright © 2017 Tobias Geerinckx-Rice <me@tobias.gr>
 # Copyright © 2018 Chris Marusich <cmmarusich@gmail.com>
 #
@@ -130,6 +130,26 @@ else
     fi
 fi
 
+cat > "$tmpfile" <<EOF
+(use-modules (gnu))                                    ; 1
+
+(operating-system                                      ; 3
+  (file-systems (cons (file-system                     ; 4
+                        (device (file-system-label "root"))
+                        (mount-point "/")              ; 6
+                        (type "ext4"))))               ; 7 (!!)
+                      %base-file-systems)
+EOF
+
+if guix system build "$tmpfile" -n 2> "$errorfile"
+then false
+else
+    # Here '%base-file-systems' appears as if it were a field specified of the
+    # enclosing 'operating-system' form due to parenthesis mismatch.
+    grep "$tmpfile:3:[0-9]\+:.*%base-file-system.*invalid field specifier" \
+	 "$errorfile"
+fi
+
 OS_BASE='
   (host-name "antelope")
   (timezone "Europe/Paris")
diff --git a/tests/records.scm b/tests/records.scm
index 16b7a9c35e..2c55a61720 100644
--- a/tests/records.scm
+++ b/tests/records.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2018, 2019 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2018, 2019, 2020 Ludovic Courtès <ludo@gnu.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -286,10 +286,11 @@
       (lambda ()
         (eval exp (test-module))
         #f)
-      (lambda (key proc message location form . args)
+      (lambda (key proc message location form subform . _)
         (and (eq? proc 'foo)
              (string-match "invalid field" message)
-             (equal? form '(baz 1 2 3 4 5))
+             (equal? subform '(baz 1 2 3 4 5))
+             (equal? form '(foo (baz 1 2 3 4 5)))
 
              ;; Make sure the location is that of the field specifier.
              ;; See <http://bugs.gnu.org/23969>.
@@ -299,6 +300,33 @@
                           ,@(alist-delete 'line loc)))
                     (pk 'actual-loc location)))))))
 
+(test-assert "define-record-type* & wrong field specifier, identifier"
+  (let ((exp '(begin
+                (define-record-type* <foo> foo make-foo
+                  foo?
+                  (bar foo-bar (default 42))
+                  (baz foo-baz))
+
+                (foo
+                 baz)))                           ;syntax error
+        (loc    (current-source-location)))       ;keep this alignment!
+    (catch 'syntax-error
+      (lambda ()
+        (eval exp (test-module))
+        #f)
+      (lambda (key proc message location form subform . _)
+        (and (eq? proc 'foo)
+             (string-match "invalid field" message)
+             (equal? subform 'baz)
+             (equal? form '(foo baz))
+
+             ;; Here the location is that of the parent form.
+             (lset= equal?
+                    (pk 'expected-loc
+                        `((line . ,(- (assq-ref loc 'line) 2))
+                          ,@(alist-delete 'line loc)))
+                    (pk 'actual-loc location)))))))
+
 (test-assert "define-record-type* & missing initializers"
   (catch 'syntax-error
     (lambda ()
-- 
cgit v1.2.3