diff --git a/CHANGELOG.md b/CHANGELOG.md index 8fe7c213b..9b4450639 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -462,6 +462,10 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - For Hermes Agent, `codegraph install` and `upgrade` no longer add `mcp-codegraph` to `platform_toolsets.cli`. Hermes up to 0.22.2 reported that name as an unknown toolset on every start, and Hermes 0.22.4 reads it as the only MCP server to load, hiding your others from CLI sessions; an entry an earlier install added is taken out again. A refresh also no longer resets your own `mcp_servers.codegraph` settings, such as an absolute command path, timeouts or `enabled: false`, and a config indented or written differently from Hermes's default no longer gets a duplicate or unreadable entry. Thanks @Huy3ko for the report and @conifercone for confirming it. (#2485) - `codegraph upgrade` now upgrades a global npm install when run from your home folder, which holds npm's global folder on Windows and with tools like nvm: before, it ran `npm install` without `-g` there, which left the global copy on the old version and put a second copy in your home folder. A copy installed in a project is now upgraded in that project wherever you run the command, with a warning if npm left it on the old version, and a copy installed with pnpm, yarn or bun gets the command that upgrades it instead of npm. Thanks @timlincool for the report and @maxmilian for the first fix. (#2477) - `codegraph status` no longer says the index is up to date while indexed files are missing their symbols: it now names files the parser couldn't read and files stored without their symbols (which `codegraph sync` repairs), `status --json` counts both, and `codegraph files --json` lists each file's recorded errors. Thanks @lipchey for the report. (#2336) +- A qualified name given to `codegraph callers`, `callees` or `impact`, or to `codegraph_node`, like `Format.Price`, now names only the symbol whose scope is spelled that way, so its answer no longer merges in `LegacyFormat.Price` or any other class whose name only ends with `Format`, nor a class that just sits in a file named `Format.cs`. In C#, `BasketService.AddItemToBasket` no longer also selects the `IBasketService` interface's method; ask for `IBasketService.AddItemToBasket` for that one. A file or folder still names the module in Rust, Python and Go, and a PHP namespace can be written with its own backslashes, like `Billing\Format::price`. No re-index needed. Thanks @Gobakabana for the report and @yang0228. (#2908) +- `codegraph affected --filter` now matches its glob against whole names: `tests/**/*.py` also finds the tests directly inside `tests/`, including a changed test file itself, `tests/*.py` no longer counts a changed `tests/x.py.bak` as a test, and `e2e/*.spec.ts` no longer matches a folder that only ends in `e2e`, like `xe2e/`. A pattern with a folder in it still matches that folder at any depth, so `e2e/*.spec.ts` keeps finding `apps/web/e2e/login.spec.ts`; start it with `/` to match from the project root only. Thanks @Jseca666. +- `codegraph init` and `codegraph index` no longer report a file you deleted as one that "could not be read" while git still lists it, which it does until the deletion is staged: such a file is now skipped, and so is a symlink that points at nothing. A file that is there but can't be read is still reported. Thanks @xianzhi-xzw for the report and the first fix. (#2904) +- A full index of a project that holds a video named `.ts`, like an MPEG-TS clip among test fixtures, no longer warns that the index is PARTIAL and missing a file, and `codegraph status` no longer reports the index as partial: the video is skipped on purpose, and it now counts as skipped. Thanks @Hetox for the original report. (#1910) - In CFML, a `` or `` block the parser can't read no longer marks its whole file as contributing nothing to the graph: `codegraph init` no longer warns about the file, and `codegraph status` no longer lists it among files whose symbols are missing, since its component, functions and calls are all indexed. The file's recorded errors now name the block and its line instead. Thanks @HarryMuc for the report. (#2654) - In Erlang, a comment inside an argument or parameter list, like the one in `add_to_includes( % give access to .hrl in app's src/` before its two arguments, no longer counts as an argument. The call now links to the function of its real arity, and a function whose parameter list holds a comment keeps its own arity. Before, the comment raised the arity by one, so the call linked to nothing. Re-index Erlang projects after upgrading. Thanks @Dshuishui. (#1619) - Indexing large Python projects is much faster again and needs less memory: since 1.6.2, resolving Python references re-read source files over and over, so a project the size of CPython took several times as long to index. The graph it builds is unchanged. Thanks @bompus for the report. (#2332) @@ -512,6 +516,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - 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 Spring projects, a route now links to the method its mapping annotation is written on when that method has no access modifier, like `@GetMapping List
list()` in a package-private controller, and a method-level `@RequestMapping(value = "/feed", method = GET)` on such a method now makes a route. Before, such a handler was skipped, so its route linked the next method declared `public`, `private` or `protected`, or nothing, and the handler looked unused. An overloaded handler under a tall stack of annotations, like `@Operation` and `@ApiResponses` above `@GetMapping`, now links the overload it annotates rather than its neighbour. Re-index Spring projects after upgrading. Thanks @Liurchen. - 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) - Indexing large Python projects is much faster again and needs less memory: since 1.6.2, resolving Python references re-read source files over and over, so a project the size of CPython took several times as long to index. The graph it builds is unchanged. Thanks @bompus for the report. (#2332) diff --git a/README.md b/README.md index af3bc07e1..2012c6148 100644 --- a/README.md +++ b/README.md @@ -52,7 +52,7 @@ Follow [@getcodegraph](https://x.com/getcodegraph) on X for updates. ## About this fork -This is **bompus/codegraph**, a fork of [colbymchenry/codegraph](https://github.com/colbymchenry/codegraph). Its default branch, `fork/consolidated`, contains upstream `main` through [`69af19ac`](https://github.com/colbymchenry/codegraph/commit/69af19ace270f2e940444d514de6dde971df12df) (after v1.6.2) plus the fork's own work, and it takes upstream changes as they land. Changes that suit upstream are also offered there as pull requests. +This is **bompus/codegraph**, a fork of [colbymchenry/codegraph](https://github.com/colbymchenry/codegraph). Its default branch, `fork/consolidated`, contains upstream `main` through [`60b0d7a9`](https://github.com/colbymchenry/codegraph/commit/60b0d7a92dc001de1be37943ef3cd1cc955d693d) (after v1.6.2) plus the fork's own work, and it takes upstream changes as they land. Changes that suit upstream are also offered there as pull requests. The fork publishes no releases. The install scripts, npm package, badges and `codegraph upgrade` further down this page install **upstream's** releases. To run the fork, build it from source (below). @@ -80,7 +80,7 @@ Session search uses `.codegraph/sessions-v2.db`. The first session search import ### What the fork adds -Compared with upstream `main` at `69af19ac` (after v1.6.2). Each item here and in the dispatch and framework lists below was checked against upstream's tree at that commit. +Compared with upstream `main` at `60b0d7a9` (after v1.6.2). Each item here and in the dispatch and framework lists below was checked against upstream's tree at that commit. | Feature | Upstream | Fork | What it does | |---|:-:|:-:|---| @@ -812,14 +812,14 @@ Traces import dependencies transitively to find which test files are affected by ```bash codegraph affected src/utils.ts src/api.ts # Pass files as arguments git diff --name-only | codegraph affected --stdin # Pipe from git diff -codegraph affected src/auth.ts --filter "e2e/*" # Custom test file pattern +codegraph affected src/auth.ts --filter "e2e/*" # Whole-name glob; matches at any folder depth ``` | Option | Description | Default | |--------|-------------|---------| | `--stdin` | Read file list from stdin | `false` | | `-d, --depth ` | Max dependency traversal depth | `5` | -| `-f, --filter ` | Custom glob to identify test files | auto-detect | +| `-f, --filter ` | Whole-name glob for test files: a pattern with no slash matches the file name at any depth; one with `/` matches from any folder boundary; a leading `/` or `./` anchors it at the project root; a trailing `/` takes everything under that folder | auto-detect | | `-j, --json` | Output as JSON | `false` | | `-q, --quiet` | Output file paths only | `false` | diff --git a/__tests__/cli-affected-filter-globs.test.ts b/__tests__/cli-affected-filter-globs.test.ts new file mode 100644 index 000000000..0f4a48f86 --- /dev/null +++ b/__tests__/cli-affected-filter-globs.test.ts @@ -0,0 +1,122 @@ +// `codegraph affected --filter ` matches whole names (#1273). +// +// The filter used to be hand-converted to an unanchored regex (`**` → `.+`, +// `*` → `[^/]*`), so it matched anywhere inside a path: `tests/**/*.py` +// missed `tests/x.py` (`.+` needs a folder), `tests/*.py` took a changed +// `tests/x.py.bak` for a test, and `e2e/*.spec.ts` took an `xe2e/` folder. +// A pattern with a slash still matches from any folder boundary, as the help +// text's own `e2e/*.spec.ts` always did for `apps/web/e2e/…`; a leading `/` +// anchors it at the project root. Exercised end-to-end against the built binary. +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { execFileSync } from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; + +const BIN = path.resolve(__dirname, '../dist/bin/codegraph.js'); + +describe('codegraph affected --filter globs (#1273)', () => { + let dir: string; + + function affected(changed: string, filter: string): string[] { + const out = execFileSync(process.execPath, [BIN, 'affected', changed, '--filter', filter, '--quiet', '-p', dir], { + encoding: 'utf-8', + env: { ...process.env, CODEGRAPH_NO_DAEMON: '1', CODEGRAPH_WASM_RELAUNCHED: '1' }, + stdio: ['ignore', 'pipe', 'pipe'], + }); + return out.split('\n').map((s) => s.trim()).filter(Boolean); + } + + beforeAll(async () => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-affected-globs-')); + const w = (rel: string, body: string) => { + fs.mkdirSync(path.dirname(path.join(dir, rel)), { recursive: true }); + fs.writeFileSync(path.join(dir, rel), body); + }; + const pyTest = 'from math_utils import add_one\n\ndef test_add_one():\n assert add_one(1) == 2\n'; + w('math_utils.py', 'def add_one(value):\n return value + 1\n'); + w('tests/test_math_utils.py', pyTest); + w('tests/unit/test_math_utils.py', pyTest); + w('tests/unit/deep/test_math_utils.py', pyTest); + w('other/tests/test_math_utils.py', pyTest); + w('checks/math_check.py', pyTest); + w('tests/test_math_utils.py.bak', '# a backup, not a test\n'); + w('tests/.hidden.py', '# a dotfile the filter names\n'); + w('src/login.ts', 'export function login(user: string): boolean { return user.length > 0; }\n'); + const spec = (rel: string) => `import { login } from '${rel}';\ntest('logs in', () => login('a'));\n`; + w('e2e/login.spec.ts', spec('../src/login')); + w('apps/web/e2e/login.spec.ts', spec('../../../src/login')); + w('xe2e/login.spec.ts', spec('../src/login')); + const cg = CodeGraph.initSync(dir); + try { + await cg.indexAll(); + } finally { + cg.close(); + } + }, 60_000); + + afterAll(() => { + fs.rmSync(dir, { recursive: true, force: true }); + }); + + it('lets ** match zero folders, so a test directly in tests/ counts', () => { + // main: only the two files below tests/unit/. + expect(affected('math_utils.py', 'tests/**/*.py')).toEqual([ + 'other/tests/test_math_utils.py', + 'tests/test_math_utils.py', + 'tests/unit/deep/test_math_utils.py', + 'tests/unit/test_math_utils.py', + ]); + // main: [] — the changed test itself didn't match its own filter. + expect(affected('tests/test_math_utils.py', 'tests/**/*.py')).toEqual(['tests/test_math_utils.py']); + }); + + it('matches the whole file name, so a .bak file is not a test', () => { + // main: listed the backup as an affected test. + expect(affected('tests/test_math_utils.py.bak', 'tests/*.py')).toEqual([]); + }); + + it('matches a slash pattern at folder boundaries only, at any depth', () => { + // main: also listed xe2e/login.spec.ts. + expect(affected('src/login.ts', 'e2e/*.spec.ts')).toEqual(['apps/web/e2e/login.spec.ts', 'e2e/login.spec.ts']); + // Depth-free by design: tests/*.py keeps matching a nested tests/ folder. + expect(affected('math_utils.py', 'tests/*.py')).toEqual(['other/tests/test_math_utils.py', 'tests/test_math_utils.py']); + }); + + it('anchors a pattern that starts with / or ./ at the project root', () => { + expect(affected('math_utils.py', '/tests/*.py')).toEqual(['tests/test_math_utils.py']); + expect(affected('math_utils.py', './tests/*.py')).toEqual(['tests/test_math_utils.py']); + expect(affected('math_utils.py', '/tests/**/*.py')).toEqual([ + 'tests/test_math_utils.py', + 'tests/unit/deep/test_math_utils.py', + 'tests/unit/test_math_utils.py', + ]); + expect(affected('src/login.ts', '/e2e/*.spec.ts')).toEqual(['e2e/login.spec.ts']); + }); + + it('takes everything under a folder named with a trailing slash', () => { + expect(affected('math_utils.py', 'tests/')).toEqual([ + 'other/tests/test_math_utils.py', + 'tests/test_math_utils.py', + 'tests/unit/deep/test_math_utils.py', + 'tests/unit/test_math_utils.py', + ]); + }); + + it('keeps file-name patterns at any depth, folders outside the test conventions, and dotfiles', () => { + expect(affected('math_utils.py', 'test_*.py')).toEqual([ + 'other/tests/test_math_utils.py', + 'tests/test_math_utils.py', + 'tests/unit/deep/test_math_utils.py', + 'tests/unit/test_math_utils.py', + ]); + expect(affected('src/login.ts', '*.spec.ts')).toEqual([ + 'apps/web/e2e/login.spec.ts', + 'e2e/login.spec.ts', + 'xe2e/login.spec.ts', + ]); + expect(affected('math_utils.py', 'checks/*.py')).toEqual(['checks/math_check.py']); + expect(affected('tests/.hidden.py', 'tests/*.py')).toEqual(['tests/.hidden.py']); + }); +}); diff --git a/__tests__/evaluation/results/precision-petclinic-818c413-98213654.json b/__tests__/evaluation/results/precision-petclinic-818c413-98213654.json new file mode 100644 index 000000000..f0b611843 --- /dev/null +++ b/__tests__/evaluation/results/precision-petclinic-818c413-98213654.json @@ -0,0 +1,44 @@ +{ + "timestamp": "2026-10-30T05:02:30.605Z", + "corpus": "petclinic", + "repo": "https://github.com/spring-projects/spring-petclinic.git", + "commit": "818c413", + "codegraphSha": "98213654", + "summary": { + "absentCases": 0, + "absentHeld": 0, + "presentCases": 1, + "presentHeld": 1, + "unjudgeable": 0 + }, + "resolvedBy": { + "(none)": 986, + "instance-method": 270, + "exact-match": 152, + "import": 44, + "framework": 27, + "function-ref": 2, + "file-path": 1 + }, + "edges": 1482, + "results": [ + { + "caseId": "petclinic-bounded-type-parameter", + "expect": "present", + "pass": true, + "found": [ + { + "from": "method:2160e6191fa145dc29200531c30d1a5d", + "to": "method:efb8586338e2f81ea9d7cb6f694a2f0d", + "resolvedBy": "instance-method" + }, + { + "from": "method:2160e6191fa145dc29200531c30d1a5d", + "to": "method:efb8586338e2f81ea9d7cb6f694a2f0d", + "resolvedBy": "instance-method" + } + ], + "missingEndpoints": [] + } + ] +} \ No newline at end of file diff --git a/__tests__/frameworks.test.ts b/__tests__/frameworks.test.ts index 5d1a4911c..75e37a3e5 100644 --- a/__tests__/frameworks.test.ts +++ b/__tests__/frameworks.test.ts @@ -990,6 +990,93 @@ class OwnerController { expect(nodes[0].name).toBe('GET /owners/{ownerId}'); expect(references[0].referenceName).toBe('showOwner'); }); + + // #851: a package-private handler (no access modifier) used to be skipped, + // so the next method with a modifier took its route. + it('links each package-private handler to its own name, not the next public method (#851)', () => { + const src = `@RestController +@RequestMapping("/api/articles") +class ArticleController { + @GetMapping + List list() { return List.of(); } + + @GetMapping("/{slug}") + String get(@PathVariable String slug) { return slug; } + + @PostMapping + void create(@RequestBody String body) { } + + @DeleteMapping("/{slug}") + public void delete(@PathVariable String slug) { } + + @RequestMapping(value = "/feed", method = RequestMethod.GET) + ResponseEntity feed() { return null; } +}`; + const { nodes, references } = springResolver.extract!('ArticleController.java', src); + expect(nodes.map(n => n.name)).toEqual([ + 'GET /api/articles', 'GET /api/articles/{slug}', 'POST /api/articles', 'DELETE /api/articles/{slug}', 'GET /api/articles/feed', + ]); + expect(references.map(r => [r.fromNodeId, r.referenceName])).toEqual( + nodes.map((n, i) => [n.id, ['list', 'get', 'create', 'delete', 'feed'][i]]), + ); + }); + + it.each([ + ['List generic()', 'generic'], + ['void none(@RequestBody String body)', 'none'], + ['int primitive()', 'primitive'], + ['String[][] array()', 'array'], + ['org.example.dto.Page qualified()', 'qualified'], + ['ResponseEntity>> nested()', 'nested'], + ['static > java.util.Map>[] typeParams()', 'typeParams'], + ['public @ResponseBody int[] annotatedType()', 'annotatedType'], + ['final synchronized Object modifiers()', 'modifiers'], + ['default ResponseEntity fromInterface()', 'fromInterface'], + ['@Operation(summary = "List (all) items", tags = {"a)"})\n @ApiResponses({ @ApiResponse(responseCode = "200"), @ApiResponse(responseCode = "404") })\n List stacked()', 'stacked'], + ])('reads the handler declared under the mapping: %s (#851)', (declaration, name) => { + const src = `@RestController +@RequestMapping("/base") +class ItemController { + @GetMapping("/x") + ${declaration} { return null; } + + public void next() { } +}`; + const { references } = springResolver.extract!('ItemController.java', src); + expect(references.map(r => r.referenceName)).toEqual([name]); + }); + + it.each([ + ['fun plain(): String', 'plain'], + ['internal suspend fun modified(): String', 'modified'], + ['fun generic(): List', 'generic'], + ['@ResponseBody\n override fun stacked(): String', 'stacked'], + ])('reads the Kotlin handler declared under the mapping: %s (#851)', (declaration, name) => { + const src = `@RestController +@RequestMapping("/base") +class ItemController { + @GetMapping("/x") + ${declaration} = "ok" + + fun next(): String = "next" +}`; + const { references } = springResolver.extract!('ItemController.kt', src); + expect(references.map(r => r.referenceName)).toEqual([name]); + }); + + it.each([ + ['private final ItemService service;', 'a field'], + ['ItemController(ItemService service) { }', 'a constructor'], + ])('links no later method when the mapping is on %s (%s, #851)', (declaration) => { + const src = `@RestController +class ItemController { + @GetMapping("/x") + ${declaration} + + public String next() { return "next"; } +}`; + expect(springResolver.extract!('ItemController.java', src).references).toEqual([]); + }); }); describe('springResolver.resolve — DI heuristics are gated to Java/Kotlin non-inheritance refs', () => { diff --git a/__tests__/spring-route-handlers.test.ts b/__tests__/spring-route-handlers.test.ts new file mode 100644 index 000000000..a780d084d --- /dev/null +++ b/__tests__/spring-route-handlers.test.ts @@ -0,0 +1,230 @@ +/** + * A Spring route links the method its mapping annotation is written on (#851). + * + * The handler used to be found by a pattern that wanted `public`, `private` or + * `protected` (or Kotlin's `fun`) before the name, so a package-private + * method was skipped and the next method with a modifier took its route: + * realworld-java21's controllers, which declare no modifiers, had most routes + * pointing at the wrong method or at nothing. A method-level + * `@RequestMapping(value = …, method = GET)` on such a method made no route. + * + * The route is also bound by position: of two overloads, the one the + * annotation sits on, even when a tall annotation stack above it leaves the + * other overload's declaration nearer the annotation. + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; +import { getKernel, resetKernelForTests } from '../src/extraction/kernel'; + +const kernelBuilt = fs.existsSync(path.join(__dirname, '../codegraph-kernel/prebuilds', + `${process.platform}-${process.arch}`, 'codegraph-kernel.node')); + +const POM = 'org.springframework.boot' + + 'spring-boot-starter-web\n'; + +const ARTICLES = 'src/main/java/com/example/ArticleController.java'; +const ARTICLES_SRC = `package com.example; + +import java.util.List; +import org.springframework.http.ResponseEntity; +import org.springframework.web.bind.annotation.*; + +@RestController +@RequestMapping("/api/articles") +class ArticleController { + + @GetMapping + List list() { + return List.of(); + } + + @GetMapping("/{slug}") + String get(@PathVariable String slug) { + return slug; + } + + @PostMapping + void create(@RequestBody String body) { + } + + @DeleteMapping("/{slug}") + public void delete(@PathVariable String slug) { + } + + @RequestMapping(value = "/feed", method = RequestMethod.GET) + ResponseEntity feed() { + return null; + } +} +`; + +const SEARCH = 'src/main/java/com/example/SearchController.java'; +const SEARCH_SRC = `package com.example; + +import org.springframework.web.bind.annotation.*; + +@RestController +public class SearchController { + @Operation(summary = "Find all") + @ApiResponses({ + @ApiResponse(responseCode = "200"), + @ApiResponse(responseCode = "404") + }) + @GetMapping("/find") + public String find() { + return "all"; + } + @GetMapping("/find/{id}") + public String find(@PathVariable int id) { + return "one"; + } +} +`; + +const ANNOTATED = 'src/main/java/com/example/AnnotatedController.java'; +const ANNOTATED_SRC = `package com.example; + +import java.util.List; +import org.springframework.lang.NonNull; +import org.springframework.lang.Nullable; +import org.springframework.web.bind.annotation.*; + +@RestController +@RequestMapping("/annotated") +class AnnotatedController { + @GetMapping("/names") + public String @Nullable [] names() { + return null; + } + + @GetMapping("/qualified") + public java.util.@Nullable @NonNull List qualified() { + return null; + } + + @RequestMapping(value = "/multi", method = RequestMethod.GET) + public java.util.@Nullable @NonNull List multi() { + return null; + } + + @RequestMapping(value = "/feed", method = RequestMethod.GET) + public String @Nullable [] feed() { + return null; + } +} +`; + +const OWNERS = 'src/main/kotlin/com/example/OwnerController.kt'; +const OWNERS_SRC = `package com.example + +import org.springframework.web.bind.annotation.* + +@RestController +@RequestMapping("/owners") +class OwnerController { + @GetMapping("/{ownerId}") + @ResponseBody + fun showOwner(@PathVariable ownerId: Int): String = "owner" + + @GetMapping("/find") + fun find(): List = emptyList() + + @PostMapping + internal fun create(@RequestBody body: String): String { + return body + } +} +`; + +/** The 1-based line of the first line of `src` that contains `needle`. */ +function lineOf(src: string, needle: string): number { + const at = src.split(/\r?\n/).findIndex((l) => l.includes(needle)); + expect(at, needle).toBeGreaterThanOrEqual(0); + return at + 1; +} + +describe.each(['native', 'wasm'].filter((backend) => backend === 'wasm' || kernelBuilt))( + 'Spring routes link the method they annotate (%s, #851)', (backend) => { + let dir: string; + let cg: CodeGraph | undefined; + + beforeEach(() => { + vi.stubEnv('CODEGRAPH_KERNEL', backend === 'wasm' ? '0' : '1'); + vi.stubEnv('CODEGRAPH_KERNEL_LANGS', 'all'); + resetKernelForTests(); + if (backend === 'native') expect(getKernel()).not.toBeNull(); + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-851-')); + }); + + afterEach(() => { + cg?.close(); + cg = undefined; + fs.rmSync(dir, { recursive: true, force: true }); + vi.unstubAllEnvs(); + resetKernelForTests(); + }); + + async function index(files: Record): Promise { + for (const [rel, body] of Object.entries(files)) { + fs.mkdirSync(path.dirname(path.join(dir, rel)), { recursive: true }); + fs.writeFileSync(path.join(dir, rel), body); + } + cg = await CodeGraph.init(dir, { index: true }); + return cg; + } + + /** Each route of `file` → its handler, as `name@startLine` (`-` for none). */ + function handlers(g: CodeGraph, file: string): Record { + const out: Record = {}; + for (const route of g.getNodesInFile(file).filter((n) => n.kind === 'route')) { + const targets = g.getOutgoingEdges(route.id) + .filter((e) => e.kind === 'references') + .map((e) => g.getNode(e.target)!) + .map((t) => `${t.kind}:${t.name}@${t.startLine}`); + out[route.name] = targets.length > 0 ? targets.join(',') : '-'; + } + return out; + } + + it('binds every route of a package-private controller to its own method', async () => { + const g = await index({ 'pom.xml': POM, [ARTICLES]: ARTICLES_SRC }); + expect(handlers(g, ARTICLES)).toEqual({ + 'GET /api/articles': `method:list@${lineOf(ARTICLES_SRC, 'List list()') - 1}`, + 'GET /api/articles/{slug}': `method:get@${lineOf(ARTICLES_SRC, '@GetMapping("/{slug}")')}`, + 'POST /api/articles': `method:create@${lineOf(ARTICLES_SRC, '@PostMapping')}`, + 'DELETE /api/articles/{slug}': `method:delete@${lineOf(ARTICLES_SRC, '@DeleteMapping')}`, + 'GET /api/articles/feed': `method:feed@${lineOf(ARTICLES_SRC, '@RequestMapping(value = "/feed"')}`, + }); + }); + + it('binds each overload to the route written on it, under a tall annotation stack', async () => { + const g = await index({ 'pom.xml': POM, [SEARCH]: SEARCH_SRC }); + expect(handlers(g, SEARCH)).toEqual({ + 'GET /find': `method:find@${lineOf(SEARCH_SRC, '@Operation')}`, + 'GET /find/{id}': `method:find@${lineOf(SEARCH_SRC, '@GetMapping("/find/{id}")')}`, + }); + }); + + it('reads type-use annotations inside qualified and array return types', async () => { + const g = await index({ 'pom.xml': POM, [ANNOTATED]: ANNOTATED_SRC }); + expect(handlers(g, ANNOTATED)).toEqual({ + 'GET /annotated/names': `method:names@${lineOf(ANNOTATED_SRC, '@GetMapping("/names")')}`, + 'GET /annotated/qualified': `method:qualified@${lineOf(ANNOTATED_SRC, '@GetMapping("/qualified")')}`, + 'GET /annotated/multi': `method:multi@${lineOf(ANNOTATED_SRC, '@RequestMapping(value = "/multi"')}`, + 'GET /annotated/feed': `method:feed@${lineOf(ANNOTATED_SRC, '@RequestMapping(value = "/feed"')}`, + }); + }); + + it('binds Kotlin handlers, generic and with modifiers', async () => { + const g = await index({ 'pom.xml': POM, [OWNERS]: OWNERS_SRC }); + expect(handlers(g, OWNERS)).toEqual({ + 'GET /owners/{ownerId}': `method:showOwner@${lineOf(OWNERS_SRC, '@GetMapping("/{ownerId}")')}`, + 'GET /owners/find': `method:find@${lineOf(OWNERS_SRC, '@GetMapping("/find")')}`, + 'POST /owners': `method:create@${lineOf(OWNERS_SRC, '@PostMapping')}`, + }); + }); +}); diff --git a/site/src/content/docs/guides/affected-tests.md b/site/src/content/docs/guides/affected-tests.md index b51910561..0a44ca2b9 100644 --- a/site/src/content/docs/guides/affected-tests.md +++ b/site/src/content/docs/guides/affected-tests.md @@ -8,7 +8,7 @@ description: Run only the tests a change actually touches. ```bash codegraph affected src/utils.ts src/api.ts # pass files as arguments git diff --name-only | codegraph affected --stdin # pipe from git diff -codegraph affected src/auth.ts --filter "e2e/*" # custom test-file pattern +codegraph affected src/auth.ts --filter "e2e/*" # whole-name glob; matches at any depth ``` ## Options @@ -17,7 +17,7 @@ codegraph affected src/auth.ts --filter "e2e/*" # custom test-file pattern |---|---|---| | `--stdin` | Read the file list from stdin | `false` | | `-d, --depth ` | Max dependency traversal depth | `5` | -| `-f, --filter ` | Custom glob to identify test files | auto-detect | +| `-f, --filter ` | Whole-name glob for test files: no slash matches the file name at any depth, a pattern with `/` matches from any folder boundary, a leading `/` or `./` anchors it at the project root, a trailing `/` takes everything under that folder | auto-detect | | `-j, --json` | Output as JSON | `false` | | `-q, --quiet` | Output file paths only | `false` | diff --git a/src/bin/codegraph.ts b/src/bin/codegraph.ts index a90b207f9..5238b9168 100644 --- a/src/bin/codegraph.ts +++ b/src/bin/codegraph.ts @@ -2673,6 +2673,37 @@ program } }); +// The matcher for `affected --filter `, tested against the +// project-relative, forward-slash paths the index stores (#1273). +// +// - No slash (`*_test.go`, `test_*.py`): the file name, at any depth. +// - A slash (`e2e/*.spec.ts`, `tests/**/*.py`): the path from any folder +// boundary, so `e2e/*.spec.ts` also matches `apps/web/e2e/login.spec.ts` +// (an implicit leading `**/`), as the filter always has. +// - A leading `/` or `./`: from the project root only. +// - A trailing `/` (`tests/`): everything under that folder. +// +// The whole name must match: `tests/*.py` no longer takes `tests/x.py.bak`, +// nor `e2e/*.spec.ts` an `xe2e/` folder, and `tests/**/*.py` now takes +// `tests/x.py` (`**` matches zero folders). Dotfiles count. picomatch is +// loaded here, not at startup, since only this command needs it. +async function affectedFilterMatcher(pattern: string): Promise<(filePath: string) => boolean> { + const { default: picomatch } = await import('picomatch'); + // `windows: false`: the stored paths always use `/`, so the glob reads the + // same on every platform (picomatch otherwise follows the host OS). + const options = { dot: true, windows: false }; + let glob = pattern.trim(); + if (glob.endsWith('/')) glob += '**'; + if (!glob.includes('/')) { + const matchName = picomatch(glob, options); + return (filePath) => matchName(path.posix.basename(filePath)); + } + const anchored = /^\.?\//.test(glob); + glob = glob.replace(/^\.?\/+/, ''); + if (!anchored && !glob.startsWith('**/')) glob = `**/${glob}`; + return picomatch(glob, options); +} + /** * codegraph affected [files...] * @@ -2689,7 +2720,7 @@ program .option('-p, --path ', 'Project path') .option('--stdin', 'Read file list from stdin (one per line)') .option('-d, --depth ', 'Max dependency traversal depth', '5') - .option('-f, --filter ', 'Custom glob filter for test files (e.g. "e2e/*.spec.ts")') + .option('-f, --filter ', 'Glob for the files that count as tests, matched against the whole name: "*_test.go" matches the file name at any depth, "e2e/*.spec.ts" that folder at any depth, "/tests/**/*.py" from the project root only') .option('-j, --json', 'Output as JSON') .option('-q, --quiet', 'Only output file paths, no decoration') .action(async (fileArgs: string[], options: { path?: string; stdin?: boolean; depth?: string; filter?: string; json?: boolean; quiet?: boolean }) => { @@ -2723,29 +2754,29 @@ program process.exit(0); } + // Custom filter pattern, compiled before the index is opened so a bad + // pattern fails fast. + let customFilter: ((filePath: string) => boolean) | null = null; + if (options.filter) { + try { + customFilter = await affectedFilterMatcher(options.filter); + } catch (err) { + error(`Invalid --filter pattern "${options.filter}": ${err instanceof Error ? err.message : String(err)}`); + process.exit(1); + } + } + const { default: CodeGraph } = await loadCodeGraph(); const cg = await CodeGraph.open(projectPath); const maxDepth = parseInt(options.depth || '5', 10); - // Custom filter pattern - let customFilter: RegExp | null = null; - if (options.filter) { - // Convert glob to regex: ** → .+, * → [^/]*, . → \. - const regex = options.filter - .replace(/[+[\]{}()^$|\\]/g, '\\$&') - .replace(/\./g, '\\.') - .replace(/\*\*/g, '.+') - .replace(/\*/g, '[^/]*'); - customFilter = new RegExp(regex); - } - // One notion of "a test" for the whole tool (#1507): the CLI used to keep // its own six regexes here, which knew `.test.` and `/tests/` but not Go's // `_test.go`, Python's `test_x.py` or the JVM's `FooTest.kt` — so // `affected` reported "no tests" for whole ecosystems while `search` and // the MCP tools counted those very files as tests. function isTestFile(filePath: string): boolean { - if (customFilter) return customFilter.test(filePath); + if (customFilter) return customFilter(filePath); return isTestPath(filePath); } diff --git a/src/resolution/frameworks/java.ts b/src/resolution/frameworks/java.ts index 50c684919..66f1f03d6 100644 --- a/src/resolution/frameworks/java.ts +++ b/src/resolution/frameworks/java.ts @@ -59,6 +59,13 @@ export const springResolver: FrameworkResolver = { }, resolve(ref: UnresolvedRef, context: ResolutionContext): ResolvedRef | null { + // A route's handler is the method its mapping annotation is written on, + // told apart from an overload by position: under a tall `@Operation` / + // `@ApiResponses` stack, the next overload's declaration can be nearer the + // annotation than the method it annotates (#851). + const handler = routeHandlerAt(ref, context); + if (handler) return { original: ref, targetNodeId: handler.id, confidence: 0.95, resolvedBy: 'framework' }; + // Spring config-key references — `@Value("${key}")` (single leaf) and // `@ConfigurationProperties(prefix="X")` (entire subtree, marked with the // `:prefix` suffix in extractSpringValueBindings). Lookup goes through @@ -237,16 +244,16 @@ export const springResolver: FrameworkResolver = { GetMapping: 'GET', PostMapping: 'POST', PutMapping: 'PUT', PatchMapping: 'PATCH', DeleteMapping: 'DELETE', }; // Verb-specific method mappings — always method-level, BARE or with a path. - const mappingRegex = /@(GetMapping|PostMapping|PutMapping|PatchMapping|DeleteMapping)\b\s*(\([^)]*\))?/g; + const mappingRegex = /@(GetMapping|PostMapping|PutMapping|PatchMapping|DeleteMapping)\b/g; let match: RegExpExecArray | null; while ((match = mappingRegex.exec(safe)) !== null) { + const annotation = readAnnotation(safe, match.index, lang); + if (!annotation) continue; const method = VERB[match[1]!]!; - const paths = parseMappingPaths((match[2] || '').replace(/^\(|\)$/g, ''), consts); + const paths = parseMappingPaths(annotation.args, consts); const line = safe.slice(0, match.index).split('\n').length; - // Method it decorates: first declared method after (skip stacked annotations; - // Java puts the return type before the name). Bounded so we don't grab a far one. - const tail = safe.slice(match.index + match[0].length, match.index + match[0].length + 600); - const methodMatch = tail.match(/\bfun\s+(\w+)\s*\(|\b(?:public|private|protected)\s+[^;{=]*?\s+(\w+)\s*\(/); + // The method it decorates: the declaration it is written on (#851). + const handler = annotatedMethodName(safe, match.index, lang); for (const routePath of classPrefixes.flatMap(prefix => paths.map(sub => joinPath(prefix, sub)))) { const routeNode: Node = { id: `route:${filePath}:${line}:${method}:${routePath}`, @@ -257,16 +264,16 @@ export const springResolver: FrameworkResolver = { startLine: line, endLine: line, startColumn: 0, - endColumn: match[0].length, + endColumn: annotation.end - match.index, language: lang, updatedAt: now, }; nodes.push(routeNode); - if (methodMatch) { + if (handler) { references.push({ fromNodeId: routeNode.id, - referenceName: (methodMatch[1] ?? methodMatch[2])!, + referenceName: handler, referenceKind: 'references', line, column: 0, @@ -280,13 +287,16 @@ export const springResolver: FrameworkResolver = { // Method-level @RequestMapping (older style: `@RequestMapping(value="/x", // method=RequestMethod.GET)` on a method). The class-level @RequestMapping is // the prefix (handled above) — skip it here so it isn't double-counted. - const reqRe = /@RequestMapping\b\s*(\([^)]*\))?/g; + const reqRe = /@RequestMapping\b/g; while ((match = reqRe.exec(safe)) !== null) { - const args = (match[1] || '').replace(/^\(|\)$/g, ''); - const after = safe.slice(match.index + match[0].length, match.index + match[0].length + 600); + const annotation = readAnnotation(safe, match.index, lang); + if (!annotation) continue; + const args = annotation.args; + const after = safe.slice(annotation.end, annotation.end + 600); if (/^\s*(?:@[\w.]+(?:\([^)]*\))?\s*)*(?:public\s+|final\s+|abstract\s+|open\s+|data\s+|sealed\s+)*class\b/.test(after)) continue; // class-level prefix - const methodMatch = after.match(/\bfun\s+(\w+)\s*\(|\b(?:public|private|protected)\s+[^;{=]*?\s+(\w+)\s*\(/); - if (!methodMatch) continue; + // Only on a method: the declaration it is written on (#851). + const handler = annotatedMethodName(safe, match.index, lang); + if (!handler) continue; const verbM = args.match(/method\s*=\s*[{\[]?\s*(?:RequestMethod\.)?(\w+)/); const method = verbM ? verbM[1]!.toUpperCase() : 'ANY'; const paths = parseMappingPaths(args, consts); @@ -297,12 +307,12 @@ export const springResolver: FrameworkResolver = { kind: 'route', name: `${method} ${routePath}`, qualifiedName: `${filePath}::route:${routePath}`, - filePath, startLine: line, endLine: line, startColumn: 0, endColumn: match[0].length, language: lang, updatedAt: now, + filePath, startLine: line, endLine: line, startColumn: 0, endColumn: annotation.end - match.index, language: lang, updatedAt: now, }; nodes.push(routeNode); references.push({ fromNodeId: routeNode.id, - referenceName: (methodMatch[1] ?? methodMatch[2])!, + referenceName: handler, referenceKind: 'references', line, column: 0, filePath, language: lang, }); @@ -554,6 +564,202 @@ function joinPath(prefix: string, sub: string): string { return '/' + parts.join('/'); } +/** Java modifiers a method may carry before its type parameters and return type. */ +const JAVA_METHOD_MODIFIERS = new Set([ + 'public', 'protected', 'private', 'static', 'final', 'abstract', 'synchronized', 'native', 'strictfp', 'default', +]); +/** Kotlin modifiers a function may carry before `fun`. */ +const KOTLIN_FUN_MODIFIERS = new Set([ + 'public', 'protected', 'private', 'internal', 'open', 'override', 'final', 'abstract', + 'suspend', 'inline', 'operator', 'infix', 'tailrec', 'external', 'actual', 'expect', +]); +/** Words that begin a declaration other than a method where a Java return type would stand. */ +const NOT_A_RETURN_TYPE = new Set(['class', 'interface', 'enum', 'record']); + +const IDENTIFIER = /[A-Za-z_$][\w$]*/y; + +function readIdentifier(s: string, i: number): string | null { + IDENTIFIER.lastIndex = i; + return IDENTIFIER.exec(s)?.[0] ?? null; +} + +function skipSpace(s: string, i: number): number { + while (i < s.length && /\s/.test(s[i]!)) i++; + return i; +} + +/** The index past the string or char literal (or `"""` text block) opening at `i`; -1 when it never closes. */ +function skipQuoted(s: string, i: number): number { + const quote = s[i]!; + if (s.startsWith('"""', i)) { + const end = s.indexOf('"""', i + 3); + return end < 0 ? -1 : end + 3; + } + for (let j = i + 1; j < s.length; j++) { + if (s[j] === '\\') j++; + else if (s[j] === quote) return j + 1; + else if (s[j] === '\n') return -1; + } + return -1; +} + +/** The index past the bracketed group opening at `i` — nested groups and literals inside it included; -1 when it never closes. */ +function skipGroup(s: string, i: number): number { + const open = s[i]!; + const close = open === '(' ? ')' : open === '[' ? ']' : open === '{' ? '}' : '>'; + let depth = 0; + for (let j = i; j < s.length; j++) { + const c = s[j]!; + if (c === '"' || (c === "'" && open !== '<')) { + const end = skipQuoted(s, j); + if (end < 0) return -1; + j = end - 1; + } else if (c === open) { + depth++; + } else if (c === close && --depth === 0) { + return j + 1; + } + } + return -1; +} + +/** + * The annotation at `at` — `@Name`, `@a.b.Name`, Kotlin's `@field:Name` or + * `@[A B]` — with its arguments read as a balanced group, so a `)` in a string + * or a nested annotation doesn't end it. `end` is past the arguments, or past + * the whitespace after a bare name. Null when no annotation is there, or its + * arguments never close. + */ +function readAnnotation(s: string, at: number, lang: 'java' | 'kotlin'): { args: string; end: number } | null { + if (s[at] !== '@') return null; + let i = at + 1; + if (lang === 'kotlin' && s[i] === '[') { + const end = skipGroup(s, i); + return end < 0 ? null : { args: '', end }; + } + const readName = (from: number): number => { + let j = from; + for (;;) { + const part = readIdentifier(s, j); + if (!part) return -1; + j += part.length; + if (s[j] !== '.') return j; + j++; + } + }; + i = readName(i); + if (i < 0) return null; + // A Kotlin use-site target: `@field:Name`, `@get:Name`. + if (lang === 'kotlin' && s[i] === ':') { + i = readName(i + 1); + if (i < 0) return null; + } + const next = skipSpace(s, i); + if (s[next] !== '(') return { args: '', end: next }; + const end = skipGroup(s, next); + return end < 0 ? null : { args: s.slice(next + 1, end - 1), end }; +} + +/** + * The name of the method the annotation at `at` is written on (#851): read + * past it and every annotation stacked with it, the modifiers — none at all + * for a package-private Java method — the type parameters and the return type + * (`void`, a primitive, a qualified or nested generic type, an array) to + * `name(`; for Kotlin, past the modifiers to `fun`, its type parameters, and + * `name(`. Null when what the annotation is written on is no method: a field, + * a constructor, a class. + */ +function annotatedMethodName(s: string, at: number, lang: 'java' | 'kotlin'): string | null { + const modifiers = lang === 'kotlin' ? KOTLIN_FUN_MODIFIERS : JAVA_METHOD_MODIFIERS; + // Annotations and modifiers come in any order (`public @ResponseBody String x()`). + const skipAnnotationsAndModifiers = (from: number): number => { + let i = from; + for (;;) { + i = skipSpace(s, i); + if (s[i] === '@') { + const annotation = readAnnotation(s, i, lang); + if (!annotation) return -1; + i = annotation.end; + continue; + } + const word = readIdentifier(s, i); + if (!word || !modifiers.has(word)) return i; + i += word.length; + } + }; + let i = skipAnnotationsAndModifiers(at); + if (i < 0) return null; + if (lang === 'kotlin') { + if (readIdentifier(s, i) !== 'fun') return null; + i = skipSpace(s, i + 3); + if (s[i] === '<') { + i = skipGroup(s, i); + if (i < 0) return null; + i = skipSpace(s, i); + } + } else { + if (s[i] === '<') { + i = skipGroup(s, i); + if (i < 0) return null; + } + // Annotations on the return type: ` @Nullable T x()`. + i = skipAnnotationsAndModifiers(i); + if (i < 0) return null; + const type = readIdentifier(s, i); + if (!type || NOT_A_RETURN_TYPE.has(type)) return null; + i += type.length; + // Its qualifiers, type arguments, array dimensions and type-use + // annotations (`String @Nullable []`, `java.util.@Nullable List`). + for (;;) { + const j = skipSpace(s, i); + if (s[j] === '@') { + const annotation = readAnnotation(s, j, lang); + if (!annotation) return null; + i = annotation.end; + } else if (s[j] === '<') { + i = skipGroup(s, j); + if (i < 0) return null; + } else if (s[j] === '.') { + let k = skipSpace(s, j + 1); + while (s[k] === '@') { + const annotation = readAnnotation(s, k, lang); + if (!annotation) return null; + k = skipSpace(s, annotation.end); + } + const part = readIdentifier(s, k); + if (!part) return null; + i = k + part.length; + } else if (s[j] === '[' && s[skipSpace(s, j + 1)] === ']') { + i = skipSpace(s, j + 1) + 1; + } else { + break; + } + } + i = skipSpace(s, i); + } + const name = readIdentifier(s, i); + if (!name) return null; + return s[skipSpace(s, i + name.length)] === '(' ? name : null; +} + +/** + * The method a Spring route's handler reference names, found where the + * route's mapping annotation is written: the innermost method of that name + * whose span — which starts at its first annotation — holds the annotation's + * line. Null for any other reference, and when no such method is indexed. + */ +function routeHandlerAt(ref: UnresolvedRef, context: ResolutionContext): Node | null { + if (ref.referenceKind !== 'references' || (ref.language !== 'java' && ref.language !== 'kotlin')) return null; + if (!ref.fromNodeId.startsWith(`route:${ref.filePath}:${ref.line}:`)) return null; + let handler: Node | null = null; + for (const node of context.getNodesInFile(ref.filePath)) { + if ((node.kind !== 'method' && node.kind !== 'function') || node.name !== ref.referenceName) continue; + if (node.startLine > ref.line || node.endLine < ref.line) continue; + if (!handler || node.startLine > handler.startLine) handler = node; + } + return handler; +} + /** A framework name heuristic's pick (see name-heuristic.ts), preferring these folders. */ function resolveByNameAndKind( ref: UnresolvedRef,