Skip to content

Commit 40145a4

Browse files
committed
fix(decorators): avoid mutating original function metadata on functools.wraps
When decorators like SetParseFn and SetParseFns are applied to functions wrapped with functools.wraps, copying existing parse_fns and metadata dictionaries prevents mutating the underlying function's metadata in place. Fixes #728. Signed-off-by: Aryan Sachin <aryansachinixd@gmail.com>
1 parent 716bbc2 commit 40145a4

2 files changed

Lines changed: 40 additions & 4 deletions

File tree

‎fire/decorators.py‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,12 @@ def SetParseFn(fn, *arguments):
3737
The decorated function, which now has metadata telling Fire how to perform.
3838
"""
3939
def _Decorator(func):
40-
parse_fns = GetParseFns(func)
40+
existing = GetParseFns(func)
41+
parse_fns = {
42+
'default': existing.get('default'),
43+
'positional': list(existing.get('positional', [])),
44+
'named': dict(existing.get('named', {})),
45+
}
4146
if not arguments:
4247
parse_fns['default'] = fn
4348
else:
@@ -66,8 +71,12 @@ def SetParseFns(*positional, **named):
6671
The decorated function, which now has metadata telling Fire how to perform.
6772
"""
6873
def _Decorator(fn):
69-
parse_fns = GetParseFns(fn)
70-
parse_fns['positional'] = positional
74+
existing = GetParseFns(fn)
75+
parse_fns = {
76+
'default': existing.get('default'),
77+
'positional': positional if positional else tuple(existing.get('positional', ())),
78+
'named': dict(existing.get('named', {})),
79+
}
7180
parse_fns['named'].update(named)
7281
_SetMetadata(fn, FIRE_PARSE_FNS, parse_fns)
7382
return fn
@@ -76,7 +85,7 @@ def _Decorator(fn):
7685

7786

7887
def _SetMetadata(fn, attribute, value):
79-
metadata = GetMetadata(fn)
88+
metadata = dict(GetMetadata(fn))
8089
metadata[attribute] = value
8190
setattr(fn, FIRE_METADATA, metadata)
8291

‎fire/decorators_test.py‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
"""Tests for the decorators module."""
1616

17+
import functools
1718
from fire import core
1819
from fire import decorators
1920
from fire import testutils
@@ -165,6 +166,32 @@ def testSetParseFn(self):
165166
command=['example7', '1', '--arg2=2', '3', '4', '--kwarg=5']),
166167
('1', '2', ('3', '4'), {'kwarg': '5'}))
167168

169+
def testSetParseFnsFunctoolsWraps(self):
170+
@decorators.SetParseFns(value=int)
171+
def original(value):
172+
return type(value).__name__
173+
174+
@decorators.SetParseFns(value=str)
175+
@functools.wraps(original)
176+
def wrapped(value):
177+
return original(value)
178+
179+
self.assertEqual(core.Fire(original, command=['01']), 'int')
180+
self.assertEqual(core.Fire(wrapped, command=['01']), 'str')
181+
182+
def testSetParseFnFunctoolsWraps(self):
183+
@decorators.SetParseFn(int, 'value')
184+
def original(value):
185+
return type(value).__name__
186+
187+
@decorators.SetParseFn(str, 'value')
188+
@functools.wraps(original)
189+
def wrapped(value):
190+
return original(value)
191+
192+
self.assertEqual(core.Fire(original, command=['01']), 'int')
193+
self.assertEqual(core.Fire(wrapped, command=['01']), 'str')
194+
168195

169196
if __name__ == '__main__':
170197
testutils.main()

0 commit comments

Comments
 (0)