diff --git a/CHANGELOG.md b/CHANGELOG.md index ce0ab3a8..ff54e641 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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. diff --git a/__tests__/frameworks-integration.test.ts b/__tests__/frameworks-integration.test.ts index 1bd0bf0d..41d658b5 100644 --- a/__tests__/frameworks-integration.test.ts +++ b/__tests__/frameworks-integration.test.ts @@ -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(); + } + }); +}); diff --git a/src/resolution/frameworks/play.ts b/src/resolution/frameworks/play.ts index a10a88c9..1f40609e 100644 --- a/src/resolution/frameworks/play.ts +++ b/src/resolution/frameworks/play.ts @@ -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(); + 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; }, diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index a49f4ef9..6b582f6d 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -1937,7 +1937,7 @@ const JVM_PACKAGES = new WeakMap>(); * `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 } | 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.