From: Ryan McNair <rdmcnair@us.ibm.com>
Date: Tue, 27 Oct 2015 22:46:46 +0000 (+0000)
Subject: Update register_opts hacking check to allow tuples
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=cf4378382a33363532b58af19af25e98db0c681c;p=openstack-build%2Fcinder-build.git

Update register_opts hacking check to allow tuples

Update the hacking check for register_opts to allow tuples to be
passed in directly as an argument. Previously tuples were getting
flagged as invalid due to a missing pair of parenthesis in the
hacking check.

Change-Id: Ida2ae47eae4ebc6188696dc25943226acd67b5de
Closes-Bug: 1511010
---

diff --git a/cinder/hacking/checks.py b/cinder/hacking/checks.py
index a50f17f36..8324a9706 100644
--- a/cinder/hacking/checks.py
+++ b/cinder/hacking/checks.py
@@ -304,6 +304,9 @@ class CheckOptRegistrationArgs(BaseASTChecker):
         else:  # could be Subscript, Call or many more
             return None
 
+    def _is_list_or_tuple(self, obj):
+        return isinstance(obj, ast.List) or isinstance(obj, ast.Tuple)
+
     def visit_Call(self, node):
         """Look for the register_opt/register_opts calls."""
         # extract the obj_name and method_name
@@ -319,7 +322,6 @@ class CheckOptRegistrationArgs(BaseASTChecker):
                 return (super(CheckOptRegistrationArgs,
                               self).generic_visit(node))
 
-            # argument should be a list with register_opts()
             if len(node.args) > 0:
                 argument_name = self._find_name(node.args[0])
                 if argument_name:
@@ -335,12 +337,10 @@ class CheckOptRegistrationArgs(BaseASTChecker):
                     # passing in a variable referencing the options being
                     # registered.
                     if (method_name == self.singular_method and
-                            (isinstance(node.args[0], ast.List)) or
-                            isinstance(node.args[0], ast.Tuple)):
+                            self._is_list_or_tuple(node.args[0])):
                         self.add_error(node.args[0])
-                    elif (method_name == self.plural_method and
-                            (not isinstance(node.args[0], ast.List)) or
-                            isinstance(node.args[0], ast.Tuple)):
+                    elif (method_name == self.plural_method and not
+                            self._is_list_or_tuple(node.args[0])):
                         self.add_error(node.args[0])
 
         return super(CheckOptRegistrationArgs, self).generic_visit(node)
diff --git a/cinder/tests/unit/test_hacking.py b/cinder/tests/unit/test_hacking.py
index 5502f949b..6e19716b1 100644
--- a/cinder/tests/unit/test_hacking.py
+++ b/cinder/tests/unit/test_hacking.py
@@ -179,6 +179,7 @@ class HackingTestCase(test.TestCase):
         checker = checks.CheckOptRegistrationArgs
         code = """
                CONF.register_opts([opt1, opt2, opt3])
+               CONF.register_opts((opt4, opt5))
                CONF.register_opt(lonely_opt)
                CONF.register_opts([OPT1, OPT2], group="group_of_opts")
                CONF.register_opt(single_opt, group=blah)
@@ -187,14 +188,15 @@ class HackingTestCase(test.TestCase):
 
         code = """
                CONF.register_opt([opt4, opt5, opt6])
+               CONF.register_opt((opt7, opt8))
                CONF.register_opts(lonely_opt)
                CONF.register_opt((an_opt, another_opt))
                """
-        for method in checker.register_methods:
-            self._assert_has_errors(code.format(method), checker,
-                                    expected_errors=[(1, 18, 'C311'),
-                                                     (2, 19, 'C311'),
-                                                     (3, 19, 'C311')])
+        self._assert_has_errors(code, checker,
+                                expected_errors=[(1, 18, 'C311'),
+                                                 (2, 19, 'C311'),
+                                                 (3, 19, 'C311'),
+                                                 (4, 19, 'C311')])
 
         code = """
                CONF.register_opt(single_opt)