Swift: skip QL code generation on untouched files

This is a developer QoL improvement, where running codegen will skip
writing (and especially formatting) any files that were not changed.

**Why?** While code generation in itself was pretty much instant, QL
formatting of generated code was starting to take a long time. This made
unconditionally running codegen quite annoying, for example before each
test run as part of an IDE workflow or as part of the pre-commit hook.

**How?** This was not completely straightforward as we could not work
with the contents of the file prior to code generation as that was
already post-processed by the QL formatting, so we had no chance of
comparing the output of template rendering with that. We therefore store
the hashes of the files _prior_ to QL formatting in a checked-in file
(`swift/ql/.generated.list`). We can therefore load those hashes at
the beginning of code generation, use them to compare the template
rendering output and update them in this special registry file.

**What else?** We also extend this mechanism to detect accidental
modification of generated files in a more robust way. Before this patch,
we were doing it with a rough regexp based heuristic. Now, we just store
the hashes of the files _after_ QL formatting in the same checked file,
so we can check that and stop generation if a generated file was
modified, or a stub was modified without removing the `// generated`
header.

Author: redsun82

.github/workflows/swift.yml

@@ -39,6 +39,7 @@ jobs:
               - 'swift/ql/lib/codeql/swift/elements/**'
               - 'swift/ql/lib/codeql/swift/generated/**'
               - 'swift/ql/test/extractor-tests/generated/**'
+              - 'swift/ql/.generated.list'
             ql:
               - 'github/workflows/swift.yml'
               - 'swift/**/*.ql'

.pre-commit-config.yaml

@@ -44,7 +44,7 @@ repos:
 
       - id: swift-codegen
         name: Run Swift checked in code generation
-        files: ^swift/(schema.py$|codegen/|.*/generated/|ql/lib/(swift\.dbscheme$|codeql/swift/elements))
+        files: ^swift/(schema.py$|codegen/|.*/generated/|ql/lib/(swift\.dbscheme$|codeql/swift/elements)|ql/\.generated.list)
         language: system
         entry: bazel run //swift/codegen -- --quiet
         pass_filenames: false

swift/codegen/codegen.py

@@ -40,11 +40,13 @@ def _parse_args() -> argparse.Namespace:
     p.add_argument("--codeql-binary", default="codeql", help="command to use for QL formatting (default %(default)s)")
     p.add_argument("--cpp-output", type=_abspath,
                    help="output directory for generated C++ files, required if trap or cpp is provided to --generate")
+    p.add_argument("--generated-registry", type=_abspath, default=paths.swift_dir / "ql/.generated.list",
+                   help="registry file containing information about checked-in generated code")
     return p.parse_args()
 
 
-def _abspath(x: str) -> pathlib.Path:
-    return pathlib.Path(x).resolve()
+def _abspath(x: str) -> typing.Optional[pathlib.Path]:
+    return pathlib.Path(x).resolve() if x else None
 
 
 def run():
@@ -56,9 +58,8 @@ def run():
     else:
         log_level = logging.INFO
     logging.basicConfig(format="{levelname} {message}", style='{', level=log_level)
-    exe_path = paths.exe_file.relative_to(opts.swift_dir)
     for target in opts.generate:
-        generate(target, opts, render.Renderer(exe_path))
+        generate(target, opts, render.Renderer(opts.swift_dir))
 
 
 if __name__ == "__main__":

swift/codegen/generators/qlgen.py

@@ -216,32 +216,11 @@ def get_classes_used_by(cls: ql.Class) -> typing.List[str]:
     return sorted(set(t for t in get_types_used_by(cls) if t[0].isupper() and t != cls.name))
 
 
-_generated_stub_re = re.compile(r"\n*private import .*\n+class \w+ extends Generated::\w+ \{[ \n]?\}", re.MULTILINE)
-
-
-def _is_generated_stub(file: pathlib.Path) -> bool:
-    with open(file) as contents:
-        for line in contents:
-            if not line.startswith("// generated"):
-                return False
-            break
-        else:
-            # no lines
-            return False
-        # we still do not detect modified synth constructors
-        if not file.name.endswith("Constructor.qll"):
-            # one line already read, if we can read 5 other we are past the normal stub generation
-            line_threshold = 5
-            first_lines = list(itertools.islice(contents, line_threshold))
-            if len(first_lines) == line_threshold or not _generated_stub_re.match("".join(first_lines)):
-                raise ModifiedStubMarkedAsGeneratedError(
-                    f"{file.name} stub was modified but is still marked as generated")
-        return True
-
-
 def format(codeql, files):
