From 01acfc4d094d2a66e2ede3bfd0f46915d97e3f28 Mon Sep 17 00:00:00 2001 From: bompus Date: Fri, 30 Oct 2026 12:52:11 -0600 Subject: [PATCH 1/9] fix(python): a parameter or local named like an imported module is not that module (#3046) A Python call through a name the file imports as a module, like other.update(...) after `from pkg import other`, linked to the module's function whatever the calling function bound `other` to: a with/except/loop target, a walrus, a case capture, a lambda parameter or a comprehension variable. Parameters and plain assignments were already scoped. The Python walker now emits a function-scoped local binding row for loop, `with ... as`, `except ... as`, walrus and `case` capture targets, and for tuple and augmented assignment targets. A lambda parameter or comprehension variable binds only inside its own expression, narrower than the line-wide scope a row carries, so the resolver reads those from the syntax tree. Ported from upstream b700a3ee --- CHANGELOG.md | 1 + __tests__/python-receiver-rebinding.test.ts | 224 ++++++++++++++++++ codegraph-kernel/src/python.rs | 70 +++++- codegraph-kernel/src/resolve/imports.rs | 3 + codegraph-kernel/src/resolve/mod.rs | 1 + .../src/resolve/python_expression_scope.rs | 78 ++++++ 6 files changed, 366 insertions(+), 11 deletions(-) create mode 100644 __tests__/python-receiver-rebinding.test.ts create mode 100644 codegraph-kernel/src/resolve/python_expression_scope.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index deb774534a..85aded1919 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -395,6 +395,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In JavaScript and TypeScript, react-dom's `root.render()` no longer links to your own class component's `render()`, such as an error boundary's, when `root` is declared outside the function making the call, like a module's `let root: Root | null`, or holds what `createRoot(el)` returned. A call on a variable declared outside the calling function now goes by the type that declaration gives it, so one declared with your own class, like `const store = new Store()` at the top of a file, links to that class's method, and a class component's `render()` and lifecycle methods, which only React calls, are no longer a guess for an untyped value. Re-index JavaScript and TypeScript projects after upgrading. Thanks @inth3shadows for the report. (#2589) - In JavaScript and TypeScript, a method called on a freshly constructed object, like `new RegExp(p).exec(s)`, `new URL(u).toString()` or `new Runner().run()`, now links only to that class's own or inherited method, on the class the file imports when several share its name, and no longer to an unrelated project method of the same name; a class from outside your project links nothing. Re-index JavaScript and TypeScript projects after upgrading. Thanks @danusha2345. (#1945) - In Python, `import a.b as X` written inside a function now links its calls like `X.f()`, as the same import at module level already did. Thanks @Egrn for the report. (#2600) +- In Python, a call through a name the file imports as a module, like `other.update(…)` after `from pkg import other`, now links to that module only where nothing closer gives the name another value: a parameter, a local, a `with … as` or `except … as` target, a loop or comprehension variable, a lambda parameter or a `case` capture named `other` is that value, so its calls no longer show up as calls into the module. A `from pkg import third as other` written inside a function now links that function's `other.get()` to `third`, and no longer the rest of the file's. Re-index Python projects after upgrading. Thanks @BGamboa13. (#2632) - In Python, `from app import email` no longer links the importing file to a project variable named `app`, such as a FastAPI or Flask `app = FastAPI()`. (#2584) - In Python, a from-import of a module whose name has no dot, like `from loader import Registry` with `loader.py` at the repository root, or `from mypackage import helper` in a `src/` layout, now links the importing file to that module's file, as `import loader` and `from pkg.loader import Registry` already did. Before, the file's dependency on the module itself was left out, so `codegraph affected` and impact could miss the files that import it. When the file also imported something else under the module's name, like `from django.core import mail` beside `from mail import …`, the module was linked to that instead. Re-index Python projects after upgrading. - In Python, a call through a module-level instance, like `_svc.delete(obj)` after `_svc = SceneService()`, now links to the method its class has or inherits, even when other classes have a method of that name. (#2582) diff --git a/__tests__/python-receiver-rebinding.test.ts b/__tests__/python-receiver-rebinding.test.ts new file mode 100644 index 0000000000..ca47d938e1 --- /dev/null +++ b/__tests__/python-receiver-rebinding.test.ts @@ -0,0 +1,224 @@ +/** + * A Python call through a name the file imports as a module reads that module + * only where nothing nearer binds the name: + * + * from pkg import other + * + * def real(): + * other.update({}) # pkg/other.py's update + * + * def by_param(other): + * other.update({}) # dict.update, or whatever the caller passed + * + * The import resolver read the file's imports alone, so a parameter, a local, + * a `with … as` / `except … as` / loop target, a walrus, a `case` capture or a + * lambda or comprehension variable named like the module still linked the + * call to the module's function. A function-local `from pkg import third as + * other` linked to `pkg.other` too, and so did, from every other function of + * the file, though it binds the name only where it is written. + */ +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import CodeGraph from '../src/index'; + +const FILES: Record = { + 'pyproject.toml': '[project]\nname = "pkg"\n', + 'src/pkg/__init__.py': '', + 'src/pkg/other.py': [ + 'def update(x=None):', + ' return x', + '', + '', + 'def persist(x=None):', + ' return x', + '', + '', + 'def get(key=None):', + ' return key', + '', + ].join('\n'), + 'src/pkg/third.py': ['def get(key=None):', ' return key', ''].join('\n'), + 'tests/shadow.py': [ + 'from pkg import other', + '', + '', + 'def real():', + ' other.update({"b": 2})', + ' return other.persist(1)', + '', + '', + 'def by_param(other):', + ' other.update({"a": 1})', + ' return other.persist(2)', + '', + '', + 'def by_with(path):', + ' with open(path, "w") as other:', + ' return other.persist(3)', + '', + '', + 'def by_assign():', + ' other = {}', + ' other.update({"c": 3})', + '', + '', + 'def by_for(items):', + ' for other in items:', + ' other.persist(4)', + '', + '', + 'def by_except():', + ' try:', + ' pass', + ' except OSError as other:', + ' return other.persist(5)', + '', + '', + 'def by_lambda():', + ' return lambda other: other.persist(6)', + '', + '', + 'def by_comprehension(items):', + ' return [other.persist(7) for other in items]', + '', + '', + 'def by_walrus(load):', + ' if (other := load()):', + ' return other.persist(8)', + '', + '', + 'def by_closure(other):', + ' def inner():', + ' return other.persist(9)', + ' return inner', + '', + '', + 'def by_capture(v):', + ' match v:', + ' case other:', + ' return other.persist(10)', + '', + '', + 'def by_local_import():', + ' from pkg import third as other', + ' return other.get(11)', + '', + '', + // Each of these reads the module the file imports. + 'def keyword_argument(run):', + ' run(', + ' other=1,', + ' )', + ' return other.persist(12)', + '', + '', + 'def in_a_string():', + ' print("with x as other", "other = 1") # other = 2', + ' return other.persist(13)', + '', + '', + 'def comprehension_of_others(items):', + ' return [x for x in other.persist(items)]', + '', + '', + 'def lambda_beside(run):', + ' return run(lambda other: 1, other.persist(14))', + '', + '', + 'def declared_global():', + ' global other', + ' return other.persist(15)', + '', + '', + 'def keyword_pattern(v):', + ' match v:', + ' case Point(other=1):', + ' return other.persist(16)', + '', + '', + 'def same_module_again():', + ' from pkg import other', + ' return other.get(17)', + '', + ].join('\n'), + // The module binds `other` with a plain import, which the file's mappings + // list after every from-import, the function-local one included. + 'tests/leak.py': [ + 'import pkg.other as other', + '', + '', + 'def local_third():', + ' from pkg import third as other', + ' return other.get(1)', + '', + '', + 'def untouched():', + ' return other.get(2)', + '', + ].join('\n'), +}; + +describe.each(['LF', 'CRLF'])('a Python receiver the function binds itself is not the module the file imports (%s)', (eol) => { + let root = ''; + let cg: CodeGraph | undefined; + + beforeAll(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-py-rebind-')); + for (const [rel, content] of Object.entries(FILES)) { + fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); + fs.writeFileSync(path.join(root, rel), eol === 'CRLF' ? content.replace(/\n/g, '\r\n') : content); + } + cg = await CodeGraph.init(root, { index: true }); + }, 60_000); + + afterAll(() => { + cg?.destroy(); + if (root) fs.rmSync(root, { recursive: true, force: true }); + }); + + /** `file:name` of each project function `name` in `file` calls, sorted. */ + const callsFrom = (file: string, name: string): string[] => { + const from = cg!.getNodesInFile(file).filter((n) => n.name === name && (n.kind === 'function' || n.kind === 'method')); + expect(from, `${name} in ${file}`).toHaveLength(1); + return cg!.getOutgoingEdgesFrom([from[0]!.id]) + .filter((e) => e.kind === 'calls') + .map((e) => cg!.getNode(e.target)!) + .filter((t) => t.filePath.startsWith('src/')) + .map((t) => `${t.filePath}:${t.name}`) + .sort(); + }; + + it('a call where nothing nearer binds the name reaches the module', () => { + expect(callsFrom('tests/shadow.py', 'real')).toEqual(['src/pkg/other.py:persist', 'src/pkg/other.py:update']); + }); + + it.each([ + 'by_param', 'by_with', 'by_assign', 'by_for', 'by_except', 'by_lambda', + 'by_comprehension', 'by_walrus', 'inner', 'by_capture', + ])('%s: a name the code around the call binds links no module function', (fn) => { + expect(callsFrom('tests/shadow.py', fn)).toEqual([]); + }); + + it('a function-local `from pkg import third as other` is that module', () => { + expect(callsFrom('tests/shadow.py', 'by_local_import')).toEqual(['src/pkg/third.py:get']); + expect(callsFrom('tests/leak.py', 'local_third')).toEqual(['src/pkg/third.py:get']); + }); + + it('a function-local import binds the name in that function only', () => { + expect(callsFrom('tests/leak.py', 'untouched')).toEqual(['src/pkg/other.py:get']); + }); + + it.each([ + ['keyword_argument', 'src/pkg/other.py:persist'], + ['in_a_string', 'src/pkg/other.py:persist'], + ['comprehension_of_others', 'src/pkg/other.py:persist'], + ['lambda_beside', 'src/pkg/other.py:persist'], + ['declared_global', 'src/pkg/other.py:persist'], + ['keyword_pattern', 'src/pkg/other.py:persist'], + ['same_module_again', 'src/pkg/other.py:get'], + ])('%s binds nothing of that name: the call reaches the module', (fn, target) => { + expect(callsFrom('tests/shadow.py', fn)).toEqual([target]); + }); +}); diff --git a/codegraph-kernel/src/python.rs b/codegraph-kernel/src/python.rs index a65a264016..a0269f65d5 100644 --- a/codegraph-kernel/src/python.rs +++ b/codegraph-kernel/src/python.rs @@ -72,6 +72,21 @@ fn body_docstring(node: Node, src: &str) -> Option { if cleaned.is_empty() { None } else { Some(cleaned) } } +/// The identifiers a Python assignment or loop target binds: a bare name, or +/// the names inside tuple, list and star targets. An attribute or subscript +/// store binds nothing. +fn target_names<'t>(node: Node<'t>, out: &mut Vec>) { + match node.kind() { + "identifier" => out.push(node), + "attribute" | "subscript" => {} + _ => { + for c in named_kids(node) { + target_names(c, out); + } + } + } +} + fn docstring_space(c: char) -> bool { // Match JavaScript trim, including BOM but excluding NEXT LINE. (c.is_whitespace() && c != '\u{85}') || c == '\u{feff}' @@ -344,9 +359,7 @@ impl<'t> Walker<'t> { if kind == "call" { self.extract_call(node); } - if kind == "assignment" { - self.emit_local_assignment_binding(node); - } + self.emit_statement_bindings(node); // An import inside a function body binds a name for that body: a row // scoped to the function (no import node, no `imports` ref — the // module-level walk owns those, as before). @@ -633,16 +646,51 @@ impl<'t> Walker<'t> { } } - /// `x = …` inside a function body: a nodeless `local` row for the name. - fn emit_local_assignment_binding(&mut self, node: Node<'t>) { + /// The names a function-body statement or pattern binds besides a + /// definition or import: assignment (augmented and tuple targets too), + /// `for` and `with`/`except … as` targets, a walrus, and a `case` capture. + /// Each is a nodeless `local` row scoped to the function. A comprehension + /// variable or lambda parameter binds in its own expression, narrower + /// than the line-granular scope a row carries; the resolver reads those + /// from the syntax tree (`python_expression_binds`). + fn emit_statement_bindings(&mut self, node: Node<'t>) { + let mut targets = Vec::new(); + match node.kind() { + "assignment" | "augmented_assignment" | "for_statement" => { + if let Some(left) = node.child_by_field_name("left") { + target_names(left, &mut targets); + } + } + "as_pattern" => { + if let Some(alias) = node.child_by_field_name("alias") { + target_names(alias, &mut targets); + } + } + "except_clause" | "except_group_clause" => { + if let Some(alias) = node.child_by_field_name("alias") { + target_names(alias, &mut targets); + } + } + "named_expression" => { + if let Some(name) = node.child_by_field_name("name") { + targets.push(name); + } + } + // `case other:` and `case Point(x=other)` capture; `other.MODE` is a value. + "dotted_name" if matches!(node.parent().map(|p| p.kind()), Some("case_pattern" | "keyword_pattern")) => { + if node.named_child_count() == 1 { + targets.extend(node.named_child(0)); + } + } + "splat_pattern" => targets.extend(node.named_child(0).filter(|c| c.kind() == "identifier")), + _ => {} + } let Some(scope) = self.enclosing_scope() else { return }; - let Some(left) = node.child_by_field_name("left") else { return }; - if left.kind() != "identifier" { - return; + for name_node in targets { + let name = self.text(name_node).to_string(); + let line = self.line_of(name_node); + self.push_binding_row(BINDING_LOCAL, &name, NONE, scope, line, None, false, None); } - let name = self.text(left).to_string(); - let line = self.line_of(left); - self.push_binding_row(BINDING_LOCAL, &name, NONE, scope, line, None, false, None); } /// Rows for an import statement without the node and refs `extract_import` diff --git a/codegraph-kernel/src/resolve/imports.rs b/codegraph-kernel/src/resolve/imports.rs index 2cc1032016..1459446fba 100644 --- a/codegraph-kernel/src/resolve/imports.rs +++ b/codegraph-kernel/src/resolve/imports.rs @@ -888,6 +888,9 @@ impl KernelResolver { } let imports = if r.language == "python" && r.reference_kind != "imports" { let root = r.reference_name.split('.').next().unwrap(); + if self.python_expression_binds(root, r) { + return Ok(None); + } match self.receiver_binding(root, r)? { Some(binding) if binding.kind == "import" => { // Unaliased imports share the package object across scopes, diff --git a/codegraph-kernel/src/resolve/mod.rs b/codegraph-kernel/src/resolve/mod.rs index 60a12eb928..0d39dfd999 100644 --- a/codegraph-kernel/src/resolve/mod.rs +++ b/codegraph-kernel/src/resolve/mod.rs @@ -644,6 +644,7 @@ mod kotlin_receiver; mod this_member; mod member_fn_ref; mod python_globals; +mod python_expression_scope; mod object_literal; mod cpp; mod cpp_types; diff --git a/codegraph-kernel/src/resolve/python_expression_scope.rs b/codegraph-kernel/src/resolve/python_expression_scope.rs new file mode 100644 index 0000000000..034b6ddcca --- /dev/null +++ b/codegraph-kernel/src/resolve/python_expression_scope.rs @@ -0,0 +1,78 @@ +//! A Python name bound by the expression around a reference: a lambda +//! parameter or a comprehension variable. Binding rows are scoped to whole +//! lines, so these single-expression scopes are read from the syntax tree. + +use super::iteration::{descendant_for_position, named_children}; +use super::*; + +/// Every identifier `node` names as a binding target, tuples and stars included. +fn binds(node: tree_sitter::Node, text: &str, name: &str) -> bool { + match node.kind() { + "identifier" => &text[node.start_byte()..node.end_byte()] == name, + "attribute" | "subscript" => false, + _ => named_children(node).into_iter().any(|c| binds(c, text, name)), + } +} + +impl KernelResolver { + /// Whether the lambda or comprehension the reference sits in binds `name` + /// there: `lambda other: other.f()`, `[other.f() for other in items]`. + /// A comprehension's first iterable is evaluated outside its scope. + pub(super) fn python_expression_binds(&mut self, name: &str, r: &ResolveRefIn) -> bool { + let Some(file) = self.read_file(&r.file_path) else { return false }; + let text = file.text(); + let Some(tree) = self.parsed_tree(&file, r) else { return false }; + let leaf = descendant_for_position( + tree.root_node(), + text, + ((r.line - 1).max(0) as usize, r.column.max(0) as usize), + ); + let at = leaf.start_byte(); + let inside = |n: tree_sitter::Node| n.start_byte() <= at && at < n.end_byte(); + let mut current = Some(leaf); + while let Some(node) = current { + current = node.parent(); + match node.kind() { + "lambda" => { + let in_body = node.child_by_field_name("body").is_some_and(inside); + let named = node.child_by_field_name("parameters").is_some_and(|params| { + named_children(params).into_iter().any(|param| { + let target = param + .child_by_field_name("name") + .or_else(|| { + matches!(param.kind(), "list_splat_pattern" | "dictionary_splat_pattern") + .then(|| param.named_child(0)) + .flatten() + }) + .unwrap_or(param); + binds(target, text, name) + }) + }); + if in_body && named { + return true; + } + } + "list_comprehension" | "set_comprehension" | "dictionary_comprehension" | "generator_expression" => { + let clauses: Vec<_> = named_children(node) + .into_iter() + .filter(|c| c.kind() == "for_in_clause") + .collect(); + let in_first_iterable = clauses + .first() + .and_then(|c| c.child_by_field_name("right")) + .is_some_and(inside); + if !in_first_iterable + && clauses + .iter() + .filter_map(|c| c.child_by_field_name("left")) + .any(|target| binds(target, text, name)) + { + return true; + } + } + _ => {} + } + } + false + } +} From cec50372c16cea5b87b230e49ffb54d070b3a9cd Mon Sep 17 00:00:00 2001 From: bompus Date: Fri, 30 Oct 2026 12:53:21 -0600 Subject: [PATCH 2/9] test(python): a base's type argument is no supertype, so Repo.save() is not User.save The fork's extractor emits an extends edge only for a plain or dotted base name, so `class Repo(Base[User])` never reached User.save. Adds upstream's regression test for that; the check that Base's own method is still reached through a parameterized base stays a todo, since the fork emits no edge for it. Ported from upstream cd04b21a (test only) --- .../python-supertype-type-arguments.test.ts | 136 ++++++++++++++++++ 1 file changed, 136 insertions(+) create mode 100644 __tests__/python-supertype-type-arguments.test.ts diff --git a/__tests__/python-supertype-type-arguments.test.ts b/__tests__/python-supertype-type-arguments.test.ts new file mode 100644 index 0000000000..510f5c0438 --- /dev/null +++ b/__tests__/python-supertype-type-arguments.test.ts @@ -0,0 +1,136 @@ +/** + * A Python class's supertypes, as its head declares them, are the classes its + * bases name — not their type arguments. `class Repo(Base[User])` extends + * `Base`; reading every word of the base list also counted `User`, so a call on + * the class or on a receiver named after it, `Repo.save()` / `repo.save()`, + * reached `User.save` when `Base` declares no `save`. + */ +import { describe, it, expect, afterAll, beforeAll } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; + +let root = ''; +let cg: CodeGraph; + +beforeAll(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-py-supertype-type-args-')); + fs.mkdirSync(path.join(root, 'app')); + fs.writeFileSync(path.join(root, 'app/__init__.py'), ''); + fs.writeFileSync(path.join(root, 'app/m.py'), `from typing import Generic, Optional, TypeVar + +T = TypeVar("T") +U = TypeVar("U") + + +class Base(Generic[T]): + def load(self): + pass + + +class Pair(Generic[T, U]): + def load(self): + pass + + +class User: + def save(self): + pass + + +class Other: + def save(self): + pass + + +class Repo(Base[User]): + pass + + +class Store(Pair[str, dict[str, User]]): + pass + + +class Keyed(Base[User], metaclass=type): + pass + + +class Forward(Base["Pair[str, User]"]): + pass + + +class Mixin: + def save(self): + pass + + +class Mixed(Base[Optional[User]], Mixin): + pass + + +def save_by_class(): + Repo.save() + + +def save_by_receiver(repo): + repo.save() + + +def save_nested(): + Store.save() + + +def save_keyed(): + Keyed.save() + + +def save_forward(): + Forward.save() + + +def save_mixed(): + Mixed.save() + + +def load_by_class(): + Repo.load() + + +def load_nested(): + Store.load() +`); + cg = await CodeGraph.init(root, { index: true }); +}); + +afterAll(() => { + cg?.close(); + if (root) fs.rmSync(root, { recursive: true, force: true }); +}); + +/** Qualified names of the methods `fn` calls. */ +const calls = (fn: string): string[] => { + const node = cg.getNodesInFile('app/m.py').find((n) => n.name === fn && n.kind === 'function')!; + expect(node, fn).toBeDefined(); + return cg.getOutgoingEdges(node.id).filter((e) => e.kind === 'calls') + .map((e) => cg.getNode(e.target)!).filter((n) => n.kind === 'method').map((n) => n.qualifiedName); +}; + +describe('a Python type argument is no supertype', () => { + it('does not reach a method of the type argument', () => { + expect(calls('save_by_class')).not.toContain('User::save'); + expect(calls('save_by_receiver')).not.toContain('User::save'); + expect(calls('save_nested')).not.toContain('User::save'); + expect(calls('save_keyed')).not.toContain('User::save'); + expect(calls('save_forward')).not.toContain('User::save'); + }); + + // The extractor emits an `extends` edge only for a plain or dotted base name, + // so a parameterized base like `Base[User]` links no inherited method at all. + // Upstream reads the class head's text and reaches `Base::load` here. + it.todo('reaches the method a parameterized base declares'); + + it('reaches a later base past the type argument that came first', () => { + expect(calls('save_mixed')).toEqual(['Mixin::save']); + }); +}); From e24528d9f5191242b3048dddb18c2744dc7eb143 Mon Sep 17 00:00:00 2001 From: bompus Date: Fri, 30 Oct 2026 12:58:32 -0600 Subject: [PATCH 3/9] fix(ruby): calls to `!`/`?` methods and calls on what `.new` made link their method (#3049) A call to a method whose name ends in `!` or `?` on a receiver (`writer.flush!`, `writer.ready?`, `CacheWriter.create!(1)`) failed the method-call pattern. A method called straight on a new object (`CacheWriter.new(key).write_cache`, `Billing::Ledger.new.charge!`) was recorded with the receiver's raw text, arguments and blocks included, which no strategy matched. The Ruby walker now records a call on `Const.new(...)`, `A::B.new` or `::Foo.new` as `new Const().m`, the constructed-receiver shape other languages use. The resolver claims that shape ahead of name matching: the class by Ruby's constant lookup from the namespaces around the call, the method as the class's own instance method or the first its ancestry declares, and nothing when either is missing. The Ruby dot pattern takes a trailing `!` or `?`, the receiver-word scoring among several project methods of the name stays off for such names on a value receiver, and a module a call returns (`include RouteSet.new.tap {...}.url_helpers`) is no longer read as a mixin of the constant the call starts at. Ported from upstream 3a4cd3bf --- CHANGELOG.md | 1 + __tests__/ruby-constructed-receivers.test.ts | 324 +++++++++++++++++++ codegraph-kernel/src/resolve/lang_scope.rs | 23 +- codegraph-kernel/src/resolve/method_call.rs | 21 +- codegraph-kernel/src/resolve/ruby_paths.rs | 29 +- codegraph-kernel/src/ruby.rs | 20 ++ 6 files changed, 407 insertions(+), 11 deletions(-) create mode 100644 __tests__/ruby-constructed-receivers.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 85aded1919..0a8d3e6296 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -632,6 +632,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In C++, a base class like `Message` in `class DynamicMessage final : public Message` now links to the class C++ itself would pick: the one declared in the namespaces around the class, or brought in by the file's `using` declarations and namespace aliases, and the class a `typedef` or `using` alias names. Before, a base class was matched by its name alone, so it could land on a class of the same name in another namespace or another language: in protocolbuffers/protobuf, `DynamicMessage` derived from the JSON parser's internal `Message` class, and in google/leveldb and RocksDB, iterators derived from an unrelated nested `Iterator` class such as the skip list's. The wrong base showed up in `codegraph_explore`'s type hierarchy and in impact, and tied the base's methods to the wrong overrides. A template parameter used as a base no longer links to a class that shares its name, and a base written from the global scope, like `public ::testing::Test`, now links to its class. Re-index C++ projects after upgrading. - In C++ and Rust, a call written with a qualifier, like `Status::OK()`, now links to a method of the class it names, never to one of a class whose name only ends the same way. Before, facebook/rocksdb's calls to `Status::OK()`, `Status::InvalidArgument(…)` and `Status::Corruption(…)` linked to `IOStatus`'s methods of those names, a subclass calling its base, like `return FileSystemWrapper::NewWritableFile(…)` or google/leveldb's `return DB::Put(…)`, linked to another subclass or to the calling method itself, and BurntSushi/ripgrep's `Path::new(…)` on the standard library's `Path` linked to its own `PrinterPath::new`. When classes in several languages share the name, the one in the call's own language is taken, so protocolbuffers/protobuf's C++ `DescriptorPool::generated_pool()` calls, which met its Ruby namesake first and linked to nothing, now link to the C++ method. A C++ call through a template parameter, like `CacheShard::ComputeHash(key)` in rocksdb's `template class ShardedCache`, no longer links to a class that merely shares words with the parameter's name, unless that class is the parameter's default, like `TcParser` in `template `. A template's value parameter, like `const PrefetchOpts& kOpts`, is no longer mistaken for a type parameter, so references to `PrefetchOpts` inside it now link. Re-index C++ and Rust projects after upgrading. - In Ruby, a call on a class written with its namespace, like `Billing::ChargeCard.call(…)` or `::Billing::ChargeCard.call(…)`, now links the caller to that class and to the class method it calls, as `ChargeCard.call(…)` already linked to the class. Before, a class used only through such calls looked like it had no callers. The name is looked up the way Ruby does it, from the namespaces around the call outward, so `Users::Create.call` reaches `Users::Create` and never a `Billing::Create`, a call inside `module Api; module V2` reaches `Api::V2`'s class before another version's, and a library's class like `ActiveRecord::Base` is never taken for a project class of the same last name. Re-index Ruby projects after upgrading. Thanks @james-jang for the report and @drakeo338 for the pull request. (#2503) +- In Ruby, a call to a method whose name ends in `!` or `?` now links when its receiver's class is known, like `writer.flush!` or `writer.ready?` after `writer = CacheWriter.new`, or `CacheWriter.create!(…)` on the class. A method called straight on a new object, like `CacheWriter.new(key).write_cache` or `Billing::Ledger.new.charge!`, now links to that class's method or to one it inherits. Before, all of these linked nothing. The class is looked up the way Ruby does it, from the namespaces around the call outward, and one from outside your project, like rack's `Request` used inside `Rack::Protection`, links nothing rather than a project class of the same name. Re-index Ruby projects after upgrading. Thanks @ricobl. - In PHP, a class written through a namespace alias in a different case than its `use` statement, like `new field\FirstName()` after `use App\Fields as Field;`, now links to that class, since PHP reads namespace names without regard to case. Before, the name was read relative to the file's own namespace and stayed unlinked. Re-index PHP projects after upgrading. Thanks @danusha2345. (#2369) - In C++, a call through a class template's specialization written with its arguments, like fmtlib/fmt's `formatter::format(42, ctx)`, no longer links to a member defined for another specialization, like a test's `formatter::format`, and when the template declares no such member, as for `formatter::parse(ctx)`, the call is no longer matched to another class's method of that name, like `native_formatter::parse`. A call to the specialization a member is defined for, and to a template's own members, links as before. Re-index C++ projects after upgrading. - In C++, a function is now named by its declarator, never by something written in the clauses after its parameters. nlohmann/json's constructor `basic_json(CompatibleType&& val) noexcept(noexcept(JSONSerializer::to_json(…)))` was missing from the index, replaced by a method `JSONSerializer::to_json` that doesn't exist and that other `JSONSerializer::to_json(…)` calls linked to; a method declared with `throw(std::bad_alloc)` was indexed as `std::bad_alloc`, and a function with a trailing `requires std::integral` as `std::integral`. Re-index C++ projects after upgrading. diff --git a/__tests__/ruby-constructed-receivers.test.ts b/__tests__/ruby-constructed-receivers.test.ts new file mode 100644 index 0000000000..93f86c568b --- /dev/null +++ b/__tests__/ruby-constructed-receivers.test.ts @@ -0,0 +1,324 @@ +/** + * Ruby calls on a typed receiver whose method name ends in `!` or `?`, and + * calls on what `.new` made (#684). + * + * `writer.flush!` and `writer.ready?` after `writer = CacheWriter.new` linked + * nothing, while `writer.write_cache` linked: the method-call pattern took no + * `!` or `?`. `CacheWriter.create!` linked nothing where `CacheWriter.create` + * did, for the same reason. + * + * `CacheWriter.new(key).write_cache` was recorded with the receiver's text, + * arguments and all, so it never linked. It is now `new CacheWriter() + * .write_cache`, and links an instance method of the class Ruby's constant + * lookup finds for `CacheWriter` where the call is written, its own or one + * its ancestry declares. A class the project doesn't declare links nothing: + * rack-protection's `Request.new(env)` is rack's `Rack::Request`, not the + * project's `Sinatra::Request`. + * + */ +import { describe, it, expect, afterEach } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; +import { extractFromSource } from '../src/extraction/tree-sitter'; + +const FILES: Record = { + 'app/writer.rb': `class CacheWriter + def self.create(key) + new + end + + def self.create!(key) + new + end + + def write_cache(key) + key + end + + def flush! + nil + end + + def ready? + true + end +end + +class OtherWriter + def write_cache(key) + key + end + + def flush! + nil + end + + def ready? + false + end +end +`, + 'app/billing.rb': `module Billing + class Card + def charge!(amount) + amount + end + end + + class Ledger < Card + def post + Card.new.charge!(1) + end + end +end + +module Admin + class Card + def charge!(amount) + amount + end + end +end +`, + 'app/counter.rb': `class Counter + def bang + writer = CacheWriter.new + writer.flush! + end + + def question + writer = CacheWriter.new + writer.ready? + end + + def chained(key) + CacheWriter.new.write_cache(key) + end + + def chained_args(key) + CacheWriter.new(key).write_cache(key) + end + + def chained_bang + CacheWriter.new(1).flush! + end + + def namespaced + Billing::Ledger.new.charge!(2) + end + + def top_level + ::CacheWriter.new.ready? + end + + def class_bang + CacheWriter.create!(1) + end + + def outside(env) + Request.new(env).preferred_type + end +end +`, + 'lib/rack/show_exceptions.rb': `module Rack + module Protection + class ShowExceptions + def call(env) + Request.new(env).preferred_type("text/html") + end + end + end +end +`, + 'lib/sinatra/request.rb': `module Sinatra + class Request + def preferred_type(*types) + types.first + end + end +end +`, + // A module a call returns is no mixin of the constant the call starts at: + // UrlForHelper doesn't get RouteSet's methods (actionpack's url_for tests). + 'app/url_helper.rb': `class RouteSet + def url_for(options) + options + end + + def url_helpers + Module.new + end +end + +class UrlForHelper + include RouteSet.new.url_helpers +end + +class UrlForTest + def test_url + UrlForHelper.new.url_for(1) + end +end +`, + // Two project methods named `key?`: a `?` call on an untyped receiver is not + // guessed between them by the receiver's words. + 'lib/options.rb': `class InheritableOptions + def key?(key) + true + end +end + +class FlashHash + def key?(name) + false + end +end + +class Store + def merged(options) + options.key?(:namespace) + end +end +`, +}; + +let root = ''; +let cg: CodeGraph | undefined; + +afterEach(() => { + cg?.close(); + cg = undefined; + if (root) fs.rmSync(root, { recursive: true, force: true }); + root = ''; +}); + +function write(files: Record): void { + for (const [rel, content] of Object.entries(files)) { + fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); + fs.writeFileSync(path.join(root, rel), content); + } +} + +async function index(files: Record = FILES): Promise { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-ruby-made-')); + write(files); + cg = await CodeGraph.init(root, { index: true }); + return cg; +} + +/** `:` of every `calls` edge written on `line` of `file`. */ +function callsAt(graph: CodeGraph, file: string, line: number): string[] { + const ids = graph.getNodesInFile(file).map((n) => n.id); + return graph + .getOutgoingEdgesFrom(ids) + .filter((e) => e.kind === 'calls' && e.line === line) + .map((e) => { + const t = graph.getNode(e.target)!; + return `${t.qualifiedName}:${t.startLine}`; + }) + .sort(); +} + +describe('Ruby bang and question methods, and calls on .new (#684)', () => { + it('links a `!` or `?` method called on a typed local', async () => { + const graph = await index(); + expect(callsAt(graph, 'app/counter.rb', 4)).toEqual(['CacheWriter::flush!:14']); + expect(callsAt(graph, 'app/counter.rb', 9)).toEqual(['CacheWriter::ready?:18']); + }); + + it('links a `!` class method', async () => { + const graph = await index(); + expect(callsAt(graph, 'app/counter.rb', 33)).toEqual(['CacheWriter::create!:6']); + }); + + it('links a call on what `.new` made, with or without arguments', async () => { + const graph = await index(); + expect(callsAt(graph, 'app/counter.rb', 13)).toEqual(['CacheWriter::write_cache:10']); + expect(callsAt(graph, 'app/counter.rb', 17)).toEqual(['CacheWriter::write_cache:10']); + expect(callsAt(graph, 'app/counter.rb', 21)).toEqual(['CacheWriter::flush!:14']); + expect(callsAt(graph, 'app/counter.rb', 29)).toEqual(['CacheWriter::ready?:18']); + }); + + it('finds the class by Ruby constant lookup, and the method in its ancestry', async () => { + const graph = await index(); + // `Card` inside `Billing::Ledger` is Billing::Card, not Admin::Card. + expect(callsAt(graph, 'app/billing.rb', 10)).toEqual(['Billing::Card::charge!:3']); + // Billing::Ledger has no `charge!`; its superclass Billing::Card does. + expect(callsAt(graph, 'app/counter.rb', 25)).toEqual(['Billing::Card::charge!:3']); + }); + + it('links nothing for a class the project does not declare', async () => { + const graph = await index(); + expect(callsAt(graph, 'app/counter.rb', 37)).toEqual([]); + expect(callsAt(graph, 'lib/rack/show_exceptions.rb', 5)).toEqual([]); + }); + + it("finds no method in a module a call returns, which the class's ancestry doesn't name", async () => { + const graph = await index(); + expect(callsAt(graph, 'app/url_helper.rb', 17)).toEqual([]); + }); + + it('guesses no `?` method by the receiver word among several of the name', async () => { + const graph = await index(); + // `options` shares a word with InheritableOptions, but a Hash answers `key?` too. + expect(callsAt(graph, 'lib/options.rb', 15)).toEqual([]); + }); +}); + +describe('Ruby call on .new, after a sync (#684)', () => { + it('links the call once a sync adds the class it constructs', async () => { + const { 'app/writer.rb': writer, ...rest } = FILES; + const graph = await index(rest); + expect(callsAt(graph, 'app/counter.rb', 17)).toEqual([]); + + write({ 'app/writer.rb': writer! }); + expect((await graph.sync()).filesAdded).toBe(1); + // counter.rb did not change: only the retry of its parked refs links it. + expect(callsAt(graph, 'app/counter.rb', 17)).toEqual(['CacheWriter::write_cache:10']); + }, 60_000); +}); + +describe('Ruby extraction of a call on .new (#684)', () => { + // A call on what `.new` made is recorded as `new CacheWriter().write_cache`: + // the class as written, without the arguments the receiver's text carried. A + // `new` on a value (`klass.new.run`, `self.class.new`) or a later link of a + // chain keeps the receiver's text. + it('records the constructed class, not the receiver text', () => { + const src = [ + 'class Callers', + ' def a(key, klass)', + ' CacheWriter.new.write_cache(key)', + ' CacheWriter.new(key, ttl: 5).flush!', + ' Billing::Ledger.new { |l| l }.post', + ' ::CacheWriter.new.ready?', + ' CacheWriter.new&.write_cache(key)', + ' klass.new.run', + ' self.class.new(key).run', + ' Widget.create(key).save', + ' CacheWriter.new.first.second', + ' end', + 'end', + '', + ].join('\n'); + const calls = extractFromSource('src/callers.rb', src, 'ruby').unresolvedReferences + .filter((r) => r.referenceKind === 'calls' && /[.(]/.test(r.referenceName)) + .map((r) => `${r.referenceName} ${r.line}`); + expect(calls.sort()).toEqual([ + 'CacheWriter.new.first.second 11', + 'Widget.create 10', + 'Widget.create(key).save 10', + 'klass.new 8', + 'klass.new.run 8', + 'new ::CacheWriter().ready? 6', + 'new Billing::Ledger().post 5', + 'new CacheWriter().first 11', + 'new CacheWriter().flush! 4', + 'new CacheWriter().write_cache 3', + 'new CacheWriter().write_cache 7', + 'self.class.new 9', + 'self.class.new(key).run 9', + ]); + }); +}); diff --git a/codegraph-kernel/src/resolve/lang_scope.rs b/codegraph-kernel/src/resolve/lang_scope.rs index 25a8d5303e..38657c514a 100644 --- a/codegraph-kernel/src/resolve/lang_scope.rs +++ b/codegraph-kernel/src/resolve/lang_scope.rs @@ -598,11 +598,7 @@ impl KernelResolver { let to = (decl.end_line.max(0) as usize).min(lines.len()); let mixes: Vec = lines[from..to.max(from)] .iter() - .filter_map(|line| { - re!(r"^\s*(?:include|extend|prepend)\s+([A-Z:][A-Za-z0-9_:]*(?:\s*,\s*[A-Z:][A-Za-z0-9_:]*)*)") - .captures(line) - .map(|m| m[1].to_string()) - }) + .filter_map(|line| ruby_mixin_list(line)) .collect(); for mix in mixes { for name in re!(r"\s*,\s*").split(&mix) { @@ -967,3 +963,20 @@ impl KernelResolver { })) } } + +/// The constants an `include` / `extend` / `prepend` line names, comma +/// separated. A module a call returns (`include RouteSet.new.tap { … }.url_helpers`) +/// is no mixin of the constant the call starts at: the list ends before a +/// name that is followed by a `.` or `(`, as a name's own trailing word +/// characters never do. +fn ruby_mixin_list(line: &str) -> Option { + let m = re!(r"^\s*(?:include|extend|prepend)\s+([A-Z:][A-Za-z0-9_:]*(?:\s*,\s*[A-Z:][A-Za-z0-9_:]*)*)").captures(line)?; + let whole = m.get(1)?; + let mut names: Vec<&str> = re!(r"\s*,\s*").split(whole.as_str()).collect(); + let rest = line[whole.end()..].trim_start(); + if rest.starts_with(['.', '(']) { + // Backtracking drops the last name; the one before it is followed by `,`. + names.pop(); + } + (!names.is_empty()).then(|| names.join(",")) +} diff --git a/codegraph-kernel/src/resolve/method_call.rs b/codegraph-kernel/src/resolve/method_call.rs index 2ca88f29f3..7a36ad3525 100644 --- a/codegraph-kernel/src/resolve/method_call.rs +++ b/codegraph-kernel/src/resolve/method_call.rs @@ -660,6 +660,14 @@ impl KernelResolver { } else { None } + .or_else(|| { + // A Ruby method name may end in `!` or `?`: `writer.flush!`, `writer.ready?`. + if r.language == "ruby" { + re!(r"^([A-Za-z0-9_.]+)\.([A-Za-z0-9_]+[!?])$").captures(&r.reference_name) + } else { + None + } + }) .or_else(|| dot_re.captures(&r.reference_name)); if dot_match.is_none() && r.language == "cpp" { let op_re = re!(r"^([A-Za-z0-9_.]+)\.(operator[^A-Za-z0-9_\s.]+)$"); @@ -1490,7 +1498,16 @@ if inferred.is_none() && r.language == "dart" { resolved_by: "instance-method", })); } - if target.len() > 1 { + // Not for a Ruby name ending in `!` or `?` called on a value: + // `key?`, `empty?`, `respond_to?`, `is_a?` and `merge!` are what the + // core classes answer on nearly every receiver, and the receiver's + // words can't tell which project method of the name, if any, is + // meant. A constant receiver (`MarshalWithFallback.dumped?`) still + // names its owner. + let ruby_bang_or_predicate = r.language == "ruby" + && method_name.ends_with(['!', '?']) + && super::call_shape::receiver_link(&object_or_class).starts_with(|c: char| c.is_ascii_lowercase() || c == '_'); + if target.len() > 1 && !ruby_bang_or_predicate { let receiver_words = split_camel_case(super::call_shape::receiver_link(&object_or_class)); // Same-file candidates first, so a score tie resolves to the // call site's own file (`score > bestScore` keeps first seen). @@ -1663,7 +1680,7 @@ if inferred.is_none() && r.language == "dart" { // method (`def self.m`) when it and `def m` share the qualified // name, then the owner's Ruby ancestry (#2834). if c.language == "ruby" { - if let Some((found, own)) = self.ruby_method_of(&c.qualified_name, method, r)? { + if let Some((found, own)) = self.ruby_method_of(&c.qualified_name, method, r, "class")? { return Ok(Some(KCand { node: found, confidence: if own { confidence } else { 0.8 }, resolved_by })); } continue; diff --git a/codegraph-kernel/src/resolve/ruby_paths.rs b/codegraph-kernel/src/resolve/ruby_paths.rs index a1f1ce3218..9e84a21541 100644 --- a/codegraph-kernel/src/resolve/ruby_paths.rs +++ b/codegraph-kernel/src/resolve/ruby_paths.rs @@ -45,6 +45,27 @@ impl KernelResolver { }; } if r.reference_kind == "calls" { + // A call on an instance `Foo.new(…)` made, bare or namespaced + // (`new Foo().m`, `new A::B().m!`): an instance method of the class + // Ruby's lookup finds for `Foo` there, its own or one its ancestry + // declares — never a namesake's by the method name alone. A class + // the project doesn't declare (rack-protection's `Request.new(env)` + // is rack's) has no method here. + if let Some(c) = re!(r"^new (::)?([A-Z]\w*(?:::[A-Z]\w*)*)\(\)\.([A-Za-z_]\w*[?!=]?)$").captures(&r.reference_name) { + let class = self + .ruby_constants_named(&c[2], r, c.get(1).is_some())? + .into_iter() + .next() + .filter(|n| n.kind == "class"); + let method = match class { + Some(class) => self.ruby_method_of(&class.qualified_name, &c[3], r, "instance")?, + None => None, + }; + return Ok(Some(match method { + Some((method, own)) => ResolveOutcome::resolved(&method, if own { 0.9 } else { 0.85 }, "instance-method", true, None), + None => ResolveOutcome::unresolved(), + })); + } let Some(c) = re!(r"^(::)?([A-Z]\w*(?:::[A-Z]\w*)*)\.([A-Za-z_]\w*[?!=]?)$").captures(r.reference_name.trim_start_matches("()")) else { return Ok(None); }; @@ -58,7 +79,7 @@ impl KernelResolver { let Some(owner) = owner else { return Ok(Some(ResolveOutcome::unresolved())); }; - return match self.ruby_method_of(&owner.qualified_name, &c[3], r)? { + return match self.ruby_method_of(&owner.qualified_name, &c[3], r, "class")? { Some((method, own)) => Ok(Some(ResolveOutcome::resolved( &method, if own { 0.85 } else { 0.8 }, @@ -121,7 +142,7 @@ impl KernelResolver { /// else on the first ancestor in its Ruby ancestry that declares it. /// Returns the method and whether it is the owner's own. `r` is the call /// site: production code never reaches a method only a test declares. - pub(super) fn ruby_method_of(&mut self, owner_qn: &str, name: &str, r: &ResolveRefIn) -> Res, bool)>> { + pub(super) fn ruby_method_of(&mut self, owner_qn: &str, name: &str, r: &ResolveRefIn, kind: &str) -> Res, bool)>> { let production = !super::resolver_upstream::test_suite_path(&r.file_path); let own: Vec> = self .nodes_by_qualified_name(&format!("{owner_qn}::{name}"))? @@ -129,7 +150,7 @@ impl KernelResolver { .filter(|n| n.language == "ruby" && n.kind == "method" && !(production && super::resolver_upstream::test_suite_path(&n.file_path))) .cloned() .collect(); - if let Some(m) = ruby_methods_of_kind_first(ruby_ordered(own, r), "class").into_iter().next() { + if let Some(m) = ruby_methods_of_kind_first(ruby_ordered(own, r), kind).into_iter().next() { return Ok(Some((m, true))); } let ancestry = self.ruby_ancestry(owner_qn)?; @@ -142,7 +163,7 @@ impl KernelResolver { .filter(|n| n.language == "ruby" && n.kind == "method" && !(production && super::resolver_upstream::test_suite_path(&n.file_path))) .cloned() .collect(); - if let Some(m) = ruby_methods_of_kind_first(ruby_ordered(found, r), "class").into_iter().next() { + if let Some(m) = ruby_methods_of_kind_first(ruby_ordered(found, r), kind).into_iter().next() { return Ok(Some((m, false))); } } diff --git a/codegraph-kernel/src/ruby.rs b/codegraph-kernel/src/ruby.rs index bb239b341c..2882ee15b0 100644 --- a/codegraph-kernel/src/ruby.rs +++ b/codegraph-kernel/src/ruby.rs @@ -633,6 +633,22 @@ impl<'t> Walker<'t> { self.push_ref_at(parent, &ref_path, imports_kind, node); } + /// rubyConstructedClass (tree-sitter.ts): the class a call receiver + /// constructs, as written — `Foo` for `Foo.new(key)`, `Billing::Card` for + /// `Billing::Card.new`, `::Foo` for `::Foo.new`; None for any other + /// receiver, a `new` on a value (`klass.new`) among them. + fn constructed_class(&self, receiver: Node<'t>) -> Option<&'t str> { + if receiver.kind() != "call" { + return None; + } + let method = receiver.child_by_field_name("method")?; + let made = receiver.child_by_field_name("receiver")?; + if self.text(method) != "new" || !is_constant_path(made) { + return None; + } + Some(self.text(made)) + } + /// extractCall — the bespoke ruby branch (tree-sitter.ts:3905-3960), /// reached only from the body walker. fn extract_call(&mut self, node: Node<'t>) { @@ -680,6 +696,10 @@ impl<'t> Walker<'t> { let skip = matches!(receiver_name, "self" | "super"); let callee = if skip { method_name.to_string() + } else if let Some(class_name) = self.constructed_class(receiver) { + // A call on what `Foo.new(…)` made: `new Foo().m`, not the + // receiver's text with its arguments in it. + format!("new {class_name}().{method_name}") } else { format!("{receiver_name}.{method_name}") }; From 21397fd77d900f28acfa39eba24aae7028b72982 Mon Sep 17 00:00:00 2001 From: bompus Date: Fri, 30 Oct 2026 17:12:14 -0600 Subject: [PATCH 4/9] fix(ruby): a call on an instance variable that initialize types links to its class's method (#3053) Ported from upstream cdfa31be547f0896c23dd0dd00381b150b75e11d. --- CHANGELOG.md | 1 + __tests__/ruby-ivar-receivers.test.ts | 152 ++++++++++++ codegraph-kernel/src/resolve/awaited.rs | 2 +- codegraph-kernel/src/resolve/mod.rs | 5 + codegraph-kernel/src/resolve/ruby_paths.rs | 276 ++++++++++++++++++++- 5 files changed, 434 insertions(+), 2 deletions(-) create mode 100644 __tests__/ruby-ivar-receivers.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a8d3e6296..996949de3f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -633,6 +633,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In C++ and Rust, a call written with a qualifier, like `Status::OK()`, now links to a method of the class it names, never to one of a class whose name only ends the same way. Before, facebook/rocksdb's calls to `Status::OK()`, `Status::InvalidArgument(…)` and `Status::Corruption(…)` linked to `IOStatus`'s methods of those names, a subclass calling its base, like `return FileSystemWrapper::NewWritableFile(…)` or google/leveldb's `return DB::Put(…)`, linked to another subclass or to the calling method itself, and BurntSushi/ripgrep's `Path::new(…)` on the standard library's `Path` linked to its own `PrinterPath::new`. When classes in several languages share the name, the one in the call's own language is taken, so protocolbuffers/protobuf's C++ `DescriptorPool::generated_pool()` calls, which met its Ruby namesake first and linked to nothing, now link to the C++ method. A C++ call through a template parameter, like `CacheShard::ComputeHash(key)` in rocksdb's `template class ShardedCache`, no longer links to a class that merely shares words with the parameter's name, unless that class is the parameter's default, like `TcParser` in `template `. A template's value parameter, like `const PrefetchOpts& kOpts`, is no longer mistaken for a type parameter, so references to `PrefetchOpts` inside it now link. Re-index C++ and Rust projects after upgrading. - In Ruby, a call on a class written with its namespace, like `Billing::ChargeCard.call(…)` or `::Billing::ChargeCard.call(…)`, now links the caller to that class and to the class method it calls, as `ChargeCard.call(…)` already linked to the class. Before, a class used only through such calls looked like it had no callers. The name is looked up the way Ruby does it, from the namespaces around the call outward, so `Users::Create.call` reaches `Users::Create` and never a `Billing::Create`, a call inside `module Api; module V2` reaches `Api::V2`'s class before another version's, and a library's class like `ActiveRecord::Base` is never taken for a project class of the same last name. Re-index Ruby projects after upgrading. Thanks @james-jang for the report and @drakeo338 for the pull request. (#2503) - In Ruby, a call to a method whose name ends in `!` or `?` now links when its receiver's class is known, like `writer.flush!` or `writer.ready?` after `writer = CacheWriter.new`, or `CacheWriter.create!(…)` on the class. A method called straight on a new object, like `CacheWriter.new(key).write_cache` or `Billing::Ledger.new.charge!`, now links to that class's method or to one it inherits. Before, all of these linked nothing. The class is looked up the way Ruby does it, from the namespaces around the call outward, and one from outside your project, like rack's `Request` used inside `Rack::Protection`, links nothing rather than a project class of the same name. Re-index Ruby projects after upgrading. Thanks @ricobl. +- In Ruby, a call on an instance variable, like `@writer.flush`, now links to the method of the class the variable holds when the class's `initialize` sets it to a new object of one class, like `@writer = CacheWriter.new`. Before, no call on an instance variable linked. A variable that `initialize` sets to anything else, or to objects of two classes, stays unlinked rather than guessed. Re-index Ruby projects after upgrading. - In PHP, a class written through a namespace alias in a different case than its `use` statement, like `new field\FirstName()` after `use App\Fields as Field;`, now links to that class, since PHP reads namespace names without regard to case. Before, the name was read relative to the file's own namespace and stayed unlinked. Re-index PHP projects after upgrading. Thanks @danusha2345. (#2369) - In C++, a call through a class template's specialization written with its arguments, like fmtlib/fmt's `formatter::format(42, ctx)`, no longer links to a member defined for another specialization, like a test's `formatter::format`, and when the template declares no such member, as for `formatter::parse(ctx)`, the call is no longer matched to another class's method of that name, like `native_formatter::parse`. A call to the specialization a member is defined for, and to a template's own members, links as before. Re-index C++ projects after upgrading. - In C++, a function is now named by its declarator, never by something written in the clauses after its parameters. nlohmann/json's constructor `basic_json(CompatibleType&& val) noexcept(noexcept(JSONSerializer::to_json(…)))` was missing from the index, replaced by a method `JSONSerializer::to_json` that doesn't exist and that other `JSONSerializer::to_json(…)` calls linked to; a method declared with `throw(std::bad_alloc)` was indexed as `std::bad_alloc`, and a function with a trailing `requires std::integral` as `std::integral`. Re-index C++ projects after upgrading. diff --git a/__tests__/ruby-ivar-receivers.test.ts b/__tests__/ruby-ivar-receivers.test.ts new file mode 100644 index 0000000000..dfd30813be --- /dev/null +++ b/__tests__/ruby-ivar-receivers.test.ts @@ -0,0 +1,152 @@ +/** + * A Ruby call on an instance variable, like `@writer.flush`, links to the + * method of the class the class's `initialize` gives the variable: + * `@writer = CacheWriter.new` makes it a `CacheWriter`. Before, no call on an + * instance variable linked at all. + * + * Only `initialize` is read, and only a lone `Const.new(…)` types the + * variable there: one assigned two classes, a conditional, a value passed + * through `.tap`, or anything but `.new` leaves it untyped, and a class the + * project doesn't declare (`Logger.new($stdout)`) links nothing. A class + * method's `@x` is the class's own, and a module's methods run on whatever + * includes it, so neither is typed. + */ +import { describe, it, expect, afterEach } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; + +const FILES: Record = { + 'app/writer.rb': `class CacheWriter + def flush + nil + end + + def ready? + true + end +end + +class OtherWriter + def flush + nil + end +end + +module Billing + class Card + def charge!(amount) + amount + end + end + + class Ledger < Card + end +end +`, + 'app/holder.rb': `class Holder + def initialize(key) + @writer = CacheWriter.new + @lazy ||= OtherWriter.new( + key + ) + @either = key ? CacheWriter.new : OtherWriter.new + @twice = CacheWriter.new + @twice = OtherWriter.new if key + @ledger = Billing::Ledger.new key do |l| + l + end + @tapped = CacheWriter.new.tap { |w| w } + @outside = Logger.new($stdout) + @a, @b = CacheWriter.new, OtherWriter.new + end + + def via_ivar + @writer.flush + @writer.ready? + @lazy.flush + @either.flush + @twice.flush + @ledger.charge!(1) + @tapped.flush + @outside.info("x") + @b.flush + @unset.flush + end + + def self.klass + @writer.flush + end +end + +module Mixin + def mixed + @writer.flush + end +end +`, +}; + +let root = ''; +let cg: CodeGraph | undefined; + +afterEach(() => { + cg?.close(); + cg = undefined; + if (root) fs.rmSync(root, { recursive: true, force: true }); + root = ''; +}); + +async function index(): Promise { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-ruby-ivar-')); + for (const [rel, content] of Object.entries(FILES)) { + fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); + fs.writeFileSync(path.join(root, rel), content); + } + cg = await CodeGraph.init(root, { index: true }); + return cg; +} + +/** `:` of every `calls` edge written on `line` of `file`. */ +function callsAt(graph: CodeGraph, file: string, line: number): string[] { + const ids = graph.getNodesInFile(file).map((n) => n.id); + return graph + .getOutgoingEdgesFrom(ids) + .filter((e) => e.kind === 'calls' && e.line === line) + .map((e) => { + const t = graph.getNode(e.target)!; + return `${t.qualifiedName}:${t.startLine}`; + }) + .sort(); +} + +describe('Ruby calls on an instance variable initialize types', () => { + it('links the method of the class initialize constructs', async () => { + const graph = await index(); + const holder = 'app/holder.rb'; + expect(callsAt(graph, holder, 19)).toEqual(['CacheWriter::flush:2']); + expect(callsAt(graph, holder, 20)).toEqual(['CacheWriter::ready?:6']); + // `||=` with its arguments over several lines. + expect(callsAt(graph, holder, 21)).toEqual(['OtherWriter::flush:12']); + // A namespaced class, built with a block: the method its superclass has. + expect(callsAt(graph, holder, 24)).toEqual(['Billing::Card::charge!:19']); + }); + + it('types nothing initialize leaves ambiguous', async () => { + const graph = await index(); + const holder = 'app/holder.rb'; + for (const line of [22, 23, 25, 27, 28]) expect(callsAt(graph, holder, line)).toEqual([]); + }); + + it('links nothing for a class the project does not declare', async () => { + const graph = await index(); + expect(callsAt(graph, 'app/holder.rb', 26)).toEqual([]); + }); + + it("leaves a class method's and a module's instance variables untyped", async () => { + const graph = await index(); + expect(callsAt(graph, 'app/holder.rb', 32)).toEqual([]); + expect(callsAt(graph, 'app/holder.rb', 38)).toEqual([]); + }); +}); diff --git a/codegraph-kernel/src/resolve/awaited.rs b/codegraph-kernel/src/resolve/awaited.rs index 7797936bcb..ef2b5145a1 100644 --- a/codegraph-kernel/src/resolve/awaited.rs +++ b/codegraph-kernel/src/resolve/awaited.rs @@ -57,7 +57,7 @@ fn awaited_names(text: &str) -> HashSet { } /// Push `c` blanked: one space per UTF-16 unit, a newline kept. -fn push_blank(out: &mut String, c: char) { +pub(super) fn push_blank(out: &mut String, c: char) { if c == '\n' { out.push('\n'); } else { diff --git a/codegraph-kernel/src/resolve/mod.rs b/codegraph-kernel/src/resolve/mod.rs index 0d39dfd999..11c161a778 100644 --- a/codegraph-kernel/src/resolve/mod.rs +++ b/codegraph-kernel/src/resolve/mod.rs @@ -786,6 +786,10 @@ pub struct KernelResolver { dart_hierarchy_memo: HashMap<(String, i64), Rc>>, /// Kotlin supertypes named in a type's head, by type name. kotlin_supers_memo: HashMap>>, + /// rubyIvarClass (#3053): the class `initialize` gives an instance + /// variable, by `owner\0name`. `None` = untyped (left to the other + /// strategies); `Some(None)` = typed to a class the project lacks. + ruby_ivar_memo: HashMap>>>, /// kotlinFileScope, by file. declared_member_memo: HashMap<(String, String), Option>, declared_member_lines: HashMap>>, @@ -1122,6 +1126,7 @@ impl KernelResolver { kotlin_receiver_types_memo: None, kotlin_scope_memo: HashMap::new(), kotlin_supers_memo: HashMap::new(), + ruby_ivar_memo: HashMap::new(), py_descendants_memo: HashMap::new(), py_bases_memo: HashMap::new(), cpp_visible_memo: None, diff --git a/codegraph-kernel/src/resolve/ruby_paths.rs b/codegraph-kernel/src/resolve/ruby_paths.rs index 9e84a21541..35cb548ac3 100644 --- a/codegraph-kernel/src/resolve/ruby_paths.rs +++ b/codegraph-kernel/src/resolve/ruby_paths.rs @@ -66,6 +66,24 @@ impl KernelResolver { None => ResolveOutcome::unresolved(), })); } + // A call on an instance variable `@writer.flush`, typed by the + // class's `initialize`: an instance method of the class every + // assignment there gives the variable — or nothing when that + // class is not the project's. A variable `initialize` does not + // type is left to the other strategies, as before (#3053). + if let Some(c) = re!(r"^@(\w+)\.([A-Za-z_]\w*[?!=]?)$").captures(&r.reference_name) { + let Some(class) = self.ruby_ivar_class(&c[1], r)? else { + return Ok(None); + }; + let method = match class { + Some(class) => self.ruby_method_of(&class.qualified_name, &c[2], r, "instance")?, + None => None, + }; + return Ok(Some(match method { + Some((method, own)) => ResolveOutcome::resolved(&method, if own { 0.85 } else { 0.8 }, "instance-method", true, None), + None => ResolveOutcome::unresolved(), + })); + } let Some(c) = re!(r"^(::)?([A-Z]\w*(?:::[A-Z]\w*)*)\.([A-Za-z_]\w*[?!=]?)$").captures(r.reference_name.trim_start_matches("()")) else { return Ok(None); }; @@ -126,6 +144,11 @@ impl KernelResolver { /// lexicalScope: the qualified name of the innermost class or module /// around `r`, or empty at the top level of its file. fn ruby_lexical_scope(&mut self, r: &ResolveRefIn) -> Res { + Ok(self.ruby_innermost_namespace(r)?.map(|i| i.qualified_name.clone()).unwrap_or_default()) + } + + /// innermostNamespace: the innermost class or module around `r`, if any. + fn ruby_innermost_namespace(&mut self, r: &ResolveRefIn) -> Res>> { let mut inner: Option> = None; for n in self.nodes_in_file(&r.file_path)?.iter() { if n.language != "ruby" || !matches!(n.kind.as_str(), "class" | "module") || n.start_line > r.line || n.end_line < r.line { @@ -135,7 +158,79 @@ impl KernelResolver { inner = Some(n.clone()); } } - Ok(inner.map(|i| i.qualified_name.clone()).unwrap_or_default()) + Ok(inner) + } + + /// ivarClass (#3053): `Some` the class of what `@name` holds where `r` + /// is written — `Some(None)` when that class is not the project's — + /// or `None` for the other strategies when `r` is not in an instance + /// method of a class, or `initialize` does not type the variable. + fn ruby_ivar_class(&mut self, name: &str, r: &ResolveRefIn) -> Res>>> { + let caller = self.node_by_id(&r.from_node_id)?; + match &caller { + Some(c) if c.language == "ruby" && c.kind == "method" && !c.is_static => {} + _ => return Ok(None), + } + let Some(owner) = self.ruby_innermost_namespace(r)? else { + return Ok(None); + }; + if owner.kind != "class" { + return Ok(None); + } + let key = format!("{}\0{}", owner.qualified_name, name); + if let Some(hit) = self.ruby_ivar_memo.get(&key) { + return Ok(hit.clone()); + } + let found = self.ruby_ivar_class_from_initialize(name, &owner.qualified_name, r)?; + self.ruby_ivar_memo.insert(key, found.clone()); + Ok(found) + } + + /// ivarClassFromInitialize: every assignment to `@name` in the owner's + /// `initialize` must be at a statement's start and a lone `Const.new(…)` + /// of one constant — then the class Ruby's lookup finds for that + /// constant from there. + fn ruby_ivar_class_from_initialize(&mut self, name: &str, owner: &str, r: &ResolveRefIn) -> Res>>> { + let mut written: Option<(String, bool, String, i64)> = None; + for init in self.nodes_by_qualified_name(&format!("{owner}::initialize"))?.iter() { + if init.kind != "method" || init.language != "ruby" || init.is_static { + continue; + } + let Some(lines) = self.read_file(&init.file_path) else { + return Ok(None); + }; + let mut body = String::new(); + for i in init.start_line - 1..init.end_line { + match lines.line(i as usize) { + Some(l) => { + body.push_str(l); + body.push('\n'); + } + None => break, + } + } + let body = strip_ruby_comments(&body); + let Some(assigned) = constructed_assignments(&body, name) else { + return Ok(None); + }; + for (path, absolute, line) in assigned { + match &written { + Some((p, a, _, _)) if *p != path || *a != absolute => return Ok(None), + None => written = Some((path, absolute, init.file_path.clone(), init.start_line + line)), + _ => {} + } + } + } + let Some((path, absolute, file, line)) = written else { + return Ok(None); + }; + let mut at = r.clone(); + at.file_path = file; + at.line = line; + at.reference_name = path.clone(); + at.reference_kind = "references".to_string(); + let class = self.ruby_constants_named(&path, &at, absolute)?.into_iter().next(); + Ok(Some(class.filter(|n| n.kind == "class"))) } /// methodOf: `name` declared on `owner_qn` itself (kind `class` first), @@ -225,3 +320,182 @@ pub(super) fn ruby_methods_of_kind_first(methods: Vec>, kind: &str) - same.extend(other); same } + +/// constructedAssignments (#3053): each assignment to `@name` in `body` +/// (an `initialize`, comments blanked): the constant whose `.new` it +/// assigns, and the assignment's line within `body`. None when one of +/// them is anything else: not at the start of a statement (`@a, @b = …`), +/// or not a lone `Const.new(…)` (`Foo.new.tap {…}`, `writer || Foo.new`, +/// `build_writer`). +fn constructed_assignments(body: &str, name: &str) -> Option> { + // Upstream's `/(?])/g`, minus the + // look-around Rust's regex lacks: the boundary char is captured and + // skipped, the `=~>` tail is checked by hand. + let target = super::cached_pattern(&format!(r"(^|[^@\w])@{}\s*(?:\|\||&&|\*\*|<<|>>|[-+*/%|&^])?=", regex::escape(name))).ok()?; + let mut out = Vec::new(); + for m in target.captures_iter(body) { + let whole = m.get(0).unwrap(); + let at = whole.start() + m.get(1).unwrap().as_str().len(); + let after_eq = whole.end(); + match body.as_bytes().get(after_eq) { + Some(b'=') | Some(b'~') | Some(b'>') => continue, + _ => {} + } + let statement_start = body[..at].rfind(['\n', ';']).map_or(0, |i| i + 1); + if !body[statement_start..at].trim().is_empty() { + return None; + } + let (path, absolute) = constructed_value(body, after_eq)?; + let line = body[..at].matches('\n').count() as i64; + out.push((path, absolute, line)); + } + Some(out) +} + +/// constructedValue: the constant a value written at `from` constructs +/// when it is a lone `Const.new`, `A::B.new(…)` or `::Foo.new …`: its +/// arguments in parentheses or without them, a block, or a statement +/// modifier may follow, but nothing the value is passed through +/// (`.tap`, `||`). None otherwise. +fn constructed_value(s: &str, from: usize) -> Option<(String, bool)> { + let m = re!(r"^\s*(::)?([A-Z]\w*(?:::[A-Z]\w*)*)\s*\.\s*new").captures(&s[from..])?; + let mut i = from + m.get(0).unwrap().end(); + // upstream's `new(?![\w?!])` (`\w` is ASCII): `new?`/`new!`/`newly` + // are method names. + match s[i..].chars().next() { + Some(c) if c.is_ascii_alphanumeric() || matches!(c, '_' | '?' | '!') => return None, + _ => {} + } + if s.as_bytes().get(i) == Some(&b'(') { + i = skip_parens(s, i)?; + } + // The end of the statement, a block, or (after a space) a modifier or + // arguments without parentheses: anything but a chain or an operator. + let tail = &s[i.min(s.len())..]; + let ok = match tail.chars().next() { + None | Some('\n') | Some(';') | Some('{') => true, + _ => re!(r#"^[ \t]+(?:$|\n|;|\{|[\w:@$"'\[])"#).is_match(&tail[..tail.len().min(200)]), + }; + // A chain may go on at the start of the next line: `Foo.new\n .tap {…}`. + let continued = re!(r"^[ \t]*\n\s*&?\.").is_match(&tail[..tail.len().min(200)]); + (ok && !continued).then(|| (m[2].to_string(), m.get(1).is_some())) +} + +/// skipParens: the index past the parenthesized group opening at `i`, +/// Ruby strings inside it skipped; None when it never closes. +fn skip_parens(s: &str, i: usize) -> Option { + let b = s.as_bytes(); + let mut depth = 0i32; + let mut j = i; + while j < b.len() { + let c = b[j]; + if c == b'"' || c == b'\'' { + j += 1; + while j < b.len() && b[j] != c { + if b[j] == b'\\' { + j += 1; + } + j += 1; + } + j += 1; + continue; + } + if c == b'(' { + depth += 1; + } else if c == b')' { + depth -= 1; + if depth == 0 { + return Some(j + 1); + } + } + j += 1; + } + None +} + +/// stripCommentsForRegex(…, 'ruby') (upstream strip-comments.ts): +/// `#` line comments and `=begin`/`=end` blocks blanked (newlines kept, +/// so offsets still map); string literals skipped intact. +fn strip_ruby_comments(src: &str) -> String { + let s: Vec = src.chars().collect(); + let n = s.len(); + let mut out = String::with_capacity(src.len()); + let mut i = 0; + let mut at_line_start = true; + while i < n { + let c = s[i]; + // `=begin` opens a block comment only at a line's start (after + // optional whitespace); `=end` must sit at a line's start too. + if at_line_start && c == '=' && s[i..].iter().take(6).collect::() == "=begin" { + let start = i; + i += 6; + while i < n { + if s[i] == '\n' { + let mut j = i + 1; + while j < n && (s[j] == ' ' || s[j] == '\t') { + j += 1; + } + if s[j..].iter().take(4).collect::() == "=end" { + i = j + 4; + while i < n && s[i] != '\n' { + i += 1; + } + break; + } + } + i += 1; + } + for &ch in &s[start..i.min(n)] { + super::awaited::push_blank(&mut out, ch); + } + at_line_start = i > 0 && s[i - 1] == '\n'; + continue; + } + if c == '"' || c == '\'' { + let start = i; + i += 1; + while i < n && s[i] != c { + if s[i] == '\\' && i + 1 < n { + i += 2; + continue; + } + if s[i] == '\n' { + break; + } + i += 1; + } + if i < n && s[i] == c { + i += 1; + } + out.extend(&s[start..i.min(n)]); + at_line_start = false; + continue; + } + if c == '#' { + let start = i; + while i < n && s[i] != '\n' { + i += 1; + } + for &ch in &s[start..i] { + super::awaited::push_blank(&mut out, ch); + } + at_line_start = false; + continue; + } + if c == '\n' { + at_line_start = true; + out.push(c); + i += 1; + continue; + } + if c == ' ' || c == '\t' { + out.push(c); + i += 1; + continue; + } + at_line_start = false; + out.push(c); + i += 1; + } + out +} From f57fd22367bbffeadf996e1f251a3516595e64f6 Mon Sep 17 00:00:00 2001 From: bompus Date: Fri, 30 Oct 2026 17:16:32 -0600 Subject: [PATCH 5/9] fix(vue): a self-closing `: the ` +`; + const result = extractFromSource('src/components/Page.vue', code); + + expect(result.errors).toEqual([]); + expect(result.nodes.find((n) => n.name === 'ready')?.startLine).toBe(7); + expect(result.nodes.find((n) => n.name === 'go')?.startLine).toBe(8); + expect(calls(result, 'helper', 8)).toBe(true); + expect(calls(result, 'formatDate', 2)).toBe(true); + expect(renders(result, 'Header', 3)).toBe(true); + }); + + it('keeps the template it comes before', () => { + const code = ` +`; + const result = extractFromSource('src/components/First.vue', code); + + expect(result.errors).toEqual([]); + expect(result.nodes.find((n) => n.name === 'ready')?.startLine).toBe(7); + expect(calls(result, 'formatDate', 3)).toBe(true); + expect(renders(result, 'Header', 4)).toBe(true); + }); + + it("still reads a call in a self-closing +`; + const result = extractFromSource('src/components/Post.vue', code); + + expect(result.errors).toEqual([]); + expect(result.nodes.find((n) => n.name === 'ready')?.startLine).toBe(9); + expect(calls(result, 'toJsonLd', 3)).toBe(true); + expect(calls(result, 'formatDate', 4)).toBe(true); + expect(renders(result, 'Header', 5)).toBe(true); + }); + + // The template-call scan always took `` as a closing tag; with one + // pattern for every site, the script reader does too. + it('ends a block at a closing tag written with a space, ``', () => { + const code = ` + +`; + const result = extractFromSource('src/components/Spaced.vue', code); + + expect(result.errors).toEqual([]); + expect(result.nodes.find((n) => n.name === 'ready')?.startLine).toBe(5); + expect(calls(result, 'formatDate', 2)).toBe(true); + }); +}); diff --git a/src/extraction/astro-extractor.ts b/src/extraction/astro-extractor.ts index 0d048dae5c..e6740cd492 100644 --- a/src/extraction/astro-extractor.ts +++ b/src/extraction/astro-extractor.ts @@ -2,7 +2,7 @@ import { Binding, Node, Edge, ExtractionResult, ExtractionError, UnresolvedRefer import { generateNodeId } from './tree-sitter-helpers'; import { extractEmbeddedBlock } from './block-extract'; import { isLanguageSupported } from './grammars'; -import { arrowScopes, declaredByArrow, foldScriptResult, sfcFileNode } from './sfc-script'; +import { arrowScopes, declaredByArrow, foldScriptResult, sfcFileNode, SFC_SCRIPT_ELEMENT, sfcBlockLineRanges } from './sfc-script'; /** * Astro built-in components — compiler-provided (``) or shipped by @@ -163,7 +163,7 @@ export class AstroExtractor { // A self-closing ``. - const scriptRegex = /]*?)?(?:\/>|>(?[\s\S]*?)<\/script>)/g; + const scriptRegex = new RegExp(SFC_SCRIPT_ELEMENT); let match; while ((match = scriptRegex.exec(this.source)) !== null) { @@ -227,16 +227,9 @@ export class AstroExtractor { coveredRanges.push([frontmatter.startLine - 1, frontmatter.endLine]); } - const tagRegex = /<(script|style)(\s[^>]*?)?(?:\/>|>[\s\S]*?<\/\1>)/g; - let tagMatch; - while ((tagMatch = tagRegex.exec(this.source)) !== null) { - // A self-closing tag has no body to skip, and its attributes - // (`set:html={serializeJsonLd(schema)}`) are template expressions. - if (tagMatch[0].endsWith('/>')) continue; - const startLine = (this.source.substring(0, tagMatch.index).match(/\n/g) || []).length; - const endLine = startLine + (tagMatch[0].match(/\n/g) || []).length; - coveredRanges.push([startLine, endLine]); - } + // A self-closing tag covers nothing: its attributes + // (`set:html={serializeJsonLd(schema)}`) are template expressions. + coveredRanges.push(...sfcBlockLineRanges(this.source)); return coveredRanges; } diff --git a/src/extraction/sfc-script.ts b/src/extraction/sfc-script.ts index eec289c400..83c100abdc 100644 --- a/src/extraction/sfc-script.ts +++ b/src/extraction/sfc-script.ts @@ -42,6 +42,51 @@ export function sfcFileNode(filePath: string, source: string, language: Language }; } +/** + * A ``, and that script was parsed with the markup in between and + * contributed nothing (#2989). + * + * Shared by the three extractors and the viewer's syntax regions. It carries + * the `g` flag, so each loop runs on its own copy (`new RegExp(...)`). + */ +export const SFC_SCRIPT_ELEMENT = /]*?)?(?:\/>|>(?[\s\S]*?)<\/script\s*>)/g; + +/** + * A ``. - const scriptRegex = /]*?)?(?:\/>|>(?[\s\S]*?)<\/script>)/g; + const scriptRegex = new RegExp(SFC_SCRIPT_ELEMENT); let match; while ((match = scriptRegex.exec(this.source)) !== null) { @@ -236,20 +236,10 @@ export class SvelteExtractor { componentNodeId: string, _scriptBlocks: Array<{ content: string; startLine: number }> ): void { - // Build a set of line ranges covered by and ranges - const tagRegex = /<(script|style)(\s[^>]*?)?(?:\/>|>[\s\S]*?<\/\1>)/g; - let tagMatch; - while ((tagMatch = tagRegex.exec(this.source)) !== null) { - // A self-closing tag has no body to skip, and its attributes - // (`src={assetUrl('/analytics.js')}`) are template expressions. - if (tagMatch[0].endsWith('/>')) continue; - const startLine = (this.source.substring(0, tagMatch.index).match(/\n/g) || []).length; - const endLine = startLine + (tagMatch[0].match(/\n/g) || []).length; - coveredRanges.push([startLine, endLine]); - } + // Line ranges covered by `. -const SCRIPT_BLOCK = /]*?)?(?:\/>|>([\s\S]*?)<\/script>)/gi; +// The extractors' own script element, read case-insensitively here. A +// self-closing ``. +const SCRIPT_BLOCK = new RegExp(SFC_SCRIPT_ELEMENT.source, 'gi'); /** Astro's frontmatter: a `---` fence at the very top of the file. */ const ASTRO_FRONTMATTER = /^(---\r?\n)([\s\S]*?)\r?\n---/; @@ -389,7 +390,8 @@ export function syntaxRegionsFor(source: string, language: Language): SyntaxRegi while ((match = SCRIPT_BLOCK.exec(source)) !== null) { const body = match[2] ?? ''; if (body.trim() === '') continue; - const start = match.index + match[0].length - body.length - ''.length; + // The body starts right after the opening tag, whose attributes hold no `>`. + const start = match.index + match[0].indexOf('>') + 1; regions.push({ start, end: start + body.length, diff --git a/src/extraction/vue-extractor.ts b/src/extraction/vue-extractor.ts index 584631d02a..a0415571c2 100644 --- a/src/extraction/vue-extractor.ts +++ b/src/extraction/vue-extractor.ts @@ -2,7 +2,7 @@ import { Binding, Node, Edge, ExtractionResult, ExtractionError, UnresolvedRefer import { generateNodeId } from './tree-sitter-helpers'; import { extractEmbeddedBlock } from './block-extract'; import { isLanguageSupported } from './grammars'; -import { foldScriptResult, sfcFileNode, sfcScriptLanguage } from './sfc-script'; +import { foldScriptResult, sfcFileNode, sfcScriptLanguage, SFC_SCRIPT_ELEMENT, sfcBlockLineRanges } from './sfc-script'; import { vueOptionsMembers } from './vue-options-api'; import { vueTemplateCalls } from './vue-template-calls'; @@ -203,12 +203,15 @@ export class VueExtractor { language: Language; }> = []; - const scriptRegex = /]*)?>(?[\s\S]*?)<\/script>/g; + // A self-closing ``. + const scriptRegex = new RegExp(SFC_SCRIPT_ELEMENT); let match; while ((match = scriptRegex.exec(this.source)) !== null) { + if (match.groups?.content === undefined) continue; const attrs = match[1] || ''; - const content = match.groups?.content || match[2] || ''; + const content = match.groups.content; // TypeScript, TSX or JavaScript, from the lang attribute const language = sfcScriptLanguage(attrs, 'vue'); @@ -289,15 +292,9 @@ export class VueExtractor { // Ranges covered by