[PATCH 0/8] Linter improvements (eliminate false positives)

  • Done
  • quality assurance status badge
Details
3 participants
  • Gabriel Wicki
  • Ludovic Courtès
  • Christopher Baines
Owner
unassigned
Submitted by
Gabriel Wicki
Severity
normal
G
G
Gabriel Wicki wrote on 21 Nov 2024 13:43
[PATCH 1/8] guix: lint: Fix indentation.
(address . 74459@debbugs.gnu.org)
x4qkln6crsokqg6r3q5ba7dzd4nmchmmde5nk476vyhbjqqu2l@dcifornpa3ca
* guix/lint.scm(check-synopsis-style): Add white space.
* tests/lint.scm: Fix indentation.
---
guix/lint.scm | 2 +-
tests/lint.scm | 16 ++++++++--------
2 files changed, 9 insertions(+), 9 deletions(-)

Toggle diff (61 lines)
diff --git a/guix/lint.scm b/guix/lint.scm
index 8c6c20c723..31d366af46 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -737,7 +737,7 @@ (define (check-synopsis-style package)
(define (check-start-with-package-name synopsis)
(if (and (regexp-exec (package-name-regexp package) synopsis)
- (not (starts-with-abbreviation? synopsis)))
+ (not (starts-with-abbreviation? synopsis)))
(list
(make-warning package
(G_ "synopsis should not start with the package name")
diff --git a/tests/lint.scm b/tests/lint.scm
index 95d82d7490..b899ebc700 100644
--- a/tests/lint.scm
+++ b/tests/lint.scm
@@ -171,14 +171,14 @@ (define (warning-contains? str warnings)
"description contains leading whitespace"
(single-lint-warning-message
(let ((pkg (dummy-package "x"
- (description " Whitespace."))))
+ (description " Whitespace."))))
(check-description-style pkg))))
(test-equal "description: trailing whitespace"
"description contains trailing whitespace"
(single-lint-warning-message
(let ((pkg (dummy-package "x"
- (description "Whitespace. "))))
+ (description "Whitespace. "))))
(check-description-style pkg))))
(test-equal "description: pluralized 'This package'"
@@ -359,18 +359,18 @@ (define (warning-contains? str warnings)
'()
(check-compiler-for-target
(dummy-package "x"
- (arguments
- (list #:make-flags
- #~(list (string-append "CC=" (cc-for-target))))))))
+ (arguments
+ (list #:make-flags
+ #~(list (string-append "CC=" (cc-for-target))))))))
(test-equal "compiler-for-target: CC=gcc is acceptable when target=#false"
'()
(check-compiler-for-target
;; This (dummy) package consists purely of architecture-independent data.
(dummy-package "tzdata"
- (arguments
- (list #:target #false
- #:make-flags #~(list "CC=gcc"))))))
+ (arguments
+ (list #:target #false
+ #:make-flags #~(list "CC=gcc"))))))
;; The emacs-build-system sets #:tests? #f by default.
(test-equal "tests-true: #:tests? #t acceptable for emacs packages"
--
2.46.0
G
G
Gabriel Wicki wrote on 21 Nov 2024 13:44
[PATCH 2/8] guix: lint: Refine description start check logic.
(address . 74459@debbugs.gnu.org)
virti5p4tcdrfczmkguwlfok6biaxqmu3q2dt737worygor2la@frziltjtdpkc
Fix linter warnings for the following:

- packages that belong to some programming language or ecosystem,
e.g. python-foo or texlive-bar,
- packages whose names end in a version distinction, e.g. wlroots-0.16 and
- packages where the software's real name contains an underscore `_'
character where our package name replaced that with a hyphen `-',
e.g. wpa_supplicant and wpa-supplicant-minimal.

* guix/lint.scm (check-description-style)[check-proper-start]: Add conditions.
* tests/lint.scm: New tests.

Change-Id: Ifc9f5cda04db59e460e287cd93afae89c7f17e3c
---
guix/lint.scm | 24 +++++++++++++++---------
tests/lint.scm | 25 +++++++++++++++++++++++++
2 files changed, 40 insertions(+), 9 deletions(-)

Toggle diff (87 lines)
diff --git a/guix/lint.scm b/guix/lint.scm
index 31d366af46..39edf93219 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -14,6 +14,7 @@
;;; Copyright � 2021 Xinglu Chen <public@yoctocell.xyz>
;;; Copyright � 2021-2023 Maxime Devos <maximedevos@telenet.be>
;;; Copyright � 2021 Brice Waegeneire <brice@waegenei.re>
+;;; Copyright � 2024 Gabriel Wicki <gabriel@erlikon.ch>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -437,15 +438,20 @@ (define (check-description-style package)
'()))
(define (check-proper-start description)
- (if (or (string-null? description)
- (properly-starts-sentence? description)
- (string-prefix-ci? (package-name package) description))
- '()
- (list
- (make-warning
- package
- (G_ "description should start with an upper-case letter or digit")
- #:field 'description))))
+ (let* ((initial (car (string-split description #\space)))
+ (first-word
+ (regexp-substitute/global #f "_" initial
+ 'pre "-" 'post)))
+ (if (or (string-null? description)
+ (properly-starts-sentence? description)
+ (string-prefix-ci? first-word (package-name package))
+ (string-suffix-ci? first-word (package-name package)))
+ '()
+ (list
+ (make-warning
+ package
+ (G_ "description should start with an upper-case letter or digit")
+ #:field 'description)))))
(define (check-end-of-sentence-space description)
"Check that an end-of-sentence period is followed by two spaces."
diff --git a/tests/lint.scm b/tests/lint.scm
index b899ebc700..9297bfbaac 100644
--- a/tests/lint.scm
+++ b/tests/lint.scm
@@ -11,6 +11,7 @@
;;; Copyright � 2020 Tobias Geerinckx-Rice <me@tobias.gr>
;;; Copyright � 2021 Xinglu Chen <public@yoctocell.xyz>
;;; Copyright � 2021, 2023 Maxime Devos <maximedevos@telenet.be>
+;;; Copyright � 2024 Gabriel Wicki <gabriel@erlikon.ch>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -132,6 +133,30 @@ (define (warning-contains? str warnings)
(description "x is a dummy package."))))
(check-description-style pkg)))
+(test-equal "description: may start with beginning of package name"
+ '()
+ (let ((pkg (dummy-package "xyz-0.1"
+ (description "xyz is a dummy package."))))
+ (check-description-style pkg)))
+
+(test-equal "description: may start with end of package name"
+ '()
+ (let ((pkg (dummy-package "foobar-xyz"
+ (description "xyz is a dummy package."))))
+ (check-description-style pkg)))
+
+(test-equal "description: may start with non-hyphenated package name"
+ '()
+ (let ((pkg (dummy-package "foobar-xyz-minimal"
+ (description "foobar_xyz is a dummy package."))))
+ (check-description-style pkg)))
+
+(test-equal "description: may start with end of package name"
+ '()
+ (let ((pkg (dummy-package "foo-bar"
+ (description "bar is some thing in foo."))))
+ (check-description-style pkg)))
+
(test-equal "description: two spaces after end of sentence"
"sentences in description should be followed by two spaces; possible infraction at 3"
(single-lint-warning-message
--
2.46.0
G
G
Gabriel Wicki wrote on 21 Nov 2024 13:45
[PATCH 3/8] guix: lint: Allow texinfo markup at beginning of description.
(address . 74459@debbugs.gnu.org)
bhf7mieoarw3rcsgzox6a25b35etlj37r6ruc4ldzjpm37eotq@fdfehe6jmpqf
* guix/lint.scm(starts-with-texinfo-markup?): New function.
(check-description-style)[check-proper-start]: Add condition.
* tests/lint.scm: Add test case.

Change-Id: I674988882265d9e2041d48dba0f9627cd68bf292
---
guix/lint.scm | 8 +++++++-
tests/lint.scm | 5 +++++
2 files changed, 12 insertions(+), 1 deletion(-)

Toggle diff (51 lines)
diff --git a/guix/lint.scm b/guix/lint.scm
index 39edf93219..4ea02a7faa 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -370,6 +370,9 @@ (define (check-compiler-for-target package)
(define (properly-starts-sentence? s)
(string-match "^[(\"'`[:upper:][:digit:]]" s))
+(define (starts-with-texinfo-markup? s)
+ (string-match "^@(acronym|dfn|code|command|emph|file|quotation|samp|uref|url)\\{.*?\\}" s))
+
(define (starts-with-abbreviation? s)
"Return #t if S starts with what looks like an abbreviation or acronym."
(string-match "^[A-Z][A-Z0-9]+\\>" s))
@@ -444,6 +447,7 @@ (define (check-description-style package)
'pre "-" 'post)))
(if (or (string-null? description)
(properly-starts-sentence? description)
+ (starts-with-texinfo-markup? description)
(string-prefix-ci? first-word (package-name package))
(string-suffix-ci? first-word (package-name package)))
'()
@@ -510,7 +514,9 @@ (define (check-description-style package)
(match (check-texinfo-markup description)
((and warning (? lint-warning?)) (list warning))
(plain-description
- (check-proper-start plain-description))))
+ (if (string-prefix? "@" description)
+ '()
+ (check-proper-start plain-description)))))
(list
(make-warning package
(G_ "invalid description: ~s")
diff --git a/tests/lint.scm b/tests/lint.scm
index 9297bfbaac..df7042c470 100644
--- a/tests/lint.scm
+++ b/tests/lint.scm
@@ -121,6 +121,11 @@ (define (warning-contains? str warnings)
(description "bad description."))))
(check-description-style pkg))))
+(test-equal "description: may start with texinfo markup"
+ '()
+ (check-description-style
+ (dummy-package "x" (description "@emph{Maxwell Equations of Software}"))))
+
(test-equal "description: may start with a digit"
'()
(let ((pkg (dummy-package "x"
--
2.46.0
G
G
Gabriel Wicki wrote on 21 Nov 2024 13:46
[PATCH 4/8] guix: lint: Allow texinfo markup at beginning of synopsis.
(address . 74459@debbugs.gnu.org)
kz6wumbn5qkz47v3grpiqfedfy42wh74hbq2kai6rms4qyqd4p@lprr2xhllsho
* guix/lint.scm(check-synopsis-style)[check-proper-start]: Add condition.
* tests/lint.scm: Add test case.

Change-Id: I2509b3a4e7e51c6a274697ceb5f776c22e27c5f9
---
guix/lint.scm | 3 ++-
tests/lint.scm | 6 ++++++
2 files changed, 8 insertions(+), 1 deletion(-)

Toggle diff (33 lines)
diff --git a/guix/lint.scm b/guix/lint.scm
index 4ea02a7faa..9fa22c92cc 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -740,7 +740,8 @@ (define (check-synopsis-style package)
'()))
(define (check-proper-start synopsis)
- (if (properly-starts-sentence? synopsis)
+ (if (or (properly-starts-sentence? synopsis)
+ (starts-with-texinfo-markup? synopsis))
'()
(list
(make-warning package
diff --git a/tests/lint.scm b/tests/lint.scm
index df7042c470..6631034151 100644
--- a/tests/lint.scm
+++ b/tests/lint.scm
@@ -307,6 +307,12 @@ (define (warning-contains? str warnings)
(check-synopsis-style pkg)))
string<?))
+(test-equal "synopsis: starts with texinfo markup"
+ '()
+ (let ((pkg (dummy-package "x"
+ (synopsis "@code{help}"))))
+ (check-synopsis-style pkg)))
+
(test-equal "synopsis: too long"
"synopsis should be less than 80 characters long"
(single-lint-warning-message
--
2.46.0
G
G
Gabriel Wicki wrote on 21 Nov 2024 13:47
[PATCH 5/8] guix: lint: Prevent false positives in description typo check.
(address . 74459@debbugs.gnu.org)
pb2steq2cn233kvvmdwyvk6wjvpx3javthngwbjoaeijuwjai7@i2k7w4mglncn
* guix/lint.scm(check-description-style)[check-description-typo]: Add spaces
to match strings to prevent matching false positives, like "allows tokens" or
"prevents torpedoes".
* tests/lint.scm: Add test.

Change-Id: Ifc2ec6167a590b9d2e742dd86fecd798c4bfaa24
---
guix/lint.scm | 4 ++--
tests/lint.scm | 8 +++++++-
2 files changed, 9 insertions(+), 3 deletions(-)

Toggle diff (41 lines)
diff --git a/guix/lint.scm b/guix/lint.scm
index 9fa22c92cc..6122a9c8e3 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -504,8 +504,8 @@ (define (check-description-style package)
(check-trademarks description)
(check-description-typo description '(("This packages" . "This package")
("This modules" . "This module")
- ("allows to" . #f)
- ("permits to" . #f)))
+ ("allows to " . #f)
+ ("permits to " . #f)))
;; Use raw description for this because Texinfo rendering
;; automatically fixes end of sentence space.
(check-end-of-sentence-space description)
diff --git a/tests/lint.scm b/tests/lint.scm
index 6631034151..47e31a69bf 100644
--- a/tests/lint.scm
+++ b/tests/lint.scm
@@ -219,12 +219,18 @@ (define (warning-contains? str warnings)
(check-description-style pkg))))
(test-equal "description: grammar 'allows to'"
- "description contains typo 'allows to'"
+ "description contains typo 'allows to '"
(single-lint-warning-message
(let ((pkg (dummy-package "x"
(description "This package allows to do stuff."))))
(check-description-style pkg))))
+(test-equal "description: grammar 'allows to' 2"
+ '()
+ (let ((pkg (dummy-package "x"
+ (description "This package allows tokenization."))))
+ (check-description-style pkg)))
+
(test-equal "synopsis: not a string"
"invalid synopsis: #f"
(single-lint-warning-message
--
2.46.0
G
G
Gabriel Wicki wrote on 21 Nov 2024 13:48
[PATCH 6/8] guix: lint: Ignore initials from double space check.
(address . 74459@debbugs.gnu.org)
bdbu7esmeykn6tts7pnwjvnymdpnlpwfelewu6myxupk33jgsd@3qlccb2yc65u
Prevent false positives in initials as the are commonly used in names, e.g.
Margaret E. Hamilton - which obviously do not end sentences. Check whether a
period character `.' is preceded by at least two characters. This should save
us from false positives when linting.

* guix/lint.scm(check-description-style)[check-end-of-sentence-space] Add
condition.
* tests/lint.scm: Add test case.

Change-Id: I42a1365aaaed2afc7308b88ebd4b0720ad362761
---
guix/lint.scm | 15 ++++++++++-----
tests/lint.scm | 2 +-
2 files changed, 11 insertions(+), 6 deletions(-)

Toggle diff (41 lines)
diff --git a/guix/lint.scm b/guix/lint.scm
index 6122a9c8e3..f2e8e95e61 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -463,11 +463,16 @@ (define (check-description-style package)
(reverse (fold-matches
"\\. [A-Z]" description '()
(lambda (m r)
- ;; Filter out matches of common abbreviations.
- (if (find (lambda (s)
- (string-suffix-ci? s (match:prefix m)))
- '("i.e" "e.g" "a.k.a" "resp"))
- r (cons (match:start m) r)))))))
+ ;; Filter out matches of common abbreviations and
+ ;; initials.
+ (let ((pre (match:prefix m)))
+ (if (or
+ (string-match "[A-Z]$" pre) ;; Initial found
+ (find (lambda (s)
+ (string-suffix-ci? s pre))
+ '("i.e" "e.g" "a.k.a" "resp")))
+ r
+ (cons (match:start m) r))))))))
(if (null? infractions)
'()
(list
diff --git a/tests/lint.scm b/tests/lint.scm
index 47e31a69bf..09be160f5d 100644
--- a/tests/lint.scm
+++ b/tests/lint.scm
@@ -173,7 +173,7 @@ (define (warning-contains? str warnings)
'()
(let ((pkg (dummy-package "x"
(description
- "E.g. Foo, i.e. Bar resp. Baz (a.k.a. DVD)."))))
+ "E.g. Foo, i.e. Bar resp. Baz (a.k.a. DVD). Name O. Person"))))
(check-description-style pkg)))
(test-equal "description: may not contain trademark signs: ™"
--
2.46.0
G
G
Gabriel Wicki wrote on 21 Nov 2024 13:49
[PATCH 7/8] guix: lint: More abbreviations.
(address . 74459@debbugs.gnu.org)
xzj62uql6hfntptg7gxszbps7mc65kz6j5c5ek7aqpoz5tax5l@w2i64pv6etwz
* guix/lint.scm: Allow more common abbreviations in double-space-after
sentence check.
* tests/lint.scm: Add tests.

Change-Id: I0eedf73e5fcd0a8c67b3ae3dfa979a57fe0f6253
---
guix/lint.scm | 2 +-
tests/lint.scm | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

Toggle diff (28 lines)
diff --git a/guix/lint.scm b/guix/lint.scm
index f2e8e95e61..b1c8834c5f 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -470,7 +470,7 @@ (define (check-description-style package)
(string-match "[A-Z]$" pre) ;; Initial found
(find (lambda (s)
(string-suffix-ci? s pre))
- '("i.e" "e.g" "a.k.a" "resp")))
+ '("i.e" "e.g" "a.k.a" "resp" "cf" "al")))
r
(cons (match:start m) r))))))))
(if (null? infractions)
diff --git a/tests/lint.scm b/tests/lint.scm
index 09be160f5d..3e9dbd29db 100644
--- a/tests/lint.scm
+++ b/tests/lint.scm
@@ -173,7 +173,7 @@ (define (warning-contains? str warnings)
'()
(let ((pkg (dummy-package "x"
(description
- "E.g. Foo, i.e. Bar resp. Baz (a.k.a. DVD). Name O. Person"))))
+ "O. Person e.g. Foo, i.e. Bar resp. Baz (a.k.a. DVD). Name et al. cf. some paper."))))
(check-description-style pkg)))
(test-equal "description: may not contain trademark signs: ™"
--
2.46.0
G
G
Gabriel Wicki wrote on 21 Nov 2024 23:25
comments
(address . 74459@debbugs.gnu.org)
ilzjnvtdlufewbxi7axblxz2cdpglbafb263gu2nxaqbtc4kj2@kwofwtn5qr5b
some notes: i failed when creating the series and included a
patch that is unrelated to the changes here. should i send in a
revision?

and did the software just swallow my cover letter?

i was btw inspired to these changes when linting all package syopses and
descriptions (issue #74329)
C
C
Christopher Baines wrote on 22 Nov 2024 18:00
(no subject)
(address . control@debbugs.gnu.org)
87serjdwzq.fsf@cbaines.net
retitle 74459 [PATCH 0/8] Linter improvements (eliminate false positives)
thanks
L
L
Ludovic Courtès wrote on 29 Nov 2024 14:54
Re: [bug#74459] [PATCH 2/8] guix: lint: Refine description start check logic.
(name . Gabriel Wicki)(address . gabriel@erlikon.ch)(address . 74459@debbugs.gnu.org)
874j3q16yv.fsf@gnu.org
Hi,

Gabriel Wicki <gabriel@erlikon.ch> skribis:

Toggle quote (14 lines)
> Fix linter warnings for the following:
>
> - packages that belong to some programming language or ecosystem,
> e.g. python-foo or texlive-bar,
> - packages whose names end in a version distinction, e.g. wlroots-0.16 and
> - packages where the software's real name contains an underscore `_'
> character where our package name replaced that with a hyphen `-',
> e.g. wpa_supplicant and wpa-supplicant-minimal.
>
> * guix/lint.scm (check-description-style)[check-proper-start]: Add conditions.
> * tests/lint.scm: New tests.
>
> Change-Id: Ifc9f5cda04db59e460e287cd93afae89c7f17e3c

[...]

Toggle quote (12 lines)
> (define (check-proper-start description)
> - (if (or (string-null? description)
> - (properly-starts-sentence? description)
> - (string-prefix-ci? (package-name package) description))
> - '()
> - (list
> - (make-warning
> - package
> - (G_ "description should start with an upper-case letter or digit")
> - #:field 'description))))
> + (let* ((initial (car (string-split description #\space)))

In general we try to avoid ‘car’ and ‘cdr’:


But also, instead of traversing all of ‘description’, perhaps you could
have something like (untested):

(define (first-word str)
(let* ((str (string-trim str))
(length (or (and=> (string-index str #\space) 1+) (string-length str))))
(string-take str length)))

Ludo’.
L
L
Ludovic Courtès wrote on 29 Nov 2024 14:55
Re: [bug#74459] comments
(name . Gabriel Wicki)(address . gabriel@erlikon.ch)(address . 74459@debbugs.gnu.org)
87zfliywiw.fsf@gnu.org
Gabriel Wicki <gabriel@erlikon.ch> skribis:

Toggle quote (9 lines)
> some notes: i failed when creating the series and included a
> patch that is unrelated to the changes here. should i send in a
> revision?
>
> and did the software just swallow my cover letter?
>
> i was btw inspired to these changes when linting all package syopses and
> descriptions (issue #74329)

I think it went well!

Apart from the minor suggestions I sent, it LGTM.

Could you send updated patches?

Thanks,
Ludo’.
G
G
Gabriel Wicki wrote on 2 Dec 2024 23:34
[PATCH v2 1/7] guix: lint: Fix indentation.
(address . 74459@debbugs.gnu.org)
fsh4b5xxlrqfso36qynhgza4nhpvjle72zwzkomv3ludhpwf6d@dpnfvsjnlgex
* guix/lint.scm(check-synopsis-style): Add white space.
* tests/lint.scm: Fix indentation.
---
guix/lint.scm | 2 +-
tests/lint.scm | 16 ++++++++--------
2 files changed, 9 insertions(+), 9 deletions(-)

Toggle diff (63 lines)
diff --git a/guix/lint.scm b/guix/lint.scm
index 8c6c20c723..31d366af46 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -737,7 +737,7 @@ (define (check-synopsis-style package)
(define (check-start-with-package-name synopsis)
(if (and (regexp-exec (package-name-regexp package) synopsis)
- (not (starts-with-abbreviation? synopsis)))
+ (not (starts-with-abbreviation? synopsis)))
(list
(make-warning package
(G_ "synopsis should not start with the package name")
diff --git a/tests/lint.scm b/tests/lint.scm
index 95d82d7490..b899ebc700 100644
--- a/tests/lint.scm
+++ b/tests/lint.scm
@@ -171,14 +171,14 @@ (define (warning-contains? str warnings)
"description contains leading whitespace"
(single-lint-warning-message
(let ((pkg (dummy-package "x"
- (description " Whitespace."))))
+ (description " Whitespace."))))
(check-description-style pkg))))
(test-equal "description: trailing whitespace"
"description contains trailing whitespace"
(single-lint-warning-message
(let ((pkg (dummy-package "x"
- (description "Whitespace. "))))
+ (description "Whitespace. "))))
(check-description-style pkg))))
(test-equal "description: pluralized 'This package'"
@@ -359,18 +359,18 @@ (define (warning-contains? str warnings)
'()
(check-compiler-for-target
(dummy-package "x"
- (arguments
- (list #:make-flags
- #~(list (string-append "CC=" (cc-for-target))))))))
+ (arguments
+ (list #:make-flags
+ #~(list (string-append "CC=" (cc-for-target))))))))
(test-equal "compiler-for-target: CC=gcc is acceptable when target=#false"
'()
(check-compiler-for-target
;; This (dummy) package consists purely of architecture-independent data.
(dummy-package "tzdata"
- (arguments
- (list #:target #false
- #:make-flags #~(list "CC=gcc"))))))
+ (arguments
+ (list #:target #false
+ #:make-flags #~(list "CC=gcc"))))))
;; The emacs-build-system sets #:tests? #f by default.
(test-equal "tests-true: #:tests? #t acceptable for emacs packages"

base-commit: 33665c52c4670bc3b4d337c89ac9cc6c4c69b26f
--
2.46.0
G
G
Gabriel Wicki wrote on 2 Dec 2024 23:35
[PATCH v2 2/7] guix: lint: Refine description start check logic.
(address . 74459@debbugs.gnu.org)
m4upzdd34ue3oxpydpt65yaah2r66zfflwzt62byzbuiqxkv5o@iu5ocslqhrop
Fix linter warnings for the following:

- packages that belong to some programming language or ecosystem,
e.g. python-foo or texlive-bar,
- packages whose names end in a version distinction, e.g. wlroots-0.16 and
- packages where the software's real name contains an underscore `_'
character where our package name replaced that with a hyphen `-',
e.g. wpa_supplicant and wpa-supplicant-minimal.

* guix/lint.scm (check-description-style)[check-proper-start]: Add conditions.
* tests/lint.scm: New tests.

Change-Id: Ifc9f5cda04db59e460e287cd93afae89c7f17e3c
---
guix/lint.scm | 27 ++++++++++++++++++---------
tests/lint.scm | 25 +++++++++++++++++++++++++
2 files changed, 43 insertions(+), 9 deletions(-)

Toggle diff (90 lines)
diff --git a/guix/lint.scm b/guix/lint.scm
index 31d366af46..63d101ebf9 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -14,6 +14,7 @@
;;; Copyright � 2021 Xinglu Chen <public@yoctocell.xyz>
;;; Copyright � 2021-2023 Maxime Devos <maximedevos@telenet.be>
;;; Copyright � 2021 Brice Waegeneire <brice@waegenei.re>
+;;; Copyright � 2024 Gabriel Wicki <gabriel@erlikon.ch>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -437,15 +438,23 @@ (define (check-description-style package)
'()))
(define (check-proper-start description)
- (if (or (string-null? description)
- (properly-starts-sentence? description)
- (string-prefix-ci? (package-name package) description))
- '()
- (list
- (make-warning
- package
- (G_ "description should start with an upper-case letter or digit")
- #:field 'description))))
+ (let* ((initial
+ (string-take description
+ (or (string-index description #\space)
+ 0)))
+ (first-word
+ (regexp-substitute/global #f "_" initial
+ 'pre "-" 'post)))
+ (if (or (string-null? description)
+ (properly-starts-sentence? description)
+ (string-prefix-ci? first-word (package-name package))
+ (string-suffix-ci? first-word (package-name package)))
+ '()
+ (list
+ (make-warning
+ package
+ (G_ "description should start with an upper-case letter or digit")
+ #:field 'description)))))
(define (check-end-of-sentence-space description)
"Check that an end-of-sentence period is followed by two spaces."
diff --git a/tests/lint.scm b/tests/lint.scm
index b899ebc700..9297bfbaac 100644
--- a/tests/lint.scm
+++ b/tests/lint.scm
@@ -11,6 +11,7 @@
;;; Copyright � 2020 Tobias Geerinckx-Rice <me@tobias.gr>
;;; Copyright � 2021 Xinglu Chen <public@yoctocell.xyz>
;;; Copyright � 2021, 2023 Maxime Devos <maximedevos@telenet.be>
+;;; Copyright � 2024 Gabriel Wicki <gabriel@erlikon.ch>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -132,6 +133,30 @@ (define (warning-contains? str warnings)
(description "x is a dummy package."))))
(check-description-style pkg)))
+(test-equal "description: may start with beginning of package name"
+ '()
+ (let ((pkg (dummy-package "xyz-0.1"
+ (description "xyz is a dummy package."))))
+ (check-description-style pkg)))
+
+(test-equal "description: may start with end of package name"
+ '()
+ (let ((pkg (dummy-package "foobar-xyz"
+ (description "xyz is a dummy package."))))
+ (check-description-style pkg)))
+
+(test-equal "description: may start with non-hyphenated package name"
+ '()
+ (let ((pkg (dummy-package "foobar-xyz-minimal"
+ (description "foobar_xyz is a dummy package."))))
+ (check-description-style pkg)))
+
+(test-equal "description: may start with end of package name"
+ '()
+ (let ((pkg (dummy-package "foo-bar"
+ (description "bar is some thing in foo."))))
+ (check-description-style pkg)))
+
(test-equal "description: two spaces after end of sentence"
"sentences in description should be followed by two spaces; possible infraction at 3"
(single-lint-warning-message
--
2.46.0
G
G
Gabriel Wicki wrote on 2 Dec 2024 23:36
[PATCH v2 3/7] guix: lint: Allow texinfo markup at beginning of description.
(address . 74459@debbugs.gnu.org)
kfgenr4mjqo3vsdg6rpuzrhwyrfzdnkuvydnjkwyv4afrvzzkx@k3weeqoimq7c
* guix/lint.scm(starts-with-texinfo-markup?): New function.
(check-description-style)[check-proper-start]: Add condition.
* tests/lint.scm: Add test case.

Change-Id: I674988882265d9e2041d48dba0f9627cd68bf292
---
guix/lint.scm | 8 +++++++-
tests/lint.scm | 5 +++++
2 files changed, 12 insertions(+), 1 deletion(-)

Toggle diff (51 lines)
diff --git a/guix/lint.scm b/guix/lint.scm
index 63d101ebf9..d6d48ad27c 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -370,6 +370,9 @@ (define (check-compiler-for-target package)
(define (properly-starts-sentence? s)
(string-match "^[(\"'`[:upper:][:digit:]]" s))
+(define (starts-with-texinfo-markup? s)
+ (string-match "^@(acronym|dfn|code|command|emph|file|quotation|samp|uref|url)\\{.*?\\}" s))
+
(define (starts-with-abbreviation? s)
"Return #t if S starts with what looks like an abbreviation or acronym."
(string-match "^[A-Z][A-Z0-9]+\\>" s))
@@ -447,6 +450,7 @@ (define (check-description-style package)
'pre "-" 'post)))
(if (or (string-null? description)
(properly-starts-sentence? description)
+ (starts-with-texinfo-markup? description)
(string-prefix-ci? first-word (package-name package))
(string-suffix-ci? first-word (package-name package)))
'()
@@ -513,7 +517,9 @@ (define (check-description-style package)
(match (check-texinfo-markup description)
((and warning (? lint-warning?)) (list warning))
(plain-description
- (check-proper-start plain-description))))
+ (if (string-prefix? "@" description)
+ '()
+ (check-proper-start plain-description)))))
(list
(make-warning package
(G_ "invalid description: ~s")
diff --git a/tests/lint.scm b/tests/lint.scm
index 9297bfbaac..df7042c470 100644
--- a/tests/lint.scm
+++ b/tests/lint.scm
@@ -121,6 +121,11 @@ (define (warning-contains? str warnings)
(description "bad description."))))
(check-description-style pkg))))
+(test-equal "description: may start with texinfo markup"
+ '()
+ (check-description-style
+ (dummy-package "x" (description "@emph{Maxwell Equations of Software}"))))
+
(test-equal "description: may start with a digit"
'()
(let ((pkg (dummy-package "x"
--
2.46.0
G
G
Gabriel Wicki wrote on 2 Dec 2024 23:37
[PATCH v2 4/7] guix: lint: Allow texinfo markup at beginning of synopsis.
(address . 74459@debbugs.gnu.org)
5z4dvh5gzjudtvzzp3pvglimxxm45uby6lnkirer2hhujy26ld@zezljqwf3342
* guix/lint.scm(check-synopsis-style)[check-proper-start]: Add condition.
* tests/lint.scm: Add test case.

Change-Id: I2509b3a4e7e51c6a274697ceb5f776c22e27c5f9
---
guix/lint.scm | 3 ++-
tests/lint.scm | 6 ++++++
2 files changed, 8 insertions(+), 1 deletion(-)

Toggle diff (33 lines)
diff --git a/guix/lint.scm b/guix/lint.scm
index d6d48ad27c..396ee01fed 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -743,7 +743,8 @@ (define (check-synopsis-style package)
'()))
(define (check-proper-start synopsis)
- (if (properly-starts-sentence? synopsis)
+ (if (or (properly-starts-sentence? synopsis)
+ (starts-with-texinfo-markup? synopsis))
'()
(list
(make-warning package
diff --git a/tests/lint.scm b/tests/lint.scm
index df7042c470..6631034151 100644
--- a/tests/lint.scm
+++ b/tests/lint.scm
@@ -307,6 +307,12 @@ (define (warning-contains? str warnings)
(check-synopsis-style pkg)))
string<?))
+(test-equal "synopsis: starts with texinfo markup"
+ '()
+ (let ((pkg (dummy-package "x"
+ (synopsis "@code{help}"))))
+ (check-synopsis-style pkg)))
+
(test-equal "synopsis: too long"
"synopsis should be less than 80 characters long"
(single-lint-warning-message
--
2.46.0
G
G
Gabriel Wicki wrote on 2 Dec 2024 23:38
[PATCH v2 5/7] guix: lint: Prevent false positives in description typo check.
(address . 74459@debbugs.gnu.org)
tqki7zilnhcjk7fplo326tqw7skzt5q2jgtgn2dinwipjxeda3@cnmdspptj7gg
* guix/lint.scm(check-description-style)[check-description-typo]: Add spaces
to match strings to prevent matching false positives, like "allows tokens" or
"prevents torpedoes".
* tests/lint.scm: Add test.

Change-Id: Ifc2ec6167a590b9d2e742dd86fecd798c4bfaa24
---
guix/lint.scm | 4 ++--
tests/lint.scm | 8 +++++++-
2 files changed, 9 insertions(+), 3 deletions(-)

Toggle diff (41 lines)
diff --git a/guix/lint.scm b/guix/lint.scm
index 396ee01fed..ee2059d812 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -507,8 +507,8 @@ (define (check-description-style package)
(check-trademarks description)
(check-description-typo description '(("This packages" . "This package")
("This modules" . "This module")
- ("allows to" . #f)
- ("permits to" . #f)))
+ ("allows to " . #f)
+ ("permits to " . #f)))
;; Use raw description for this because Texinfo rendering
;; automatically fixes end of sentence space.
(check-end-of-sentence-space description)
diff --git a/tests/lint.scm b/tests/lint.scm
index 6631034151..47e31a69bf 100644
--- a/tests/lint.scm
+++ b/tests/lint.scm
@@ -219,12 +219,18 @@ (define (warning-contains? str warnings)
(check-description-style pkg))))
(test-equal "description: grammar 'allows to'"
- "description contains typo 'allows to'"
+ "description contains typo 'allows to '"
(single-lint-warning-message
(let ((pkg (dummy-package "x"
(description "This package allows to do stuff."))))
(check-description-style pkg))))
+(test-equal "description: grammar 'allows to' 2"
+ '()
+ (let ((pkg (dummy-package "x"
+ (description "This package allows tokenization."))))
+ (check-description-style pkg)))
+
(test-equal "synopsis: not a string"
"invalid synopsis: #f"
(single-lint-warning-message
--
2.46.0
G
G
Gabriel Wicki wrote on 2 Dec 2024 23:39
[PATCH v2 6/7] guix: lint: Ignore initials from double space check.
(address . 74459@debbugs.gnu.org)
dgphupy57h3oltuscutuht43ilsy5fpqzs6m6jumutnwsiekiu@k2udrxldwy7j
Prevent false positives in initials as the are commonly used in names, e.g.
Margaret E. Hamilton - which obviously do not end sentences. Check whether a
period character `.' is preceded by at least two characters. This should save
us from false positives when linting.

* guix/lint.scm(check-description-style)[check-end-of-sentence-space] Add
condition.
* tests/lint.scm: Add test case.

Change-Id: I42a1365aaaed2afc7308b88ebd4b0720ad362761
---
guix/lint.scm | 15 ++++++++++-----
tests/lint.scm | 2 +-
2 files changed, 11 insertions(+), 6 deletions(-)

Toggle diff (41 lines)
diff --git a/guix/lint.scm b/guix/lint.scm
index ee2059d812..1c8be911eb 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -466,11 +466,16 @@ (define (check-description-style package)
(reverse (fold-matches
"\\. [A-Z]" description '()
(lambda (m r)
- ;; Filter out matches of common abbreviations.
- (if (find (lambda (s)
- (string-suffix-ci? s (match:prefix m)))
- '("i.e" "e.g" "a.k.a" "resp"))
- r (cons (match:start m) r)))))))
+ ;; Filter out matches of common abbreviations and
+ ;; initials.
+ (let ((pre (match:prefix m)))
+ (if (or
+ (string-match "[A-Z]$" pre) ;; Initial found
+ (find (lambda (s)
+ (string-suffix-ci? s pre))
+ '("i.e" "e.g" "a.k.a" "resp")))
+ r
+ (cons (match:start m) r))))))))
(if (null? infractions)
'()
(list
diff --git a/tests/lint.scm b/tests/lint.scm
index 47e31a69bf..09be160f5d 100644
--- a/tests/lint.scm
+++ b/tests/lint.scm
@@ -173,7 +173,7 @@ (define (warning-contains? str warnings)
'()
(let ((pkg (dummy-package "x"
(description
- "E.g. Foo, i.e. Bar resp. Baz (a.k.a. DVD)."))))
+ "E.g. Foo, i.e. Bar resp. Baz (a.k.a. DVD). Name O. Person"))))
(check-description-style pkg)))
(test-equal "description: may not contain trademark signs: ™"
--
2.46.0
G
G
Gabriel Wicki wrote on 2 Dec 2024 23:40
[PATCH v2 7/7] guix: lint: More abbreviations.
(address . 74459@debbugs.gnu.org)
yjqn4sluy72edcqe7kph4anx2n3shoosswdbmjb2yhgolqkazu@pfun55ub45ye
* guix/lint.scm: Allow more common abbreviations in double-space-after
sentence check.
* tests/lint.scm: Add tests.

Change-Id: I0eedf73e5fcd0a8c67b3ae3dfa979a57fe0f6253
---
guix/lint.scm | 2 +-
tests/lint.scm | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

Toggle diff (28 lines)
diff --git a/guix/lint.scm b/guix/lint.scm
index 1c8be911eb..1ea43df6b3 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -473,7 +473,7 @@ (define (check-description-style package)
(string-match "[A-Z]$" pre) ;; Initial found
(find (lambda (s)
(string-suffix-ci? s pre))
- '("i.e" "e.g" "a.k.a" "resp")))
+ '("i.e" "e.g" "a.k.a" "resp" "cf" "al")))
r
(cons (match:start m) r))))))))
(if (null? infractions)
diff --git a/tests/lint.scm b/tests/lint.scm
index 09be160f5d..3e9dbd29db 100644
--- a/tests/lint.scm
+++ b/tests/lint.scm
@@ -173,7 +173,7 @@ (define (warning-contains? str warnings)
'()
(let ((pkg (dummy-package "x"
(description
- "E.g. Foo, i.e. Bar resp. Baz (a.k.a. DVD). Name O. Person"))))
+ "O. Person e.g. Foo, i.e. Bar resp. Baz (a.k.a. DVD). Name et al. cf. some paper."))))
(check-description-style pkg)))
(test-equal "description: may not contain trademark signs: ™"
--
2.46.0
G
G
Gabriel Wicki wrote on 2 Dec 2024 23:46
Re: [bug#74459] comments
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 74459@debbugs.gnu.org)
ufwwb7m666cl6v2q3tnewqicokivjqda3xloorlcp2yc5nbkuk@xdonxddicsmj
Thanks for your review, Ludo'! I've addressed your concern and came up
with an even more readable solution -- I hope you agree.

Thanks for merging and have a nice week,

gabber
L
L
Ludovic Courtès wrote on 12 Dec 2024 12:07
(name . Gabriel Wicki)(address . gabriel@erlikon.ch)(address . 74459-done@debbugs.gnu.org)
8734itcg76.fsf@gnu.org
Hi,

Gabriel Wicki <gabriel@erlikon.ch> skribis:

Toggle quote (3 lines)
> Thanks for your review, Ludo'! I've addressed your concern and came up
> with an even more readable solution -- I hope you agree.

Yes, that’s nice. Applied, thanks!

I removed “guix:” from commit subject lines and also followed up with a
commit that pre-compiles the regexp instead of recompiling it every time
via ‘string-match’ (it can make a difference when linting a large number
of packages as is the case in the Data Service for example).

Thanks!

Ludo’.
Closed
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 74459@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 74459
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch