Removed unnecessary parantheses and fixed formation

Some place in Nova, return value has unnecessary parentheses,
or return keyword is not followed by a space when returning a value,
which is a bad format and might be ambigous. For example:

    return(1)
    return{
        'a': 1
    }

As noone would write returns like:

    return1
    return'(empty)'

In this patchset we get rid of this bad formation.
Also, I've added a hacking rule which helps to prevent this in future.

TrivialFix

Change-Id: I4ff85d0240bf8719188ebd06fc0e98a81c1d1203

HACKING.rst

@@ -69,6 +69,7 @@ Nova Specific Commandments
 - [N356] Enforce use of assertIs/assertIsNot
 - [N357] Use oslo_utils.uuidutils or uuidsentinel(in case of test cases) to
   generate UUID instead of uuid4().
+- [N358] Return must always be followed by a space when returning a value.
 
 Creating Unit Tests
 -------------------

nova/cmd/dhcpbridge.py

@@ -131,7 +131,7 @@ def main():
             network_id = int(os.environ.get('NETWORK_ID'))
         except TypeError:
             LOG.error(_LE("Environment variable 'NETWORK_ID' must be set."))
-            return(1)
+            return 1
 
         print(init_leases(network_id))
 

nova/cmd/manage.py

@@ -235,14 +235,14 @@ class ProjectCommands(object):
                     value = -1
                 if int(value) < -1:
                     print(_('Quota limit must be -1 or greater.'))
-                    return(2)
+                    return 2
                 if ((int(value) < minimum) and
                    (maximum != -1 or (maximum == -1 and int(value) != -1))):
                     print(_('Quota limit must be greater than %s.') % minimum)
-                    return(2)
+                    return 2
                 if maximum != -1 and int(value) > maximum:
                     print(_('Quota limit must be less than %s.') % maximum)
-                    return(2)
+                    return 2
                 try:
                     db.quota_create(ctxt, project_id, key, value,
                                     user_id=user_id)
@@ -253,7 +253,7 @@ class ProjectCommands(object):
                 print(_('%(key)s is not a valid quota key. Valid options are: '
                         '%(options)s.') % {'key': key,
                                            'options': ', '.join(quota)})
