Add full expansion scopes to visited syntax (#279)

Closes #276. This allows a complete rework and immense simplification of the `let-to-define` logic. It also removes the need to pass the full source code analysis object to each refactoring rule, since all of the information produced by the analysis is now embedded in the syntax object handed to the refactoring rule.

This commit also adds some miscellaneous observability improvements, including more debug logging and better error messages.

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

@@ -263,7 +263,8 @@ test: "for/and with or to filter clause"
 
 test: "for/fold building hash to for/hash"
 ------------------------------
-(for/fold ([h (hash)]) ([x (in-range 0 10)])
+(for/fold ([h (hash)])
+          ([x (in-range 0 10)])
   (hash-set h x 'foo))
 ------------------------------
 ------------------------------
@@ -274,7 +275,8 @@ test: "for/fold building hash to for/hash"
 
 test: "for*/fold building hash to for*/hash"
 ------------------------------
-(for*/fold ([h (hash)]) ([x (in-range 0 10)])
+(for*/fold ([h (hash)])
+           ([x (in-range 0 10)])
   (hash-set h x 'foo))
 ------------------------------
 ------------------------------
@@ -285,18 +287,18 @@ test: "for*/fold building hash to for*/hash"
 
 test: "for/fold building hash can't be refactored when referring to hash"
 ------------------------------
-(for/fold ([h (hash)]) ([x (in-range 0 10)])
-  (displayln
-   (hash-has-key? h x))
+(for/fold ([h (hash)])
+          ([x (in-range 0 10)])
+  (displayln (hash-has-key? h x))
   (hash-set h x 'foo))
 ------------------------------
 
 
 test: "for*/fold building hash can't be refactored when referring to hash"
 ------------------------------
-(for*/fold ([h (hash)]) ([x (in-range 0 10)])
-  (displayln
-   (hash-has-key? h x))
+(for*/fold ([h (hash)])
+           ([x (in-range 0 10)])
+  (displayln (hash-has-key? h x))
   (hash-set h x 'foo))
 ------------------------------
 

default-recommendations/for-loop-shortcuts.rkt

@@ -23,6 +23,7 @@
          resyntax/default-recommendations/private/syntax-identifier-sets
          resyntax/default-recommendations/private/syntax-lines
          resyntax/private/identifier-naming
+         resyntax/private/logger
          resyntax/private/syntax-neighbors
          resyntax/private/syntax-traversal
          syntax/parse)
@@ -204,13 +205,21 @@
 
 (define-refactoring-rule for/fold-building-hash-to-for/hash
   #:description "This `for` loop is building a hash and can be simplified."
-  #:literals (for/fold for*/fold hash make-immutable-hash)
-  ((~or (~and for/fold (~bind [loop #'for/hash])) (~and for*/fold (~bind [loop #'for*/hash])))
+  #:literals (for/fold for*/fold hash make-immutable-hash hash-set)
+  ((~or (~and for/fold (~bind [loop #'for/hash]))
+        (~and for*/fold (~bind [loop #'for*/hash])))
    ([h:id (~or (hash) (make-immutable-hash))]) iteration-clauses
    body ...
    (hash-set h-usage:id key value))
-  #:when (free-identifier=? #'h #'h-usage)
-  #:when (not (set-member? (syntax-free-identifiers #'(body ...)) #'h))
+
+  ;; The expansion of for/fold is very complex, and one thing it does is mess with the accumulator ids
+  ;; and their uses such that free-identifier=? on an accumulator's use and its binder breaks. To work
+  ;; around this, we compare the hash usage and hash accumulator ids by symbol here.
+  #:when (equal? (syntax-e #'h) (syntax-e #'h-usage))
+
+  #:do [(define body-ids (syntax-free-identifiers #'(body ...)))]
+  #:when (and (not (set-member? body-ids #'h))
+              (not (set-member? body-ids #'h-usage)))
   (loop iteration-clauses body ... (values key value)))
 
 

default-recommendations/function-definition-shortcuts.rkt

@@ -80,14 +80,26 @@
 (define-refactoring-rule define-case-lambda-to-define
   #:description "This use of `case-lambda` is equivalent to using `define` with optional arguments."
   #:literals (define case-lambda)
+
   (define id:id
     (case-lambda
       [(case1-arg:id ...) (usage:id usage1:id ... default:expr)]
       [(case2-arg:id ... bonus-arg:id) body ...]))
+
   #:when (oneline-syntax? #'default)
   #:when (free-identifier=? #'id #'usage)
-  #:when (free-identifiers=? #'(case1-arg ...) #'(case2-arg ...))
-  #:when (free-identifiers=? #'(case1-arg ...) #'(usage1 ...))
+
+  #:when (and (equal? (length (attribute case1-arg))
+                      (length (attribute case2-arg)))
+              (equal? (length (attribute case1-arg))
+                      (length (attribute usage1))))
+
+  #:when (for/and ([case1-arg-id (in-list (attribute case1-arg))]
+                   [case2-arg-id (in-list (attribute case2-arg))]
+                   [usage1-id (in-list (attribute usage1))])
+           (and (equal? (syntax-e case1-arg-id) (syntax-e case2-arg-id))
+                (equal? (syntax-e case1-arg-id) (syntax-e usage1-id))))
+
   (define (id case2-arg ... [bonus-arg default])
     body ...))
 

default-recommendations/let-binding-suggestions-test.rkt

@@ -235,6 +235,33 @@ test: "self-shadowing let*-values binding clause isn't refactorable"
 ------------------------------
 
 
+test: "let* with later right-hand-sides referring to earlier bindings is refactorable"
+------------------------------
+(define (f a)
+  (let* ([b (+ a 1)]
+         [c (+ b 1)]
+         [d (+ c 1)])
+    d))
+------------------------------
+------------------------------
+(define (f a)
+  (define b (+ a 1))
+  (define c (+ b 1))
+  (define d (+ c 1))
+  d)
+------------------------------
+
+
+test: "let* with later bindings shadowing earlier right-hand-sides not refactorable"
+------------------------------
+(define y 1)
+(define (f)
+  (let* ([x (+ y 1)]
+         [y (+ x 1)])
+    1))
+------------------------------
+
+
 test: "let forms inside lambdas"
 ------------------------------
 (λ ()
@@ -503,6 +530,15 @@ test: "variable definition with nested let binding of name bound earlier not ref
 ------------------------------
 
 
+test: "variable definition with nested let binding shadowing name used later not refactorable"
+------------------------------
+(define x 5)
+(define (f)
+  (define y (let ([x 1]) (* x 2)))
+  (* x y))
+------------------------------
+
+
 test: "let binding nested in begin0 extractable to definition"
 ------------------------------
 (define (f)

default-recommendations/let-binding-suggestions.rkt

@@ -31,6 +31,7 @@
          resyntax/refactoring-rule
          resyntax/refactoring-suite
          resyntax/private/syntax-neighbors
+         syntax/id-set
          syntax/parse)
 
 
@@ -71,8 +72,11 @@
   (~seq body-before ...
         (define id:id (let ([nested-id:id nested-expr:expr]) expr:expr))
         body-after ...)
-  #:when (not
-          (set-member? (syntax-bound-identifiers #'(body-before ... id body-after ...)) #'nested-id))
+  #:when (identifier-binding-unchanged-in-context? (attribute id) (attribute nested-expr))
+  #:when (for/and ([body-free-id
+                    (in-free-id-set
+                     (syntax-free-identifiers #'(body-before ... nested-expr body-after ...)))])
+           (identifier-binding-unchanged-in-context? body-free-id (attribute nested-id)))
   (body-before ...
    (define nested-id nested-expr)
    (define id expr)
@@ -97,7 +101,7 @@
   #:description "This `let` binding does nothing and can be removed."
   #:literals (let)
   (let ([left-id:id right-id:id]) body)
-  #:when (bound-identifier=? #'left-id #'right-id)
+  #:when (equal? (syntax-e (attribute left-id)) (syntax-e (attribute right-id)))
   body)