[bug#61266] [PATCH 1/1] lint: Add unused-modules linter.

[bug#61266] [PATCH 1/1] lint: Add unused-modules linter.

[Reily Siegel]

* guix/lint.scm (file-module): New function.
(module-dependencies): New function.
(sexp-symbols): New function.
(file-symbols): New function.
(report-unused-module): New function.
(check-unused-modules): New function.
(%local-checkers): Add check-unused-modules.
---
 guix/lint.scm | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 63 insertions(+), 1 deletion(-)

diff --git a/guix/lint.scm b/guix/lint.scm
index 8e3976171f..38c8595bd6 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -14,6 +14,7 @@
 ;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz>
 ;;; Copyright © 2021, 2022 Maxime Devos <maximedevos@telenet.be>
 ;;; Copyright © 2021 Brice Waegeneire <brice@waegenei.re>
+;;; Copyright © 2023 Reily Siegel <mail@reilysiegel.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -37,6 +38,7 @@ (define-module (guix lint)
   #:autoload   (guix base64) (base64-encode)
   #:use-module (guix build-system)
   #:use-module (guix diagnostics)
+  #:use-module (guix discovery)
   #:use-module (guix download)
   #:use-module (guix ftp-client)
   #:use-module (guix http-client)
@@ -1843,6 +1845,62 @@ (define (check-formatting package)
                                        (G_ "source file not found"))))))))
         '())))
 
+(define (file-module file)
+  "Return the resolved module defined in FILE."
+  (call-with-input-file file
+    (lambda (port)
+      (let loop ()
+        (match (read port)
+          (('define-module module . _) (resolve-module module))
+          ((? eof-object?) #f)
+          (_ (loop)))))))
+
+(define (module-dependencies module)
+  "Return an alist of (module . public-exports) for each of MODULE's imports."
+  (fold-module-public-variables*
+   (lambda (module sym _ alist)
+     (assoc-set! alist module (cons sym (or (assoc-ref alist module) '()))))
+   '()
+   (module-uses module)))
+
+(define (sexp-symbols sexp)
+  "Return all symols in SEXP."
+  (match sexp
+    ((? symbol?) (list sexp))
+    ((? list?) (apply append (map sexp-symbols sexp)))
+    (_ '())))
+
+(define (file-symbols file)
+  "Return all symbols in FILE."
+  (call-with-input-file file
+    (lambda (port)
+      (let loop ((res '()))
+        (let ((sexp (read port)))
+          (if (eof-object? sexp)
+              res
+              (loop (append res (sexp-symbols sexp)))))))))
+
+(define (report-unused-module package module)
+  "Report a warning that MODULE is not used in the file where PACKAGE is defined."
+  (make-warning package
+                (G_ "Imported module ~a is not used.")
+                (list (module-name module))))
+
+(define (check-unused-modules package)
+  "Check if the file in which PACKAGE is defined contains unused module imports."
+  (let* ((file (package-file package))
+         (symbols (file-symbols file))
+         (dependencies (module-dependencies (file-module file))))
+    (fold
+     (match-lambda*
+       (((module . publics) res)
+        (if (null? (lset-intersection eq? publics symbols))
+            (cons (report-unused-module package module)
+                  res)
+            res)))
+     '()
+     dependencies)))
+
 \f
 ;;;
 ;;; List of checkers.
@@ -1922,7 +1980,11 @@ (define %local-checkers
    (lint-checker
      (name        'formatting)
      (description "Look for formatting issues in the source")
-     (check       check-formatting))))
+     (check       check-formatting))
+   (lint-checker
+     (name        'unused-modules)
+     (description "Look for unused modules in the package file")
+     (check       check-unused-modules))))
 
 (define %network-dependent-checkers
   (list

[bug#61266] [PATCH 1/1] lint: Add unused-modules linter.

[Ludovic Courtès]

Hi Reily,

Reily Siegel <mail@reilysiegel.com> skribis:

> * guix/lint.scm (file-module): New function.
> (module-dependencies): New function.
> (sexp-symbols): New function.
> (file-symbols): New function.
> (report-unused-module): New function.
> (check-unused-modules): New function.
> (%local-checkers): Add check-unused-modules.

Nice!  This is a hot topic, discussed at the Guix Days last week!

  https://lists.gnu.org/archive/html/guix-devel/2023-02/msg00028.html

[...]

> +(define (check-unused-modules package)
> +  "Check if the file in which PACKAGE is defined contains unused module imports."
> +  (let* ((file (package-file package))
> +         (symbols (file-symbols file))
> +         (dependencies (module-dependencies (file-module file))))
> +    (fold
> +     (match-lambda*
> +       (((module . publics) res)
> +        (if (null? (lset-intersection eq? publics symbols))
> +            (cons (report-unused-module package module)
> +                  res)
> +            res)))
> +     '()
> +     dependencies)))

As you may know, this is an approximation: it doesn’t take into account
shadowed bindings, module import renamers and selections, and so on; it
might think a module is used just because a symbol with the same name as
one it exports appears somewhere in the code.  In practice, it probably
works quite well for package modules though, right?

I have just submitted a patch adding a ‘-Wunused-module’ warning to
Guile’s compiler, which avoids these issues:

  https://lists.gnu.org/archive/html/guile-devel/2023-02/msg00026.html

I wonder if we should stick to that and avoid having a lint warning
altogether.  WDYT?

Thanks,
Ludo’.

[bug#61266] [PATCH 1/1] lint: Add unused-modules linter.

[Reily Siegel]

Ludovic Courtès <ludo@gnu.org> writes:

> I have just submitted a patch adding a ‘-Wunused-module’ warning to
> Guile’s compiler, which avoids these issues:
>
>   https://lists.gnu.org/archive/html/guile-devel/2023-02/msg00026.html
>
> I wonder if we should stick to that and avoid having a lint warning
> altogether.  WDYT?

This is probably a much more workable approach. Another problem with
lint checkers that I forgot to mention in my initial patch is that they
are really designed to work on packages, not files.

--
Reily Siegel

bug#61266: [PATCH 1/1] lint: Add unused-modules linter.

[Ludovic Courtès]

Hi Reily,

Reily Siegel <mail@reilysiegel.com> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>> I have just submitted a patch adding a ‘-Wunused-module’ warning to
>> Guile’s compiler, which avoids these issues:
>>
>>   https://lists.gnu.org/archive/html/guile-devel/2023-02/msg00026.html
>>
>> I wonder if we should stick to that and avoid having a lint warning
>> altogether.  WDYT?
>
> This is probably a much more workable approach. Another problem with
> lint checkers that I forgot to mention in my initial patch is that they
> are really designed to work on packages, not files.

Alright, I’m closing this issue but let’s reopen it if we eventually
change our mind.

Thanks for looking into this!

Ludo’.