-    format_cmd = [codeql, "query", "format", "--in-place", "--"]
-    format_cmd.extend(str(f) for f in files if f.suffix in (".qll", ".ql"))
+    ql_files = [str(f) for f in files if f.suffix in (".qll", ".ql")]
+    if not ql_files:
+        return
+    format_cmd = [codeql, "query", "format", "--in-place", "--"] + ql_files
     res = subprocess.run(format_cmd, stderr=subprocess.PIPE, text=True)
     if res.returncode:
         for line in res.stderr.splitlines():
@@ -307,11 +286,14 @@ def generate(opts, renderer):
     stub_out = opts.ql_stub_output
     test_out = opts.ql_test_output
     missing_test_source_filename = "MISSING_SOURCE.txt"
+    include_file = stub_out.with_suffix(".qll")
+
+    generated = {q for q in out.rglob("*.qll")}
+    generated.add(include_file)
+    generated.update(q for q in test_out.rglob("*.ql"))
+    generated.update(q for q in test_out.rglob(missing_test_source_filename))
 
-    existing = {q for q in out.rglob("*.qll")}
-    existing |= {q for q in stub_out.rglob("*.qll") if _is_generated_stub(q)}
-    existing |= {q for q in test_out.rglob("*.ql")}
-    existing |= {q for q in test_out.rglob(missing_test_source_filename)}
+    stubs = {q for q in stub_out.rglob("*.qll")}
 
     data = schema.load_file(input)
 
@@ -324,77 +306,75 @@ def generate(opts, renderer):
 
     imports = {}
 
