Skip to content

Commit 065fbf8

Browse files
Matt McDonaldcopybara-github
Matt McDonald
authored andcommitted
Surface inappropriate usage of a FlagHolder object as a runtime error
With the aim of steering users away from potential pitfalls in usage of the `FlagHolder` API, this change introduces implementation overrides for the class `__eq__` and `__bool__` methods which turn any such usage into a `TypeError` at runtime. This is to help avoid situations where a user neglects to use the `FlagHolder.value` property in cases where they intended to, e.g., ``` >>> SOME_FLAG = flags.DEFINE_boolean(...) >>> if SOME_FLAG: do_something() Traceback (most recent call last): File "<stdin>", line 1, in <module> TypeError: bool() not supported for instances of type 'FlagHolder' (did you mean to use 'FlagHolder.value' instead?) ``` PiperOrigin-RevId: 338473759 Change-Id: If3b6505a3bdca03db6d96ec7cc28e7214cb1a5e7
1 parent 9a0552c commit 065fbf8

File tree

3 files changed

+29
-0
lines changed

3 files changed

+29
-0
lines changed

absl/CHANGELOG.md

+3
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com).
1515
* (testing) Multiple flags are now set together before their validators run.
1616
This resolves an issue where multi-flag validators rely on specific flag
1717
combinations.
18+
* (flags) As a deterrent for misuse, FlagHolder objects will now raise a
19+
TypeError exception when used in a conditional statement or equality
20+
expression.
1821

1922
## 0.10.0 (2020-08-19)
2023

absl/flags/_flagvalues.py

+14
Original file line numberDiff line numberDiff line change
@@ -1357,6 +1357,20 @@ def __init__(self, flag_values, flag, ensure_non_none_value=False):
13571357
# This allows future use of this for "required flags with None default"
13581358
self._ensure_non_none_value = ensure_non_none_value
13591359

1360+
def __eq__(self, other):
1361+
raise TypeError(
1362+
"unsupported operand type(s) for ==: '{0}' and '{1}' "
1363+
"(did you mean to use '{0}.value' instead?)".format(
1364+
type(self).__name__, type(other).__name__))
1365+
1366+
def __bool__(self):
1367+
raise TypeError(
1368+
"bool() not supported for instances of type '{0}' "
1369+
"(did you mean to use '{0}.value' instead?)".format(
1370+
type(self).__name__))
1371+
1372+
__nonzero__ = __bool__
1373+
13601374
@property
13611375
def name(self):
13621376
return self._name

absl/flags/tests/_flagvalues_test.py

+12
Original file line numberDiff line numberDiff line change
@@ -914,6 +914,18 @@ def test_allow_override(self):
914914
self.assertEqual(3, first.value)
915915
self.assertEqual(3, second.value)
916916

917+
def test_eq(self):
918+
with self.assertRaises(TypeError):
919+
self.name_flag == 'value' # pylint: disable=pointless-statement
920+
921+
def test_eq_reflection(self):
922+
with self.assertRaises(TypeError):
923+
'value' == self.name_flag # pylint: disable=pointless-statement
924+
925+
def test_bool(self):
926+
with self.assertRaises(TypeError):
927+
bool(self.name_flag)
928+
917929

918930
if __name__ == '__main__':
919931
absltest.main()

0 commit comments

Comments
 (0)