Replacements should show less context
Consider this code:
(define (f x)
(do-some-stuff)
(let ([y (whatever)])
(do-more-stuff)))
This is what the command line UI currently outputs:
Internal definitions are recommended instead of let expressions, to reduce nesting.
(define (f x)
(do-some-stuff)
(let ([y (whatever)])
(do-more-stuff)))
(define (f x)
(do-some-stuff)
(define y (whatever))
(do-more-stuff))
It's a little hard to see what the actual change being suggested is. It would be better if the output looked like this:
Internal definitions are recommended instead of let expressions, to reduce nesting.
(let ([y (whatever)])
(do-more-stuff))
(define y (whatever))
(do-more-stuff)
The problem is that refactoring rules analyze a syntax object stx and generate a replacement syntax object, fixed-stx. So they have to replace everything in the object they analyze, and the let-to-define rule has to analyze the whole (define (f x) ...) form so it can know that the let expression can be rewritten. This could be fixed by changing the syntax-replacement model to include an outer "context" syntax object, which is the full form being analyzed, of which the original-syntax object being replaced is one small part. Additionally, replacements would need to be able to specify that they replace the original syntax object with a splice of multiple syntax objects.
Note: this would also help with #5, since it lets refactoring rules avoid destroying comments in parts of the code they don't need to change.