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

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 61266 in the body.
You can then email your comments to 61266 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Reily Siegel <mail <at> reilysiegel.com>
To: guix-patches <at> gnu.org
Subject: [PATCH 1/1] lint: Add unused-modules linter.
Date: Sat, 4 Feb 2023 00:59:42 -0500

* 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 <at> yoctocell.xyz>
 ;;; Copyright © 2021, 2022 Maxime Devos <maximedevos <at> telenet.be>
 ;;; Copyright © 2021 Brice Waegeneire <brice <at> waegenei.re>
+;;; Copyright © 2023 Reily Siegel <mail <at> 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)))
+
 
 ;;;
 ;;; 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

Message #12 received at 61266 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Reily Siegel <mail <at> reilysiegel.com>
Cc: 61266 <at> debbugs.gnu.org
Subject: Re: bug#61266: [PATCH 1/1] lint: Add unused-modules linter.
Date: Sun, 12 Feb 2023 00:55:58 +0100

Hi Reily,

Reily Siegel <mail <at> 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’.

Message #15 received at 61266 <at> debbugs.gnu.org (full text, mbox):

From: Reily Siegel <mail <at> reilysiegel.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 61266 <at> debbugs.gnu.org
Subject: Re: bug#61266: [PATCH 1/1] lint: Add unused-modules linter.
Date: Mon, 13 Feb 2023 13:20:49 -0500

Ludovic Courtès <ludo <at> 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

Message #20 received at 61266-done <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Reily Siegel <mail <at> reilysiegel.com>
Cc: 61266-done <at> debbugs.gnu.org
Subject: Re: bug#61266: [PATCH 1/1] lint: Add unused-modules linter.
Date: Mon, 20 Feb 2023 11:53:45 +0100

Hi Reily,

Reily Siegel <mail <at> reilysiegel.com> skribis:

> Ludovic Courtès <ludo <at> 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’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.