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

  • Open
  • quality assurance status badge
Details
2 participants
  • Gabriel Wicki
  • Christopher Baines
Owner
unassigned
Submitted by
Gabriel Wicki
Severity
normal
G
G
Gabriel Wicki wrote 7 days ago
[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 7 days ago
[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 7 days ago
[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 7 days ago
[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 7 days ago
[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 7 days ago
[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 7 days ago
[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 6 days ago
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 5 days ago
(no subject)
(address . control@debbugs.gnu.org)
87serjdwzq.fsf@cbaines.net
retitle 74459 [PATCH 0/8] Linter improvements (eliminate false positives)
thanks
?
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