diff mbox series

[bug#67564,v2,3/3] guix: import: Parse cabal layout blocks correctly

Message ID d72b586f76a03b9bfa1e17698c9f58312dbea147.1701537651.git.saku@laesvuori.fi
State New
Headers show
Series [bug#67564,v2,1/3] guix: import: hackage: Fix crash on recursive import | expand

Commit Message

Saku Laesvuori Dec. 2, 2023, 5:23 p.m. UTC 
Cabal consideres lines to be part of a layout block if they are indented
at least one space more than the field line the block belongs to.
Previously Guix considered lines to be a part of the block if they were
indented at least as much as the first line in it.

This also makes a workaround that enabled if statements to have multiple
elses redundant and removes it.

Fixes: https://issues.guix.gnu.org/35743

* guix/import/cabal.scm (current-indentation*): Renamed from
current-indentation.
(previous-indentation, current-indentation): New variables.
(make-cabal-parser): Remove outdated comment.
[open]: Use previous-indentation + 1 instead of
current-indentation.
[elif-else]: Split to elif and else to allow only one ELSE in an if
statement.
(read-cabal)[parameterize]: Use current-indentation* and previous-indentation.
* tests/hackage.scm (hackage->guix-package test mixed layout): Expect to
  pass.

Change-Id: I3a1495b1588a022fabbfe8dad9f3231e578af4f3
---
 guix/import/cabal.scm | 42 +++++++++++++++++++-----------------------
 tests/hackage.scm     |  2 --
 2 files changed, 19 insertions(+), 25 deletions(-)
diff mbox series

Patch

diff --git a/guix/import/cabal.scm b/guix/import/cabal.scm
index fe03c30254..b969197455 100644
--- a/guix/import/cabal.scm
+++ b/guix/import/cabal.scm
@@ -130,8 +130,17 @@  (define (context-stack-pop!) ((context-stack) 'pop!))
 
 (define (context-stack-clear!) ((context-stack) 'clear!))
 
-;; Indentation of the line being parsed.
-(define current-indentation (make-parameter 0))
+;; Indentation of the line being parsed and that of the previous line.
+(define current-indentation* (make-parameter 0))
+
+(define previous-indentation (make-parameter 0))
+
+(define* (current-indentation #:optional value)
+  (if value
+    (begin
+      (previous-indentation (current-indentation*))
+      (current-indentation* value))
+    (current-indentation*)))
 
 ;; Signal to reprocess the beginning of line, in case we need to close more
 ;; than one indentation level.
@@ -196,27 +205,13 @@  (define (make-cabal-parser)
                 (exprs elif-else)          : (append $1 (list ($2 '(()))))
                 (elif-else)                : (list ($1 '(()))))
    ;; LALR(1) parsers prefer to be left-recursive, which make if-statements slightly involved.
-   ;; XXX: This technically allows multiple else statements.
-   (elif-else   (elif-else ELIF tests OCURLY exprs CCURLY) : (lambda (y) ($1 (list (append (list 'if $3 $5) y))))
-                (elif-else ELIF tests open exprs close) : (lambda (y) ($1 (list (append (list 'if $3 $5) y))))
-                (elif-else ELSE OCURLY exprs CCURLY) : (lambda (y) ($1 (list $4)))
-                ;; The 'open' token after 'tests' is shifted after an 'exprs'
-                ;; is found.  This is because, instead of 'exprs' a 'OCURLY'
-                ;; token is a valid alternative.  For this reason, 'open'
-                ;; pushes a <parse-context> with a line indentation equal to
-                ;; the indentation of 'exprs'.
-                ;;
-                ;; Differently from this, without the rule above this
-                ;; comment, when an 'ELSE' token is found, the 'open' token
-                ;; following the 'ELSE' would be shifted immediately, before
-                ;; the 'exprs' is found (because there are no other valid
-                ;; tokens).  The 'open' would therefore create a
-                ;; <parse-context> with the indentation of 'ELSE' and not
-                ;; 'exprs', creating an inconsistency.  We therefore allow
-                ;; mixed style conditionals.
-                (elif-else ELSE open exprs close) : (lambda (y) ($1 (list $4)))
+   (elif        (elif ELIF tests OCURLY exprs CCURLY) : (lambda (y) ($1 (list (append (list 'if $3 $5) y))))
+                (elif ELIF tests open exprs close) : (lambda (y) ($1 (list (append (list 'if $3 $5) y))))
                 ;; Terminating rule.
                 (if-then) : (lambda (y) (append $1 y)))
+   (elif-else   (elif ELSE OCURLY exprs CCURLY) : (lambda (y) ($1 (list $4)))
+                (elif ELSE open exprs close)    : (lambda (y) ($1 (list $4)))
+                (elif)                          : $1)
    (if-then     (IF tests OCURLY exprs CCURLY) : (list 'if $2 $4)
                 (IF tests open exprs close)    : (list 'if $2 $4))
    (tests       (TEST OPAREN ID CPAREN)        : `(,$1 ,$3)
@@ -237,7 +232,7 @@  (define (make-cabal-parser)
                (OPAREN tests CPAREN)           : $2)
    (open       () : (context-stack-push!
                                    (make-parse-context (context layout)
-                                                       (current-indentation))))
+                                                       (+ 1 (previous-indentation)))))
    (close      (VCCURLY))))
 
 (define (peek-next-line-indent port)
@@ -655,7 +650,8 @@  (define* (read-cabal #:optional (port (current-input-port))
   (let ((cabal-parser (make-cabal-parser)))
     (parameterize ((cabal-file-name
                     (or file-name (port-filename port) "standard input"))
-                   (current-indentation 0)
+                   (current-indentation* 0)
+                   (previous-indentation 0)
                    (check-bol? #f)
                    (context-stack (make-stack)))
       (cabal-parser (make-lexer port) (errorp)))))
diff --git a/tests/hackage.scm b/tests/hackage.scm
index 8eea818ebd..32e5f39329 100644
--- a/tests/hackage.scm
+++ b/tests/hackage.scm
@@ -306,8 +306,6 @@  (define test-cabal-mixed-layout
   ghc-options: -Wall
 ")
 
-;; Fails: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=35743
-(test-expect-fail 1)
 (test-assert "hackage->guix-package test mixed layout"
   (eval-test-with-cabal test-cabal-mixed-layout match-ghc-foo))