Skip to content

Commit

Permalink
Rip out NEWLINE formatting
Browse files Browse the repository at this point in the history
The `fmt` integration makes it obsolete.
  • Loading branch information
jackfirth committed Aug 10, 2024
1 parent 70810ad commit 164c8ba
Show file tree
Hide file tree
Showing 12 changed files with 73 additions and 130 deletions.
22 changes: 11 additions & 11 deletions default-recommendations/conditional-shortcuts.rkt
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,11 @@
#:literals (if)

(pattern (if cond then nested:nested-if-else)
#:with (branch ...) #'(NEWLINE [cond then] nested.branch ...)
#:with (branch ...) #'([cond then] nested.branch ...)
#:attr branches-size (+ 1 (attribute nested.branches-size)))

(pattern (if cond then default)
#:with (branch ...) #'(NEWLINE [cond then] NEWLINE [else default])
#:with (branch ...) #'([cond then] [else default])
#:attr branches-size 2))


Expand Down Expand Up @@ -89,7 +89,7 @@
#:description equivalent-conditional-description
[conditional:when-or-unless-equivalent-conditional
((~if conditional.negated? unless when)
conditional.condition NEWLINE (ORIGINAL-SPLICE conditional.body ...))])
conditional.condition (ORIGINAL-SPLICE conditional.body ...))])


(define-refactoring-rule always-throwing-if-to-when
Expand All @@ -101,8 +101,8 @@
fail:always-throwing-expression
else-expression))
(header.formatted
... NEWLINE
((~if condition.negated? unless when) condition.base-condition NEWLINE fail) NEWLINE
...
((~if condition.negated? unless when) condition.base-condition fail)
else-expression)])


Expand All @@ -117,8 +117,8 @@
[else
body ...]))
(header.formatted
... NEWLINE
((~if condition.negated? unless when) condition.base-condition NEWLINE fail) NEWLINE
...
((~if condition.negated? unless when) condition.base-condition fail)
(ORIGINAL-SPLICE body ...))])


