Merge pull request #17444 from hvitved/rust/final-classes

Rust/Swift: Make all public AST classes `final`

.pre-commit-config.yaml

@@ -15,7 +15,7 @@ repos:
       - id: clang-format
 
   - repo: https://github.com/pre-commit/mirrors-autopep8
-    rev: v1.6.0
+    rev: v2.0.4
     hooks:
       - id: autopep8
         files: ^misc/codegen/.*\.py

misc/codegen/generators/qlgen.py

@@ -2,18 +2,19 @@
 QL code generation
 
 `generate(opts, renderer)` will generate in the library directory:
- * generated/Raw.qll with thin class wrappers around DB types
- * generated/Synth.qll with the base algebraic datatypes for AST entities
- * generated/<group>/<Class>.qll with generated properties for each class
- * if not already modified, a elements/<group>/<Class>.qll stub to customize the above classes
- * elements.qll importing all the above stubs
- * if not already modified, a elements/<group>/<Class>Constructor.qll stub to customize the algebraic datatype
+ * `generated/Raw.qll` with thin class wrappers around DB types
+ * `generated/Synth.qll` with the base algebraic datatypes for AST entities
+ * `generated/<group>/<Class>.qll` with generated properties for each class
+ * if not already modified, an `elements/<group>/<Class>Impl.qll` stub to customize the above classes
+ * `elements/<group>/<Class>.qll` that wraps the internal `<Class>Impl.qll` file in a public `final` class.
+ * `elements.qll` importing all the above public classes
+ * if not already modified, an `elements/<group>/<Class>Constructor.qll` stub to customize the algebraic datatype
    characteristic predicate
- * generated/SynthConstructors.qll importing all the above constructor stubs
- * generated/PureSynthConstructors.qll importing constructor stubs for pure synthesized types (that is, not
+ * `generated/SynthConstructors.qll` importing all the above constructor stubs
+ * `generated/PureSynthConstructors.qll` importing constructor stubs for pure synthesized types (that is, not
    corresponding to raw types)
 Moreover in the test directory for each <Class> in <group> it will generate beneath the
-extractor-tests/generated/<group>/<Class> directory either
+`extractor-tests/generated/<group>/<Class>` directory either
  * a `MISSING_SOURCE.txt` explanation file if no source is present, or
  * one `<Class>.ql` test query for all single properties and on `<Class>_<property>.ql` test query for each optional or
    repeated property
@@ -164,6 +165,7 @@ def get_ql_class(cls: schema.Class, lookup: typing.Dict[str, schema.Class]) -> q
     return ql.Class(
         name=cls.name,
         bases=cls.bases,
+        bases_impl=[base + "Impl::" + base for base in cls.bases],
         final=not cls.derived,
         properties=properties,
         dir=pathlib.Path(cls.group or ""),
@@ -210,15 +212,17 @@ def get_import(file: pathlib.Path, root_dir: pathlib.Path):
     return str(stem).replace("/", ".")
 
 
-def get_types_used_by(cls: ql.Class) -> typing.Iterable[str]:
+def get_types_used_by(cls: ql.Class, is_impl: bool) -> typing.Iterable[str]:
     for b in cls.bases:
-        yield b.base
+        yield b.base + "Impl" if is_impl else b.base
     for p in cls.properties:
         yield p.type
+    if cls.root:
+        yield cls.name  # used in `getResolveStep` and `resolve`
 
 
-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))
+def get_classes_used_by(cls: ql.Class, is_impl: bool) -> typing.List[str]:
+    return sorted(set(t for t in get_types_used_by(cls, is_impl) if t[0].isupper() and (is_impl or t != cls.name)))
 
 
 def format(codeql, files):
@@ -239,6 +243,10 @@ def _get_path(cls: schema.Class) -> pathlib.Path:
     return pathlib.Path(cls.group or "", cls.name).with_suffix(".qll")
 
 
+def _get_path_impl(cls: schema.Class) -> pathlib.Path:
+    return pathlib.Path(cls.group or "", cls.name+"Impl").with_suffix(".qll")
+
+
 def _get_all_properties(cls: schema.Class, lookup: typing.Dict[str, schema.Class],
                         already_seen: typing.Optional[typing.Set[int]] = None) -> \
         typing.Iterable[typing.Tuple[schema.Class, schema.Property]]:
