Merge pull request #9690 from github/redsun82/swift-stop-stub-reverts

Swift: prevent accidental revert of modified stub

Author: MathiasVP

swift/codegen/generators/qlgen.py

@@ -12,7 +12,16 @@
 log = logging.getLogger(__name__)
 
 
-class FormatError(Exception):
+class Error(Exception):
+    def __str__(self):
+        return self.args[0]
+
+
+class FormatError(Error):
+    pass
+
+
+class ModifiedStubMarkedAsGeneratedError(Error):
     pass
 
 
@@ -84,11 +93,22 @@ def get_classes_used_by(cls: ql.Class):
     return sorted(set(t for t in get_types_used_by(cls) if t[0].isupper()))
 
 
-def is_generated(file):
+def _is_generated_stub(file, check_modification=False):
     with open(file) as contents:
         for line in contents:
-            return line.startswith("// generated")
-        return False
+            if not line.startswith("// generated"):
+                return False
+            break
+        else:
+            # no lines
+            return False
+        if check_modification:
+            # one line already read, if we can read 4 other we are past the normal stub generation
+            line_threshold = 4
+            if sum(1 for _ in zip(range(line_threshold), contents)) == line_threshold:
+                raise ModifiedStubMarkedAsGeneratedError(
+                    f"{file.name} stub was modified but is still marked as generated")
+        return True
 
 
 def format(codeql, files):
@@ -138,7 +158,7 @@ def generate(opts, renderer):
     test_out = opts.ql_test_output
     missing_test_source_filename = "MISSING_SOURCE.txt"
     existing = {q for q in out.rglob("*.qll")}
-    existing |= {q for q in stub_out.rglob("*.qll") if is_generated(q)}
+    existing |= {q for q in stub_out.rglob("*.qll") if _is_generated_stub(q, check_modification=True)}
     existing |= {q for q in test_out.rglob("*.ql")}
     existing |= {q for q in test_out.rglob(missing_test_source_filename)}
 
@@ -157,7 +177,7 @@ def generate(opts, renderer):
         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_file):
+        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)

swift/codegen/test/test_qlgen.py

@@ -2,6 +2,8 @@
 import subprocess
 import sys
 
+import pytest
+
 from swift.codegen.generators import qlgen
 from swift.codegen.lib import ql
 from swift.codegen.test.utils import *
@@ -36,17 +38,22 @@ def children_file(): return ql_output_path() / "GetImmediateParent.qll"
 
 
 @pytest.fixture
-def generate(input, opts, renderer):
+def qlgen_opts(opts):
     opts.ql_stub_output = stub_path()
     opts.ql_output = ql_output_path()
     opts.ql_test_output = ql_test_output_path()
     opts.ql_format = True
     opts.swift_dir = paths.swift_dir
+    return opts
+
+
+@pytest.fixture
+def generate(input, qlgen_opts, renderer):
     renderer.written = []
 
     def func(classes):
         input.classes = classes
-        return run_generation(qlgen.generate, opts, renderer)
+        return run_generation(qlgen.generate, qlgen_opts, renderer)
 
     return func
 
@@ -349,7 +356,7 @@ def test_empty_cleanup(generate, renderer):
     assert renderer.mock_calls[-1] == mock.call.cleanup(set())
 
 
-def test_non_empty_cleanup(opts, generate, renderer, tmp_path):
+def test_non_empty_cleanup(opts, generate, renderer):
     ql_a = opts.ql_output / "A.qll"
     ql_b = opts.ql_output / "B.qll"
     stub_a = opts.ql_stub_output / "A.qll"
@@ -369,6 +376,13 @@ def test_non_empty_cleanup(opts, generate, renderer, tmp_path):
         {ql_a, ql_b, stub_a, test_a, test_b})
 
 
+def test_modified_stub_still_generated(qlgen_opts, renderer):
+    stub = qlgen_opts.ql_stub_output / "A.qll"
+    write(stub, "// generated\n\n\n\nsomething\n")
+    with pytest.raises(qlgen.ModifiedStubMarkedAsGeneratedError):
+        run_generation(qlgen.generate, qlgen_opts, renderer)
+
+
 def test_test_missing_source(generate_tests):
     generate_tests([
         schema.Class("A"),