-                return(2)
+                return 2
         print_format = "%-36s %-10s %-10s %-10s"
         print(print_format % (
                     _('Quota'),
@@ -353,7 +353,7 @@ class FloatingIpCommands(object):
             # instead of printing, but logging isn't used here and I
             # don't know why.
             print('error: %s' % exc)
-            return(1)
+            return 1
 
     @args('--ip_range', metavar='<range>', help='IP range')
     def delete(self, ip_range):
@@ -398,7 +398,7 @@ def validate_network_plugin(f, *args, **kwargs):
     if utils.is_neutron():
         print(_("ERROR: Network commands are not supported when using the "
                 "Neutron API.  Use python-neutronclient instead."))
-        return(2)
+        return 2
     return f(*args, **kwargs)
 
 
@@ -554,7 +554,7 @@ class NetworkCommands(object):
                 error_msg = "ERROR: Unexpected arguments provided. Please " \
                     "use separate commands."
                 print(error_msg)
-                return(1)
+                return 1
             db.network_update(admin_context, network['id'], net)
             return
 
@@ -666,11 +666,11 @@ class DbCommands(object):
         max_rows = int(max_rows)
         if max_rows < 0:
             print(_("Must supply a positive value for max_rows"))
-            return(2)
+            return 2
         if max_rows > db.MAX_INT:
             print(_('max rows must be <= %(max_value)d') %
                   {'max_value': db.MAX_INT})
-            return(2)
+            return 2
 
         table_to_rows_archived = {}
         if until_complete and verbose:
@@ -935,7 +935,7 @@ class GetLogCommands(object):
             log_file = '/var/log/messages'
         else:
             print(_('Unable to find system log file!'))
-            return(1)
+            return 1
         lines = [line.strip() for line in open(log_file, "r")]
         lines.reverse()
         print(_('Last %s nova syslog entries:-') % (entries))
@@ -1020,7 +1020,7 @@ class CellCommands(object):
         if cell_type not in ['parent', 'child', 'api', 'compute']:
             print("Error: cell type must be 'parent'/'api' or "
                 "'child'/'compute'")
-            return(2)
+            return 2
 
         # Set up the transport URL
         transport_hosts = self._create_transport_hosts(
@@ -1582,21 +1582,21 @@ def main():
 
     if CONF.category.name == "version":
         print(version.version_string_with_package())
-        return(0)
+        return 0
 
     if CONF.category.name == "bash-completion":
         cmd_common.print_bash_completion(CATEGORIES)
-        return(0)
+        return 0
 
     try:
         fn, fn_args, fn_kwargs = cmd_common.get_action_fn()
         ret = fn(*fn_args, **fn_kwargs)
         rpc.cleanup()
-        return(ret)
+        return ret
     except Exception:
         if CONF.post_mortem:
             import pdb
             pdb.post_mortem()
         else:
             print(_("An error has occurred:\n%s") % traceback.format_exc())
-        return(1)
+        return 1

nova/cmd/policy_check.py

@@ -168,7 +168,7 @@ def main():
     try:
         fn, fn_args, fn_kwargs = cmd_common.get_action_fn()
         ret = fn(*fn_args, **fn_kwargs)
-        return(ret)
+        return ret
     except Exception as ex:
         print(_("error: %s") % ex)
         return 1

nova/cmd/status.py

@@ -452,7 +452,7 @@ def main():
     try:
         fn, fn_args, fn_kwargs = cmd_common.get_action_fn()
         ret = fn(*fn_args, **fn_kwargs)
-        return(ret)
+        return ret
     except Exception:
         print(_('Error:\n%s') % traceback.format_exc())
         # This is 255 so it's not confused with the upgrade check exit codes.

nova/hacking/checks.py

@@ -108,6 +108,7 @@ log_remove_context = re.compile(
 log_string_interpolation = re.compile(r".*LOG\.(error|warning|info"
                                       r"|critical|exception|debug)"
                                       r"\([^,]*%[^,]*[,)]")
+return_not_followed_by_space = re.compile(r"^\s*return(?:\(|{|\"|'|#).*$")
 
 
 class BaseASTChecker(ast.NodeVisitor):
@@ -876,6 +877,23 @@ def check_uuid4(logical_line):
         yield (0, msg)
 
 
+def return_followed_by_space(logical_line):
+    """Return should be followed by a space.
+
+    Return should be followed by a space to clarify that return is
+    not a function. Adding a space may force the developer to rethink
+    if there are unnecessary parentheses in the written code.
+
+    Not correct: return(42), return(a, b)
+    Correct: return 42, return (a, b), return a, b
+
+    N358
+    """
+    if return_not_followed_by_space.match(logical_line):
+        yield (0,
+               "N357: Return keyword should be followed by a space.")
+
+
 def factory(register):
     register(import_no_db_in_virt)
     register(no_db_session_in_public_api)
@@ -920,3 +938,4 @@ def factory(register):
     register(no_assert_equal_true_false)
     register(no_assert_true_false_is_not)
     register(check_uuid4)
+    register(return_followed_by_space)

nova/tests/unit/fake_network.py

@@ -219,16 +219,16 @@ def fake_network_obj(context, network_id=1, ipv6=None):
 
 
 def fake_vif(x):
-    return{'id': x,
+    return {'id': x,
-           'created_at': None,
+            'created_at': None,
-           'updated_at': None,
+            'updated_at': None,
-           'deleted_at': None,
+            'deleted_at': None,
-           'deleted': 0,
+            'deleted': 0,
-           'address': 'DE:AD:BE:EF:00:%02x' % x,
+            'address': 'DE:AD:BE:EF:00:%02x' % x,
-           'uuid': getattr(uuids, 'vif%i' % x),
+            'uuid': getattr(uuids, 'vif%i' % x),
-           'network_id': x,
+            'network_id': x,
-           'instance_uuid': uuids.vifs_1,
+            'instance_uuid': uuids.vifs_1,
-           'tag': 'fake-tag'}
+            'tag': 'fake-tag'}
 
 
 def floating_ip_ids():

nova/tests/unit/matchers.py

@@ -167,10 +167,10 @@ class SubDictMismatch(object):
         if self.keys:
             return "Keys between dictionaries did not match"
         else:
-            return("Dictionaries do not match at %s. d1: %s d2: %s"
+            return ("Dictionaries do not match at %s. d1: %s d2: %s"
-                   % (self.key,
+                    % (self.key,
-                      self.super_value,
+                       self.super_value,
-                      self.sub_value))
+                       self.sub_value))
 
     def get_details(self):
         return {}

nova/tests/unit/test_hacking.py

@@ -893,3 +893,15 @@ class HackingTestCase(test.NoDBTestCase):
                   version_uuid = uuid.uuid4().version
                """
         self._assert_has_no_errors(code, checks.check_uuid4)
+
+    def test_return_followed_by_space(self):
+        self.assertEqual(1, len(list(checks.return_followed_by_space(
+            "return(42)"))))
+        self.assertEqual(1, len(list(checks.return_followed_by_space(
+            "return(' some string ')"))))
+        self.assertEqual(0, len(list(checks.return_followed_by_space(
+            "return 42"))))
+        self.assertEqual(0, len(list(checks.return_followed_by_space(
+            "return ' some string '"))))
+        self.assertEqual(0, len(list(checks.return_followed_by_space(
+            "return (int('40') + 2)"))))

nova/virt/xenapi/vm_utils.py

@@ -157,7 +157,7 @@ def get_vm_device_id(session, image_meta):
 
 def _hypervisor_supports_device_id(version):
     version_as_string = '.'.join(str(v) for v in version)
-    return(versionutils.is_compatible('6.1', version_as_string))
+    return versionutils.is_compatible('6.1', version_as_string)
 
 
 def create_vm(session, instance, name_label, kernel, ramdisk,