@@ -315,11 +323,14 @@ def _get_stub(cls: schema.Class, base_import: str, generated_import_prefix: str)
     else:
         accessors = []
     return ql.Stub(name=cls.name, base_import=base_import, import_prefix=generated_import_prefix,
-                   doc=cls.doc, synth_accessors=accessors,
-                   internal="ql_internal" in cls.pragmas)
+                   doc=cls.doc, synth_accessors=accessors)
+
 
+def _get_class_public(cls: schema.Class) -> ql.ClassPublic:
+    return ql.ClassPublic(name=cls.name, doc=cls.doc, internal="ql_internal" in cls.pragmas)
 
-_stub_qldoc_header = "// the following QLdoc is generated: if you need to edit it, do it in the schema file\n"
+
+_stub_qldoc_header = "// the following QLdoc is generated: if you need to edit it, do it in the schema file\n  "
 
 _class_qldoc_re = re.compile(
     rf"(?P<qldoc>(?:{re.escape(_stub_qldoc_header)})?/\*\*.*?\*/\s*|^\s*)(?:class\s+(?P<class>\w+))?",
@@ -330,13 +341,13 @@ def _patch_class_qldoc(cls: str, qldoc: str, stub_file: pathlib.Path):
     """ Replace or insert `qldoc` as the QLdoc of class `cls` in `stub_file` """
     if not qldoc or not stub_file.exists():
         return
-    qldoc = "\n".join(l.rstrip() for l in qldoc.splitlines())
+    qldoc = "\n  ".join(l.rstrip() for l in qldoc.splitlines())
     with open(stub_file) as input:
         contents = input.read()
     for match in _class_qldoc_re.finditer(contents):
         if match["class"] == cls:
             qldoc_start, qldoc_end = match.span("qldoc")
-            contents = f"{contents[:qldoc_start]}{_stub_qldoc_header}{qldoc}\n{contents[qldoc_end:]}"
+            contents = f"{contents[:qldoc_start]}{_stub_qldoc_header}{qldoc}\n  {contents[qldoc_end:]}"
             tmp = stub_file.with_suffix(f"{stub_file.suffix}.bkp")
             with open(tmp, "w") as out:
                 out.write(contents)
@@ -370,6 +381,8 @@ def generate(opts, renderer):
         raise RootElementHasChildren(root)
 
     imports = {}
+    imports_impl = {}
+    classes_used_by = {}
     generated_import_prefix = get_import(out, opts.root_dir)
     registry = opts.generated_registry or pathlib.Path(
         os.path.commonpath((out, stub_out, test_out)), ".generated.list")
@@ -382,24 +395,34 @@ def generate(opts, renderer):
 
         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.root_dir)
+            path = get_import(stub_out / c.path, opts.root_dir)
+            imports[c.name] = path
+            imports_impl[c.name + "Impl"] = path + "Impl"
 
         for c in classes.values():
             qll = out / c.path.with_suffix(".qll")
-            c.imports = [imports[t] for t in get_classes_used_by(c)]
+            c.imports = [imports[t] if t in imports else imports_impl[t] +
+                         "::Impl as " + t for t in get_classes_used_by(c, is_impl=True)]
+            classes_used_by[c.name] = get_classes_used_by(c, is_impl=False)
             c.import_prefix = generated_import_prefix
             renderer.render(c, qll)
 
         for c in data.classes.values():
             path = _get_path(c)
-            stub_file = stub_out / path
+            path_impl = _get_path_impl(c)
+            stub_file = stub_out / path_impl
             base_import = get_import(out / path, opts.root_dir)
             stub = _get_stub(c, base_import, generated_import_prefix)
+
             if not renderer.is_customized_stub(stub_file):
                 renderer.render(stub, stub_file)
             else:
                 qldoc = renderer.render_str(stub, template='ql_stub_class_qldoc')
                 _patch_class_qldoc(c.name, qldoc, stub_file)