Expand Down Expand Up @@ -198,11 +198,11 @@
(if (or (multiline-syntax? #'condition)
(multiline-syntax? #'then-expr)
(attribute then-expr.uses-begin?))
#'(NEWLINE [condition (ORIGINAL-GAP condition then-expr) then-expr.refactored ...])
#'([condition (ORIGINAL-GAP condition then-expr) then-expr.refactored ...])
#'((ORIGINAL-GAP condition then-expr) [condition then-expr.refactored ...]))
#:with (false-branch ...)
(if (attribute else-expr.uses-begin?)
#'(NEWLINE [else (ORIGINAL-GAP then-expr else-expr) else-expr.refactored ...])
#'([else (ORIGINAL-GAP then-expr else-expr) else-expr.refactored ...])
#'((ORIGINAL-GAP then-expr else-expr) [else else-expr.refactored ...]))
(cond
true-branch ...
Expand All @@ -221,11 +221,11 @@
(if (or (multiline-syntax? #'condition)
(multiline-syntax? #'then-expr)
(attribute then-expr.uses-let?))
#'(NEWLINE [condition (ORIGINAL-GAP condition then-expr) then-expr.refactored ...])
#'([condition (ORIGINAL-GAP condition then-expr) then-expr.refactored ...])
#'((ORIGINAL-GAP condition then-expr) [condition then-expr.refactored ...]))
#:with (false-branch ...)
(if (attribute else-expr.uses-let?)
#'(NEWLINE [else (ORIGINAL-GAP then-expr else-expr) else-expr.refactored ...])
#'([else (ORIGINAL-GAP then-expr else-expr) else-expr.refactored ...])
#'((ORIGINAL-GAP then-expr else-expr) [else else-expr.refactored ...]))
(cond
true-branch ...
Expand Down
8 changes: 1 addition & 7 deletions default-recommendations/definition-shortcuts.rkt
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,8 @@
[(header:header-form-allowing-internal-definitions
(define-values (id:id ...) (values expr:expr ...))
rest ...)

#:with ((gap ...) ...)
(for/list ([e (in-syntax #'(expr ...))])
(if (multiline-syntax? e) #'(NEWLINE) #'()))

(header.formatted ...
NEWLINE
(~@ (define id gap ... expr) NEWLINE) ...
(define id expr) ...
(ORIGINAL-SPLICE rest ...))])


Expand Down
39 changes: 19 additions & 20 deletions default-recommendations/for-loop-shortcuts.rkt
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@
list-expression:sequence-syntax-convertible-list-expression))
#:when (bound-identifier=? #'x #'y)
#:with nesting-loop? #false
#:with loop-clauses #'([x list-expression.refactored] NEWLINE #:when filter-body)
#:with loop-clauses #'([x list-expression.refactored] #:when filter-body)
#:with loop #'(for/list loop-clauses loop-body ...))

(pattern
Expand All @@ -98,7 +98,7 @@
list-expression:sequence-syntax-convertible-list-expression))
#:when (not (bound-identifier=? #'x #'y))
#:with nesting-loop? #true
#:with loop-clauses #'([y list-expression.refactored] NEWLINE [x append-map-body.refactored])
#:with loop-clauses #'([y list-expression.refactored] [x append-map-body.refactored])
#:with loop #'(for*/list loop-clauses loop-body ...))

(pattern
Expand All @@ -114,7 +114,7 @@
#:description "Applying `+` to a list of numbers can be replaced with a `for/sum` loop."
#:literals (apply +)
[(apply + loop:for-loop-convertible-list-expression)
((~if loop.nesting-loop? for*/sum for/sum) loop.loop-clauses (~@ NEWLINE loop.loop-body) ...)])
((~if loop.nesting-loop? for*/sum for/sum) loop.loop-clauses loop.loop-body ...)])


;; A loop body function is a lambda expression that is passed to a function like map, for-each, or
Expand All @@ -132,23 +132,23 @@
;; assume all such lambdas are multi-line, and multi-line for-each functions are typically easier
;; to read when they're in the body of a for loop.
(pattern (_:lambda-by-any-name (x) first-body remaining-body ...+)
#:with (body ...) #'(NEWLINE (ORIGINAL-SPLICE first-body remaining-body ...)))
#:with (body ...) #'((ORIGINAL-SPLICE first-body remaining-body ...)))

;; We don't bother migrating for-each forms with only a single body form unless the body form is
;; exceptionally long, so that forms which span multiple lines tend to get migrated. By not
;; migrating short forms, we avoid bothering reviewers with changes to loops that aren't complex
;; enough to need a lot of refactoring in the first place.
(pattern (_:lambda-by-any-name (x) only-body)
#:when (>= (syntax-span #'only-body) 60)
#:with (body ...) #'(NEWLINE only-body)))
#:with (body ...) #'(only-body)))


(define-refactoring-rule for-each-to-for
#:description "This `for-each` operation can be replaced with a `for` loop."
#:literals (for-each)
[(for-each function:worthwhile-loop-body-function loop:for-clause-convertible-list-expression)
((~if loop.flat? for for*)
((~@ loop.leading-clause NEWLINE) ... [function.x loop.trailing-expression])
(loop.leading-clause ... [function.x loop.trailing-expression])
function.body ...)])


Expand All @@ -157,17 +157,17 @@
#:literals (ormap)
[(ormap function:worthwhile-loop-body-function loop:for-clause-convertible-list-expression)
((~if loop.flat? for/or for*/or)
((~@ loop.leading-clause NEWLINE) ... [function.x loop.trailing-expression])
function.body ...)])
(loop.leading-clause ... [function.x loop.trailing-expression])
function.body ...)])


(define-refactoring-rule andmap-to-for/and
#:description "This `andmap` operation can be replaced with a `for/and` loop."
#:literals (andmap)
[(andmap function:worthwhile-loop-body-function loop:for-clause-convertible-list-expression)
((~if loop.flat? for/and for*/and)
((~@ loop.leading-clause NEWLINE) ... [function.x loop.trailing-expression])
function.body ...)])
(loop.leading-clause ... [function.x loop.trailing-expression])
function.body ...)])


(define-syntax-class for-list-id
Expand Down Expand Up @@ -200,7 +200,7 @@
(hash-set h-usage:id key value))
#:when (free-identifier=? #'h #'h-usage)
#:when (not (set-member? (syntax-free-identifiers #'(body ...)) #'h))
(loop (ORIGINAL-SPLICE iteration-clauses body ...) NEWLINE (values key value))])
(loop (ORIGINAL-SPLICE iteration-clauses body ...) (values key value))])


