Skip to content

Commit 2a182d3

Browse files
committed
Enforce baseline checks in fast mode (#45)
Keep nil-child and duplicate-name guards always on, document the risk with benchmark guidance, and adjust the checks-disabled tests accordingly.
1 parent 68c16b0 commit 2a182d3

5 files changed

Lines changed: 20 additions & 18 deletions

File tree

API-CHANGES.md

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,12 @@ smooth transition to the new APIs.
3131
(see #104).
3232

3333
* Added per-tree validation toggles via `checks: false` on `Tree::TreeNode.new`
34-
to allow disabling guard checks in performance-critical code paths. This is
35-
potentially dangerous and can lead to unexpected behavior if invalid data
36-
enters the tree. Only disable checks with clear performance benchmark data
37-
supporting the risk (see #45).
34+
to allow disabling guard checks in performance-critical code paths. Some
35+
baseline guards (nil child checks, duplicate child name checks) are always
36+
enforced to avoid corrupting the tree. Disabling checks is still potentially
37+
dangerous and can lead to unexpected behavior if invalid data enters the tree.
38+
Only disable checks with clear performance benchmark data supporting the risk
39+
(see #45).
3840

3941
## Release 2.2.0 Changes
4042

History.md

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,10 @@
2323
`HashWithIndifferentAccess` data (see #104).
2424

2525
* Add a per-tree `checks: false` option to skip validation checks when
26-
performance matters. This is risky and can yield unexpected behavior if
27-
invalid data is introduced. Only disable checks with benchmark data that
28-
justifies the risk (see #45).
26+
performance matters. Some baseline guards (nil children, duplicate child
27+
names) are always enforced to avoid corrupting the tree. This is still risky
28+
and can yield unexpected behavior if invalid data is introduced. Only
29+
disable checks with benchmark data that justifies the risk (see #45).
2930

3031
### 2.2.1pre / 2026-02-07
3132

lib/tree.rb

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -256,10 +256,11 @@ def validate_acyclic!
256256
#
257257
# @param [Object] content Content of the node.
258258
# @param [Hash, nil] options Optional settings such as { checks: false } to
259-
# disable validation checks for performance-critical usage. Disabling
260-
# checks is risky and can lead to unexpected behavior if invalid data is
261-
# added to the tree. Only disable checks with benchmark data that justifies
262-
# the risk.
259+
# disable validation checks for performance-critical usage. Some baseline
260+
# guards (nil children, duplicate child names) are always enforced to
261+
# avoid corrupting the tree. Disabling checks is risky and can lead to
262+
# unexpected behavior if invalid data is added to the tree. Only disable
263+
# checks with benchmark data that justifies the risk.
263264
#
264265
# @raise [ArgumentError] Raised if the node name is empty.
265266
#

lib/tree/utils/structure_methods.rb

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -143,9 +143,9 @@ def freeze_tree!
143143
end
144144

145145
def validate_add_child!(child)
146+
raise ArgumentError, 'Attempting to add a nil node' unless child
146147
return unless checks_enabled?
147148

148-
raise ArgumentError, 'Attempting to add a nil node' unless child
149149
raise ArgumentError, 'Attempting add node to itself' if equal?(child)
150150
raise ArgumentError, 'Attempting add root as a child' if child.equal?(root)
151151

@@ -155,8 +155,6 @@ def validate_add_child!(child)
155155
end
156156

157157
def ensure_unique_child_name!(child)
158-
return unless checks_enabled?
159-
160158
return unless @children_hash.include?(child.name)
161159

162160
raise "Child #{child.name} already added!"

test/test_tree_checks.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ module TestTree
3636
class TestTreeChecks < Test::Unit::TestCase
3737
def test_checks_disabled_skips_validation
3838
root = Tree::TreeNode.new('root', nil, { checks: false })
39-
child1 = Tree::TreeNode.new('dup', nil, { checks: false })
40-
child2 = Tree::TreeNode.new('dup', nil, { checks: false })
39+
child1 = Tree::TreeNode.new('dup-1', nil, { checks: false })
40+
child2 = Tree::TreeNode.new('dup-2', nil, { checks: false })
4141

4242
root.add(child1)
4343
assert_nothing_raised { root.add(child2) }
@@ -72,8 +72,8 @@ def test_checks_setting_propagates_to_children
7272
root.add(child)
7373
assert_equal(false, child.checks_enabled?)
7474

75-
dup = Tree::TreeNode.new('child')
76-
assert_nothing_raised { root.add(dup) }
75+
other = Tree::TreeNode.new('child2')
76+
assert_nothing_raised { root.add(other) }
7777
end
7878
end
7979
end

0 commit comments

Comments
 (0)