hacking: Check for missing ignore_missing calls

This comes up in reviews frequently. Let's automate it.

Change-Id: Ia7ebd7cf29fe4550b22921e898bebaaa5f7bb4f6
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

Author: stephenfin

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',
+        )

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