Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions default-recommendations/one-line-reformatting-test.rkt
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
#lang resyntax/test
header: - #lang racket/base


test: "regression test for issue #605"
----------------------------------------
(require racket/match)
(define (f expr)
(let loop ([expr expr]
[env '()])
(match expr
[`(,(and (or '+ '- '* '/ 'and 'or) op) ,as ..2 ,b) `(,op ,(loop `(,op ,@as) env) ,(loop b env))]
[_ (void)])))
========================================
(require racket/match)
(define (f expr)
(let loop ([expr expr]
[env '()])
(match expr
[`(,(and (or '+ '- '* '/ 'and 'or) op) ,as ..2 ,b) (list op
(loop `(,op ,@as) env)
(loop b env))]
[_ (void)])))
----------------------------------------
74 changes: 71 additions & 3 deletions private/syntax-replacement.rkt
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
rebellion/collection/range-set
rebellion/private/static-name
rebellion/type/record
resyntax/private/linemap
resyntax/private/logger
resyntax/private/source
resyntax/private/string-indent
Expand All @@ -56,7 +57,9 @@


(module+ test
(require racket/port
(require fmt
racket/port
racket/string
rackunit
(submod "..")))

Expand Down Expand Up @@ -255,7 +258,20 @@
;; horizontal space is available and indenting the resulting string. However, fmt has some odd
;; behavior in how it handles formatting regions with multiple expressions (sorawee/fmt#70) and
;; regions with commented top-level expressions (sorawee/fmt#68).
(define allowed-width (- (current-width) initial-columns))
(define base-allowed-width (- (current-width) initial-columns))

;; For single-line replacements, we need to account for trailing text after the replacement
;; to ensure the formatted code doesn't exceed the line length limit.
(define trailing-text-length
(let ([linemap (string-linemap original)]
[orig-end (string-replacement-original-end replacement)])
;; Convert from 0-based string indices to 1-based positions for linemap
(if (= (linemap-position-to-line linemap (add1 start))
(linemap-position-to-line linemap (add1 orig-end)))
(- (linemap-position-to-end-of-line linemap (add1 orig-end)) (add1 orig-end))
0)))

(define allowed-width (- base-allowed-width trailing-text-length))
(define formatted-code-substring
(string-hanging-indent (program-format changed-code-substring #:width allowed-width)
#:amount initial-columns))
Expand Down Expand Up @@ -315,7 +331,59 @@
#:uses-universal-tagged-syntax? #false))
(define expected
(string-replacement #:start 0 #:end 13 #:contents (list (inserted-string "(+ 1 2 3)"))))
(check-equal? (syntax-replacement-render replacement) expected)))
(check-equal? (syntax-replacement-render replacement) expected))

(test-case "string-replacement-format considers trailing text on single-line replacements"
;; Test that when formatting a single-line replacement, we account for trailing text
;; to avoid exceeding the line length limit. This test simulates the issue from
;; herbie-fp/herbie#1391 where a quasiquote is replaced with a list call.
(define orig-code " [x `(a b c)]")
(define replacement
(string-replacement #:start 9
#:end 18
#:contents (list (inserted-string "(list a b c)"))))
;; The original code is 18 characters. With trailing "]", it's 19 characters total.
;; If we format without accounting for the trailing "]", the formatted result might
;; exceed the desired line length.
(parameterize ([current-width 102])
(define formatted (string-replacement-format replacement orig-code))
(define result (string-apply-replacement orig-code formatted))
;; The result should fit within a reasonable line length and shouldn't be multiline
(check-false (string-contains? result "\n")
"Result should remain on a single line")
;; Verify the replacement happened
(check-true (string-contains? result "(list a b c)")
"Result should contain the list form")))

(test-case "realistic example from herbie-fp/herbie#1391"
;; This test simulates the actual issue where a 102-character line would become 103 characters
;; Without the fix, the formatter would allow the code to exceed the line length limit.
;; With the fix, the formatter either keeps it on one line within the limit, or breaks it
;; across multiple lines if it cannot fit.
(define orig-line " [`(,(and (or '+ '- '* '/ 'and 'or) op) ,as ..2 ,b) `(,op ,(loop `(,op ,@as) env) ,(loop b env))]")
(check-equal? (string-length orig-line) 102 "Original line should be 102 characters")

;; The quasiquote expression starts at position 57 and ends at position 101
;; Original: `(,op ,(loop `(,op ,@as) env) ,(loop b env))
;; Replacement: (list op (loop `(,op ,@as) env) (loop b env))
(define quasiquote-start 57)
(define quasiquote-end 101) ; just before the final ]

(define replacement
(string-replacement #:start quasiquote-start
#:end quasiquote-end
#:contents (list (inserted-string "(list op (loop `(,op ,@as) env) (loop b env))"))))

;; Test with Racket's standard line width of 102
(parameterize ([current-width 102])
(define formatted (string-replacement-format replacement orig-line))
(define result (string-apply-replacement orig-line formatted))

;; If the result is a single line, it should not exceed 102 characters
;; If it's multi-line, each line should not exceed 102 characters
(for ([line (in-list (string-split result "\n"))])
(check-true (<= (string-length line) 102)
(format "Line exceeds length limit: ~a chars (should be <= 102)" (string-length line)))))))


(define (syntax-replacement-introduces-incorrect-bindings? replacement)
Expand Down