Coverage for nova/hacking/checks.py: 85%
396 statements
« prev ^ index » next coverage.py v7.6.12, created at 2025-04-17 15:08 +0000
1# Copyright (c) 2012, Cloudscaling
2# All Rights Reserved.
3#
4# Licensed under the Apache License, Version 2.0 (the "License"); you may
5# not use this file except in compliance with the License. You may obtain
6# a copy of the License at
7#
8# http://www.apache.org/licenses/LICENSE-2.0
9#
10# Unless required by applicable law or agreed to in writing, software
11# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
12# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
13# License for the specific language governing permissions and limitations
14# under the License.
16"""
17Guidelines for writing new hacking checks
19 - Use only for Nova specific tests. OpenStack general tests
20 should be submitted to the common 'hacking' module.
21 - Pick numbers in the range N3xx. Find the current test with
22 the highest allocated number and then pick the next value.
23 - Keep the test method code in the source file ordered based
24 on the N3xx value.
25 - List the new rule in the top level HACKING.rst file
26 - Add test cases for each new rule to nova/tests/unit/test_hacking.py
28"""
30import ast
31import os
32import re
34from hacking import core
37UNDERSCORE_IMPORT_FILES = []
39session_check = re.compile(r"\w*def [a-zA-Z0-9].*[(].*session.*[)]")
40cfg_re = re.compile(r".*\scfg\.")
41# Excludes oslo.config OptGroup objects
42cfg_opt_re = re.compile(r".*[\s\[]cfg\.[a-zA-Z]*Opt\(")
43rule_default_re = re.compile(r".*RuleDefault\(")
44policy_enforce_re = re.compile(r".*_ENFORCER\.enforce\(")
45virt_file_re = re.compile(r"\./nova/(?:tests/)?virt/(\w+)/")
46virt_import_re = re.compile(
47 r"^\s*(?:import|from) nova\.(?:tests\.)?virt\.(\w+)")
48virt_config_re = re.compile(
49 r"CONF\.import_opt\('.*?', 'nova\.virt\.(\w+)('|.)")
50asse_trueinst_re = re.compile(
51 r"(.)*assertTrue\(isinstance\((\w|\.|\'|\"|\[|\])+, "
52 r"(\w|\.|\'|\"|\[|\])+\)\)")
53asse_equal_type_re = re.compile(
54 r"(.)*assertEqual\(type\((\w|\.|\'|\"|\[|\])+\), "
55 r"(\w|\.|\'|\"|\[|\])+\)")
56asse_equal_in_end_with_true_or_false_re = re.compile(r"assertEqual\("
57 r"(\w|[][.'\"])+ in (\w|[][.'\", ])+, (True|False)\)")
58asse_equal_in_start_with_true_or_false_re = re.compile(r"assertEqual\("
59 r"(True|False), (\w|[][.'\"])+ in (\w|[][.'\", ])+\)")
60# NOTE(snikitin): Next two regexes weren't united to one for more readability.
61# asse_true_false_with_in_or_not_in regex checks
62# assertTrue/False(A in B) cases where B argument has no spaces
63# asse_true_false_with_in_or_not_in_spaces regex checks cases
64# where B argument has spaces and starts/ends with [, ', ".
65# For example: [1, 2, 3], "some string", 'another string'.
66# We have to separate these regexes to escape a false positives
67# results. B argument should have spaces only if it starts
68# with [, ", '. Otherwise checking of string
69# "assertFalse(A in B and C in D)" will be false positives.
70# In this case B argument is "B and C in D".
71asse_true_false_with_in_or_not_in = re.compile(r"assert(True|False)\("
72 r"(\w|[][.'\"])+( not)? in (\w|[][.'\",])+(, .*)?\)")
73asse_true_false_with_in_or_not_in_spaces = re.compile(r"assert(True|False)"
74 r"\((\w|[][.'\"])+( not)? in [\[|'|\"](\w|[][.'\", ])+"
75 r"[\[|'|\"](, .*)?\)")
76asse_raises_regexp = re.compile(r"assertRaisesRegexp\(")
77conf_attribute_set_re = re.compile(r"CONF\.[a-z0-9_.]+\s*=\s*\w")
78translated_log = re.compile(r"(.)*LOG\.\w+\(\s*_\(\s*('|\")")
79mutable_default_args = re.compile(r"^\s*def .+\((.+=\{\}|.+=\[\])")
80string_translation = re.compile(r"[^_]*_\(\s*('|\")")
81underscore_import_check = re.compile(r"(.)*import _(.)*")
82import_translation_for_log_or_exception = re.compile(
83 r"(.)*(from\snova.i18n\simport)\s_")
84# We need this for cases where they have created their own _ function.
85custom_underscore_check = re.compile(r"(.)*_\s*=\s*(.)*")
86api_version_re = re.compile(r"@.*\bapi_version\b")
87dict_constructor_with_list_copy_re = re.compile(r".*\bdict\((\[)?(\(|\[)")
88decorator_re = re.compile(r"@.*")
89http_not_implemented_re = re.compile(r"raise .*HTTPNotImplemented\(")
90spawn_re = re.compile(
91 r".*(eventlet|greenthread)\.(?P<spawn_part>spawn(_n)?)\(.*\)")
92contextlib_nested = re.compile(r"^with (contextlib\.)?nested\(")
93doubled_words_re = re.compile(
94 r"\b(then?|[iao]n|i[fst]|but|f?or|at|and|[dt]o)\s+\1\b")
95log_remove_context = re.compile(
96 r"(.)*LOG\.(.*)\(.*(context=[_a-zA-Z0-9].*)+.*\)")
97return_not_followed_by_space = re.compile(r"^\s*return(?:\(|{|\"|'|#).*$")
98uuid4_re = re.compile(r"uuid4\(\)($|[^\.]|\.hex)")
99redundant_import_alias_re = re.compile(r"import (?:.*\.)?(.+) as \1$")
100yield_not_followed_by_space = re.compile(r"^\s*yield(?:\(|{|\[|\"|').*$")
101asse_regexpmatches = re.compile(
102 r"(assertRegexpMatches|assertNotRegexpMatches)\(")
103privsep_file_re = re.compile('^nova/privsep[./]')
104privsep_import_re = re.compile(
105 r"^(?:import|from).*\bprivsep\b")
106# Redundant parenthetical masquerading as a tuple, used with ``in``:
107# Space, "in", space, open paren
108# Optional single or double quote (so we match strings or symbols)
109# A sequence of the characters that can make up a symbol. (This is weak: a
110# string can contain other characters; and a numeric symbol can start with a
111# minus, and a method call has a param list, and... Not sure this gets better
112# without a lexer.)
113# The same closing quote
114# Close paren
115disguised_as_tuple_re = re.compile(r''' in \((['"]?)[a-zA-Z0-9_.]+\1\)''')
117# NOTE(takashin): The patterns of nox-existent mock assertion methods and
118# attributes do not cover all cases. If you find a new pattern,
119# add the pattern in the following regex patterns.
120mock_assert_method_re = re.compile(
121 r"\.((called_once(_with)*|has_calls)|"
122 r"mock_assert_(called(_(once|with|once_with))?"
123 r"|any_call|has_calls|not_called)|"
124 r"(asser|asset|asssert|assset)_(called(_(once|with|once_with))?"
125 r"|any_call|has_calls|not_called))\(")
126mock_attribute_re = re.compile(r"[\.\(](retrun_value)[,=\s]")
127# Regex for useless assertions
128useless_assertion_re = re.compile(
129 r"\.((assertIsNone)\(None|(assertTrue)\((True|\d+|'.+'|\".+\")),")
130# Regex for misuse of assert_has_calls
131mock_assert_has_calls_re = re.compile(r"\.assert_has_calls\s?=")
133# Regex for catching aliasing mock.Mock class in test
134mock_class_aliasing_re = re.compile(
135 r"^[A-Za-z0-9_.]+\s*=\s*mock\.(Magic|NonCallable)?Mock$")
136# Regex for catching aliasing mock.Mock class in test
137mock_class_as_new_value_in_patching_re = re.compile(
138 r"mock\.patch(\.object)?.* new=mock\.(Magic|NonCallable)?Mock[^(]")
139# Regex for direct use of oslo.concurrency lockutils.ReaderWriterLock
140rwlock_re = re.compile(
141 r"(?P<module_part>(oslo_concurrency\.)?(lockutils|fasteners))"
142 r"\.ReaderWriterLock\(.*\)")
143six_re = re.compile(r"^(import six(\..*)?|from six(\..*)? import .*)$")
144# Regex for catching the setDaemon method
145set_daemon_re = re.compile(r"\.setDaemon\(")
148class BaseASTChecker(ast.NodeVisitor):
149 """Provides a simple framework for writing AST-based checks.
151 Subclasses should implement visit_* methods like any other AST visitor
152 implementation. When they detect an error for a particular node the
153 method should call ``self.add_error(offending_node)``. Details about
154 where in the code the error occurred will be pulled from the node
155 object.
157 Subclasses should also provide a class variable named CHECK_DESC to
158 be used for the human readable error message.
160 """
162 def __init__(self, tree, filename):
163 """This object is created automatically by pycodestyle.
165 :param tree: an AST tree
166 :param filename: name of the file being analyzed
167 (ignored by our checks)
168 """
169 self._tree = tree
170 self._errors = []
172 def run(self):
173 """Called automatically by pycodestyle."""
174 self.visit(self._tree)
175 return self._errors
177 def add_error(self, node, message=None):
178 """Add an error caused by a node to the list of errors."""
179 message = message or self.CHECK_DESC
180 error = (node.lineno, node.col_offset, message, self.__class__)
181 self._errors.append(error)
183 def _check_call_names(self, call_node, names):
184 if isinstance(call_node, ast.Call):
185 if isinstance(call_node.func, ast.Name):
186 if call_node.func.id in names:
187 return True
188 return False
191@core.flake8ext
192def import_no_db_in_virt(logical_line, filename):
193 """Check for db calls from nova/virt
195 As of grizzly-2 all the database calls have been removed from
196 nova/virt, and we want to keep it that way.
198 N307
199 """
200 if "nova/virt" in filename and not filename.endswith("fake.py"):
201 if logical_line.startswith("from nova.db.main import api"):
202 yield (0, "N307: nova.db.* import not allowed in nova/virt/*")
205@core.flake8ext
206def no_db_session_in_public_api(logical_line, filename):
207 if "db/api.py" in filename:
208 if session_check.match(logical_line):
209 yield (0, "N309: public db api methods may not accept session")
212@core.flake8ext
213def use_timeutils_utcnow(logical_line, filename):
214 # tools are OK to use the standard datetime module
215 if "/tools/" in filename:
216 return
218 msg = "N310: timeutils.utcnow() must be used instead of datetime.%s()"
220 datetime_funcs = ['now', 'utcnow']
221 for f in datetime_funcs:
222 pos = logical_line.find('datetime.%s' % f)
223 if pos != -1:
224 yield (pos, msg % f)
227def _get_virt_name(regex, data):
228 m = regex.match(data)
229 if m is None: 229 ↛ 230line 229 didn't jump to line 230 because the condition on line 229 was never true
230 return None
231 driver = m.group(1)
232 # Ignore things we mis-detect as virt drivers in the regex
233 if driver in ["test_virt_drivers", "driver", "disk", "api", "imagecache", 233 ↛ 235line 233 didn't jump to line 235 because the condition on line 233 was never true
234 "cpu", "hardware", "image"]:
235 return None
236 return driver
239@core.flake8ext
240def import_no_virt_driver_import_deps(physical_line, filename):
241 """Check virt drivers' modules aren't imported by other drivers
243 Modules under each virt driver's directory are
244 considered private to that virt driver. Other drivers
245 in Nova must not access those drivers. Any code that
246 is to be shared should be refactored into a common
247 module
249 N311
250 """
251 thisdriver = _get_virt_name(virt_file_re, filename)
252 thatdriver = _get_virt_name(virt_import_re, physical_line)
253 if (thatdriver is not None and
254 thisdriver is not None and
255 thisdriver != thatdriver):
256 return (0, "N311: importing code from other virt drivers forbidden")
259@core.flake8ext
260def import_no_virt_driver_config_deps(physical_line, filename):
261 """Check virt drivers' config vars aren't used by other drivers
263 Modules under each virt driver's directory are
264 considered private to that virt driver. Other drivers
265 in Nova must not use their config vars. Any config vars
266 that are to be shared should be moved into a common module
268 N312
269 """
270 thisdriver = _get_virt_name(virt_file_re, filename)
271 thatdriver = _get_virt_name(virt_config_re, physical_line)
272 if (thatdriver is not None and
273 thisdriver is not None and
274 thisdriver != thatdriver):
275 return (0, "N312: using config vars from other virt drivers forbidden")
278@core.flake8ext
279def capital_cfg_help(logical_line, tokens):
280 msg = "N313: capitalize help string"
282 if cfg_re.match(logical_line):
283 for t in range(len(tokens)):
284 if tokens[t][1] == "help":
285 txt = tokens[t + 2][1]
286 if len(txt) > 1 and txt[1].islower():
287 yield (0, msg)
290@core.flake8ext
291def assert_true_instance(logical_line):
292 """Check for assertTrue(isinstance(a, b)) sentences
294 N316
295 """
296 if asse_trueinst_re.match(logical_line):
297 yield (0, "N316: assertTrue(isinstance(a, b)) sentences not allowed")
300@core.flake8ext
301def assert_equal_type(logical_line):
302 """Check for assertEqual(type(A), B) sentences
304 N317
305 """
306 if asse_equal_type_re.match(logical_line):
307 yield (0, "N317: assertEqual(type(A), B) sentences not allowed")
310@core.flake8ext
311def no_translate_logs(logical_line, filename):
312 """Check for 'LOG.foo(_('
314 As per our translation policy, we shouldn't translate logs.
315 This check assumes that 'LOG' is a logger.
317 N319
318 """
319 if translated_log.match(logical_line):
320 yield (0, "N319 Don't translate logs")
323@core.flake8ext
324def no_import_translation_in_tests(logical_line, filename):
325 """Check for 'from nova.i18n import _'
326 N337
327 """
328 if 'nova/tests/' in filename:
329 res = import_translation_for_log_or_exception.match(logical_line)
330 if res:
331 yield (0, "N337 Don't import translation in tests")
334@core.flake8ext
335def no_setting_conf_directly_in_tests(logical_line, filename):
336 """Check for setting CONF.* attributes directly in tests
338 The value can leak out of tests affecting how subsequent tests run.
339 Using self.flags(option=value) is the preferred method to temporarily
340 set config options in tests.
342 N320
343 """
344 if 'nova/tests/' in filename:
345 res = conf_attribute_set_re.match(logical_line)
346 if res:
347 yield (0, "N320: Setting CONF.* attributes directly in tests is "
348 "forbidden. Use self.flags(option=value) instead")
351@core.flake8ext
352def no_mutable_default_args(logical_line):
353 msg = "N322: Method's default argument shouldn't be mutable!"
354 if mutable_default_args.match(logical_line):
355 yield (0, msg)
358@core.flake8ext
359def check_explicit_underscore_import(logical_line, filename):
360 """Check for explicit import of the _ function
362 We need to ensure that any files that are using the _() function
363 to translate logs are explicitly importing the _ function. We
364 can't trust unit test to catch whether the import has been
365 added so we need to check for it here.
366 """
368 # Build a list of the files that have _ imported. No further
369 # checking needed once it is found.
370 if filename in UNDERSCORE_IMPORT_FILES:
371 pass
372 elif (underscore_import_check.match(logical_line) or
373 custom_underscore_check.match(logical_line)):
374 UNDERSCORE_IMPORT_FILES.append(filename)
375 elif string_translation.match(logical_line): 375 ↛ exitline 375 didn't return from function 'check_explicit_underscore_import' because the condition on line 375 was always true
376 yield (0, "N323: Found use of _() without explicit import of _ !")
379@core.flake8ext
380def use_jsonutils(logical_line, filename):
381 # tools are OK to use the standard json module
382 if "/tools/" in filename: 382 ↛ 383line 382 didn't jump to line 383 because the condition on line 382 was never true
383 return
385 msg = "N324: jsonutils.%(fun)s must be used instead of json.%(fun)s"
387 if "json." in logical_line:
388 json_funcs = ['dumps(', 'dump(', 'loads(', 'load(']
389 for f in json_funcs:
390 pos = logical_line.find('json.%s' % f)
391 if pos != -1:
392 yield (pos, msg % {'fun': f[:-1]})
395@core.flake8ext
396def check_api_version_decorator(logical_line, previous_logical, blank_before,
397 filename):
398 msg = ("N332: the api_version decorator must be the first decorator"
399 " on a method.")
400 if blank_before == 0 and re.match(api_version_re, logical_line) \
401 and re.match(decorator_re, previous_logical):
402 yield (0, msg)
405class CheckForTransAdd(BaseASTChecker):
406 """Checks for the use of concatenation on a translated string.
408 Translations should not be concatenated with other strings, but
409 should instead include the string being added to the translated
410 string to give the translators the most information.
411 """
413 name = 'check_for_trans_add'
414 version = '0.1'
416 CHECK_DESC = ('N326 Translated messages cannot be concatenated. '
417 'String should be included in translated message.')
419 TRANS_FUNC = ['_']
421 def visit_BinOp(self, node):
422 if isinstance(node.op, ast.Add): 422 ↛ 428line 422 didn't jump to line 428 because the condition on line 422 was always true
423 for node_x in (node.left, node.right):
424 if isinstance(node_x, ast.Call):
425 if isinstance(node_x.func, ast.Name): 425 ↛ 423line 425 didn't jump to line 423 because the condition on line 425 was always true
426 if node_x.func.id == '_': 426 ↛ 423line 426 didn't jump to line 423 because the condition on line 426 was always true
427 self.add_error(node_x)
428 super(CheckForTransAdd, self).generic_visit(node)
431class _FindVariableReferences(ast.NodeVisitor):
432 def __init__(self):
433 super(_FindVariableReferences, self).__init__()
434 self._references = []
436 def visit_Name(self, node):
437 if isinstance(node.ctx, ast.Load): 437 ↛ 444line 437 didn't jump to line 444 because the condition on line 437 was always true
438 # This means the value of a variable was loaded. For example a
439 # variable 'foo' was used like:
440 # mocked_thing.bar = foo
441 # foo()
442 # self.assertRaises(exception, foo)
443 self._references.append(node.id)
444 super(_FindVariableReferences, self).generic_visit(node)
447class CheckForUncalledTestClosure(BaseASTChecker):
448 """Look for closures that are never called in tests.
450 A recurring pattern when using multiple mocks is to create a closure
451 decorated with mocks like:
453 def test_thing(self):
454 @mock.patch.object(self.compute, 'foo')
455 @mock.patch.object(self.compute, 'bar')
456 def _do_test(mock_bar, mock_foo):
457 # Test things
458 _do_test()
460 However it is easy to leave off the _do_test() and have the test pass
461 because nothing runs. This check looks for methods defined within a test
462 method and ensures that there is a reference to them. Only methods defined
463 one level deep are checked. Something like:
465 def test_thing(self):
466 class FakeThing:
467 def foo(self):
469 would not ensure that foo is referenced.
471 N349
472 """
474 name = 'check_for_uncalled_test_closure'
475 version = '0.1'
477 def __init__(self, tree, filename):
478 super(CheckForUncalledTestClosure, self).__init__(tree, filename)
479 self._filename = filename
481 def visit_FunctionDef(self, node):
482 # self._filename is 'stdin' in the unit test for this check.
483 if (not os.path.basename(self._filename).startswith('test_') and 483 ↛ 485line 483 didn't jump to line 485 because the condition on line 483 was never true
484 os.path.basename(self._filename) != 'stdin'):
485 return
487 closures = []
488 references = []
489 # Walk just the direct nodes of the test method
490 for child_node in ast.iter_child_nodes(node):
491 if isinstance(child_node, ast.FunctionDef):
492 closures.append(child_node.name)
494 # Walk all nodes to find references
495 find_references = _FindVariableReferences()
496 find_references.generic_visit(node)
497 references = find_references._references
499 missed = set(closures) - set(references)
500 if missed:
501 self.add_error(node, 'N349: Test closures not called: %s'
502 % ','.join(missed))
505@core.flake8ext
506def assert_true_or_false_with_in(logical_line):
507 """Check for assertTrue/False(A in B), assertTrue/False(A not in B),
508 assertTrue/False(A in B, message) or assertTrue/False(A not in B, message)
509 sentences.
511 N334
512 """
513 res = (asse_true_false_with_in_or_not_in.search(logical_line) or
514 asse_true_false_with_in_or_not_in_spaces.search(logical_line))
515 if res:
516 yield (0, "N334: Use assertIn/NotIn(A, B) rather than "
517 "assertTrue/False(A in/not in B) when checking collection "
518 "contents.")
521@core.flake8ext
522def assert_raises_regexp(logical_line):
523 """Check for usage of deprecated assertRaisesRegexp
525 N335
526 """
527 res = asse_raises_regexp.search(logical_line)
528 if res:
529 yield (0, "N335: assertRaisesRegex must be used instead "
530 "of assertRaisesRegexp")
533@core.flake8ext
534def dict_constructor_with_list_copy(logical_line):
535 msg = ("N336: Must use a dict comprehension instead of a dict constructor"
536 " with a sequence of key-value pairs."
537 )
538 if dict_constructor_with_list_copy_re.match(logical_line):
539 yield (0, msg)
542@core.flake8ext
543def assert_equal_in(logical_line):
544 """Check for assertEqual(A in B, True), assertEqual(True, A in B),
545 assertEqual(A in B, False) or assertEqual(False, A in B) sentences
547 N338
548 """
549 res = (asse_equal_in_start_with_true_or_false_re.search(logical_line) or
550 asse_equal_in_end_with_true_or_false_re.search(logical_line))
551 if res:
552 yield (0, "N338: Use assertIn/NotIn(A, B) rather than "
553 "assertEqual(A in B, True/False) when checking collection "
554 "contents.")
557@core.flake8ext
558def check_http_not_implemented(logical_line, filename, noqa):
559 msg = ("N339: HTTPNotImplemented response must be implemented with"
560 " common raise_feature_not_supported().")
561 if noqa: 561 ↛ 562line 561 didn't jump to line 562 because the condition on line 561 was never true
562 return
563 if ("nova/api/openstack/compute" not in filename):
564 return
565 if re.match(http_not_implemented_re, logical_line):
566 yield (0, msg)
569@core.flake8ext
570def check_greenthread_spawns(logical_line, filename):
571 """Check for use of greenthread.spawn(), greenthread.spawn_n(),
572 eventlet.spawn(), and eventlet.spawn_n()
574 N340
575 """
576 msg = ("N340: Use nova.utils.%(spawn)s() rather than "
577 "greenthread.%(spawn)s() and eventlet.%(spawn)s()")
578 if "nova/utils.py" in filename or "nova/tests/" in filename: 578 ↛ 579line 578 didn't jump to line 579 because the condition on line 578 was never true
579 return
581 match = re.match(spawn_re, logical_line)
583 if match:
584 yield (0, msg % {'spawn': match.group('spawn_part')})
587@core.flake8ext
588def check_no_contextlib_nested(logical_line, filename):
589 msg = ("N341: contextlib.nested is deprecated. With Python 2.7 and later "
590 "the with-statement supports multiple nested objects. See https://"
591 "docs.python.org/2/library/contextlib.html#contextlib.nested for "
592 "more information. nova.test.nested() is an alternative as well.")
594 if contextlib_nested.match(logical_line):
595 yield (0, msg)
598@core.flake8ext
599def check_config_option_in_central_place(logical_line, filename):
600 msg = ("N342: Config options should be in the central location "
601 "'/nova/conf/*'. Do not declare new config options outside "
602 "of that folder.")
603 # That's the correct location
604 if "nova/conf/" in filename:
605 return
607 # (macsz) All config options (with exceptions that are clarified
608 # in the list below) were moved to the central place. List below is for
609 # all options that were impossible to move without doing a major impact
610 # on code. Add full path to a module or folder.
611 conf_exceptions = [
612 # CLI opts are allowed to be outside of nova/conf directory
613 'nova/cmd/manage.py',
614 'nova/cmd/policy.py',
615 'nova/cmd/status.py',
616 # config options should not be declared in tests, but there is
617 # another checker for it (N320)
618 'nova/tests',
619 ]
621 if any(f in filename for f in conf_exceptions):
622 return
624 if cfg_opt_re.match(logical_line):
625 yield (0, msg)
628@core.flake8ext
629def check_policy_registration_in_central_place(logical_line, filename):
630 msg = ('N350: Policy registration should be in the central location(s) '
631 '"/nova/policies/*"')
632 # This is where registration should happen
633 if "nova/policies/" in filename:
634 return
635 # A couple of policy tests register rules
636 if "nova/tests/unit/test_policy.py" in filename: 636 ↛ 637line 636 didn't jump to line 637 because the condition on line 636 was never true
637 return
639 if rule_default_re.match(logical_line):
640 yield (0, msg)
643@core.flake8ext
644def check_policy_enforce(logical_line, filename):
645 """Look for uses of nova.policy._ENFORCER.enforce()
647 Now that policy defaults are registered in code the _ENFORCER.authorize
648 method should be used. That ensures that only registered policies are used.
649 Uses of _ENFORCER.enforce could allow unregistered policies to be used, so
650 this check looks for uses of that method.
652 N351
653 """
655 msg = ('N351: nova.policy._ENFORCER.enforce() should not be used. '
656 'Use the authorize() method instead.')
658 if policy_enforce_re.match(logical_line):
659 yield (0, msg)
662@core.flake8ext
663def check_doubled_words(physical_line, filename):
664 """Check for the common doubled-word typos
666 N343
667 """
668 msg = ("N343: Doubled word '%(word)s' typo found")
670 match = re.search(doubled_words_re, physical_line)
672 if match:
673 return (0, msg % {'word': match.group(1)})
676@core.flake8ext
677def no_os_popen(logical_line):
678 """Disallow 'os.popen('
680 Deprecated library function os.popen() Replace it using subprocess
681 https://bugs.launchpad.net/tempest/+bug/1529836
683 N348
684 """
686 if 'os.popen(' in logical_line:
687 yield (0, 'N348 Deprecated library function os.popen(). '
688 'Replace it using subprocess module. ')
691@core.flake8ext
692def no_log_warn(logical_line):
693 """Disallow 'LOG.warn('
695 Deprecated LOG.warn(), instead use LOG.warning
696 https://bugs.launchpad.net/senlin/+bug/1508442
698 N352
699 """
701 msg = ("N352: LOG.warn is deprecated, please use LOG.warning!")
702 if "LOG.warn(" in logical_line:
703 yield (0, msg)
706@core.flake8ext
707def check_context_log(logical_line, filename, noqa):
708 """check whether context is being passed to the logs
710 Not correct: LOG.info("Rebooting instance", context=context)
711 Correct: LOG.info("Rebooting instance")
712 https://bugs.launchpad.net/nova/+bug/1500896
714 N353
715 """
716 if noqa: 716 ↛ 717line 716 didn't jump to line 717 because the condition on line 716 was never true
717 return
719 if "nova/tests" in filename: 719 ↛ 720line 719 didn't jump to line 720 because the condition on line 719 was never true
720 return
722 if log_remove_context.match(logical_line):
723 yield (0,
724 "N353: Nova is using oslo.context's RequestContext "
725 "which means the context object is in scope when "
726 "doing logging using oslo.log, so no need to pass it as "
727 "kwarg.")
730@core.flake8ext
731def no_assert_equal_true_false(logical_line):
732 """Enforce use of assertTrue/assertFalse.
734 Prevent use of assertEqual(A, True|False), assertEqual(True|False, A),
735 assertNotEqual(A, True|False), and assertNotEqual(True|False, A).
737 N355
738 """
739 _start_re = re.compile(r'assert(Not)?Equal\((True|False),')
740 _end_re = re.compile(r'assert(Not)?Equal\(.*,\s+(True|False)\)$')
742 if _start_re.search(logical_line) or _end_re.search(logical_line):
743 yield (0, "N355: assertEqual(A, True|False), "
744 "assertEqual(True|False, A), assertNotEqual(A, True|False), "
745 "or assertEqual(True|False, A) sentences must not be used. "
746 "Use assertTrue(A) or assertFalse(A) instead")
749@core.flake8ext
750def no_assert_true_false_is_not(logical_line):
751 """Enforce use of assertIs/assertIsNot.
753 Prevent use of assertTrue(A is|is not B) and assertFalse(A is|is not B).
755 N356
756 """
757 _re = re.compile(r'assert(True|False)\(.+\s+is\s+(not\s+)?.+\)$')
759 if _re.search(logical_line): 759 ↛ exitline 759 didn't return from function 'no_assert_true_false_is_not' because the condition on line 759 was always true
760 yield (0, "N356: assertTrue(A is|is not B) or "
761 "assertFalse(A is|is not B) sentences must not be used. "
762 "Use assertIs(A, B) or assertIsNot(A, B) instead")
765@core.flake8ext
766def check_uuid4(logical_line):
767 """Generating UUID
769 Use oslo_utils.uuidutils or uuidsentinel(in case of test cases) to generate
770 UUID instead of uuid4().
772 N357
773 """
775 msg = ("N357: Use oslo_utils.uuidutils or uuidsentinel(in case of test "
776 "cases) to generate UUID instead of uuid4().")
778 if uuid4_re.search(logical_line):
779 yield (0, msg)
782@core.flake8ext
783def return_followed_by_space(logical_line):
784 """Return should be followed by a space.
786 Return should be followed by a space to clarify that return is
787 not a function. Adding a space may force the developer to rethink
788 if there are unnecessary parentheses in the written code.
790 Not correct: return(42), return(a, b)
791 Correct: return, return 42, return (a, b), return a, b
793 N358
794 """
795 if return_not_followed_by_space.match(logical_line):
796 yield (0,
797 "N358: Return keyword should be followed by a space.")
800@core.flake8ext
801def no_redundant_import_alias(logical_line):
802 """Check for redundant import aliases.
804 Imports should not be in the forms below.
806 from x import y as y
807 import x as x
808 import x.y as y
810 N359
811 """
812 if re.search(redundant_import_alias_re, logical_line):
813 yield (0, "N359: Import alias should not be redundant.")
816@core.flake8ext
817def yield_followed_by_space(logical_line):
818 """Yield should be followed by a space.
820 Yield should be followed by a space to clarify that yield is
821 not a function. Adding a space may force the developer to rethink
822 if there are unnecessary parentheses in the written code.
824 Not correct: yield(x), yield(a, b)
825 Correct: yield x, yield (a, b), yield a, b
827 N360
828 """
829 if yield_not_followed_by_space.match(logical_line):
830 yield (0,
831 "N360: Yield keyword should be followed by a space.")
834@core.flake8ext
835def assert_regexpmatches(logical_line):
836 """Check for usage of deprecated assertRegexpMatches/assertNotRegexpMatches
838 N361
839 """
840 res = asse_regexpmatches.search(logical_line)
841 if res:
842 yield (0, "N361: assertRegex/assertNotRegex must be used instead "
843 "of assertRegexpMatches/assertNotRegexpMatches.")
846@core.flake8ext
847def privsep_imports_not_aliased(logical_line, filename):
848 """Do not abbreviate or alias privsep module imports.
850 When accessing symbols under nova.privsep in code or tests, the full module
851 path (e.g. nova.privsep.path.readfile(...)) should be used
852 explicitly rather than importing and using an alias/abbreviation such as:
854 from nova.privsep import path
855 ...
856 path.readfile(...)
858 See Ief177dbcb018da6fbad13bb0ff153fc47292d5b9.
860 N362
861 """
862 if (
863 # Give modules under nova.privsep a pass
864 not privsep_file_re.match(filename) and
865 # Any style of import of privsep...
866 privsep_import_re.match(logical_line) and
867 # ...that isn't 'import nova.privsep[.foo...]'
868 logical_line.count(' ') > 1):
869 yield (0, "N362: always import privsep modules so that the use of "
870 "escalated permissions is obvious to callers. For example, "
871 "use 'import nova.privsep.path' instead of "
872 "'from nova.privsep import path'.")
875@core.flake8ext
876def did_you_mean_tuple(logical_line):
877 """Disallow ``(not_a_tuple)`` because you meant ``(a_tuple_of_one,)``.
879 N363
880 """
881 if disguised_as_tuple_re.search(logical_line):
882 yield (0, "N363: You said ``in (not_a_tuple)`` when you almost "
883 "certainly meant ``in (a_tuple_of_one,)``.")
886@core.flake8ext
887def nonexistent_assertion_methods_and_attributes(logical_line, filename):
888 """Check non-existent mock assertion methods and attributes.
890 The following assertion methods do not exist.
892 - called_once()
893 - called_once_with()
894 - has_calls()
895 - mock_assert_*()
897 The following typos were found in the past cases.
899 - asser_*
900 - asset_*
901 - assset_*
902 - asssert_*
903 - retrun_value
905 N364
906 """
907 msg = ("N364: Non existent mock assertion method or attribute (%s) is "
908 "used. Check a typo or whether the assertion method should begin "
909 "with 'assert_'.")
910 if 'nova/tests/' in filename:
911 match = mock_assert_method_re.search(logical_line)
912 if match:
913 yield (0, msg % match.group(1))
915 match = mock_attribute_re.search(logical_line)
916 if match:
917 yield (0, msg % match.group(1))
920@core.flake8ext
921def useless_assertion(logical_line, filename):
922 """Check useless assertions in tests.
924 The following assertions are useless.
926 - assertIsNone(None, ...)
927 - assertTrue(True, ...)
928 - assertTrue(2, ...) # Constant number
929 - assertTrue('Constant string', ...)
930 - assertTrue("Constant string", ...)
932 They are usually misuses of assertIsNone or assertTrue.
934 N365
935 """
936 msg = "N365: Misuse of %s."
937 if 'nova/tests/' in filename:
938 match = useless_assertion_re.search(logical_line)
939 if match:
940 yield (0, msg % (match.group(2) or match.group(3)))
943@core.flake8ext
944def check_assert_has_calls(logical_line, filename):
945 """Check misuse of assert_has_calls.
947 Not correct: mock_method.assert_has_calls = [mock.call(0)]
948 Correct: mock_method.assert_has_calls([mock.call(0)])
950 N366
951 """
952 msg = "N366: The assert_has_calls is a method rather than a variable."
953 if ('nova/tests/' in filename and
954 mock_assert_has_calls_re.search(logical_line)):
955 yield (0, msg)
958@core.flake8ext
959def do_not_alias_mock_class(logical_line, filename):
960 """Check for aliasing Mock class
962 Aliasing Mock class almost always a bad idea. Consider the test code
963 trying to catch the instantiation of the Rados class but instead
964 introducing a global change on the Mock object:
965 https://github.com/openstack/nova/blob/10b1dc84f47a71061340f8e0ae0fe32dca44061a/nova/tests/unit/storage/test_rbd.py#L122-L125
966 After this code every test that assumes that mock.Mock().shutdown is a new
967 auto-generated mock.Mock() object will fail a shutdown is now defined in
968 the Mock class level and therefore surviving between test cases.
970 N367
971 """
972 if 'nova/tests/' in filename:
973 res = mock_class_aliasing_re.match(logical_line)
974 if res:
975 yield (
976 0,
977 "N367: Aliasing mock.Mock class is dangerous as it easy to "
978 "introduce class level changes in Mock that survives "
979 "between test cases. If you want to mock object creation "
980 "then mock the class under test with a mock _instance_ and "
981 "set the return_value of the mock to return mock instances. "
982 "See for example: "
983 "https://review.opendev.org/c/openstack/nova/+/805657"
984 )
987@core.flake8ext
988def do_not_use_mock_class_as_new_mock_value(logical_line, filename):
989 """Check if mock.Mock class is used during set up of a patcher as new
990 kwargs.
992 The mock.patch and mock.patch.object takes a `new` kwargs and use that
993 value as the replacement during the patching. Using new=mock.Mock
994 (instead of new=mock.Mock() or new_callable=mock.Mock) results in code
995 under test pointing to the Mock class. This is almost always a wrong thing
996 as any changes on that class will leak between test cases uncontrollably.
998 N368
999 """
1000 if 'nova/tests/' in filename:
1001 res = mock_class_as_new_value_in_patching_re.search(logical_line)
1002 if res:
1003 yield (
1004 0,
1005 "N368: Using mock.patch(..., new=mock.Mock) causes that the "
1006 "patching will introduce the Mock class as replacement value "
1007 "instead of a mock object. Any change on the Mock calls will "
1008 "leak out from the test and can cause interference. "
1009 "Use new=mock.Mock() or new_callable=mock.Mock instead."
1010 )
1013@core.flake8ext
1014def check_lockutils_rwlocks(logical_line):
1015 """Check for direct use of oslo.concurrency lockutils.ReaderWriterLock()
1017 oslo.concurrency lockutils uses fasteners.ReaderWriterLock to provide
1018 read/write locks and fasteners calls threading.current_thread() to track
1019 and identify lock holders and waiters. The eventlet implementation of
1020 current_thread() only supports greenlets of type GreenThread, else it falls
1021 back on the native threading.current_thread() method.
1023 See https://github.com/eventlet/eventlet/issues/731 for details.
1025 N369
1026 """
1027 msg = ("N369: %(module)s.ReaderWriterLock() does not "
1028 "function correctly with eventlet patched code. "
1029 "Use nova.utils.ReaderWriterLock() instead.")
1030 match = re.match(rwlock_re, logical_line)
1031 if match:
1032 yield (
1033 0,
1034 msg % {'module': match.group('module_part')}
1035 )
1038@core.flake8ext
1039def check_six(logical_line):
1040 """Check for use of six
1042 nova is now Python 3-only so we don't want six. However, people might use
1043 it out of habit and it will likely work since six is a transitive
1044 dependency.
1046 N370
1047 """
1048 match = re.match(six_re, logical_line)
1049 if match:
1050 yield (0, "N370: Don't use or import six")
1053@core.flake8ext
1054def import_stock_mock(logical_line):
1055 """Use python's mock, not the mock library.
1057 Since we `dropped support for python 2`__, we no longer need to use the
1058 mock library, which existed to backport py3 functionality into py2. Change
1059 Ib44b5bff657c8e76c4f701e14d51a4efda3f6d32 cut over to importing the stock
1060 mock, which must be done by saying::
1062 from unittest import mock
1064 ...because if you say::
1066 import mock
1068 ...you may be getting the stock mock; or, due to transitive dependencies in
1069 the environment, the library mock. This check can be removed in the future
1070 (and we can start saying ``import mock`` again) if we manage to purge these
1071 transitive dependencies.
1073 .. __: https://review.opendev.org/#/c/687954/
1075 N371
1076 """
1077 if logical_line == 'import mock' or logical_line.startswith('from mock'):
1078 yield (
1079 0,
1080 "N371: You must explicitly import python's mock: "
1081 "``from unittest import mock``"
1082 )
1085@core.flake8ext
1086def check_set_daemon(logical_line):
1087 """Check for use of the setDaemon method of the threading.Thread class
1089 The setDaemon method of the threading.Thread class has been deprecated
1090 since Python 3.10. Use the daemon attribute instead.
1092 See
1093 https://docs.python.org/3.10/library/threading.html#threading.Thread.setDaemon
1094 for details.
1096 N372
1097 """
1098 res = set_daemon_re.search(logical_line)
1099 if res:
1100 yield (0, "N372: Don't use the setDaemon method. "
1101 "Use the daemon attribute instead.")