-    db_classes = [cls for cls in classes.values() if not cls.ipa]
-    renderer.render(ql.DbClasses(db_classes), out / "Raw.qll")
-
-    classes_by_dir_and_name = sorted(classes.values(), key=lambda cls: (cls.dir, cls.name))
-    for c in classes_by_dir_and_name:
-        imports[c.name] = get_import(stub_out / c.path, opts.swift_dir)
-
-    for c in classes.values():
-        qll = out / c.path.with_suffix(".qll")
-        c.imports = [imports[t] for t in get_classes_used_by(c)]
-        renderer.render(c, qll)
-        stub_file = stub_out / c.path.with_suffix(".qll")
-        if not stub_file.is_file() or _is_generated_stub(stub_file):
-            stub = ql.Stub(
-                name=c.name, base_import=get_import(qll, opts.swift_dir))
-            renderer.render(stub, stub_file)
-
-    # for example path/to/elements -> path/to/elements.qll
-    include_file = stub_out.with_suffix(".qll")
-    renderer.render(ql.ImportList(list(imports.values())), include_file)
-
-    renderer.render(ql.GetParentImplementation(list(classes.values())), out / 'ParentChild.qll')
-
-    for c in data.classes.values():
-        if _should_skip_qltest(c, data.classes):
-            continue
-        test_dir = test_out / c.group / c.name
-        test_dir.mkdir(parents=True, exist_ok=True)
-        if not any(test_dir.glob("*.swift")):
-            log.warning(f"no test source in {test_dir.relative_to(test_out)}")
-            renderer.render(ql.MissingTestInstructions(),
-                            test_dir / missing_test_source_filename)
-            continue
-        total_props, partial_props = _partition(_get_all_properties_to_be_tested(c, data.classes),
-                                                lambda p: p.is_single or p.is_predicate)
-        renderer.render(ql.ClassTester(class_name=c.name,
-                                       properties=total_props,
-                                       # in case of collapsed hierarchies we want to see the actual QL class in results
-                                       show_ql_class="qltest_collapse_hierarchy" in c.pragmas),
-                        test_dir / f"{c.name}.ql")
-        for p in partial_props:
-            renderer.render(ql.PropertyTester(class_name=c.name,
-                                              property=p), test_dir / f"{c.name}_{p.getter}.ql")
-
-    final_ipa_types = []
-    non_final_ipa_types = []
-    constructor_imports = []
-    ipa_constructor_imports = []
-    stubs = {}
-    for cls in sorted(data.classes.values(), key=lambda cls: (cls.group, cls.name)):
-        ipa_type = get_ql_ipa_class(cls)
-        if ipa_type.is_final:
-            final_ipa_types.append(ipa_type)
-            if ipa_type.has_params:
-                stub_file = stub_out / cls.group / f"{cls.name}Constructor.qll"
-                if not stub_file.is_file() or _is_generated_stub(stub_file):
-                    # stub rendering must be postponed as we might not have yet all subtracted ipa types in `ipa_type`
-                    stubs[stub_file] = ql.Synth.ConstructorStub(ipa_type)
-                constructor_import = get_import(stub_file, opts.swift_dir)
-                constructor_imports.append(constructor_import)
-                if ipa_type.is_ipa:
-                    ipa_constructor_imports.append(constructor_import)
-        else:
-            non_final_ipa_types.append(ipa_type)
-
-    for stub_file, data in stubs.items():
-        renderer.render(data, stub_file)
-    renderer.render(ql.Synth.Types(root.name, final_ipa_types, non_final_ipa_types), out / "Synth.qll")
-    renderer.render(ql.ImportList(constructor_imports), out / "SynthConstructors.qll")
-    renderer.render(ql.ImportList(ipa_constructor_imports), out / "PureSynthConstructors.qll")
-
-    renderer.cleanup(existing)
-    if opts.ql_format:
-        format(opts.codeql_binary, renderer.written)
+    with renderer.manage(generated=generated, stubs=stubs, registry=opts.generated_registry) as renderer:
+
+        db_classes = [cls for cls in classes.values() if not cls.ipa]
+        renderer.render(ql.DbClasses(db_classes), out / "Raw.qll")
+
+        classes_by_dir_and_name = sorted(classes.values(), key=lambda cls: (cls.dir, cls.name))
+        for c in classes_by_dir_and_name:
+            imports[c.name] = get_import(stub_out / c.path, opts.swift_dir)
+
+        for c in classes.values():
+            qll = out / c.path.with_suffix(".qll")
+            c.imports = [imports[t] for t in get_classes_used_by(c)]
+            renderer.render(c, qll)
+            stub_file = stub_out / c.path.with_suffix(".qll")
+            if not renderer.is_customized_stub(stub_file):
+                stub = ql.Stub(name=c.name, base_import=get_import(qll, opts.swift_dir))
+                renderer.render(stub, stub_file)
+
+        # for example path/to/elements -> path/to/elements.qll
+        renderer.render(ql.ImportList(list(imports.values())), include_file)
+
+        renderer.render(ql.GetParentImplementation(list(classes.values())), out / 'ParentChild.qll')
+
+        for c in data.classes.values():
+            if _should_skip_qltest(c, data.classes):
+                continue
+            test_dir = test_out / c.group / c.name
+            test_dir.mkdir(parents=True, exist_ok=True)
+            if not any(test_dir.glob("*.swift")):
+                log.warning(f"no test source in {test_dir.relative_to(test_out)}")
+                renderer.render(ql.MissingTestInstructions(),
+                                test_dir / missing_test_source_filename)
+                continue
+            total_props, partial_props = _partition(_get_all_properties_to_be_tested(c, data.classes),
+                                                    lambda p: p.is_single or p.is_predicate)
+            renderer.render(ql.ClassTester(class_name=c.name,
+                                           properties=total_props,
+                                           # in case of collapsed hierarchies we want to see the actual QL class in results
+                                           show_ql_class="qltest_collapse_hierarchy" in c.pragmas),
+                            test_dir / f"{c.name}.ql")
+            for p in partial_props:
+                renderer.render(ql.PropertyTester(class_name=c.name,
+                                                  property=p), test_dir / f"{c.name}_{p.getter}.ql")
+
+        final_ipa_types = []
+        non_final_ipa_types = []
+        constructor_imports = []
+        ipa_constructor_imports = []
+        stubs = {}
+        for cls in sorted(data.classes.values(), key=lambda cls: (cls.group, cls.name)):
+            ipa_type = get_ql_ipa_class(cls)
+            if ipa_type.is_final:
+                final_ipa_types.append(ipa_type)
+                if ipa_type.has_params:
+                    stub_file = stub_out / cls.group / f"{cls.name}Constructor.qll"
+                    if not renderer.is_customized_stub(stub_file):
+                        # stub rendering must be postponed as we might not have yet all subtracted ipa types in `ipa_type`
+                        stubs[stub_file] = ql.Synth.ConstructorStub(ipa_type)
+                    constructor_import = get_import(stub_file, opts.swift_dir)
+                    constructor_imports.append(constructor_import)
+                    if ipa_type.is_ipa:
+                        ipa_constructor_imports.append(constructor_import)
+            else:
+                non_final_ipa_types.append(ipa_type)
+
+        for stub_file, data in stubs.items():
+            renderer.render(data, stub_file)
+        renderer.render(ql.Synth.Types(root.name, final_ipa_types, non_final_ipa_types), out / "Synth.qll")
+        renderer.render(ql.ImportList(constructor_imports), out / "SynthConstructors.qll")
+        renderer.render(ql.ImportList(ipa_constructor_imports), out / "PureSynthConstructors.qll")
+        if opts.ql_format:
+            format(opts.codeql_binary, renderer.written)