+            class_public = _get_class_public(c)
+            class_public_file = stub_out / path
+            class_public.imports = [imports[t] for t in classes_used_by[c.name]]
+            renderer.render(class_public, class_public_file)
 
         # for example path/to/elements -> path/to/elements.qll
         renderer.render(ql.ImportList([i for name, i in imports.items() if not classes[name].internal]),

misc/codegen/lib/ql.py

@@ -101,6 +101,7 @@ class Class:
 
     name: str
     bases: List[Base] = field(default_factory=list)
+    bases_impl: List[Base] = field(default_factory=list)
     final: bool = False
     properties: List[Property] = field(default_factory=list)
     dir: pathlib.Path = pathlib.Path()
@@ -114,7 +115,9 @@ class Class:
     hideable: bool = False
 
     def __post_init__(self):
-        self.bases = [Base(str(b), str(prev)) for b, prev in zip(self.bases, itertools.chain([""], self.bases))]
+        def get_bases(bases): return [Base(str(b), str(prev)) for b, prev in zip(bases, itertools.chain([""], bases))]
+        self.bases = get_bases(self.bases)
+        self.bases_impl = get_bases(self.bases_impl)
         if self.properties:
             self.properties[0].first = True
 
@@ -159,13 +162,26 @@ class Stub:
     base_import: str
     import_prefix: str
     synth_accessors: List[SynthUnderlyingAccessor] = field(default_factory=list)
-    internal: bool = False
     doc: List[str] = field(default_factory=list)
 
     @property
     def has_synth_accessors(self) -> bool:
         return bool(self.synth_accessors)
 
+    @property
+    def has_qldoc(self) -> bool:
+        return bool(self.doc)
+
+
+@dataclass
+class ClassPublic:
+    template: ClassVar = 'ql_class_public'
+
+    name: str
+    imports: List[str] = field(default_factory=list)
+    internal: bool = False
+    doc: List[str] = field(default_factory=list)
+
     @property
     def has_qldoc(self) -> bool:
         return bool(self.doc) or self.internal

misc/codegen/templates/dbscheme.mustache

@@ -1,4 +1,4 @@
-// generated by {{generator}}
+// generated by {{generator}}, do not edit
 {{#includes}}
 
 // from {{src}}

misc/codegen/templates/ql_class.mustache

@@ -1,4 +1,4 @@
-// generated by {{generator}}
+// generated by {{generator}}, do not edit
 /**
  * This module provides the generated definition of `{{name}}`.
  * INTERNAL: Do not import directly.
@@ -9,7 +9,9 @@ private import {{import_prefix}}.Raw
 {{#imports}}
 import {{.}}
 {{/imports}}
-
+{{#root}}
+private class {{name}}Alias = {{name}};
+{{/root}}
 /**
  * INTERNAL: This module contains the fully generated definition of `{{name}}` and should not
  * be referenced directly.
@@ -22,7 +24,7 @@ module Generated {
    * INTERNAL: Do not reference the `Generated::{{name}}` class directly.
    * Use the subclass `{{name}}`, where the following predicates are available.
    */
-  class {{name}} extends Synth::T{{name}}{{#bases}}, {{.}}{{/bases}} {
+  class {{name}} extends Synth::T{{name}}{{#bases_impl}}, {{.}}{{/bases_impl}} {
   {{#root}}
     /**
      * Gets the string representation of this element.
@@ -49,13 +51,13 @@ module Generated {
      * Classes can override this to indicate this node should be in the "hidden" AST, mostly reserved
      * for conversions and syntactic sugar nodes like parentheses.
      */
-    {{name}} getResolveStep() { none() } // overridden by subclasses
+    {{name}}Alias getResolveStep() { none() } // overridden by subclasses
 
     /**
      * Gets the element that should substitute this element in the explicit AST, applying `getResolveStep`
      * transitively.
      */
-    final {{name}} resolve() {
+    final {{name}}Alias resolve() {
       not exists(this.getResolveStep()) and result = this
       or
       result = this.getResolveStep().resolve()

misc/codegen/templates/ql_class_public.mustache

@@ -0,0 +1,12 @@
+// generated by {{generator}}, do not edit
+/**
+ * This module provides the public class `{{name}}`.
+ */
+
+private import {{name}}Impl
+{{#imports}}
+import {{.}}
+{{/imports}}
+
+{{>ql_stub_class_qldoc}}
+final class {{name}} = Impl::{{name}};

misc/codegen/templates/ql_imports.mustache

@@ -1,4 +1,4 @@
-// generated by {{generator}}
+// generated by {{generator}}, do not edit
 /**
  * This module exports all modules providing `Element` subclasses.
  */

misc/codegen/templates/ql_parent.mustache

@@ -1,4 +1,4 @@
-// generated by {{generator}}
+// generated by {{generator}}, do not edit
 /**
  * This module provides the generated parent/child relationship.
  */

misc/codegen/templates/ql_stub.mustache

@@ -6,10 +6,16 @@ private import {{import_prefix}}.Raw
 private import {{import_prefix}}.Synth
 {{/has_synth_accessors}}
 
-{{>ql_stub_class_qldoc}}
-class {{name}} extends Generated::{{name}} {
+/**
+ * INTERNAL: This module contains the customizable definition of `{{name}}` and should not
+ * be referenced directly.
+ */
+module Impl {
+  {{>ql_stub_class_qldoc}}
+  class {{name}} extends Generated::{{name}} {
     {{#synth_accessors}}
     private
     cached {{type}} getUnderlying{{argument}}() { this = Synth::T{{name}}({{#constructorparams}}{{^first}},{{/first}}{{param}}{{/constructorparams}})}
     {{/synth_accessors}}
-}
+  }
+}

misc/codegen/templates/ql_stub_module_qldoc.mustache

@@ -1,6 +1,5 @@
 /**
  * This module provides a hand-modifiable wrapper around the generated class `{{name}}`.
-{{#internal}}
+ *
  * INTERNAL: Do not use.
-{{/internal}}
  */

misc/codegen/templates/ql_test_class.mustache

@@ -1,4 +1,4 @@
-// generated by {{generator}}
+// generated by {{generator}}, do not edit
 
 import {{elements_module}}
 import TestUtils

misc/codegen/templates/ql_test_missing.mustache

@@ -1,4 +1,4 @@
-// generated by {{generator}}
+// generated by {{generator}}, do not edit
 
 After a source file is added in this directory and {{generator}} is run again, test queries
 will appear and this file will be deleted

misc/codegen/templates/ql_test_property.mustache

@@ -1,4 +1,4 @@
-// generated by {{generator}}
+// generated by {{generator}}, do not edit
 
 import {{elements_module}}
 import TestUtils

misc/codegen/templates/rust_classes.mustache

@@ -1,4 +1,4 @@
-// generated by {{generator}}
+// generated by {{generator}}, do not edit
 
 #![cfg_attr(any(), rustfmt::skip)]
 

misc/codegen/templates/rust_module.mustache

@@ -1,4 +1,4 @@
-// generated by {{generator}}
+// generated by {{generator}}, do not edit
 {{#modules}}
 
 mod {{.}};

misc/codegen/templates/rust_test_code.mustache

@@ -1,4 +1,4 @@
-// generated by {{generator}}
+// generated by {{generator}}, do not edit
 
 {{#function}}
 fn {{name}}{{signature}} {

misc/codegen/test/test_ql.py

@@ -147,15 +147,14 @@ def test_class_with_children():
     assert cls.has_children is True
 
 
-@pytest.mark.parametrize("doc,internal,expected",
+@pytest.mark.parametrize("doc,expected",
                          [
-                             (["foo", "bar"], False, True),
-                             (["foo", "bar"], True, True),
-                             ([], False, False),
-                             ([], True, True),
+                             (["foo", "bar"], True),
+                             (["foo", "bar"], True),
+                             ([], False)
                          ])
-def test_has_doc(doc, internal, expected):
-    stub = ql.Stub("Class", base_import="foo", import_prefix="bar", doc=doc, internal=internal)
+def test_has_doc(doc, expected):
+    stub = ql.Stub("Class", base_import="foo", import_prefix="bar", doc=doc)
     assert stub.has_qldoc is expected