jackfirth / resyntax

A Racket refactoring engine
Apache License 2.0
51 stars 10 forks source link

Bad define-case-lambda-to-define #226

Closed sorawee closed 4 days ago

sorawee commented 8 months ago
resyntax: /Users/sorawee/projects/racket/extra-pkgs/scribble/scribble-lib/scribble/racket.rkt [define-case-lambda-to-define]

  This use of `case-lambda` is equivalent to using `define` with optional arguments.

  367 (define out
  368   (case-lambda
  369     [(v cls)
  370      (out v cls (let sz-loop ([v v])
  371                   (cond
  372                     [(string? v) (string-length v)]
  373                     [(list? v) (for/fold ([s 0]) ([v (in-list v)]) (+ s (sz-loop v)))]
  374                     [(sized-element? v) (sized-element-length v)]
  375                     [(element? v)
  376                      (sz-loop (element-content v))]
  377                     [(delayed-element? v)
  378                      (content-width v)]
  379                     [(part-relative-element? v)
  380                      (content-width v)]
  381                     [(spaces? v)
  382                      (+ (sz-loop (car (element-content v)))
  383                         (spaces-cnt v)
  384                         (sz-loop (caddr (element-content v))))]
  385                     [else 1])))]
  386     [(v cls len)
  387      (unless (equal? v "")
  388        (cond
  389          [(spaces? v)
  390           (out (car (element-content v)) cls 0)
  391           (out (cadr (element-content v)) #f 0)
  392           (out (caddr (element-content v)) cls len)]
  393          [(equal? v "\n")
  394           (if multi-line?
  395               (begin
  396                 (finish-line!)
  397                 (out prefix cls))
  398               (out " " cls))]
  399          [else
  400           (set! content (cons (elem-wrap
  401                                ((if highlight?
  402                                     (lambda (c)
  403                                       (make-element highlighted-color c))
  404                                     values)
  405                                 (if (and color? cls)
  406                                     (make-element/cache cls v)
  407                                     v)))
  408                               content))
  409           (set! dest-col (+ dest-col len))]))]))

  367 (define (out v cls [len (let sz-loop ([v v])
  368                           (cond
  369                             [(string? v) (string-length v)]
  370                             [(list? v) (for/fold ([s 0]) ([v (in-list v)]) (+ s (sz-loop v)))]
  371                             [(sized-element? v) (sized-element-length v)]
  372                             [(element? v)
  373                              (sz-loop (element-content v))]
  374                             [(delayed-element? v)
  375                              (content-width v)]
  376                             [(part-relative-element? v)
  377                              (content-width v)]
  378                             [(spaces? v)
  379                              (+ (sz-loop (car (element-content v)))
  380                                 (spaces-cnt v)
  381                                 (sz-loop (caddr (element-content v))))]
  382                             [else 1]))])
  383   (unless (equal? v "")
  384     (cond
  385       [(spaces? v)
  386        (out (car (element-content v)) cls 0)
  387        (out (cadr (element-content v)) #f 0)
  388        (out (caddr (element-content v)) cls len)]
  389       [(equal? v "\n")
  390        (if multi-line?
  391            (begin
  392              (finish-line!)
  393              (out prefix cls))
  394            (out " " cls))]
  395       [else
  396        (set! content (cons (elem-wrap
  397                             ((if highlight?
  398                                  (lambda (c)
  399                                    (make-element highlighted-color c))
  400                                  values)
  401                              (if (and color? cls)
  402                                  (make-element/cache cls v)
  403                                  v)))
  404                            content))
  405        (set! dest-col (+ dest-col len))])))
jackfirth commented 7 months ago

It certainly looks terrible. Did it break the code too, or does it just look horrendous because it's a dozen lines of code to set a default in the function header?

sorawee commented 7 months ago

It doesn't break the code. Just look terrible.

jackfirth commented 7 months ago

Adding a restriction on this rule to skip cases where the default value expression is multiple lines seems like a good fix then.