dev(ci): refactor commit linter

A new approach to make linter rules more flexible.
This commit is contained in:
Henrik Lissner 2021-09-16 02:43:13 +02:00
parent f2588b0e90
commit 650f7a82e3

View file

@ -32,199 +32,257 @@
;;; Git hooks ;;; Git hooks
(defvar doom-cli-commit-rules (defvar doom-cli-commit-rules
(list (cons "^[^\n]\\{10,\\}$" (list (fn! (&key subject)
"Subject is too short (<10) and should be more descriptive") (when (<= (length subject) 10)
(cons 'error "Subject is too short (<10) and should be more descriptive")))
(cons "^\\(\\(revert\\|bump\\)!?: \\|[^\n]\\{,72\\}\\)$" (fn! (&key subject type)
"Subject too long; <=50 is ideal, 72 is max") (unless (memq type '(bump revert))
(let ((len (length subject)))
(cond ((> len 50)
(cons 'warning
(format "Subject is %d characters; <=50 is ideal, 72 is max"
len)))
((> len 72)
(cons 'error
(format "Subject is %d characters; <=50 is ideal, 72 is max"
len)))))))
(cons (concat (fn! (&key type)
"^\\(" (unless (memq type '(bump dev docs feat fix merge module nit perf
(regexp-opt refactor release revert test tweak))
'("bump" "dev" "docs" "feat" "fix" "merge" "module" "nit" "perf" (cons 'error (format "Commit has an invalid type (%s)" type))))
"refactor" "release" "revert" "test" "tweak"))
"\\)!?[^ :]*: ")
"Invalid type")
(cons (lambda () (fn! (&key summary)
(when (re-search-forward "^[^ :]+: " nil t) (when (or (not (stringp summary))
(and (looking-at "[A-Z][^-]") (string-blank-p summary))
(not (looking-at "\\(SPC\\|TAB\\|ESC\\|LFD\\|DEL\\|RET\\)"))))) (cons 'error "Commit has no summary")))
"Do not capitalize the first word of the subject")
(cons (lambda () (fn! (&key summary subject)
(looking-at "^\\(bump\\|revert\\|release\\|merge\\|module\\)!?([^)]+):")) (and (stringp summary)
"This type's scope goes after the colon, not before") (string-match-p "^[A-Z][^-]" summary)
(not (string-match-p "\\(SPC\\|TAB\\|ESC\\|LFD\\|DEL\\|RET\\)" summary))
(cons 'error (format "%S in summary is capitalized; do not capitalize the summary"
(car (split-string summary " "))))))
(cons (lambda () (fn! (&key type scopes summary)
(when (looking-at "\\([^ :!(]\\)+!?(\\([^)]+\\)): ") (and (memq type '(bump revert release merge module))
(let ((type (match-string 1))) scopes
(or (member type '("bump" "revert" "merge" "module" "release")) (cons 'error
(not (format "Scopes for %s commits should go after the colon, not before"
(cl-loop type))))
with scopes =
(cl-loop for path
in (cdr (doom-module-load-path (list doom-modules-dir)))
for (_category . module)
= (doom-module-from-path path)
collect (symbol-name module))
with extra-scopes = '("cli")
with regexp-scopes = '("^&")
with type-scopes =
(pcase type
("docs"
(cons "install"
(mapcar #'file-name-base
(doom-glob doom-docs-dir "[a-z]*.org")))))
with scopes-re =
(concat (string-join regexp-scopes "\\|")
"\\|"
(regexp-opt (append type-scopes extra-scopes scopes)))
for scope in (split-string (match-string 2) ",")
if (or (not (stringp scope))
(string-match scopes-re scope))
return t))))))
"Invalid scope")
(cons (lambda () (fn! (&key type scopes)
(when (looking-at "\\([^ :!(]\\)+!?(\\([^)]+\\)): ") (unless (memq type '(bump revert merge module release))
(let ((scopes (split-string (match-string 1) ","))) (cl-loop with scopes =
(not (equal scopes (sort scopes #'string-lessp)))))) (cl-loop for path
"Scopes not in lexical order") in (cdr (doom-module-load-path (list doom-modules-dir)))
for (_category . module)
= (doom-module-from-path path)
collect (symbol-name module))
with extra-scopes = '("cli")
with regexp-scopes = '("^&")
with type-scopes =
(pcase type
(`docs
(cons "install"
(mapcar #'file-name-base
(doom-glob doom-docs-dir "[a-z]*.org")))))
with scopes-re =
(concat (string-join regexp-scopes "\\|")
"\\|"
(regexp-opt (append type-scopes extra-scopes scopes)))
for scope in scopes
if (not (string-match scopes-re scope))
collect scope into error-scopes
finally return
(when error-scopes
(cons 'error (format "Commit has invalid scope(s): %s"
error-scopes))))))
(cons (lambda () (fn! (&key scopes)
(catch 'found (unless (equal scopes (sort scopes #'string-lessp))
(unless (looking-at "\\(bump\\|revert\\|merge\\)") (cons 'error "Scopes are not in lexicographical order")))
(while (re-search-forward "^[^\n]\\{73,\\}" nil t)
;; Exclude ref lines, bump lines, or lines with URLs
(save-excursion
(or (re-search-backward "^\\(Ref\\|Close\\|Fix\\|Revert\\) " nil t)
(let ((bump-re "\\(https?://.+\\|[^/]+\\)/[^/]+@[a-z0-9]\\{12\\}"))
(re-search-backward (format "^%s -> %s$" bump-re bump-re) nil t))
(re-search-backward "https?://[^ ]+\\{73,\\}" nil t)
(throw 'found t)))))))
"Body line length exceeds 72 characters")
(cons (lambda () (fn! (&key type body)
(when (looking-at "[^ :!(]+![(:]") (unless (memq type '(bump revert merge))
(not (re-search-forward "^BREAKING CHANGE: .+" nil t)))) (catch 'result
"'!' present in type, but missing 'BREAKING CHANGE:' in body") (with-temp-buffer
(save-excursion (insert body))
(while (re-search-forward "^[^\n]\\{73,\\}" nil t)
;; Exclude ref lines, bump lines, comments, lines with URLs,
;; or indented lines
(save-excursion
(or (let ((bump-re "\\(https?://.+\\|[^/]+\\)/[^/]+@[a-z0-9]\\{12\\}"))
(re-search-backward (format "^%s -> %s$" bump-re bump-re) nil t))
(re-search-backward "https?://[^ ]+\\{73,\\}" nil t)
(re-search-backward "^\\(?:#\\| +\\)" nil t)
(throw 'result (cons 'error "Line(s) in commit body exceed 72 characters")))))))))
(cons (lambda () (fn! (&key bang body type)
(when (looking-at "[^ :!]+\\(([^)])\\)?: ") (if bang
(re-search-forward "^BREAKING CHANGE: .+" nil t))) (cond ((not (string-match-p "^BREAKING CHANGE:" body))
"'BREAKING CHANGE:' present in body, but missing '!' in type") (cons 'error "'!' present in commit type, but missing 'BREAKING CHANGE:' in body"))
((not (string-match-p "^BREAKING CHANGE: .+" body))
(cons 'error "'BREAKING CHANGE:' present in commit body, but missing explanation")))
(when (string-match-p "^BREAKING CHANGE:" body)
(cons 'error (format "'BREAKING CHANGE:' present in body, but missing '!' after %S"
type)))))
(cons (lambda () (fn! (&key type body)
(when (re-search-forward "^BREAKING CHANGE:" nil t) (and (eq type 'bump)
(not (looking-at " [^\n]+")))) (let ((bump-re "\\(?:https?://.+\\|[^/]+\\)/[^/]+@\\([a-z0-9]+\\)"))
"'BREAKING CHANGE:' present in body, but empty") (not (string-match-p (concat "^" bump-re " -> " bump-re "$")
body)))
(cons 'error "Bump commit is missing commit hash diffs")))
(cons (lambda () (fn! (&key body)
(when (looking-at "bump: ") (with-temp-buffer
(let ((bump-re "^\\(https?://.+\\|[^/]+\\)/[^/]+@[a-z0-9]\\{12\\}")) (insert body)
(re-search-forward (concat "^" bump-re " -> " bump-re "$") (catch 'result
nil t)))) (let ((bump-re "^\\(?:https?://.+\\|[^/]+\\)/[^/]+@\\([a-z0-9]+\\)"))
"Bump commit doesn't contain commit diff") (while (re-search-backward bump-re nil t)
(when (/= (length (match-string 1)) 12)
(throw 'result (cons 'error (format "Commit hash in %S must be 12 characters long"
(match-string 0))))))))))
;; TODO Add bump validations for revert: type. ;; TODO Add bump validations for revert: type.
(cons (lambda () (fn! (&key body)
(re-search-forward "^\\(\\(Fix\\|Clos\\|Revert\\)ed\\|Reference[sd]\\|Refs\\): " (when (string-match-p "^\\(\\(Fix\\|Clos\\|Revert\\)ed\\|Reference[sd]\\|Refs\\):? " body)
nil t)) (cons 'error "No present tense or imperative mood for a reference line")))
"Use present tense & imperative mood for references, and without a colon")
(cons (lambda () (fn! (&key refs)
(catch 'found (and (seq-filter (lambda (ref)
(while (re-search-forward "^\\(?:Fix\\|Close\\|Revert\\|Ref\\) \\([^\n]+\\)$" (string-match-p "^\\(\\(Fix\\|Close\\|Revert\\)\\|Ref\\): " ref))
nil t) refs)
(let ((ref (match-string 1))) (cons 'error "Colon after reference line keyword; omit the colon on Fix, Close, Revert, and Ref lines")))
(or (string-match "^\\(https?://.+\\|[^/]+/[^/]+\\)?\\(#[0-9]+\\|@[a-z0-9]+\\)" ref)
(string-match "^https?://" ref) (fn! (&key refs)
(and (string-match "^[a-z0-9]\\{12\\}$" ref) (catch 'found
(= (car (doom-call-process "git" "show" ref)) (dolist (line refs)
0)) (cl-destructuring-bind (type . ref) (split-string line " +")
(throw 'found t)))))) (setq ref (string-join ref " "))
"Invalid footer reference (should be an issue, PR, URL, or commit)") (or (string-match "^\\(https?://.+\\|[^/]+/[^/]+\\)?\\(#[0-9]+\\|@[a-z0-9]+\\)" ref)
(string-match "^https?://" ref)
(and (string-match "^[a-z0-9]\\{12\\}$" ref)
(= (car (doom-call-process "git" "show" ref))
0))
(throw 'found
(cons 'error
(format "%S is not a valid issue/PR, URL, or 12-char commit hash"
line))))))))
;; TODO Check that bump/revert SUBJECT list: 1) valid modules and 2) ;; TODO Check that bump/revert SUBJECT list: 1) valid modules and 2)
;; modules whose files are actually being touched. ;; modules whose files are actually being touched.
;; TODO Ensure your diff corraborates your SCOPE ;; TODO Ensure your diff corraborates your SCOPE
)) ))
(defun doom-cli--ci-hook-commit-msg (file) (defun doom-cli--ci-hook-commit-msg (file)
(with-temp-buffer (with-temp-buffer
(insert-file-contents file) (insert-file-contents file)
(let (errors) (doom-cli--ci--lint
(dolist (rule doom-cli-commit-rules) (list (cons
(cl-destructuring-bind (pred . msg) rule "CURRENT"
(goto-char (point-min)) (buffer-substring (point-min)
(save-match-data (and (re-search-forward "^# Please enter the commit message" nil t)
(when (if (functionp pred) (match-beginning 0))))))))
(funcall pred)
(if (stringp pred)
(not (re-search-forward pred nil t))
(error "Invalid predicate: %S" pred)))
(push msg errors)))))
(when errors
(print! (error "Your commit message failed to pass Doom's conventions, here's why:"))
(dolist (error (reverse errors))
(print-group! (print! (info error))))
(terpri)
(print! "See https://gist.github.com/hlissner/4d78e396acb897d9b2d8be07a103a854 for details")
(throw 'exit 0))
t)))
;; ;;
;;; ;;;
(defun doom-cli--ci-lint-commits (from &optional to) (defun doom-cli--ci--lint (commits)
(let ((errors? 0) (let ((errors? 0)
commits) (warnings? 0))
(with-temp-buffer (print! (start "Linting %d commits" (length commits)))
(insert (print-group!
(cdr (doom-call-process (dolist (commit commits)
"git" "log" (let (subject body refs summary type scopes bang refs errors warnings)
(format "%s..%s" from (or to "HEAD"))))) (with-temp-buffer
(while (re-search-backward "^commit \\([a-z0-9]\\{40\\}\\)" nil t) (save-excursion (insert (cdr commit)))
(push (cons (match-string 1) (setq subject (buffer-substring (point-min) (line-end-position))
(replace-regexp-in-string body (buffer-substring
"^ " "" (line-beginning-position 3)
(save-excursion (save-excursion
(buffer-substring-no-properties (or (and (re-search-forward (format "\n\n%s "
(search-forward "\n\n") (regexp-opt '("Co-authored-by:" "Signed-off-by:" "Fix" "Ref" "Close" "Revert")
(if (re-search-forward "\ncommit \\([a-z0-9]\\{40\\}\\)" nil t) t))
(match-beginning 0) nil t)
(point-max)))))) (match-beginning 1))
commits))) (point-max))))
(dolist (commit commits) refs (split-string
(with-temp-buffer (save-excursion
(let (errors) (buffer-substring
(save-excursion (insert (cdr commit))) (or (and (re-search-forward (format "\n\n%s "
(dolist (rule doom-cli-commit-rules) (regexp-opt '("Co-authored-by:" "Signed-off-by:" "Fix" "Ref" "Close" "Revert")
(save-excursion t))
(save-match-data nil t)
(cl-destructuring-bind (pred . msg) rule (match-beginning 1))
(and (cond ((functionp pred) (point-max))
(funcall pred)) (point-max)))
((stringp pred) "\n" t))
(not (re-search-forward pred nil t))) (save-match-data
((error "Invalid predicate: %S" pred))) (when (looking-at "^\\([a-zA-Z0-9_-]+\\)\\(!?\\)\\(?:(\\([^)]+\\))\\)?: \\([^\n]+\\)")
(push msg errors)))))) (setq type (intern (match-string 1))
(if (not errors) bang (equal (match-string 2) "!")
(print! (success "Commit %s") (car commit)) scopes (ignore-errors (split-string (match-string 3) ","))
summary (match-string 4)))))
(dolist (fn doom-cli-commit-rules)
(pcase (funcall fn
:bang bang
:body body
:refs refs
:scopes scopes
:subject subject
:summary summary
:type type)
(`(,type . ,msg)
(push msg (if (eq type 'error) errors warnings)))))
(if (and (null errors) (null warnings))
(print! (success "%s %s") (substring (car commit) 0 7) subject)
(print! (start "%s %s") (substring (car commit) 0 7) subject))
(print-group!
(when errors
(cl-incf errors?) (cl-incf errors?)
(print! (error "Commit %s") (car commit)) (dolist (e (reverse errors))
(print-group! (print! (error "%s" e))))
(print! "%S" (cdr commit)) (when warnings
(dolist (e (reverse errors)) (cl-incf warnings?)
(print! (error "%s" e)))))))) (dolist (e (reverse warnings))
(print! (warn "%s" e))))))))
(when (> warnings? 0)
(print! (warn "Warnings: %d") errors?))
(when (> errors? 0) (when (> errors? 0)
(print! (error "Failures: %d") errors?))
(if (not (or (> errors? 0) (> warnings? 0)))
(print! (success "There were no issues!"))
(terpri) (terpri)
(print! "%d commit(s) failed the linter" errors?) (print! "See https://docs.doomemacs.org/latest/#/developers/conventions/git-commits for details")
(terpri) (when (> errors? 0)
(print! "See https://doomemacs.org/project.org#commit-message-formatting for details") (throw 'exit 1)))))
(throw 'exit 1)))
t) (defun doom-cli--ci--read-commits ()
(let (commits)
(while (re-search-backward "^commit \\([a-z0-9]\\{40\\}\\)" nil t)
(push (cons (match-string 1)
(replace-regexp-in-string
"^ " ""
(save-excursion
(buffer-substring-no-properties
(search-forward "\n\n")
(if (re-search-forward "\ncommit \\([a-z0-9]\\{40\\}\\)" nil t)
(match-beginning 0)
(point-max))))))
commits))
commits))
(defun doom-cli--ci-lint-commits (from &optional to)
(with-temp-buffer
(insert
(cdr (doom-call-process
"git" "log"
(format "%s..%s" from (or to "HEAD")))))
(doom-cli--ci--lint (doom-cli--ci--read-commits))))