diff --git a/hacking/checks.py b/hacking/checks.py index 98b40370df..facb0cc6ac 100644 --- a/hacking/checks.py +++ b/hacking/checks.py @@ -10,6 +10,7 @@ # License for the specific language governing permissions and limitations # under the License. +import ast import os import re @@ -74,7 +75,7 @@ def assert_no_duplicated_setup(logical_line, filename): @core.flake8ext -def assert_use_of_client_aliases(logical_line, filename): +def assert_use_of_client_aliases(logical_line): """Ensure we use $service_client instead of $sdk_connection.service. O402 @@ -106,3 +107,73 @@ def assert_use_of_client_aliases(logical_line, filename): f"O402: {match.group(1)} is already a mock: there's no need to " f"assign a new mock.Mock instance.", ) + + +class SDKProxyFindChecker(ast.NodeVisitor): + """NodeVisitor to find ``*_client.find_*`` statements.""" + + def __init__(self): + self.error = False + + def visit_Call(self, node): + # No need to keep visiting the AST if we already found something. + if self.error: + return + + self.generic_visit(node) + + if not ( + isinstance(node.func, ast.Attribute) + and node.func.attr.startswith('find_') # and + # isinstance(node.func.value, ast.Attribute) and + # node.func.value.attr.endswith('_client') + ): + # print(f'skipping: got {node.func}') + return + + if not ( + ( + # handle calls like 'identity_client.find_project' + isinstance(node.func.value, ast.Name) + and node.func.value.id.endswith('client') + ) + or ( + # handle calls like 'self.app.client_manager.image.find_image' + isinstance(node.func.value, ast.Attribute) + and node.func.value.attr + in ('identity', 'network', 'image', 'compute') + ) + ): + return + + if not any(kw.arg == 'ignore_missing' for kw in node.keywords): + self.error = True + + +@core.flake8ext +def assert_find_ignore_missing_kwargs(logical_line, filename): + """Ensure ignore_missing is always used for ``find_*`` SDK proxy calls. + + Okay: self.compute_client.find_server(foo, ignore_missing=True) + Okay: self.image_client.find_server(foo, ignore_missing=False) + Okay: self.volume_client.volumes.find(name='foo') + O403: self.network_client.find_network(parsed_args.network) + O403: self.compute_client.find_flavor(flavor_id, get_extra_specs=True) + """ + if 'tests' in filename: + return + + checker = SDKProxyFindChecker() + try: + parsed_logical_line = ast.parse(logical_line) + except SyntaxError: + # let flake8 catch this itself + # https://github.com/PyCQA/flake8/issues/1948 + return + checker.visit(parsed_logical_line) + if checker.error: + yield ( + 0, + 'O403: Calls to find_* proxy methods must explicitly set ' + 'ignore_missing', + ) diff --git a/tox.ini b/tox.ini index 2ee2dc2cad..4392a36a8c 100644 --- a/tox.ini +++ b/tox.ini @@ -125,4 +125,5 @@ extension = O400 = checks:assert_no_oslo O401 = checks:assert_no_duplicated_setup O402 = checks:assert_use_of_client_aliases + O403 = checks:assert_find_ignore_missing_kwargs paths = ./hacking