fix(play): a Class.method reference reaches a member of the class it names

The Play resolver claims every Class.method reference in a Play project, in
routes files and in ordinary Java and Scala code alike, and took the first
method of that name anywhere in the class's file. In a file holding several
classes (Java nested classes, a Scala class with its companion and
siblings) that was usually another class's method: MediaType.parse(...)
went to Cookie.parse, a route controllers.Application.index to a nested
Helper.index, and Scala RequestAuthority.parse(...) to AuthorityPort.parse.

A method now counts only when its qualified name is the named type's own,
cls::method. A Scala object is an owner too, so a companion's own members
are found through it, and for a call in code so are a Java interface's and
enum's static methods. For a call written in code, the types of the
caller's language are tried first: Play declares RequestAuthority both as a
Scala case class and as a Java record, and each language's callers mean
their own; the other language's types still follow, since Play's Java API
calls its Scala classes. Among same-named types the caller's own file comes
first (Scala names carry no package), an overloaded member gives the
overload the call's arguments fit, and a Java file's import of the name
from outside the project leaves the call to the generic resolver. A route
keeps the plain order and takes a class or object. An inherited member is
left to the generic resolver, which finds it through declared supertypes.

Validation:
- frameworks-integration.test.ts: the PR's case (nested Http.Cookie /
  MediaType, Scala sibling objects, a route beside a nested Helper.index),
  red on main; ours, Scala and Java callers of a RequestAuthority declared
  in both languages (red on main and with the PR's own resolver) and an
  inherited route handler; and a guard for overloads, same-named objects
  in two packages and a Java interface's static method.
- A/B against main cde4cd74: playframework +113 / -257, 510 retargeted;
  CMAK 47 retargeted, all first-in-file picks corrected; play-samples -2;
  click and gin identical. The 50 Scala parse calls that records sent to
  the Java record reach their Scala type again.
- Cost: playframework process CPU, median of 3 interleaved fresh indexes,
  63.3 s on main and 62.8 s here.

Known costs, left as follow-ups:
- 177 right edges main reached only through its first-in-file rule are
  lost: 171 Assets.resourceNameAt calls in AssetsSpec through a local
  val Assets = new AssetsBuilder(...), which the generic resolver drops
  because it reads Scala's private[controllers] as file-private, and 6
  Writeable.writeableOf_* calls inherited by object Writeable extends
  DefaultWriteables, an object's supertypes the generic resolver does
  not follow.
- 8 calls in Scala files that import Play's Java EventSource, Comet,
  Environment or (in a nested scope) Accumulator reach the same-named
  Scala object: no Scala import is read.

Original PR #1950 by @danusha2345.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
danusha2345
2026-10-24 01:52:27 +00:00
committed by Colby Mchenry
co-authored by Claude Opus 5.5
parent cde4cd74eb
commit 4c572e07b6
4 changed files with 250 additions and 10 deletions
+1
View File
@@ -102,6 +102,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
- `codegraph_explore` and `codegraph context` now include some of the subtypes and implementors of a class or interface you ask about, and the type hierarchy the library returns for one (`getTypeHierarchy`) now lists all of them. Before, a bug in the hierarchy walk left every type without subtypes, so only what it extends or implements came back. Thanks @jocax for the report and @danusha2345 for the fix. (#2833)
- In Rust, code that uses an enum only through its variants, like `mode::Mode::A` in an expression, a `Mode::B =>` match arm or `Self::A` inside the enum's own `impl`, now shows up in that enum's callers and impact, linked to the enum that actually declares the variant rather than to a same-named type elsewhere. Standard-library variants like `Ordering::Less` or `Option::Some` and associated items like `Foo::new()` or `Foo::MAX` don't count as using a project enum. Re-index Rust projects after upgrading. Thanks @mg-mg-mg for the report and @danusha2345. (#2328)
- Axum routes now link to their handler when rustfmt wraps the `.route(` call so the handler sits on a later line, and when the handler's name also appears elsewhere on the line, as in `delete(handlers::user::delete)` or `.route("/login", post(handlers::user::login))`; Actix's `web::resource(…)` and `.route(…)` registrations get the same fix. A call inside a closure handler is no longer mistaken for a route, and an Actix resource no longer takes the next route's handler as its own. Thanks @mg-mg-mg for the report and @danusha2345. (#2326)
- In Play projects, a route or a call like `MediaType.parse(...)` now links to a method of the class or object it names, and a call to an overloaded one to the overload its arguments fit. Before, it went to the first method of that name anywhere in the class's file, often a nested or sibling class's. When a Scala and a Java type share a name, as Play's own `RequestAuthority` does, each language's calls now reach their own. Thanks @danusha2345.
- In JavaScript and TypeScript, the functions written inside a named object literal are now symbols of their own even when the object isn't exported: a plain `const api = { load() {…} }`, an object declared inside an IIFE or a function, and a namespace hung on the page or on another object, like `window.App = { init() {…} }`, `App.utils = {…}` or `dw_page = {…}`. Calls such as `api.load()`, `window.App.init()`, `App.utils.pad()`, a sibling's `this.render()`, and `App.init()` from another script on the page now reach them, the calls made inside a member belong to that member instead of the object, and a bare `init()` is no longer taken for `App.init`, so script-tag apps no longer lose most of their code from `callers` and impact. A member's qualified name now carries its object, exported or not (`api::load`, `window.App::init`), so asking for `App.init` finds it. Re-index JavaScript and TypeScript projects after upgrading. Thanks @tkhoaaa for the report and @danusha2345. (#2300)
- In JavaScript and TypeScript files with Windows (CRLF) line endings, as a Windows checkout usually has them, a store exported on a later line, like `const useStore = create(…)` followed by `export default useStore;`, now has its actions indexed, as it already did with Unix line endings. Before, those actions were missing from the graph on Windows, so their callers and impact came back empty. Re-index JavaScript and TypeScript projects after upgrading.
- In VB.NET, a name that begins with a keyword, like `SharedCache`, `Dimension`, `PublicKey` or `FriendlyName`, is now indexed whole, and a method like `Sub NewItem()` is no longer taken for a constructor. Before, the keyword was cut off (`Cache`, `ension`, `Key`) or the declaration was lost, and a line in a method that starts with such a name, like `Constants.Reload()` or `ConstVBV.Value = False`, was misread as a declaration, so reads of and calls to these names didn't link. A `Private Protected` member now counts as private instead of public. Re-index VB.NET projects after upgrading.
+208
View File
@@ -1444,3 +1444,211 @@ describe('Terraform follow-ups: remote-state bridge, provider alias, moved block
}
});
});
describe('Play end-to-end — Class.method resolves to a member of that class', () => {
let tmpDir: string;
afterEach(() => {
if (tmpDir && fs.existsSync(tmpDir)) fs.rmSync(tmpDir, { recursive: true, force: true });
});
const targetOf = (cg: CodeGraph, fromName: string, calleeName: string): string | undefined => {
const from = cg.getNodesByKind('method').find((n) => n.name === fromName);
expect(from).toBeDefined();
const edge = cg
.getOutgoingEdges(from!.id)
.filter((e) => e.kind === 'calls' || e.kind === 'references')
.map((e) => cg.getNode(e.target))
.find((t) => t?.name === calleeName);
return edge?.qualifiedName;
};
it('picks the named nested class member, not the first same-named method in its file', async () => {
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-play-member-'));
fs.mkdirSync(path.join(tmpDir, 'conf'), { recursive: true });
fs.mkdirSync(path.join(tmpDir, 'app/controllers'), { recursive: true });
fs.mkdirSync(path.join(tmpDir, 'app/util'), { recursive: true });
fs.writeFileSync(path.join(tmpDir, 'conf/routes'), 'GET / controllers.Application.index\n');
fs.writeFileSync(path.join(tmpDir, 'app/controllers/Application.java'), [
'package controllers;',
'public class Application {',
' public static class Helper {',
' public static String index() { return "helper"; }',
' }',
' public static String index() { return "app"; }',
'}',
].join('\n'));
fs.writeFileSync(path.join(tmpDir, 'app/util/Http.java'), [
'package util;',
'public class Http {',
' public static class Cookie {',
' public static String parse(String s) { return s; }',
' }',
' public static class MediaType {',
' public static String parse(String s) { return s; }',
' }',
'}',
].join('\n'));
fs.writeFileSync(path.join(tmpDir, 'app/util/Scheme.scala'), [
'package util',
'object Scheme {',
' def parse(s: String): String = s',
'}',
'object AuthorityPort {',
' def parse(s: String): String = s',
'}',
].join('\n'));
fs.writeFileSync(path.join(tmpDir, 'app/util/Caller.java'), [
'package util;',
'public class Caller {',
' public String javaCall() { return MediaType.parse("x"); }',
' public String javaCookie() { return Cookie.parse("x"); }',
'}',
].join('\n'));
fs.writeFileSync(path.join(tmpDir, 'app/util/ScalaCaller.scala'), [
'package util',
'class ScalaCaller {',
' def scalaCall(): String = AuthorityPort.parse("x")',
' def scalaScheme(): String = Scheme.parse("x")',
'}',
].join('\n'));
const cg = await CodeGraph.init(tmpDir, { index: true });
try {
expect(targetOf(cg, 'javaCall', 'parse')).toBe('util::Http::MediaType::parse');
expect(targetOf(cg, 'javaCookie', 'parse')).toBe('util::Http::Cookie::parse');
expect(targetOf(cg, 'scalaCall', 'parse')).toBe('AuthorityPort::parse');
expect(targetOf(cg, 'scalaScheme', 'parse')).toBe('Scheme::parse');
const route = cg.getNodesByKind('route').find((n) => n.name === 'GET /');
expect(route).toBeDefined();
const handlers = cg.getOutgoingEdges(route!.id).map((e) => cg.getNode(e.target)?.qualifiedName);
expect(handlers).toContain('controllers::Application::index');
expect(handlers).not.toContain('controllers::Application::Helper::index');
} finally {
cg.close();
}
});
it('prefers the caller\'s own language, and reaches a member the class inherits', async () => {
// Play declares `RequestAuthority` twice: a Scala case class with a
// companion object, and a Java class. Each language's call means its own.
// A route still reaches a handler the controller inherits.
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-play-lang-'));
fs.mkdirSync(path.join(tmpDir, 'conf'), { recursive: true });
fs.mkdirSync(path.join(tmpDir, 'app/controllers'), { recursive: true });
fs.mkdirSync(path.join(tmpDir, 'app/scalaapi'), { recursive: true });
fs.mkdirSync(path.join(tmpDir, 'app/javaapi'), { recursive: true });
fs.writeFileSync(path.join(tmpDir, 'conf/routes'), 'GET /assets/*file controllers.Assets.versioned(path = "/public", file: Asset)\n');
fs.writeFileSync(path.join(tmpDir, 'app/controllers/Assets.scala'), [
'package controllers',
'class Assets(meta: String) extends AssetsBuilder(meta)',
'class AssetsBuilder(meta: String) {',
' def versioned(path: String, file: String): String = path + file',
'}',
].join('\n'));
fs.writeFileSync(path.join(tmpDir, 'app/scalaapi/RequestAuthority.scala'), [
'package scalaapi',
'final case class RequestAuthority(host: String)',
'object RequestAuthority {',
' def parse(s: String): RequestAuthority = RequestAuthority(s)',
'}',
'class ScalaUser {',
' def scalaParse(): RequestAuthority = RequestAuthority.parse("y")',
'}',
].join('\n'));
fs.writeFileSync(path.join(tmpDir, 'app/javaapi/RequestAuthority.java'), [
'package javaapi;',
'public class RequestAuthority {',
' public static RequestAuthority parse(String s) { return null; }',
'}',
].join('\n'));
fs.writeFileSync(path.join(tmpDir, 'app/javaapi/JavaUser.java'), [
'package javaapi;',
'public class JavaUser {',
' RequestAuthority javaParse() { return RequestAuthority.parse("x"); }',
'}',
].join('\n'));
const cg = await CodeGraph.init(tmpDir, { index: true });
try {
expect(targetOf(cg, 'scalaParse', 'parse')).toBe('RequestAuthority::parse');
expect(targetOf(cg, 'javaParse', 'parse')).toBe('javaapi::RequestAuthority::parse');
const route = cg.getNodesByKind('route').find((n) => n.name === 'GET /assets/*file');
expect(route).toBeDefined();
const handlers = cg.getOutgoingEdges(route!.id).map((e) => cg.getNode(e.target)?.qualifiedName);
expect(handlers).toContain('AssetsBuilder::versioned');
} finally {
cg.close();
}
});
it('takes the overload the call fits, a same-named object in the caller\'s file, and a Java interface\'s static method', async () => {
// CMAK keeps one `object GroupMetadataManager` per Kafka version, and its
// `TopicConfigs.validate` has a one- and a two-argument overload. Play's
// `play.LoggerConfigurator` is a Java interface beside a Scala object.
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-play-pick-'));
fs.mkdirSync(path.join(tmpDir, 'conf'), { recursive: true });
fs.writeFileSync(path.join(tmpDir, 'conf/routes'), 'GET / controllers.Home.index\n');
const write = (rel: string, lines: string[]) => {
fs.mkdirSync(path.dirname(path.join(tmpDir, rel)), { recursive: true });
fs.writeFileSync(path.join(tmpDir, rel), lines.join('\n'));
};
write('app/kafka/Configs.scala', [
'package kafka',
'object Configs {',
' def validate(props: String): Unit = ()',
' def validate(version: Int, props: String): Unit = ()',
'}',
'class Admin {',
' def check(): Unit = Configs.validate(1, "p")',
'}',
]);
for (const v of ['one', 'two']) {
write(`app/kafka/${v}/Manager.scala`, [
`package kafka.${v}`,
'object Manager {',
' def readKey(s: String): String = s',
'}',
'class Reader {',
` def read_${v}(): String = Manager.readKey("x")`,
'}',
]);
}
write('app/play/LoggerConfigurator.java', [
'package play;',
'public interface LoggerConfigurator {',
' static String generateProperties(String s) { return s; }',
'}',
]);
write('app/play/api/LoggerConfigurator.scala', [
'package play.api',
'object LoggerConfigurator {',
' def generateProperties(s: String): String = s',
'}',
]);
write('app/japi/JavaLogger.java', [
'package japi;',
'import play.LoggerConfigurator;',
'public class JavaLogger {',
' String props() { return LoggerConfigurator.generateProperties("x"); }',
'}',
]);
const cg = await CodeGraph.init(tmpDir, { index: true });
try {
const target = (fromName: string, calleeName: string) => {
const from = cg.getNodesByKind('method').find((n) => n.name === fromName);
expect(from).toBeDefined();
return cg.getOutgoingEdges(from!.id).filter((e) => e.kind === 'calls')
.map((e) => cg.getNode(e.target)).find((t) => t?.name === calleeName);
};
expect(target('check', 'validate')?.startLine).toBe(4);
expect(target('read_one', 'readKey')?.filePath).toBe('app/kafka/one/Manager.scala');
expect(target('read_two', 'readKey')?.filePath).toBe('app/kafka/two/Manager.scala');
expect(targetOf(cg, 'props', 'generateProperties')).toBe('play::LoggerConfigurator::generateProperties');
} finally {
cg.close();
}
});
});
+39 -8
View File
@@ -18,10 +18,14 @@
import { Node } from '../../types';
import { FrameworkResolver, ResolutionContext, ResolvedRef, UnresolvedRef } from '../types';
import { isPlayRoutesFile } from '../../extraction/grammars';
import { isJavaOutsideImport, retargetSelfOverload } from '../name-matcher';
const ROUTE_LINE = /^(GET|POST|PUT|PATCH|DELETE|HEAD|OPTIONS)\s+(\S+)\s+(.+)$/;
const METHOD_KINDS = new Set(['method', 'function']);
const CLASS_KINDS = new Set(['class']);
/** A route's controller: a class, or a Scala `object` (a companion shares its class's qualified name). */
const CONTROLLER_KINDS = new Set(['class', 'module']);
/** What a call in code names: those, or a Java interface or enum, which hold static members of their own. */
const OWNER_KINDS = new Set([...CONTROLLER_KINDS, 'interface', 'enum']);
export const playResolver: FrameworkResolver = {
name: 'play',
@@ -49,15 +53,42 @@ export const playResolver: FrameworkResolver = {
const m = ref.referenceName.match(/^([A-Za-z_]\w*)\.([A-Za-z_]\w*)$/);
if (!m) return null;
const [, className, methodName] = m;
const classNodes = context.getNodesByName(className!).filter((n) => CLASS_KINDS.has(n.kind));
for (const cls of classNodes) {
const method = context
.getNodesInFile(cls.filePath)
.find((n) => METHOD_KINDS.has(n.kind) && n.name === methodName);
if (method) {
return { original: ref, targetNodeId: method.id, confidence: 0.9, resolvedBy: 'framework' };
const route = isPlayRoutesFile(ref.filePath);
// `import org.apache.logging.log4j.core.config.Configurator` makes a Java
// file's `Configurator` that library's, never a project type of the name.
if (!route && ref.language === 'java' && isJavaOutsideImport(className!, ref, context)) return null;
// A call's own language comes first: Play's `RequestAuthority` is both a
// Scala case class and a Java record, and each language's callers mean
// their own; the other language's types follow, since Play's Java API
// calls its Scala classes. A route file has no language of its own.
const kinds = route ? CONTROLLER_KINDS : OWNER_KINDS;
const owners = context.getNodesByName(className!).filter((n) => kinds.has(n.kind));
const groups = route
? [owners]
: [owners.filter((n) => n.language === ref.language), owners.filter((n) => n.language !== ref.language)];
for (const group of groups) {
// Only a member of THAT type counts: a file can hold many classes (Java
// nested classes, a Scala class + its companion + siblings), and the
// first same-named method in the file is usually another class's. A
// Scala object, a companion included, holds its own members.
const members = new Map<string, Node>();
for (const cls of group) {
const memberQn = `${cls.qualifiedName}::${methodName}`;
for (const n of context.getNodesInFile(cls.filePath)) {
if (METHOD_KINDS.has(n.kind) && n.name === methodName && n.qualifiedName === memberQn) members.set(n.id, n);
}
}
if (members.size === 0) continue;
const found = [...members.values()];
// Same-named types in several packages (Scala names carry no package):
// the one in the caller's own file comes first.
const pick = (!route && found.find((n) => n.filePath === ref.filePath)) || found[0]!;
const result: ResolvedRef = { original: ref, targetNodeId: pick.id, confidence: 0.9, resolvedBy: 'framework' };
// Overloads: the one the call's arguments fit, as the generic resolver picks it.
return route ? result : retargetSelfOverload(result, ref, context);
}
// An inherited member isn't looked up here: the generic resolver finds it
// through the class's declared supertypes.
return null;
},
+2 -2
View File
@@ -1937,7 +1937,7 @@ const JVM_PACKAGES = new WeakMap<ResolutionContext, Set<string>>();
* `import static org.junit.Assert.assertEquals;`. A nested class of a project
* type (`import com.acme.Outer.Inner;`) is under a project package, so it is not.
*/
function isJavaOutsideImport(name: string, ref: UnresolvedRef, context: ResolutionContext): boolean {
export function isJavaOutsideImport(name: string, ref: UnresolvedRef, context: ResolutionContext): boolean {
const binding = context.getImportMappings(ref.filePath, ref.language).find((m) => m.localName === name);
if (!binding) return false;
// A member the file declares itself is in scope before any import —
@@ -27201,7 +27201,7 @@ type Arity = { min: number; max: number } | null;
/** Per context: node id → its same-owner overloads with their arities (null: none). */
const OVERLOAD_SETS = new WeakMap<ResolutionContext, Map<string, { name: string; own: Arity; siblings: Array<{ id: string; arity: Arity }> } | null>>();
function retargetSelfOverload(result: ResolvedRef, ref: UnresolvedRef, context: ResolutionContext): ResolvedRef {
export function retargetSelfOverload(result: ResolvedRef, ref: UnresolvedRef, context: ResolutionContext): ResolvedRef {
if (ref.referenceKind !== 'calls' || !OVERLOADING_LANGUAGES.has(ref.language)) return result;
// C++ overload sets (templates, SFINAE tags, a `data()` on any container) are
// only trusted for a method's call to itself.