| Message ID | 87h6w2gc8n.fsf@reilysiegel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [bug#61266,1/1] lint: Add unused-modules linter. | expand |
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’.
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
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’.
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))) + ;;; ;;; 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