Skip to content

Commit 3f6ec11

Browse files
committed
performance/single-literal-merge: Add :dynamic style
Adds and defaults to `:dynamic`, which acts as `:single` or `:multiple` depending on the number of elements in the map literal. (See [#35](#35).)
1 parent 3f1720d commit 3f6ec11

File tree

5 files changed

+94
-37
lines changed

5 files changed

+94
-37
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ Update rules:
3232
- `lint/require-explicit-param-tags`: clean up docs.
3333
- `performance/into-transducer`: remove `cat` as it can't be used in the incorrect form.
3434
- `performance/into-transducer`: add configurable fn list with `:fn-0-arg` and `:fn-1-arg` (depending on how many arguments the fn accepts).
35+
- `performance/single-literal-merge`: adds and defaults to `:dynamic`, which acts as `:single` or `:multiple` depending on the number of elements in the map literal. (See [#35](https://github.com/NoahTheDuke/splint/issues/35).)
3536

3637
Patterns:
3738

resources/noahtheduke/splint/config/default.edn

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -439,10 +439,11 @@
439439
performance/single-literal-merge
440440
{:description "Prefer assoc over merge when merging a literal."
441441
:enabled false
442-
:chosen-style :single
443-
:supported-styles [:single :multiple]
442+
:chosen-style :dynamic
443+
:supported-styles [:single :multiple :dynamic]
444444
:added "1.11"
445-
:updated "1.11"}
445+
:updated "<<next>>"
446+
:links ["https://bsless.github.io/code-smells"]}
446447

447448
;; Style Rules
448449

src/noahtheduke/splint/rules/performance/into_transducer.clj

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,13 @@
2323
; avoid
2424
(into [] (map inc (range 100)))
2525
26-
; avoid (with `:fn-names [cool-fn]`)
26+
; avoid (with `:fn-1-arg [cool-fn]`)
2727
(into [] (cool-fn inc (range 100)))
2828
2929
; prefer
3030
(into [] (map inc) (range 100))
3131
32-
; prefer (with `:fn-names [cool-fn]`)
32+
; prefer (with `:fn-1-arg [cool-fn]`)
3333
(into [] (cool-fn inc) (range 100))
3434
"
3535
{:pattern '(into [?*args] (?trans ??f ?coll))

src/noahtheduke/splint/rules/performance/single_literal_merge.clj

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
(defrule performance/single-literal-merge
2727
"`clojure.core/merge` is inherently slow. Its major benefit is handling nil values. If there is only a single object to merge in and it's a map literal, that benefit is doubly unused. Better to directly assoc the values in.
2828
29+
By default, this rule suggests alternatives based on how many elements are in the map literal: 4 or less will suggest as `:single`, more than 4 will suggest as `:multiple`. Either can be set in the config to enforce one or the other.
30+
2931
@note
3032
If the chosen style is `:single` and `performance/assoc-many` is enabled, the style will be treated as `:multiple` to make the warnings consistent.
3133
@@ -34,7 +36,7 @@
3436
; avoid
3537
(merge m {:a 1 :b 2 :c 3})
3638
37-
; prefer (chosen style :single (default))
39+
; prefer (chosen style :single)
3840
(assoc m :a 1 :b 2 :c 3)
3941
4042
; prefer (chosen style :multiple)
@@ -48,7 +50,11 @@
4850
:autocorrect true
4951
:on-match (fn [ctx rule form {:syms [?given ?literal]}]
5052
(when-let [?literal (not-empty ?literal)]
51-
(let [new-form (condp = (select-style ctx rule)
53+
(let [new-form (case (select-style ctx rule)
5254
:single (single-assoc ?given ?literal)
53-
:multiple (multi-assoc ?given ?literal))]
55+
:multiple (multi-assoc ?given ?literal)
56+
;; default to dynamic
57+
#_:dynamic (if (< 4 (count ?literal))
58+
(multi-assoc ?given ?literal)
59+
(single-assoc ?given ?literal)))]
5460
(->diagnostic ctx rule form {:replace-form new-form}))))})

test/noahtheduke/splint/rules/performance/single_literal_merge_test.clj

Lines changed: 78 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -4,42 +4,67 @@
44

55
(ns noahtheduke.splint.rules.performance.single-literal-merge-test
66
(:require
7-
[lazytest.core :refer [defdescribe it]]
7+
[lazytest.core :refer [defdescribe it describe]]
88
[noahtheduke.splint.test-helpers :refer [expect-match single-rule-config]]))
99

1010
(set! *warn-on-reflection* true)
1111

1212
(def rule-name 'performance/single-literal-merge)
1313

1414
(defdescribe single-literal-merge-test
15-
(it "flattens given map"
16-
(expect-match
17-
[{:rule-name 'performance/single-literal-merge
18-
:form '(merge m {:a 1 :b 2})
19-
:message "Prefer assoc for merging literal maps"
20-
:alt '(assoc m :a 1 :b 2)}]
21-
"(merge m {:a 1 :b 2})"
22-
(single-rule-config rule-name)))
15+
(describe ":chosen-style :single"
16+
(it "flattens given map"
17+
(expect-match
18+
[{:rule-name 'performance/single-literal-merge
19+
:form '(merge m {:a 1 :b 2})
20+
:message "Prefer assoc for merging literal maps"
21+
:alt '(assoc m :a 1 :b 2)}]
22+
"(merge m {:a 1 :b 2})"
23+
(single-rule-config rule-name {:chosen-style :single})))
2324

24-
(it "works on nil"
25-
(expect-match
26-
[{:rule-name 'performance/single-literal-merge
27-
:form '(merge nil {:a 1 :b 2})
28-
:message "Prefer assoc for merging literal maps"
29-
:alt '(assoc nil :a 1 :b 2)}]
30-
"(merge nil {:a 1 :b 2})"
31-
(single-rule-config rule-name)))
25+
(it "works on nil"
26+
(expect-match
27+
[{:rule-name 'performance/single-literal-merge
28+
:form '(merge nil {:a 1 :b 2})
29+
:message "Prefer assoc for merging literal maps"
30+
:alt '(assoc nil :a 1 :b 2)}]
31+
"(merge nil {:a 1 :b 2})"
32+
(single-rule-config rule-name {:chosen-style :single}))))
3233

33-
(it "respects :chosen-style :multiple"
34-
(expect-match
35-
[{:rule-name 'performance/single-literal-merge
36-
:form '(merge m {:a 1 :b 2})
37-
:message "Prefer assoc for merging literal maps"
38-
:alt '(-> m
39-
(assoc :a 1)
40-
(assoc :b 2))}]
41-
"(merge m {:a 1 :b 2})"
42-
(single-rule-config rule-name {:chosen-style :multiple})))
34+
(describe ":chosen-style :multiple"
35+
(it "works as expected"
36+
(expect-match
37+
[{:rule-name 'performance/single-literal-merge
38+
:form '(merge m {:a 1 :b 2})
39+
:message "Prefer assoc for merging literal maps"
40+
:alt '(-> m
41+
(assoc :a 1)
42+
(assoc :b 2))}]
43+
"(merge m {:a 1 :b 2})"
44+
(single-rule-config rule-name {:chosen-style :multiple}))))
45+
46+
(describe ":chosen-style :dynamic"
47+
(it "acts as :single if map literal is small"
48+
(expect-match
49+
[{:rule-name 'performance/single-literal-merge
50+
:form '(merge m {:a 1 :b 2})
51+
:message "Prefer assoc for merging literal maps"
52+
:alt '(assoc m :a 1 :b 2)}]
53+
"(merge m {:a 1 :b 2})"
54+
(single-rule-config rule-name {:chosen-style :dynamic})))
55+
(it "acts as :multiple if map literal is large"
56+
(expect-match
57+
[{:rule-name 'performance/single-literal-merge
58+
:form '(merge m {:a :b :c :d :e :f :g :h :i :j})
59+
:message "Prefer assoc for merging literal maps"
60+
:alt '(-> m
61+
(assoc :a :b)
62+
(assoc :c :d)
63+
(assoc :e :f)
64+
(assoc :g :h)
65+
(assoc :i :j))}]
66+
"(merge m {:a :b :c :d :e :f :g :h :i :j})"
67+
(single-rule-config rule-name {:chosen-style :dynamic}))))
4368

4469
(it "it respects performance/assoc-many"
4570
(expect-match
@@ -50,7 +75,31 @@
5075
(assoc :a 1)
5176
(assoc :b 2))}]
5277
"(merge m {:a 1 :b 2})"
53-
(update (single-rule-config rule-name) 'performance/assoc-many assoc :enabled true)))
78+
(update (single-rule-config rule-name {:chosen-style :single})
79+
'performance/assoc-many assoc :enabled true)))
80+
81+
(describe ":chosen-style defaults to :dynamic"
82+
(it "acts as :single if map literal is small"
83+
(expect-match
84+
[{:rule-name 'performance/single-literal-merge
85+
:form '(merge m {:a 1 :b 2})
86+
:message "Prefer assoc for merging literal maps"
87+
:alt '(assoc m :a 1 :b 2)}]
88+
"(merge m {:a 1 :b 2})"
89+
(single-rule-config rule-name {:chosen-style nil})))
90+
(it "acts as :multiple if map literal is large"
91+
(expect-match
92+
[{:rule-name 'performance/single-literal-merge
93+
:form '(merge m {:a :b :c :d :e :f :g :h :i :j})
94+
:message "Prefer assoc for merging literal maps"
95+
:alt '(-> m
96+
(assoc :a :b)
97+
(assoc :c :d)
98+
(assoc :e :f)
99+
(assoc :g :h)
100+
(assoc :i :j))}]
101+
"(merge m {:a :b :c :d :e :f :g :h :i :j})"
102+
(single-rule-config rule-name {:chosen-style nil}))))
54103

55104
(it "keeps the order in the alt"
56105
(expect-match
@@ -67,7 +116,7 @@
67116
:message "Prefer assoc for merging literal maps"
68117
:alt '(assoc a :a :b :c :d :e :f :g :h :i :j :k :l :m :n :o :p :q :r :s :t :u :v :w :x :y :z)}]
69118
"(merge a {:a :b :c :d :e :f :g :h :i :j :k :l :m :n :o :p :q :r :s :t :u :v :w :x :y :z})"
70-
(single-rule-config rule-name)))
119+
(single-rule-config rule-name {:chosen-style :single})))
71120

72121
(it "ignores multiple maps"
73122
(expect-match

0 commit comments

Comments
 (0)