Merge pull request #2712 from DeusData/fix/py-namespace-string-future-facts

fix(python): docstrings and __future__ imports are no namespace barriers

Author: DeusData

internal/cbm/cbm.h

@@ -412,8 +412,12 @@ static inline bool cbm_import_binds_runtime_name(const CBMImport *imp) {
     return imp && !imp->type_checking_only;
 }
 
-/* Content-only ordered Python namespace facts; no resolved target identities. */
-#define CBM_PY_NAMESPACE_FACTS_VERSION 1u
+/* Content-only ordered Python namespace facts; no resolved target identities.
+ * The version travels in the lsp_surface "py" payload, and a stored payload of
+ * another version does not decode: an incremental run then re-extracts in full,
+ * so stored facts of an older extraction never mix with fresh ones.
+ * 2: string-literal statements are no effect; `from __future__` imports bind. */
+#define CBM_PY_NAMESPACE_FACTS_VERSION 2u
 
 typedef enum {
     CBM_PY_NS_NOT_CAPTURED = 0,

internal/cbm/extract_imports.c

@@ -612,6 +612,37 @@ static bool py_ns_plain_string(PyNamespaceBuilder *b, TSNode node, const char **
     return true;
 }
 
+/* One string of a literal: any quoting or prefix (r, b, u, rb), unless it is an
+ * f-string with a replacement field, whose __format__ / __str__ may run code. */
+static bool py_ns_string_without_field(TSNode node) {
+    if (strcmp(ts_node_type(node), "string") != 0)
+        return false;
+    uint32_t count = ts_node_named_child_count(node);
+    for (uint32_t i = 0; i < count; i++) {
+        if (strcmp(ts_node_type(ts_node_named_child(node, i)), "interpolation") == 0)
+            return false;
+    }
+    return true;
+}
+
+/* A string literal, implicit concatenations included, evaluates nothing: a
+ * docstring (module, class or attribute) is no namespace effect. */
+static bool py_ns_literal_string(TSNode node) {
+    if (strcmp(ts_node_type(node), "concatenated_string") != 0)
+        return py_ns_string_without_field(node);
+    uint32_t count = ts_node_named_child_count(node);
+    bool parts = false;
+    for (uint32_t i = 0; i < count; i++) {
+        TSNode part = ts_node_named_child(node, i);
+        if (strcmp(ts_node_type(part), "comment") == 0)
+            continue;
+        if (!py_ns_string_without_field(part))
+            return false;
+        parts = true;
+    }
+    return parts;
+}
+
 /* Bounded syntactic proof only. Unsupported or deep expressions become an
  * unknown-effect barrier; no evaluator or source execution is involved. */
 static bool py_ns_safe_expr(PyNamespaceBuilder *b, TSNode node, int depth) {
@@ -624,11 +655,8 @@ static bool py_ns_safe_expr(PyNamespaceBuilder *b, TSNode node, int depth) {
         strcmp(kind, "float") == 0 || strcmp(kind, "true") == 0 || strcmp(kind, "false") == 0 ||
         strcmp(kind, "none") == 0 || strcmp(kind, "comment") == 0)
         return true;
-    if (strcmp(kind, "string") == 0) {
-        const char *text = NULL;
-        size_t len = 0;
-        return py_ns_plain_string(b, node, &text, &len);
-    }
+    if (strcmp(kind, "string") == 0 || strcmp(kind, "concatenated_string") == 0)
+        return py_ns_literal_string(node);
     bool container =
         strcmp(kind, "tuple") == 0 || strcmp(kind, "list") == 0 ||
         strcmp(kind, "parenthesized_expression") == 0 || strcmp(kind, "parameters") == 0 ||
@@ -677,8 +705,7 @@ static bool py_ns_class_safe(PyNamespaceBuilder *b, TSNode node) {
                 continue;
             if (strcmp(kind, "expression_statement") == 0 &&
                 ts_node_named_child_count(child) == 1) {
-                TSNode expr = ts_node_named_child(child, 0);
-                if (strcmp(ts_node_type(expr), "string") == 0 && py_ns_safe_expr(b, expr, 0))
+                if (py_ns_literal_string(ts_node_named_child(child, 0)))
                     continue;
                 /* Even an identifier assignment can install a descriptor whose
                  * __set_name__ hook executes during class creation. */
@@ -968,6 +995,35 @@ static bool py_ns_type_checking_block(PyNamespaceBuilder *b, TSNode node) {
     return !b->failure;
 }
 
+/* `from __future__ import x [as y]` is its own node type, without a module
+ * field: a compiler directive that also binds x (or y) to the feature object,
+ * like a from-import of module `__future__`. */
+static bool py_ns_future_import(PyNamespaceBuilder *b, TSNode node) {
+    const char *module_name = py_ns_copy(b, "__future__", strlen("__future__"));
+    if (!module_name)
+        return false;
+    TSTreeCursor cursor = ts_tree_cursor_new(node);
+    bool ok = true;
+    int emitted = 0;
+    if (ts_tree_cursor_goto_first_child(&cursor)) {
+        do {
+            const char *field = ts_tree_cursor_current_field_name(&cursor);
+            if (!field || strcmp(field, "name") != 0)
+                continue;
+            CBMPyNamespaceFact f;
+            if (!py_ns_import_fact(b, ts_tree_cursor_current_node(&cursor), true, module_name, 0,
+                                   &f) ||
+                !py_ns_push(b, f)) {
+                ok = false;
+                break;
+            }
+            emitted++;
+        } while (ts_tree_cursor_goto_next_sibling(&cursor));
+    }
+    ts_tree_cursor_delete(&cursor);
+    return ok && emitted > 0;
+}
+
 static bool py_ns_statement(PyNamespaceBuilder *b, TSNode node) {
     const char *kind = ts_node_type(node);
     if (strcmp(kind, "comment") == 0 || strcmp(kind, "pass_statement") == 0)
@@ -976,6 +1032,8 @@ static bool py_ns_statement(PyNamespaceBuilder *b, TSNode node) {
         return py_ns_import(b, node, false);
     if (strcmp(kind, "import_from_statement") == 0)
         return py_ns_import(b, node, true);
+    if (strcmp(kind, "future_import_statement") == 0)
+        return py_ns_future_import(b, node);
     bool decorated = strcmp(kind, "decorated_definition") == 0;
     TSNode def = decorated ? ts_node_child_by_field_name(node, TS_FIELD("definition")) : node;
     if (!ts_node_is_null(def)) {

tests/test_extraction.c

@@ -12318,6 +12318,96 @@ TEST(python_namespace_facts_type_checking_alternatives) {
     PASS();
 }
 
+/* A string-literal statement (a module, class or attribute docstring) has no
+ * effect, and `from __future__ import x` binds x like any from-import. Neither
+ * may withdraw the names bound before it: sqlalchemy's sql/expression.py lost
+ * its 149 re-exports to `nullsfirst = nulls_first` followed by a triple-quoted
+ * docstring. An f-string with a replacement field can run code
+ * (__format__ / __str__) and stays an effect. */
+static bool py_facts_any_unknown(const CBMFileResult *r, CBMPyNamespaceUnknownReason reason) {
+    for (int i = 0; r && i < r->py_namespace.count; i++) {
+        if (r->py_namespace.items[i].kind == CBM_PY_NS_UNKNOWN &&
+            r->py_namespace.items[i].reason == reason)
+            return true;
+    }
+    return false;
+}
+
+TEST(python_namespace_facts_string_statements_and_future_imports) {
+    static const char source[] = "\"\"\"Module docstring.\"\"\"\n"
+                                 "from __future__ import annotations\n"
+                                 "from __future__ import (division as div, generator_stop)\n"
+                                 "from .impl import a\n"
+                                 "\"\"\"A triple-quoted string after an import.\"\"\"\n"
+                                 "'''Single-quoted triple.'''\n"
+                                 "r\"raw \\d\"\n"
+                                 "b\"bytes\"\n"
+                                 "rb'raw bytes'\n"
+                                 "u\"unicode\"\n"
+                                 "\"implicit\" 'concatenation'\n"
+                                 "f\"no replacement field {{escaped}}\"\n"
+                                 "from .impl import b\n"
+                                 "class Documented:\n"
+                                 "    \"\"\"Class docstring.\"\"\"\n"
+                                 "    def method(self):\n"
+                                 "        pass\n"
+                                 "x = a\n"
+                                 "\"\"\"Attribute docstring.\"\"\"\n";
+    CBMFileResult *r = extract(source, CBM_LANG_PYTHON, "t", "pkg/__init__.py");
+    bool ok = r && !r->has_error && r->py_namespace.status == CBM_PY_NS_COMPLETE &&
+              cbm_py_namespace_facts_valid(&r->py_namespace) && r->py_namespace.count == 7;
+    if (ok) {
+        const CBMPyNamespaceFact *f = r->py_namespace.items;
+        ok = f[0].kind == CBM_PY_NS_IMPORT_NAME && strcmp(f[0].module_name, "__future__") == 0 &&
+             f[0].relative_level == 0 && strcmp(f[0].member_name, "annotations") == 0 &&
+             strcmp(f[0].local_name, "annotations") == 0 && f[1].kind == CBM_PY_NS_IMPORT_NAME &&
+             strcmp(f[1].member_name, "division") == 0 && strcmp(f[1].local_name, "div") == 0 &&
+             f[2].kind == CBM_PY_NS_IMPORT_NAME && strcmp(f[2].local_name, "generator_stop") == 0 &&
+             f[3].kind == CBM_PY_NS_IMPORT_NAME && strcmp(f[3].local_name, "a") == 0 &&
+             f[4].kind == CBM_PY_NS_IMPORT_NAME && strcmp(f[4].local_name, "b") == 0 &&
+             f[5].kind == CBM_PY_NS_OWN_DEF && f[5].def_kind == CBM_PY_NS_DEF_CLASS &&
+             strcmp(f[5].local_name, "Documented") == 0 && f[6].kind == CBM_PY_NS_SHADOW &&
+             strcmp(f[6].local_name, "x") == 0;
+    }
+    if (!ok && r) {
+        for (int i = 0; i < r->py_namespace.count; i++) {
+            const CBMPyNamespaceFact *f = &r->py_namespace.items[i];
+            printf("  string/future fact %d: kind %d reason %d name %s\n", i, (int)f->kind,
+                   (int)f->reason, f->local_name ? f->local_name : "-");
+        }
+    }
+    if (r)
+        cbm_free_result(r);
+    ASSERT_TRUE(ok); /* RED while a docstring is an effect and __future__ is unsupported */
+
+    /* Controls: each stays a namespace barrier. */
+    static const struct {
+        const char *name;
+        const char *source;
+        CBMPyNamespaceUnknownReason reason;
+    } barriers[] = {
+        {"f-string field", "from .impl import a\nf\"{a}\"\n", CBM_PY_NS_REASON_EFFECT},
+        {"concatenated f-string field", "from .impl import a\n\"x\" f\"{a!r}\"\n",
+         CBM_PY_NS_REASON_EFFECT},
+        {"call", "from .impl import a\nprint(a)\n", CBM_PY_NS_REASON_EFFECT},
+        /* Python rejects it, and so does the grammar: the whole file is unknown */
+        {"star from __future__", "from .impl import a\nfrom __future__ import *\n",
+         CBM_PY_NS_REASON_PARSE},
+    };
+    for (size_t i = 0; i < sizeof(barriers) / sizeof(barriers[0]); i++) {
+        r = extract(barriers[i].source, CBM_LANG_PYTHON, "t", "pkg/__init__.py");
+        ok = r && r->py_namespace.status == CBM_PY_NS_COMPLETE &&
+             cbm_py_namespace_facts_valid(&r->py_namespace) &&
+             py_facts_any_unknown(r, barriers[i].reason);
+        if (!ok)
+            printf("  control '%s' is no namespace barrier\n", barriers[i].name);
+        if (r)
+            cbm_free_result(r);
+        ASSERT_TRUE(ok);
+    }
+    PASS();
+}
+
 TEST(python_namespace_facts_distinguish_all_operations_and_unknowns) {
     static const char source[] = "__all__ = ['old']; __all__ = []\n"
                                  "__all__ = dynamic\n"
@@ -12665,7 +12755,7 @@ TEST(python_namespace_facts_multiple_bindings_and_unsupported_effects) {
                                  "__all__ = ['escaped\\n']\n";
     CBMFileResult *r = extract(source, CBM_LANG_PYTHON, "t", "main.py");
     bool ok = r && !r->has_error && !r->parse_incomplete &&
-              r->py_namespace.status == CBM_PY_NS_COMPLETE && r->py_namespace.count == 14 &&
+              r->py_namespace.status == CBM_PY_NS_COMPLETE && r->py_namespace.count == 13 &&
               cbm_py_namespace_facts_valid(&r->py_namespace) && has_def(r, "Function", "later") &&
               has_def(r, "Class", "HasDescriptor");
     if (ok) {
@@ -12681,8 +12771,10 @@ TEST(python_namespace_facts_multiple_bindings_and_unsupported_effects) {
              strcmp(f[7].local_name, "__all__") == 0 && f[8].kind == CBM_PY_NS_UNKNOWN &&
              f[9].kind == CBM_PY_NS_SHADOW && strcmp(f[9].local_name, "values") == 0 &&
              f[10].kind == CBM_PY_NS_UNKNOWN && f[11].kind == CBM_PY_NS_SHADOW &&
-             f[11].reason == CBM_PY_NS_REASON_UNPROVEN_CLASS && f[12].kind == CBM_PY_NS_UNKNOWN &&
-             f[13].kind == CBM_PY_NS_ALL_UNKNOWN;
+             f[11].reason == CBM_PY_NS_REASON_UNPROVEN_CLASS &&
+             /* a string with an escape evaluates nothing: no effect, but no
+              * literal export name either */
+             f[12].kind == CBM_PY_NS_ALL_UNKNOWN;
     }
     if (r)
         cbm_free_result(r);
@@ -12705,6 +12797,7 @@ SUITE(extraction) {
     RUN_TEST(python_namespace_facts_unknown_effects_and_body_stability);
     RUN_TEST(python_namespace_facts_parse_errors_are_complete_unknown);
     RUN_TEST(python_namespace_facts_type_checking_alternatives);
+    RUN_TEST(python_namespace_facts_string_statements_and_future_imports);
     RUN_TEST(python_namespace_facts_capture_and_copy_fail_atomically);
     RUN_TEST(python_namespace_facts_validation_rejects_partial_and_unknown_schema);
     RUN_TEST(python_namespace_facts_compact_spill_and_copy_keep_owned_payload);

tests/test_pipeline.c

@@ -14624,6 +14624,7 @@ static const char *const py_reexport_calls[][2] = {
     {"ctl_from_cls", "direct.impl.Direct"},
     {"ctl_from_fn", "direct.impl.direct_fn"},
     {"lazy_tc", "lz.lazy_mod.Lazy"},
+    {"doc_reexport", "docp.impl.Doc"},
 };
 enum { PY_REEXPORT_N_CALLS = sizeof(py_reexport_calls) / sizeof(py_reexport_calls[0]) };
 
@@ -14633,7 +14634,7 @@ enum { PY_REEXPORT_N_CALLS = sizeof(py_reexport_calls) / sizeof(py_reexport_call
 static const char *const py_reexport_excluded[][2] = {
     {"excl_unlisted", "pkg.all_mod.Unlisted"}, {"excl_private", "pkg.star_mod._private_fn"},
     {"excl_shadowed", "shad.impl.Gone"},       {"excl_barrier", "barr.impl.Before"},
-    {"excl_compat", "cmp.types_only.Shim"},
+    {"excl_compat", "cmp.types_only.Shim"},    {"excl_fstring", "fsp.impl.Kept"},
 };
 enum { PY_REEXPORT_N_EXCLUDED = sizeof(py_reexport_excluded) / sizeof(py_reexport_excluded[0]) };
 
@@ -14693,13 +14694,28 @@ static int py_reexport_run(bool parallel, bool incremental, bool *lsp, bool *exc
                     "    except ImportError:\n        Shim = None\n");
     write_temp_file(tmp, "app/cmp/types_only.py", "class Shim:\n    pass\n");
     write_temp_file(tmp, "app/cmp/runtime_impl.py", "class Shim:\n    pass\n");
+    /* The sql/expression.py shape of sqlalchemy: docstrings and a __future__
+     * import around the re-exports; an f-string with a field stays a barrier. */
+    write_temp_file(tmp, "app/docp/__init__.py",
+                    "\"\"\"Package docstring.\"\"\"\n"
+                    "from __future__ import annotations\n"
+                    "from .impl import Doc\n"
+                    "alias = Doc\n"
+                    "\"\"\"Attribute docstring.\"\"\"\n");
+    write_temp_file(tmp, "app/docp/impl.py", "class Doc:\n    pass\n");
+    write_temp_file(tmp, "app/fsp/__init__.py",
+                    "from .impl import Kept\n"
+                    "f\"{Kept}\"\n");
+    write_temp_file(tmp, "app/fsp/impl.py", "class Kept:\n    pass\n");
     write_temp_file(tmp, "app/main.py",
                     "from app import pkg\n"
                     "from app import cyc\n"
                     "from app import shad\n"
                     "from app import barr\n"
                     "from app import lz\n"
                     "from app import cmp\n"
+                    "from app import docp\n"
+                    "from app import fsp\n"
                     "from app.db import models\n"
                     "from app.pkg import Explicit, Listed, Deep, star_fn, Renamed\n"
                     "from app.direct.impl import Direct, direct_fn\n\n\n"
@@ -14725,6 +14741,8 @@ static int py_reexport_run(bool parallel, bool incremental, bool *lsp, bool *exc
                     "def excl_barrier():\n    barr.Before()\n\n\n"
                     "def excl_compat():\n    cmp.Shim()\n\n\n"
                     "def lazy_tc():\n    lz.Lazy()\n\n\n"
+                    "def doc_reexport():\n    docp.Doc()\n\n\n"
+                    "def excl_fstring():\n    fsp.Kept()\n\n\n"
                     "def ctl_from_cls():\n    Direct()\n\n\n"
                     "def ctl_from_fn():\n    direct_fn()\n");
     for (int i = 0; parallel && i < 52; i++) {
@@ -23459,26 +23477,32 @@ TEST(pipeline_python_namespace_codec_owns_ordered_facts) {
 }
 
 TEST(pipeline_python_namespace_codec_rejects_non_authoritative_rows) {
-    /* Numeric values below are the frozen version-1 namespace wire values.
-     * Even an empty LSP array must validate its namespace envelope. */
+    /* Numeric values below are the frozen namespace wire values (version 2
+     * changed which statements extraction proves harmless, not the wire
+     * shape). Even an empty LSP array must validate its namespace envelope,
+     * and a payload of an older facts version is no authority: its facts
+     * were extracted under other rules, so the incremental run that reads
+     * it re-extracts in full. */
     const char *bad[] = {
         "{\"v\":1,\"lsp\":[]}",
         "{\"v\":3,\"lang\":1,\"lsp\":[]}",
         "{\"v\":3,\"lang\":1,\"lsp\":[],\"py\":null}",
         "{\"v\":3,\"lang\":1.0,\"lsp\":[],\"py\":null}",
         "{\"v\":3,\"v\":3,\"lang\":0,\"lsp\":[],\"py\":null}",
-        "{\"v\":3,\"lang\":1,\"lsp\":[],\"py\":{\"v\":1,\"lang\":1,"
+        "{\"v\":3,\"lang\":1,\"lsp\":[],\"py\":{\"v\":2,\"lang\":1,"
         "\"module\":\"p.m\",\"path\":\"m.py\",\"status\":3,\"failure\":2,\"events\":[]}}",
+        "{\"v\":3,\"lang\":1,\"lsp\":[],\"py\":{\"v\":1,\"lang\":1,"
+        "\"module\":\"p.m\",\"path\":\"m.py\",\"status\":2,\"failure\":0,\"events\":[]}}",
         "{\"v\":3,\"lang\":1,\"lsp\":[],\"py\":{\"v\":99,\"lang\":1,"
         "\"module\":\"p.m\",\"path\":\"m.py\",\"status\":2,\"failure\":0,\"events\":[]}}",
-        "{\"v\":3,\"lang\":1,\"lsp\":[],\"py\":{\"v\":1,\"lang\":1,"
+        "{\"v\":3,\"lang\":1,\"lsp\":[],\"py\":{\"v\":2,\"lang\":1,"
         "\"module\":\"p.m\",\"path\":\"m.py\\u0000suffix\",\"status\":2,"
         "\"failure\":0,\"events\":[]}}",
-        "{\"v\":3,\"lang\":1,\"lsp\":[],\"py\":{\"v\":1,\"lang\":1,"
+        "{\"v\":3,\"lang\":1,\"lsp\":[],\"py\":{\"v\":2,\"lang\":1,"
         "\"module\":\"p.m\",\"path\":\"m.py\",\"status\":2,\"failure\":0,\"events\":["
         "{\"k\":999,\"d\":0,\"s\":0,\"r\":0,\"f\":0,\"l\":0,"
         "\"n\":null,\"m\":null,\"i\":null,\"a\":[]}]}}",
-        "{\"v\":3,\"lang\":1,\"lsp\":[],\"py\":{\"v\":1,\"lang\":1,"
+        "{\"v\":3,\"lang\":1,\"lsp\":[],\"py\":{\"v\":2,\"lang\":1,"
         "\"module\":\"p.m\",\"path\":\"m.py\",\"status\":2,\"failure\":0,\"events\":["
         "{\"k\":8,\"d\":0,\"s\":1,\"r\":0,\"f\":0,\"l\":4294967296,"
         "\"n\":null,\"m\":null,\"i\":null,\"a\":[]}]}}",
@@ -23498,7 +23522,7 @@ TEST(pipeline_python_namespace_codec_rejects_non_authoritative_rows) {
         cbm_arena_destroy(&arena);
     }
     const char *unknown =
-        "{\"v\":3,\"lang\":1,\"lsp\":[],\"reg\":[],\"py\":{\"v\":1,\"lang\":1,"
+        "{\"v\":3,\"lang\":1,\"lsp\":[],\"reg\":[],\"py\":{\"v\":2,\"lang\":1,"
         "\"module\":\"p.m\",\"path\":\"m.py\",\"status\":2,\"failure\":0,\"events\":["
         "{\"k\":7,\"d\":0,\"s\":0,\"r\":7,\"f\":0,\"l\":0,"
         "\"n\":null,\"m\":null,\"i\":null,\"a\":[]}]}}";
@@ -23511,6 +23535,7 @@ TEST(pipeline_python_namespace_codec_rejects_non_authoritative_rows) {
                     facts.facts.items[0].kind == CBM_PY_NS_UNKNOWN &&
                     facts.facts.items[0].reason == CBM_PY_NS_REASON_PARSE;
     cbm_arena_destroy(&arena);
+    ASSERT_EQ(CBM_PY_NAMESPACE_FACTS_VERSION, 2u); /* the rows above spell it */
     ASSERT_TRUE(rejected);
     ASSERT_EQ(unknown_rc, 1);
     ASSERT_EQ(equal_rc, 0);