branch: externals/xr
commit 647dbaad55d661d89270c4c4c07820953ff61681
Author: Mattias Engdegård <matti...@acm.org>
Commit: Mattias Engdegård <matti...@acm.org>
Don't complain about (? (+ X))
An optional repetition where the repetition is one-or-more isn't
ambiguous, and was just seen in the wild.
Previously, we only suppressed the warning if there was a group around
the repetition.
---
xr-test.el | 2 ++
xr.el | 88 ++++++++++++++++++++++++++++++--------------------------------
2 files changed, 44 insertions(+), 46 deletions(-)
diff --git a/xr-test.el b/xr-test.el
index 90e5be2..2d5d885 100644
--- a/xr-test.el
+++ b/xr-test.el
@@ -388,6 +388,8 @@
'((2 . "Repetition of option")
(14 . "Repetition of repetition")
(25 . "Repetition of repetition"))))
+ (should (equal (xr-lint "\\(?:a+\\)?")
+ nil))
(should (equal (xr-lint "\\(a*\\)*\\(b+\\)*\\(c*\\)?\\(d+\\)?")
'((6 . "Repetition of repetition")
(13 . "Repetition of repetition")
diff --git a/xr.el b/xr.el
index f3ffdb8..6b080e1 100644
--- a/xr.el
+++ b/xr.el
@@ -546,53 +546,49 @@ like (* (* X) ... (* X))."
(let ((operator (match-string 0))
(operand (car sequence)))
(when warnings
- (cond
- ((and (consp operand)
- (or
- ;; (* (* X)), for any repetitions *
- (memq (car operand)
- '(opt zero-or-more one-or-more +? *? ??))
- ;; (* (group (* X))), for any repetitions *
- (and
- (eq (car operand) 'group)
- (null (cddr operand))
- (let ((inner (cadr operand)))
- (and (consp inner)
- (memq (car inner)
- '(opt zero-or-more one-or-more
- +? *? ??))
- ;; Except (? (group (+ X))), since that may
- ;; be legitimate.
- (not (and (equal operator "?")
- (memq (car inner)
- '(one-or-more +?)))))))))
- (let ((outer-opt (member operator '("?" "??")))
- (inner-opt (or (memq (car operand) '(opt ??))
- (and (eq (car operand) 'group)
- (memq (caadr operand)
- '(opt ??))))))
- (xr--report warnings (match-beginning 0)
- (if outer-opt
+ ;; Check both (OP (OP X)) and (OP (group (OP X))).
+ (let ((inner-op
+ (and (consp operand)
+ (if (eq (car operand) 'group)
+ (and (null (cddr operand))
+ (let ((inner (cadr operand)))
+ (and (consp inner)
+ (car inner))))
+ (car operand)))))
+ (cond
+ ((and
+ ;; (OP1 (OP2 X)), for any repetitions OP1, OP2
+ (memq inner-op '(opt zero-or-more one-or-more *? +? ??))
+ ;; Except (? (+ X)) which may be legitimate.
+ (not (and (equal operator "?")
+ (consp operand)
+ (memq inner-op '(one-or-more +?)))))
+ (let ((outer-opt (member operator '("?" "??")))
+ (inner-opt (memq inner-op '(opt ??))))
+ (xr--report warnings (match-beginning 0)
+ (if outer-opt
+ (if inner-opt
+ "Optional option"
+ "Optional repetition")
(if inner-opt
- "Optional option"
- "Optional repetition")
- (if inner-opt
- "Repetition of option"
- "Repetition of repetition")))))
- ((memq operand xr--zero-width-assertions)
- (xr--report warnings (match-beginning 0)
- (if (member operator '("?" "??"))
- "Optional zero-width assertion"
- "Repetition of zero-width assertion")))
- ((and (xr--matches-empty-p operand)
- ;; Rejecting repetition of the empty string
- ;; suppresses some false positives.
- (not (equal operand "")))
- (xr--report
- warnings (match-beginning 0)
- (if (member operator '("?" "??"))
- "Optional expression matching an empty string"
- "Repetition of expression matching an empty string"))))
+ "Repetition of option"
+ "Repetition of repetition")))))
+ ((memq operand xr--zero-width-assertions)
+ (xr--report warnings (match-beginning 0)
+ (if (member operator '("?" "??"))
+ "Optional zero-width assertion"
+ "Repetition of zero-width assertion")))
+ ((and (xr--matches-empty-p operand)
+ ;; Rejecting repetition of the empty string
+ ;; suppresses some false positives.
+ (not (equal operand "")))
+ (xr--report
+ warnings (match-beginning 0)
+ (concat
+ (if (member operator '("?" "??"))
+ "Optional expression"
+ "Repetition of expression")
+ " matching an empty string")))))
;; (* (* X) ... (* X)) etc: wrap-around subsumption
(when (member operator '("*" "+" "*?" "+?"))
(xr--check-wrap-around-repetition
