From 8410db42a1e8b68021d421a1503fc5ba536c0bde Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Wed, 30 Sep 2026 19:15:47 +0000 Subject: [PATCH] fix(scala): block-local vals, package imports and package objects (#2193) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - A `val` the graph files under a class but that is written inside a block of its body (cats' `test("…") { val f = … }`) is that block's alone. It took `f(…)` calls from lambda parameters in other suites and from cats' core library (`FreeT::FlatMapped::f` from option.scala, Invariant.scala…). A `val`/`const` declared inside a function's lines is likewise local (outside C/C++, whose nesting is not trusted; a Lua global stays global). - In a class body with no enclosing method, a binder (a lambda parameter, a `val`) counts as local only inside a block still open at the site. - Only an import rooted at a value (`import builder._`) lets any type's members in; a lowercase package path (`import cats.syntax.all._`) did too. - An imported object's supertypes are in scope, and `package object x extends Api` (no node in the graph) is read from source - imported, or the file's own package: sttp's `multipart(...)`, `asJson(...)`, `asEither(...)`. cats -478 +63, sttp -153 +81 (sampled losses: other tests' locals, scalatest `test`/`eventually`, libc `free`, `sttp.model.Header`). fs2, http4s, okio, koin, Exposed, lodash, excalidraw, flask, kong, jekyll, halo unchanged. Co-authored-by: Claude Opus 5.5 --- CHANGELOG.md | 1 + __tests__/scala-block-locals.test.ts | 118 +++++++++++++++++++ src/resolution/name-matcher.ts | 167 +++++++++++++++++++++++++-- 3 files changed, 274 insertions(+), 12 deletions(-) create mode 100644 __tests__/scala-block-locals.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c19bd687..377aea2f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixes +- Scala calls no longer land on another test's local value. A `val f` written inside one `test("…") { }` block was being picked for a lambda parameter `f` in another suite, and even for `f(…)` in cats' core library. A package import like `import cats.syntax.all._` no longer puts every class's members in scope. Members that really are in scope now resolve: those of an imported package object's parent types (`import sttp.client4._` gives `SttpApi`'s `multipart`), and of the file's own package object. - A barrel's `export * as core from './core'` now exports only `core`, not everything in `./core`. In zod, `z.string()` and `z.number()` had been landing on same-named regex constants deep in its core module instead of the schema functions. Calls through `import * as ns` also find members a barrel forwards by name (`export { safeParse } from '../core'`). They follow a namespace path like `z.coerce.number()` too. - JavaScript calls through CommonJS `require` bindings now reach the right function. `var express = require('../')` follows `module.exports` through an `index.js` to the function it exports, and every binding in a `var a = require('x'), b = require('y')` list is read. `require('./utils').setCharset` means that module's `setCharset` rather than its first export, and a call to a package from outside the project (`supertest`, `node:assert`) is no longer an edge to the local variable. On express, over 700 `express()` calls now reach `createApplication`. - Lua calls made through a `local` alias now reach the function it names. That covers `local splitn = require("kong.tools.string").splitn`, a module member such as `utils.select_listener`, and a function a module exports under another name (`return { check = check_phase }`). Aliases of Lua's own functions, such as `local type = type` or `local fmt = string.format`, no longer show up as calls into your project. On Kong that is over 1,000 calls that now reach their real function, and about 5,000 fewer dead-end edges. diff --git a/__tests__/scala-block-locals.test.ts b/__tests__/scala-block-locals.test.ts new file mode 100644 index 000000000..2b079ec42 --- /dev/null +++ b/__tests__/scala-block-locals.test.ts @@ -0,0 +1,118 @@ +/** + * A Scala `val` written inside a block of a class body — cats' `test("…") { + * val f = … }` — is that block's, not a member of the class: a lambda + * parameter `f` in another suite (`forAll { (e: Int, f: Int => Int) => f(e) }`) + * and a sibling test's `f(2)` are not calls to it. A lowercase package import + * (`import cats.syntax.all._`) no longer lets every class's members in; an + * imported package object's supertypes do (`package object api extends Api`), + * as does the package object of the file's own package. + */ +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-scala-block-')); + const files: Record = { + 'src/test/scala/app/ASuite.scala': `package app + +class ASuite { + def test(name: String)(body: => Unit): Unit = () + + test("a") { + val f = (i: Int) => i + f(1) + } + + test("c") { + f(2) + } +} +`, + 'src/test/scala/app/BSuite.scala': `package app + +import app.syntax.all._ + +class BSuite { + def test(name: String)(body: => Unit): Unit = () + def forAll(p: (Int, Int => Int) => Boolean): Unit = () + + test("b") { + forAll { (e: Int, f: Int => Int) => + f(e) == 1 + } + } +} +`, + 'src/main/scala/app/syntax/all.scala': `package app.syntax + +object all +`, + 'src/main/scala/app/Api.scala': `package app + +trait Api { + def multipart(s: String): String = s +} +`, + 'src/main/scala/app/api/package.scala': `package app + +package object api extends Api +`, + 'src/main/scala/app/client/Client.scala': `package app.client + +import app.api._ + +class Client { + def send(): String = multipart("x") +} +`, + 'src/main/scala/app/api/Inner.scala': `package app.api + +class Inner { + def send(): String = multipart("y") +} +`, + }; + 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 }); +}); + +afterAll(() => { + cg?.close(); + if (root) fs.rmSync(root, { recursive: true, force: true }); +}); + +/** `qualified name:line` of every call edge out of a file. */ +function callsFrom(file: string): string[] { + const ids = cg.getNodesInFile(file).map((n) => n.id); + return cg + .getOutgoingEdgesFrom(ids) + .filter((e) => e.kind === 'calls') + .map((e) => `${cg.getNode(e.target)!.qualifiedName}:${e.line}`) + .sort(); +} + +describe('Scala block-local vals', () => { + it('are called from their own block only', () => { + expect(callsFrom('src/test/scala/app/ASuite.scala').filter((c) => c.startsWith('ASuite::f'))).toEqual(['ASuite::f:8']); + }); + + it('are not another suite’s lambda parameter, whatever the file imports', () => { + expect(callsFrom('src/test/scala/app/BSuite.scala').filter((c) => c.includes('::f:'))).toEqual([]); + }); +}); + +describe('Scala package objects', () => { + it('bring their supertypes’ members in when imported, and into their own package', () => { + expect(callsFrom('src/main/scala/app/client/Client.scala')).toEqual(['Api::multipart:6']); + expect(callsFrom('src/main/scala/app/api/Inner.scala')).toEqual(['Api::multipart:4']); + }); +}); diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 098b49170..49ed35bb6 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -618,6 +618,13 @@ function isLexicallyReachable( ref: UnresolvedRef, context: ResolutionContext ): boolean { + // A `val` / `const` declared in a function body is that body's alone — + // okio's `(source as Source).buffer()` bound to a `val buffer = Buffer()` + // inside another test file's `pipe()`. + if (candidate.kind === 'variable' || candidate.kind === 'constant' || (candidate.kind === 'field' && candidate.language === 'scala')) { + const scope = localDeclarationScope(candidate, context); + return scope === null || (ref.filePath === candidate.filePath && ref.line >= scope.start && ref.line <= scope.end); + } // A function — or a type (`case class B()` in a test method), or a method // of such a type — declared inside a function is only in scope in there. if (candidate.kind !== 'function' && candidate.kind !== 'method' && !LOCAL_TYPE_KINDS.has(candidate.kind)) return true; @@ -633,6 +640,66 @@ function isLexicallyReachable( return scope === null || (ref.filePath === candidate.filePath && ref.line >= scope.start && ref.line <= scope.end); } +/** Per context: node id → the function body (or Scala block) a declaration is local to. */ +const LOCAL_DECL_MEMO = new WeakMap>(); + +/** + * The lines a variable declaration is in scope for when it is local: the + * innermost function or method of its file whose lines hold it — or, for a + * Scala `val` the graph files under its class but written inside a block of + * the class body (cats' `test("…") { val f = … }`), that block. Null for a + * declaration at file, class or object level. A Lua global assigned inside a + * function is still global; C and C++ nesting is not trusted (see below). + */ +function localDeclarationScope(candidate: Node, context: ResolutionContext): { start: number; end: number } | null { + if (NO_NESTED_FUNCTIONS.has(candidate.language)) return null; + let memo = LOCAL_DECL_MEMO.get(context); + if (!memo) LOCAL_DECL_MEMO.set(context, (memo = new Map())); + const hit = memo.get(candidate.id); + if (hit !== undefined) return hit; + let scope: { start: number; end: number } | null = null; + if (!((candidate.language === 'lua' || candidate.language === 'luau') && !isLuaLocal(candidate, context))) { + for (const n of context.getNodesInFile(candidate.filePath)) { + if ((n.kind !== 'function' && n.kind !== 'method') || n.id === candidate.id) continue; + if (n.startLine > candidate.startLine || n.endLine < candidate.startLine || n.startLine === n.endLine) continue; + if (n.startLine === candidate.startLine && (n.startColumn ?? 0) >= (candidate.startColumn ?? 0)) continue; + if (!scope || n.endLine - n.startLine < scope.end - scope.start) scope = { start: n.startLine, end: n.endLine }; + } + if (!scope && candidate.kind === 'field' && candidate.language === 'scala') scope = scalaBlockOf(candidate, context); + } + memo.set(candidate.id, scope); + return scope; +} + +/** The `{ … }` block, deeper than its class body, that a Scala `val` is written in; null for a member. */ +function scalaBlockOf(candidate: Node, context: ResolutionContext): { start: number; end: number } | null { + const owner = candidate.qualifiedName.includes('::') ? candidate.qualifiedName.slice(0, candidate.qualifiedName.lastIndexOf('::')) : ''; + const cls = context.getNodesInFile(candidate.filePath).find((n) => + n.qualifiedName === owner && (n.kind === 'class' || n.kind === 'trait' || n.kind === 'struct' || n.kind === 'module')); + if (!cls || cls.startLine >= candidate.startLine) return null; + const lines = context.getFileLines?.(candidate.filePath) ?? context.readFile(candidate.filePath)?.split(/\r?\n/) ?? []; + const clean = (l: string) => l.replace(/"(?:[^"\\]|\\.)*"|'(?:[^'\\]|\\.)'/g, '""').replace(/\/\/.*$/, ''); + // Depth at the start of each line, counted from the class header; the body is depth 1. + const opens: number[] = []; + let depth = 0; + for (let line = cls.startLine; line < candidate.startLine; line++) { + for (const ch of clean(lines[line - 1] ?? '')) { + if (ch === '{') { depth++; opens.push(line); } + else if (ch === '}') { depth = Math.max(0, depth - 1); opens.pop(); } + } + } + if (depth <= 1) return null; + const start = opens[opens.length - 1]!; + let d = depth; + for (let line = candidate.startLine; line <= cls.endLine; line++) { + for (const ch of clean(lines[line - 1] ?? '')) { + if (ch === '{') d++; + else if (ch === '}' && --d < depth) return { start, end: line }; + } + } + return { start, end: cls.endLine }; +} + /** Per context: a candidate's scoping function body, or null when nothing scopes it. */ const LEXICAL_SCOPE_MEMO = new WeakMap>(); @@ -2445,7 +2512,7 @@ const GO_STD_METHODS: ReadonlySet = new Set([ const SCALA_TYPE_KINDS: ReadonlySet = new Set(['class', 'trait', 'interface', 'enum', 'struct', 'module', 'namespace']); const SCALA_MEMBER_KINDS: ReadonlySet = new Set(['method', 'field', 'property', 'variable', 'constant']); const SCALA_SUPERS = new WeakMap>(); -const SCALA_IMPORTS = new WeakMap; members: Set }>>(); +const SCALA_IMPORTS = new WeakMap; members: Set; values: Set }>>(); /** * Whether a bare Scala name can mean the member `n`, read at its site. Three @@ -2484,7 +2551,10 @@ function isScalaMemberInScope(n: Node, ref: UnresolvedRef, context: ResolutionCo const imports = scalaImportsOf(ref.filePath, context); if (imports.owners.has(owner) || imports.members.has(`${owner}.${name}`)) return true; // `import builder._` brings in a VALUE's members, of a type the file doesn't say. - if ([...imports.owners].some((o) => /^[a-z]/.test(o))) return true; + if (imports.values.size > 0) return true; + // An imported object's inherited members: `import sttp.client4._` is + // `package object client4 extends SttpApi`, so `multipart(…)` is SttpApi's. + if (scalaImportedSupertypes(ref.filePath, imports, context).has(owner)) return true; const around = context .getNodesInFile(ref.filePath) .filter((t) => SCALA_TYPE_KINDS.has(t.kind) && t.startLine <= ref.line && t.endLine >= ref.line); @@ -2564,18 +2634,41 @@ function isScalaExtensionMethod(n: Node, context: ResolutionContext): boolean { * `val` / `var` / `def`, a lambda or `for` parameter — or null. */ function scalaLocalBinder(name: string, ref: UnresolvedRef, context: ResolutionContext): Node | null { - const fn = context - .getNodesInFile(ref.filePath) - .filter((f) => (f.kind === 'method' || f.kind === 'function') && f.startLine <= ref.line && f.endLine >= ref.line) + const nodes = context.getNodesInFile(ref.filePath); + const innermost = (kinds: ReadonlySet) => nodes + .filter((f) => kinds.has(f.kind) && f.startLine <= ref.line && f.endLine >= ref.line) .sort((a, b) => (a.endLine - a.startLine) - (b.endLine - b.startLine))[0]; - if (!fn) return null; + const fn = innermost(SCALA_FUNCTION_KINDS); + // A test suite's body runs in its class: `test("…") { forAll { (e: E, f: A => B) => f(1) } }`. + const scope = fn ?? innermost(SCALA_TYPE_KINDS); + if (!scope) return null; const lines = context.getFileLines?.(ref.filePath) ?? context.readFile(ref.filePath)?.split('\n') ?? []; - const text = lines.slice(fn.startLine - 1, ref.line).join('\n'); + const text = lines.slice(scope.startLine - 1, ref.line).join('\n'); const n = name.replace(/[$]/g, '\\$'); - const binds = new RegExp(`(?:[(,\\[]\\s*(?:implicit\\s+|using\\s+)?${n}\\s*:)|(?:\\b(?:val|var|def|lazy\\s+val)\\s+${n}\\b)|(?:(?|<-))|(?:\\(\\s*${n}\\s*(?:,[^)]*)?\\)\\s*=>)`).test(text); - return binds ? fn : null; + const binder = new RegExp(`(?:[(,\\[]\\s*(?:implicit\\s+|using\\s+)?${n}\\s*:)|(?:\\b(?:val|var|def|lazy\\s+val)\\s+${n}\\b)|(?:(?|<-))|(?:\\(\\s*${n}\\s*(?:,[^)]*)?\\)\\s*=>)`, 'g'); + if (fn) return binder.test(text) ? fn : null; + // In a class body, a binder counts only inside a block still open at the site — + // not a sibling test's `val f`, not the class's own members at its body's depth. + const blockAt: number[] = new Array(text.length); + const open: number[] = []; + let next = 0; + let depth = 0; + for (let i = 0; i < text.length; i++) { + const ch = text[i]; + if (ch === '{') { open.push(++next); depth++; } + else if (ch === '}') { open.pop(); depth--; } + blockAt[i] = open.length > 1 ? open[open.length - 1]! : 0; + } + const live = new Set(open.slice(1)); + for (const m of text.matchAll(binder)) { + const block = blockAt[m.index!] ?? 0; + if (block !== 0 && live.has(block)) return scope; + } + return null; } +const SCALA_FUNCTION_KINDS: ReadonlySet = new Set(['method', 'function']); + /** The simple names a Scala type's declarations extend or mix in. */ function scalaSupertypesOf(typeName: string, context: ResolutionContext): string[] { let memo = SCALA_SUPERS.get(context); @@ -2607,15 +2700,62 @@ function scalaSupertypesOf(typeName: string, context: ResolutionContext): string return names.slice(1); } +const SCALA_IMPORTED_SUPERS = new WeakMap>>(); +const SCALA_PACKAGE_OBJECTS = new WeakMap>(); + +/** `package object client4 extends SttpApi with …` — the graph holds no node for one. */ +function scalaPackageObjects(context: ResolutionContext): Map { + const hit = SCALA_PACKAGE_OBJECTS.get(context); + if (hit) return hit; + const out = new Map(); + for (const file of context.getAllFiles()) { + if (!file.endsWith('.scala') || (context.fileContains && !context.fileContains(file, 'package object'))) continue; + const source = context.readFile(file) ?? ''; + for (const m of source.matchAll(/\bpackage\s+object\s+([\w$]+)\s+extends\s+([^{\n]+)/g)) { + const names = m[2]!.replace(/\[[^\]]*\]/g, '').split(/\bwith\b/).map((t) => t.trim().split('.').pop()!.replace(/\(.*$/, '').trim()).filter(Boolean); + out.set(m[1]!, [...(out.get(m[1]!) ?? []), ...names]); + } + } + SCALA_PACKAGE_OBJECTS.set(context, out); + return out; +} + +/** Every supertype of the objects a Scala file imports wholesale (`import Obj._`). */ +function scalaImportedSupertypes(file: string, imports: { owners: Set }, context: ResolutionContext): Set { + let memo = SCALA_IMPORTED_SUPERS.get(context); + if (!memo) SCALA_IMPORTED_SUPERS.set(context, (memo = new Map())); + const hit = memo.get(file); + if (hit) return hit; + const seen = new Set(); + // Code in `package sttp.client4` (or under it) sees `package object client4`'s members unimported. + const packages = [...(context.readFile(file) ?? '').matchAll(/^\s*package\s+([\w.]+)\s*$/gm)].flatMap((m) => m[1]!.split('.')); + const queue = [...imports.owners, ...packages]; + const packageObjects = scalaPackageObjects(context); + while (queue.length > 0 && seen.size < 120) { + const typeName = queue.shift()!; + for (const sup of [...scalaSupertypesOf(typeName, context), ...(packageObjects.get(typeName) ?? [])]) { + if (!seen.has(sup)) { seen.add(sup); queue.push(sup); } + } + } + memo.set(file, seen); + return seen; +} + /** A Scala file's `import a.b.Obj._` / `import a.b.Obj.*` owners and `import a.b.Obj.{x, y}` / `Obj.x` members. */ -function scalaImportsOf(file: string, context: ResolutionContext): { owners: Set; members: Set } { +function scalaImportsOf(file: string, context: ResolutionContext): { owners: Set; members: Set; values: Set } { let memo = SCALA_IMPORTS.get(context); if (!memo) SCALA_IMPORTS.set(context, (memo = new Map())); const hit = memo.get(file); if (hit) return hit; - const found = { owners: new Set(), members: new Set() }; - for (const m of (context.readFile(file) ?? '').matchAll(/^\s*import\s+([\w.]+?)\.(?:(_|\*)|\{([^}]*)\}|([\w$]+))\s*$/gm)) { + const found = { owners: new Set(), members: new Set(), values: new Set() }; + const source = context.readFile(file) ?? ''; + for (const m of source.matchAll(/^\s*import\s+([\w.]+?)\.(?:(_|\*)|\{([^}]*)\}|([\w$]+))\s*$/gm)) { const owner = m[1]!.split('.').pop()!; + // `import builder._` — rooted at a value the file declares, whose type it doesn't say. + const root = m[1]!.split('.')[0]!; + if (/^[a-z]/.test(root) && (root === m[1] || new RegExp(`\\b(?:val|var|lazy\\s+val)\\s+${root}\\b|[(,]\\s*${root}\\s*:`).test(source))) { + found.values.add(owner); + } if (m[2]) found.owners.add(owner); else for (const member of (m[3] ?? m[4] ?? '').split(',')) { const id = member.trim().split(/\s*=>\s*/)[0]!; @@ -4750,6 +4890,9 @@ export function clearNameMatcherMemos(context: ResolutionContext): void { RUST_TRAIT_IMPL_MEMO.delete(context); RUST_USES.delete(context); LEXICAL_SCOPE_MEMO.delete(context); + SCALA_IMPORTED_SUPERS.delete(context); + SCALA_PACKAGE_OBJECTS.delete(context); + LOCAL_DECL_MEMO.delete(context); JAVA_SUPERS.delete(context); PHP_SUPERS.delete(context); DART_SUPERS.delete(context);