diff --git a/HACKING.rst b/HACKING.rst index 45ed114b60dd..1161a350cb3f 100644 --- a/HACKING.rst +++ b/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 ------------------- diff --git a/nova/cmd/dhcpbridge.py b/nova/cmd/dhcpbridge.py index 2550a5f653e8..11a2b3d56f0c 100644 --- a/nova/cmd/dhcpbridge.py +++ b/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)) diff --git a/nova/cmd/manage.py b/nova/cmd/manage.py index d36eb496edc0..888da92f4aa4 100644 --- a/nova/cmd/manage.py +++ b/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='', 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 diff --git a/nova/cmd/policy_check.py b/nova/cmd/policy_check.py index bc71e3bb18ab..9026aed628a9 100644 --- a/nova/cmd/policy_check.py +++ b/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 diff --git a/nova/cmd/status.py b/nova/cmd/status.py index e95256491054..a1160e7b7d63 100644 --- a/nova/cmd/status.py +++ b/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. diff --git a/nova/hacking/checks.py b/nova/hacking/checks.py index de9b49c88b24..144ad5d2a92f 100644 --- a/nova/hacking/checks.py +++ b/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) diff --git a/nova/tests/unit/fake_network.py b/nova/tests/unit/fake_network.py index 9bea09332b92..bdd470dc83a1 100644 --- a/nova/tests/unit/fake_network.py +++ b/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, - 'created_at': None, - 'updated_at': None, - 'deleted_at': None, - 'deleted': 0, - 'address': 'DE:AD:BE:EF:00:%02x' % x, - 'uuid': getattr(uuids, 'vif%i' % x), - 'network_id': x, - 'instance_uuid': uuids.vifs_1, - 'tag': 'fake-tag'} + return {'id': x, + 'created_at': None, + 'updated_at': None, + 'deleted_at': None, + 'deleted': 0, + 'address': 'DE:AD:BE:EF:00:%02x' % x, + 'uuid': getattr(uuids, 'vif%i' % x), + 'network_id': x, + 'instance_uuid': uuids.vifs_1, + 'tag': 'fake-tag'} def floating_ip_ids(): diff --git a/nova/tests/unit/matchers.py b/nova/tests/unit/matchers.py index c2df5e414996..5282b549f0d0 100644 --- a/nova/tests/unit/matchers.py +++ b/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" - % (self.key, - self.super_value, - self.sub_value)) + return ("Dictionaries do not match at %s. d1: %s d2: %s" + % (self.key, + self.super_value, + self.sub_value)) def get_details(self): return {} diff --git a/nova/tests/unit/test_hacking.py b/nova/tests/unit/test_hacking.py index 6c4ff7d94b6e..f1b603dddfcf 100644 --- a/nova/tests/unit/test_hacking.py +++ b/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)")))) diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py index b8c1f49ad484..286835811ac7 100644 --- a/nova/virt/xenapi/vm_utils.py +++ b/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,