resyntax icon indicating copy to clipboard operation
resyntax copied to clipboard

Replacements should show less context

Open jackfirth opened this issue 5 years ago • 1 comments

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.

jackfirth avatar Feb 03 '21 23:02 jackfirth

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.

jackfirth avatar Feb 04 '21 00:02 jackfirth