Prevent the truthiness of absl.flags Flag instances from being tested in a bool context (not useful) to avoid situations where someone really wanted to refer to `their_flag.value` instead. This prevents logic bugs.

There were ~10 problems found by this and cleaned up in our internal codebase.

PiperOrigin-RevId: 427295923
Change-Id: Ifc9e086133986cc14d052a06889a93c1ab0ff688

3 files changed, 19 insertions, 9 deletions

diff --git a/absl/flags/_flag.py b/absl/flags/_flag.py
index a893b22..a1c53ff 100644
--- a/absl/flags/_flag.py
+++ b/absl/flags/_flag.py
@@ -128,6 +128,10 @@ class Flag(object):
       return id(self) < id(other)
     return NotImplemented
 
+  def __bool__(self):
+    raise TypeError('A Flag instance would always be True. '
+                    'Did you mean to test the `.value` attribute?')
+
   def __getstate__(self):
     raise TypeError("can't pickle Flag objects")
 
diff --git a/absl/flags/_flagvalues.py b/absl/flags/_flagvalues.py
index dab6091..c9be720 100644
--- a/absl/flags/_flagvalues.py
+++ b/absl/flags/_flagvalues.py
@@ -774,7 +774,7 @@ class FlagValues(object):
         continue
 
       flag = flag_dict.get(name)
-      if flag:
+      if flag is not None:
         if flag.boolean and value is None:
           value = 'true'
         else:
@@ -782,13 +782,13 @@ class FlagValues(object):
       elif name.startswith('no') and len(name) > 2:
         # Boolean flags can take the form of --noflag, with no value.
         noflag = flag_dict.get(name[2:])
-        if noflag and noflag.boolean:
+        if noflag is not None and noflag.boolean:
           if value is not None:
             raise ValueError(arg + ' does not take an argument')
           flag = noflag
           value = 'false'
 
-      if retired_flag_func and not flag:
+      if retired_flag_func and flag is None:
         is_retired, is_bool = retired_flag_func(name)
 
         # If we didn't recognize that flag, but it starts with
@@ -808,7 +808,7 @@ class FlagValues(object):
               'be specified. See go/totw/90.', name)
           continue
 
-      if flag:
+      if flag is not None:
         flag.parse(value)
         flag.using_default_value = False
       else:
diff --git a/absl/flags/tests/_flag_test.py b/absl/flags/tests/_flag_test.py
index 800a00c..492f117 100644
--- a/absl/flags/tests/_flag_test.py
+++ b/absl/flags/tests/_flag_test.py
@@ -35,6 +35,7 @@ from absl.testing import parameterized
 class FlagTest(absltest.TestCase):
 
   def setUp(self):
+    super().setUp()
     self.flag = _flag.Flag(
         _argument_parser.ArgumentParser(),
         _argument_parser.ArgumentSerializer(),
@@ -59,6 +60,11 @@ class FlagTest(absltest.TestCase):
         'number', 1, 'help')
     self.assertEqual(1, flag.default_unparsed)
 
+  def test_no_truthiness(self):
+    with self.assertRaises(TypeError):
+      if self.flag:
+        self.fail('Flag instances must raise rather than be truthy.')
+
   def test_set_default_overrides_current_value(self):
     self.assertEqual('apple', self.flag.value)
     self.flag._set_default('orange')
@@ -71,14 +77,14 @@ class FlagTest(absltest.TestCase):
     self.assertEqual('apple', self.flag.value)
 
   def test_pickle(self):
-    with self.assertRaisesRegexp(TypeError, "can't pickle Flag objects"):
+    with self.assertRaisesRegex(TypeError, "can't pickle Flag objects"):
       pickle.dumps(self.flag)
 
   def test_copy(self):
     self.flag.value = 'orange'
 
-    with self.assertRaisesRegexp(
-        TypeError, 'Flag does not support shallow copies'):
+    with self.assertRaisesRegex(TypeError,
+                                'Flag does not support shallow copies'):
       copy.copy(self.flag)
 
     flag2 = copy.deepcopy(self.flag)
@@ -172,10 +178,10 @@ class EnumClassFlagTest(parameterized.TestCase):
 class MultiEnumClassFlagTest(parameterized.TestCase):
 
   @parameterized.named_parameters(
-      ('NoHelpSupplied', '', '<apple|orange>: (no help available);\n    '
+      ('NoHelpSupplied', '', '<apple|orange>: (no help available);\n    ' +
        'repeat this option to specify a list of values', False),
       ('WithHelpSupplied', 'Type of fruit.',
-       '<APPLE|ORANGE>: Type of fruit.;\n    '
+       '<APPLE|ORANGE>: Type of fruit.;\n    ' +
        'repeat this option to specify a list of values', True))
   def test_help_text(self, helptext_input, helptext_output, case_sensitive):
     f = _flag.MultiEnumClassFlag(