Fix Svelte, JS destructuring, and Rust use-declaration extraction

graphify/extract.py

@@ -137,6 +137,7 @@
     _ts_collect_type_refs,
     _ts_heritage_clause_entries,
     _ts_walk_class_members,
+    _sfc_mask_non_script,
     _vue_mask_non_script,
     _walk_js_tree,
     _walk_python_tree,
@@ -1599,56 +1600,107 @@ def _emit_rescued_import(
 
 
 def extract_svelte(path: Path) -> dict:
-    """Extract imports from .svelte files: script-block via JS AST + template regex fallback.
+    """Extract imports, symbols, and type refs from a ``.svelte`` component.
 
-    Tree-sitter only sees the <script> block. Svelte template syntax like
-    {#await import('./X.svelte')} lives in the markup layer and is invisible
-    to the JS parser, so a regex pass covers those dynamic imports.
+    Masks the non-``<script>`` regions and parses the script with the grammar
+    its ``lang`` implies (``tsx``->TSX, ``js``/``jsx``->JS, ``ts`` or unset->TS;
+    TS is a superset of JS so it is a safe default), mirroring
+    :func:`extract_vue`. Feeding the whole component to the JS grammar makes the
+    markup a top-level ERROR node, so ``import_statement`` and declaration nodes
+    are never reached and everything but a stray symbol is dropped (#713).
+
+    Both script blocks of a Svelte 5 component survive the mask, so a
+    ``<script module>`` block is parsed alongside the instance script.
+
+    A regex pass then recovers ``import('...')`` dynamic imports, which the AST
+    pass does not edge and which legally live in markup-layer template syntax
+    such as ``{#await import('./X.svelte')}`` — outside every script block, and
+    so blanked out of the masked source the AST sees.
+
+    Static imports are edged by the AST pass, so the old regex rescue for them
+    would double-emit and is used only when the script fails to parse, where
+    the AST pass reaches no ``import_statement`` at all.
     """
-    result = _extract_generic(path, _JS_CONFIG)
     try:
-        import re as _re
         src = path.read_text(encoding="utf-8", errors="replace")
+    except OSError as e:
+        # Report the failure rather than returning an empty result: an
+        # unreadable component is indistinguishable from an empty one
+        # otherwise, and extract() warns on `error` (#2551).
+        return {"nodes": [], "edges": [], "error": f"cannot read {path}: {e}"}
+
+    masked, lang = _sfc_mask_non_script(src)
+    if lang == "tsx":
+        config = _TSX_CONFIG
+    elif lang in ("js", "jsx"):
+        config = _JS_CONFIG
+    else:  # "ts" or unspecified — default to the TS grammar (superset of JS)
+        config = _TS_CONFIG
+
+    result = _extract_generic(path, config, source_override=masked.encode("utf-8"))
+
+    try:
+        import re as _re
         existing_ids = {n["id"] for n in result.get("nodes", [])}
         # Source file node ID must match the one _extract_generic creates:
         # _make_id(str(path)) - single arg, no stem prefix. Otherwise the source
         # endpoint is a phantom node and build_from_json drops the edge (#701).
         file_node_id = _make_id(str(path))
+        if result.get("error") and not result.get("nodes"):
+            # A hard failure (grammar missing, unreadable source) returns no
+            # nodes at all, so a rescued edge would have a dangling source and
+            # be dropped at build time. Mint the file node _extract_generic
+            # would have, in the same shape, so the rescue is worth running.
+            # `error` stays on the result for extract()'s own reporting.
+            result.setdefault("nodes", []).append({
+                "id": file_node_id, "label": path.name,
+                "file_type": "code", "source_file": str(path),
+                "source_location": "L1",
+            })
+            existing_ids.add(file_node_id)
         aliases = _load_tsconfig_aliases(path.parent)
         base_url = _load_tsconfig_base_url(path.parent)
+        # Scanned over the raw source, not the masked one, so template-layer
+        # dynamic imports are seen. Resolution is shared with the static pass:
+        # relative paths and tsconfig aliases probe real on-disk extensions
+        # (#716, #701), and a target that IS a real file emits an edge stamped
+        # with target_file instead of an absolute-id ghost stub (#2195).
         for m in _re.finditer(r"""import\(\s*['"]([^'"]+)['"]\s*\)""", src):
             raw = m.group(1)
             if not raw:
                 continue
-            # Resolution + emit shared with the static pass below: relative
-            # paths and tsconfig aliases probe real on-disk extensions (#716,
-            # #701), and a target that IS a real file emits an edge stamped
-            # with target_file instead of an absolute-id ghost stub (#2195).
             _emit_rescued_import(
                 result, existing_ids, file_node_id, path, raw,
                 "dynamic_import", aliases, base_url,
             )
-        # Static imports inside <script> blocks. The JS tree-sitter parser fed
-        # the full .svelte file produces a top-level ERROR node (HTML markup
-        # is not valid JS), so import_statement nodes are never reached and
-        # static imports are silently dropped (#713). Regex over each script
-        # body recovers them.
-        script_re = _re.compile(
-            r"<script\b[^>]*>([\s\S]*?)</script\s*>", _re.IGNORECASE
-        )
-        static_import_re = _re.compile(
-            r"""import\s+(?:[^'"`;]+?\s+from\s+)?['"]([^'"]+)['"]"""
-        )
-        for script_match in script_re.finditer(src):
-            script_body = script_match.group(1)
-            for m in static_import_re.finditer(script_body):
+        if result.get("parse_errors") or result.get("error"):
+            # The AST pass produced no usable tree — the masked script parsed
+            # WITH errors (`parse_errors`, so `import_statement` nodes may
+            # never have been reached), or it failed outright (`error`: the
+            # grammar is missing, the source unreadable). Both leave imports
+            # unedged, so fall back to the regex rescue the pre-mask extractor
+            # ran unconditionally.
+            # Gated on the failure so a clean parse does not double-emit: the
+            # AST already edges those specifiers. Scanned over the MASKED
+            # source, whose only surviving text is the script regions.
+            static_import_re = _re.compile(
+                r"""import\s+(?:[^'"`;]+?\s+from\s+)?['"]([^'"]+)['"]"""
+            )
+            for m in static_import_re.finditer(masked):
                 raw = m.group(1)
                 if not raw:
                     continue
                 _emit_rescued_import(
                     result, existing_ids, file_node_id, path, raw,
                     "imports_from", aliases, base_url,
                 )
+        # Both rescues can repeat an edge: a recovered parse edges some
+        # imports before the error node, and a specifier imported twice in one
+        # file (`import('./X')` in two markup branches) is matched twice.
+        # Imported inside the function, as cli.py and watch.py do, to keep
+        # extract.py free of a module-level dependency on build.py.
+        from graphify.build import dedupe_edges
+        result["edges"] = dedupe_edges(result.get("edges", []))
     except Exception:
         pass
     return result
@@ -1732,10 +1784,13 @@ def extract_vue(path: Path) -> dict:
     """
     try:
         src = path.read_text(encoding="utf-8", errors="replace")
-    except OSError:
-        return {"nodes": [], "edges": []}
+    except OSError as e:
+        # Report the failure rather than returning an empty result: an
+        # unreadable component is indistinguishable from an empty one
+        # otherwise, and extract() warns on `error` (#2551).
+        return {"nodes": [], "edges": [], "error": f"cannot read {path}: {e}"}
 
-    masked, lang = _vue_mask_non_script(src)
+    masked, lang = _sfc_mask_non_script(src)
     if lang == "tsx":
         config = _TSX_CONFIG
     elif lang in ("js", "jsx"):

graphify/extractors/engine.py

@@ -1912,6 +1912,36 @@ def _find_require_call(value_node):
         return _find_require_call(obj)
     return None
 
+def _is_require_initializer(value_node, source: bytes) -> bool:
+    """True when a declarator's initializer is a ``require(...)`` call.
+
+    The call may be reached through member access — ``require('./m').sub`` is
+    still a CJS import, and :func:`_require_imports_js` edges that form, so
+    both sides must agree on what counts or a destructured binding would be
+    both imported and shadowed by a local stub.
+
+    A STRING-LITERAL specifier is required, because that is what
+    :func:`_require_imports_js` needs to emit an edge at all. A computed
+    specifier (``require(name)``, ``require('./' + x)``) names no module it
+    can resolve, so suppressing the local binding for one would delete the
+    name from the graph outright rather than repoint it.
+
+    ``_find_require_call`` matches the call *shape* only — any
+    ``identifier(...)`` — and leaves the callee-name check to its callers, so
+    it must not be used alone to recognise a CJS import.
+    """
+    call = _find_require_call(value_node)
+    if call is None:
+        return False
+    fn = call.child_by_field_name("function")
+    if fn is None or _read_text(fn, source) != "require":
+        return False
+    args = call.child_by_field_name("arguments")
+    if args is None:
+        return False
+    return any(arg.type == "string" for arg in args.children)
+
+
 def _require_imports_js(node, source: bytes, importer_nid: str, stem: str, edges: list, str_path: str) -> bool:
     """Detect CommonJS require imports inside lexical_declaration / variable_declaration.
 
@@ -2001,6 +2031,11 @@ def _require_imports_js(node, source: bytes, importer_nid: str, stem: str, edges
 
 _JS_FUNCTION_VALUE_TYPES = frozenset({"arrow_function", "function_expression", "function", "generator_function"})
 
+# Declarator `name` node types that are a destructuring PATTERN rather than a
+# single bound identifier. Their `name` field is the pattern source, never a
+# symbol, so it must not be used as a node label.
+_JS_DESTRUCTURING_PATTERNS = frozenset({"object_pattern", "array_pattern"})
+
 
 def _scan_js_nested_function_declarations(
     container_node, parent_nid: str, *, source: bytes, config,
@@ -2130,6 +2165,61 @@ def _js_member_assignment_target(left, source: bytes):
                 return ("prototype", inner_obj_name, member_name)
     return None
 
+_JS_PATTERN_TYPES = frozenset({"object_pattern", "array_pattern"})
+
+
+def _js_pattern_bound_names(name_node, source: bytes) -> list[str]:
+    """Return the identifiers a destructuring declarator actually binds.
+
+    ``const { a, b: renamed, c = 1, ...rest } = x`` binds ``a``, ``renamed``,
+    ``c`` and ``rest`` — not the text of the pattern. Reading the declarator's
+    ``name`` field verbatim instead mints one node labelled with the whole
+    pattern source (``{ a, b: renamed, c = 1, ...rest }``), which names no
+    symbol and can never be the target of a reference. Svelte 5 makes the shape
+    universal — every component destructures ``$props()`` — but the same
+    declarator shape is ordinary JS/TS.
+
+    Walks only the binding side: a ``pair_pattern``'s value (``b: renamed``
+    binds ``renamed``, not the property key ``b``) and an assignment pattern's
+    left operand (``c = $bindable()`` binds ``c``, not ``$bindable``). Nested
+    patterns recurse, so ``{ deep: { inner } }`` binds ``inner``. Returns an
+    empty list for a non-pattern node.
+    """
+    if name_node is None or name_node.type not in _JS_PATTERN_TYPES:
+        return []
+    names: list[str] = []
+
+    def visit(node) -> None:
+        t = node.type
+        if t in ("identifier", "shorthand_property_identifier_pattern"):
+            text = _read_text(node, source)
+            if text and text not in names:
+                names.append(text)
+            return
+        if t == "pair_pattern":
+            # `key: target` — only the value side is bound.
+            value = node.child_by_field_name("value")
+            if value is not None:
+                visit(value)
+            return
+        if t in ("object_assignment_pattern", "assignment_pattern"):
+            # `target = default` — the default is an expression, not a binding.
+            left = node.child_by_field_name("left")
+            if left is None:
+                left = node.named_children[0] if node.named_children else None
+            if left is not None:
+                visit(left)
+            return
+        if t in ("rest_pattern", "object_pattern", "array_pattern"):
+            for child in node.named_children:
+                visit(child)
+            return
+        # Anything else (type annotations, holes in `[a, , c]`) binds nothing.
+
+    visit(name_node)
+    return names
+
+
 def _js_extra_walk(node, source: bytes, file_nid: str, stem: str, str_path: str,
                    nodes: list, edges: list, seen_ids: set, function_bodies: list,
                    parent_class_nid: str | None, add_node_fn, add_edge_fn,
@@ -2303,13 +2393,47 @@ def _js_extra_walk(node, source: bytes, file_nid: str, stem: str, str_path: str,
                     ):
                         # Simple exported identifiers are part of the module API
                         # regardless of initializer shape. Keep other scalar noise suppressed.
+                        const_nid = None
                         if name_node:
-                            const_name = _read_text(name_node, source)
                             line = child.start_point[0] + 1
-                            const_nid = _make_id(stem, const_name)
-                            add_node_fn(const_nid, const_name, line)
-                            add_edge_fn(file_nid, const_nid, "contains", line)
-                            const_found = True
+                            # A destructuring declarator binds several names and
+                            # its `name` field is the pattern source, not a
+                            # symbol — node each identifier it actually binds.
+                            const_names = _js_pattern_bound_names(name_node, source)
+                            if const_names and _is_require_initializer(value, source):
+                                # `const { doWork } = require('./lib')` binds an
+                                # IMPORT, not a local definition. `_require_imports_js`
+                                # already edges those names at the file level; noding
+                                # them here would shadow the real cross-file target,
+                                # so a call to `doWork()` would resolve to this file's
+                                # stub instead of the callee's definition.
+                                const_names = []
+                            elif not const_names:
+                                if name_node.type in _JS_DESTRUCTURING_PATTERNS:
+                                    # A pattern that binds nothing (`const {} =`,
+                                    # `const { a: {} } =`) has no name to node.
+                                    # Falling back to the pattern SOURCE is the
+                                    # bug `_js_pattern_bound_names` exists to
+                                    # fix — it mints `{ a: {} }` as a symbol.
+                                    const_names = []
+                                else:
+                                    const_names = [_read_text(name_node, source)]
+                            for const_name in const_names:
+                                # A name that normalizes to nothing would collapse
+                                # the id to the absolute file-stem and leak the
+                                # scan path (#1899); skip it, as the arrow branch does.
+                                if not const_name or not normalize_id(const_name):
+                                    continue
+                                nid = _make_id(stem, const_name)
+                                add_node_fn(nid, const_name, line)
+                                add_edge_fn(file_nid, nid, "contains", line)
+                                const_found = True
+                                if const_nid is None:
+                                    # Closures in the initializer are attributed to
+                                    # the first binding; a destructured initializer
+                                    # has no single owning symbol.
+                                    const_nid = nid
+                        if const_nid is not None:
                             # #2552: `const handler = wrapper(async (req) => …)`
                             # created the const node above but, unlike the arrow
                             # branch, never tracked the callback's body — so