Restrict or-in-for/and refactoring to obviously useful cases (#359)

Closes #357. Now it only fires if the last clause uses `let`, since that probably means the `or` is just serving as a way to get some simple cases out of the way before the main logic of the loop body.

Author: jackfirth

default-recommendations/for-loop-shortcuts-test.rkt

@@ -242,22 +242,29 @@ test: "andmap to for/and"
 ------------------------------
 
 
-test: "for/and with or to filter clause"
+test: "for/and with or guarding complex expression to filter clause"
 ------------------------------
-(define some-list (list 3 5 14 10 6 5 2))
+(define some-list (list 3 "foo" 5 14 "bar" 10 6 "baz" 5 2))
 (for/and ([x (in-list some-list)])
   (or (number? x)
-      (positive? x)
-      (not (even? x))
-      (< x 10)))
+      (let ([l (string-length x)])
+        (and (odd? l) (< l 10)))))
 ------------------------------
 ------------------------------
-(define some-list (list 3 5 14 10 6 5 2))
+(define some-list (list 3 "foo" 5 14 "bar" 10 6 "baz" 5 2))
 (for/and ([x (in-list some-list)]
-          #:unless (number? x)
-          #:unless (positive? x)
-          #:when (even? x))
-  (< x 10))
+          #:unless (number? x))
+  (define l (string-length x))
+  (and (odd? l) (< l 10)))
+------------------------------
+
+
+test: "for/and with or guarding simple expression not refactorable"
+------------------------------
+(define some-list (list 3 "foo" 5 14 "bar" 10 6 "baz" 5 2))
+(for/and ([x (in-list some-list)])
+  (or (number? x)
+      (string? x)))
 ------------------------------
 
 

default-recommendations/for-loop-shortcuts.rkt

@@ -408,17 +408,20 @@ return just that result."
     true-branch))
 
 
-(define-refactoring-rule or-in-for/and-to-filter-clause
-  #:description "The `or` expression in this `for` loop can be replaced by a filtering clause."
+(define-refactoring-rule or-let-in-for/and-to-filter-clause
+  #:description
+  "The `or` expression in this `for` loop can be replaced by a filtering clause, letting you use\
+ `define` instead of `let` in the loop body."
   #:literals (for/and for*/and or)
   ((~and loop-id (~or for/and for*/and))
    (~and original-clauses (clause ...))
-   (~and original-body (or condition:condition-expression ...+ last-condition)))
+   (~and original-body
+         (or condition:condition-expression ...+ last-condition:refactorable-let-expression)))
   (loop-id
    (~replacement
     (clause ... (~@ (~if condition.negated? #:when #:unless) condition.base-condition) ...)
     #:original original-clauses)
-   (~replacement last-condition #:original original-body)))
+   (~@ . (~splicing-replacement (last-condition.refactored ...) #:original original-body))))
 
 
 (define-syntax-class apply-append-refactorable-for-loop
@@ -477,7 +480,7 @@ return just that result."
            nested-for-to-for*
            nested-for/and-to-for*/and
            nested-for/or-to-for*/or
-           or-in-for/and-to-filter-clause
+           or-let-in-for/and-to-filter-clause
            ormap-to-for/or
            unless-expression-in-for-loop-to-unless-keyword
            when-expression-in-for-loop-to-when-keyword))