(define-syntax-class nested-for
Expand All @@ -209,7 +209,7 @@
#:literals (for)

(pattern (for (outer-clause) nested:nested-for)
#:with (clause ...) #'(outer-clause NEWLINE nested.clause ...)
#:with (clause ...) #'(outer-clause nested.clause ...)
#:with (body ...) #'(nested.body ...))

(pattern (for (only-clause) body ...)
Expand All @@ -220,7 +220,7 @@
#:description "These nested `for` loops can be replaced by a single `for*` loop."
[nested:nested-for
#:when (>= (length (attribute nested.clause)) 2)
(for* (nested.clause ...) NEWLINE
(for* (nested.clause ...)
(ORIGINAL-SPLICE nested.body ...))])


Expand All @@ -238,8 +238,7 @@
(free-identifier=? #'i1 #'i3)
(free-identifier=? #'i1 #'i4)
(free-identifier=? #'vec1 #'vec2))
(for/first ([x (in-vector vec1)]
NEWLINE #:when condition) NEWLINE
(for/first ([x (in-vector vec1)] #:when condition)
true-branch)])


Expand All @@ -251,7 +250,7 @@
(~and original-body (or condition:condition-expression ...+ last-condition)))
(loop-id (ORIGINAL-GAP loop-id original-clauses)
((ORIGINAL-SPLICE clause ...)
(~@ NEWLINE (~if condition.negated? #:when #:unless) condition.base-condition) ...)
(~@ (~if condition.negated? #:when #:unless) condition.base-condition) ...)
(ORIGINAL-GAP original-clauses original-body)
last-condition)])

Expand All @@ -263,14 +262,14 @@
(pattern (for/list (only-clause) only-body:expr)
#:when (oneline-syntax? #'only-body)
#:with refactored-loop
#'(for*/list (only-clause NEWLINE [v (in-list only-body)])
NEWLINE v))
#'(for*/list (only-clause [v (in-list only-body)])
v))

(pattern ((~and loop-id for*/list) (clause ...) only-body:expr)
#:when (oneline-syntax? #'only-body)
#:with refactored-loop
#'(loop-id ((ORIGINAL-SPLICE clause ...) NEWLINE [v (in-list only-body)])
NEWLINE v)))
#'(loop-id ((ORIGINAL-SPLICE clause ...) [v (in-list only-body)])
v)))


(define-refactoring-rule apply-append-for-loop-to-for-loop
Expand Down
21 changes: 3 additions & 18 deletions default-recommendations/function-definition-shortcuts-test.rkt
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,13 @@ test: "lambda variable definition with no arguments to function definition"
------------------------------


test: "one-line lambda variable definition to one-line function definition"
test: "one-line lambda variable definition to function definition"
------------------------------
(define f (λ (a b c) 1))
------------------------------
------------------------------
(define (f a b c) 1)
(define (f a b c)
1)
------------------------------


Expand Down Expand Up @@ -96,22 +97,6 @@ test: "lambda function definition with closed-over expressions not refactorable"
------------------------------


test: "lambda variable definition with long header to function definition with preserved formatting"
------------------------------
(define f
(λ (a
b
c)
1))
------------------------------
------------------------------
(define (f a
b
c)
1)
------------------------------


test: "lambda variable definition with commented body to definition with preserved comments"
------------------------------
(define f
Expand Down
2 changes: 1 addition & 1 deletion default-recommendations/function-definition-shortcuts.rkt
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@
#:when (free-identifiers=? #'(case1-arg ...) #'(case2-arg ...))
#:when (free-identifiers=? #'(case1-arg ...) #'(usage1 ...))
(define (id case2-arg ... [bonus-arg default])
(~@ NEWLINE body) ...)])
body ...)])


(define function-definition-shortcuts
Expand Down
20 changes: 10 additions & 10 deletions default-recommendations/legacy-struct-migrations-test.rkt
Original file line number Diff line number Diff line change
Expand Up @@ -106,14 +106,16 @@ test: "define-struct with options with separating whitespace"

test: "define-struct with field comments"
----------------------------------------
(define-struct point (x ;; The X coordinate of the point
y ;; The Y coordinate of the point
))
(define-struct point
(x ;; The X coordinate of the point
y ;; The Y coordinate of the point
))
----------------------------------------
----------------------------------------
(struct point (x ;; The X coordinate of the point
y ;; The Y coordinate of the point
)
(struct point
(x ;; The X coordinate of the point
y ;; The Y coordinate of the point
)
#:extra-constructor-name make-point)
----------------------------------------

Expand All @@ -124,8 +126,7 @@ test: "define-struct with comments between options"

;; Custom write implementation
#:property prop:custom-write
(λ (this out mode)
(write-string "#<point>" out))
(λ (this out mode) (write-string "#<point>" out))

;; Field guard
#:guard (λ (x y _) (values x y)))
Expand All @@ -135,8 +136,7 @@ test: "define-struct with comments between options"

;; Custom write implementation
#:property prop:custom-write
(λ (this out mode)
(write-string "#<point>" out))
(λ (this out mode) (write-string "#<point>" out))

;; Field guard
#:guard (λ (x y _) (values x y))
Expand Down
4 changes: 2 additions & 2 deletions default-recommendations/legacy-struct-migrations.rkt
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,15 @@
#:attributes (keyword [expr 1] [formatted 1] [original 1])
(pattern (~seq (~and keyword:keyword (~not :constructor-name-keyword)) expr:expr ...)
#:with (original ...) #'(keyword expr ...)
#:with (formatted ...) #'(NEWLINE (ORIGINAL-SPLICE keyword expr ...))))
#:with (formatted ...) #'((ORIGINAL-SPLICE keyword expr ...))))


(define-refactoring-rule define-struct-to-struct
#:description "The `define-struct` form exists for backwards compatibility, `struct` is preferred."
#:literals (define-struct)
[(define-struct id:id-maybe-super fields option:struct-option ...)
(struct id.migrated ... (ORIGINAL-SPLICE fields option.original ... ...)
NEWLINE #:extra-constructor-name id.make-id)])
#:extra-constructor-name id.make-id)])


(define legacy-struct-migrations
Expand Down
4 changes: 2 additions & 2 deletions default-recommendations/match-shortcuts.rkt
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@
[(header:header-form-allowing-internal-definitions
(match subject
[pattern body ...]))
(header.formatted ... NEWLINE
(match-define pattern subject) NEWLINE
(header.formatted ...
(match-define pattern subject)
(ORIGINAL-SPLICE body ...))])


Expand Down
Loading

0 comments on commit 164c8ba

Please sign in to comment.