From 5aa1df112abcaea7c5c8936246cf41d9b92e58e6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 24 Jul 2025 05:36:13 +0000 Subject: [PATCH 1/4] Initial plan From 509988ee05506a863d2caf2b2a7e14bd30687672 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 24 Jul 2025 05:57:40 +0000 Subject: [PATCH 2/4] Implement basic match conditional to #:when refactoring rule Co-authored-by: actuallyentropy <34977021+actuallyentropy@users.noreply.github.com> --- .../match-shortcuts-test.rkt | 65 +++++++++++++++++++ default-recommendations/match-shortcuts.rkt | 41 +++++++++++- 2 files changed, 105 insertions(+), 1 deletion(-) diff --git a/default-recommendations/match-shortcuts-test.rkt b/default-recommendations/match-shortcuts-test.rkt index 5e45248b..2f112b6f 100644 --- a/default-recommendations/match-shortcuts-test.rkt +++ b/default-recommendations/match-shortcuts-test.rkt @@ -198,3 +198,68 @@ test: "match patterns using ? with a commented lambda can be simplified with #:w (foo 100) ------------------------------ + +test: "match patterns with if conditionals can be simplified using #:when clauses" +------------------------------ +(define (f pt) + (match pt + [(list x y) + (if (> x y) + (list y x) + pt)] + [(list) '()])) +------------------------------ +------------------------------ +(define (f pt) + (match pt + [(list x y) + #:when (> x y) + (list y x)] + [(list x y) pt] + [(list) '()])) +------------------------------ + + +test: "match patterns with cond conditionals can be simplified using #:when clauses" +------------------------------ +(define (f pt) + (match pt + [(list x y) + (cond + [(> x y) (list y x)] + [else pt])] + [(list) '()])) +------------------------------ +------------------------------ +(define (f pt) + (match pt + [(list x y) + #:when (> x y) + (list y x)] + [(list x y) pt] + [(list) '()])) +------------------------------ + + +test: "single-clause match with if conditional should not be refactored" +------------------------------ +(define (f pt) + (match pt + [(list x y) + (if (> x y) + (list y x) + pt)])) +------------------------------ + + +test: "match patterns with complex nested patterns should not be refactored" +------------------------------ +(define (f data) + (match data + [(list (list (list x y) z) w) + (if (> x y) + (list y x) + data)] + [_ 'no-match])) +------------------------------ + diff --git a/default-recommendations/match-shortcuts.rkt b/default-recommendations/match-shortcuts.rkt index 243e7d6c..99625f8a 100644 --- a/default-recommendations/match-shortcuts.rkt +++ b/default-recommendations/match-shortcuts.rkt @@ -156,6 +156,45 @@ clause-after ...)) +(define-syntax-class simple-pattern + #:description "simple match pattern" + #:attributes () + (pattern id:id) + (pattern (~or null (quote ()) '())) + (pattern (list id:id ...)) + (pattern (cons head:id tail:id)) + (pattern literal:number) + (pattern literal:string) + (pattern literal:boolean)) + + +(define-syntax-class conditional-body + #:description "conditional expression in match clause body" + #:attributes (condition then-expr else-expr) + #:literals (if cond else) + + (pattern (if condition then-expr else-expr)) + + (pattern (cond [condition then-expr] [else else-expr]))) + + +(define-refactoring-rule match-conditional-to-when + #:description "This conditional in a `match` clause can be simplified using `#:when`." + #:literals (match) + + (match subject + clause-before ... + (~and [pattern:simple-pattern conditional:conditional-body] clause-to-replace) + clause-after ...+) + + (match subject + clause-before ... + [pattern #:when conditional.condition conditional.then-expr] + [pattern conditional.else-expr] + clause-after ...)) + + (define-refactoring-suite match-shortcuts - #:rules (predicate-pattern-with-lambda-to-when + #:rules (match-conditional-to-when + predicate-pattern-with-lambda-to-when single-clause-match-to-match-define)) From 2c5bc47bbf38aba37f44ad296f3c4383335ae24f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 24 Jul 2025 06:15:36 +0000 Subject: [PATCH 3/4] Changes before error encountered Co-authored-by: actuallyentropy <34977021+actuallyentropy@users.noreply.github.com> --- default-recommendations/match-shortcuts.rkt | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/default-recommendations/match-shortcuts.rkt b/default-recommendations/match-shortcuts.rkt index 99625f8a..bcfec5c9 100644 --- a/default-recommendations/match-shortcuts.rkt +++ b/default-recommendations/match-shortcuts.rkt @@ -156,18 +156,6 @@ clause-after ...)) -(define-syntax-class simple-pattern - #:description "simple match pattern" - #:attributes () - (pattern id:id) - (pattern (~or null (quote ()) '())) - (pattern (list id:id ...)) - (pattern (cons head:id tail:id)) - (pattern literal:number) - (pattern literal:string) - (pattern literal:boolean)) - - (define-syntax-class conditional-body #:description "conditional expression in match clause body" #:attributes (condition then-expr else-expr) @@ -184,7 +172,7 @@ (match subject clause-before ... - (~and [pattern:simple-pattern conditional:conditional-body] clause-to-replace) + (~and [pattern conditional:conditional-body] clause-to-replace) clause-after ...+) (match subject From 70faec2c8a3e63eccda4a54fc13b8a3455e54ced Mon Sep 17 00:00:00 2001 From: Jack Firth Date: Thu, 24 Jul 2025 18:28:16 -0700 Subject: [PATCH 4/4] Clean up copilot's implementation --- .../match-shortcuts-test.rkt | 43 +++++++++++++++++-- default-recommendations/match-shortcuts.rkt | 24 ++++++++--- 2 files changed, 56 insertions(+), 11 deletions(-) diff --git a/default-recommendations/match-shortcuts-test.rkt b/default-recommendations/match-shortcuts-test.rkt index 2f112b6f..430aeb29 100644 --- a/default-recommendations/match-shortcuts-test.rkt +++ b/default-recommendations/match-shortcuts-test.rkt @@ -171,6 +171,7 @@ test: "match patterns using ? with a lambda cannot be simplified when under elli [_ 'no-match])) ------------------------------ + test: "match patterns using ? with a commented lambda can be simplified with #:when clauses" ------------------------------ (define (foo x) @@ -241,7 +242,35 @@ test: "match patterns with cond conditionals can be simplified using #:when clau ------------------------------ -test: "single-clause match with if conditional should not be refactored" +test: "match patterns with multi-body cond conditionals can be simplified using #:when clauses" +------------------------------ +(define (f pt) + (match pt + [(list x y) + (cond + [(> x y) + (displayln "true case") + (list y x)] + [else + (displayln "false case") + pt])] + [(list) '()])) +------------------------------ +------------------------------ +(define (f pt) + (match pt + [(list x y) + #:when (> x y) + (displayln "true case") + (list y x)] + [(list x y) + (displayln "false case") + pt] + [(list) '()])) +------------------------------ + + +test: "single-clause match with if conditional should be refactored to match-define instead of #:when" ------------------------------ (define (f pt) (match pt @@ -250,16 +279,22 @@ test: "single-clause match with if conditional should not be refactored" (list y x) pt)])) ------------------------------ +------------------------------ +(define (f pt) + (match-define (list x y) pt) + (if (> x y) + (list y x) + pt)) +------------------------------ -test: "match patterns with complex nested patterns should not be refactored" +test: "match with if conditional and long pattern should not be refactored to use #:when" ------------------------------ (define (f data) (match data - [(list (list (list x y) z) w) + [(list x y _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _) (if (> x y) (list y x) data)] [_ 'no-match])) ------------------------------ - diff --git a/default-recommendations/match-shortcuts.rkt b/default-recommendations/match-shortcuts.rkt index bcfec5c9..4cde8de8 100644 --- a/default-recommendations/match-shortcuts.rkt +++ b/default-recommendations/match-shortcuts.rkt @@ -15,6 +15,7 @@ resyntax/base resyntax/default-recommendations/private/lambda-by-any-name resyntax/default-recommendations/private/syntax-identifier-sets + resyntax/default-recommendations/private/syntax-lines resyntax/private/syntax-traversal syntax/parse syntax/strip-context) @@ -158,12 +159,12 @@ (define-syntax-class conditional-body #:description "conditional expression in match clause body" - #:attributes (condition then-expr else-expr) + #:attributes (condition [then-expr 1] [else-expr 1]) #:literals (if cond else) - - (pattern (if condition then-expr else-expr)) - - (pattern (cond [condition then-expr] [else else-expr]))) + (pattern (if condition only-then-expr only-else-expr) + #:attr [then-expr 1] (list (attribute only-then-expr)) + #:attr [else-expr 1] (list (attribute only-else-expr))) + (pattern (cond [condition then-expr ...] [else else-expr ...]))) (define-refactoring-rule match-conditional-to-when @@ -175,10 +176,19 @@ (~and [pattern conditional:conditional-body] clause-to-replace) clause-after ...+) + #:when (positive? (+ (length (attribute clause-before)) (length (attribute clause-after)))) + + ; Duplicating a long or complex match pattern isn't worth the reduction in nesting from + ; refactoring the code to use #:when, so we don't bother if the pattern is multiple lines or if + ; it's a fairly long line. + #:when (oneline-syntax? (attribute pattern)) + #:when (<= (syntax-span (attribute pattern)) 60) + (match subject clause-before ... - [pattern #:when conditional.condition conditional.then-expr] - [pattern conditional.else-expr] + (~@ . (~splicing-replacement ([pattern #:when conditional.condition conditional.then-expr ...] + [pattern conditional.else-expr ...]) + #:original pattern)) clause-after ...))