diff --git a/CHANGELOG.md b/CHANGELOG.md index 3479d2689..21616899f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - Laravel routes are now named by the path a request takes: a leading `/` is added where the routes file leaves it out, `Route::prefix()` and `Route::group(['prefix' => …])` groups are applied, and routes in `routes/api.php` carry the `/api` prefix Laravel serves them under, read from your `RouteServiceProvider`, `bootstrap/app.php` or any file that mounts a routes file. A front-end `fetch('/api/…')` now connects to the Laravel route that serves it. Re-index Laravel projects after upgrading. - Controllers that Spring or Laravel tests exercise by URL now count as tested. That covers MockMvc's `perform(post("/owners/new"))`, WebTestClient, TestRestTemplate, RestAssured, Laravel's `$this->postJson('api/me')`, Pest's `get('/about')`, and a project's own request helpers built on them. `codegraph_explore` now names the test suite that reaches such an endpoint, where before every one of them looked untested. Re-index Spring and Laravel projects after upgrading. +- A PHP call written without a receiver, such as `redirect($url)`, `view('books.show')`, `auth()` or `basename($path)`, is a function call, and no longer links to a same-named method, field or class elsewhere in the project. These wrong links showed up in callers, impact and `codegraph_explore` answers wherever a Laravel helper or PHP built-in shared its name with a project member. Re-index PHP projects after upgrading. ## [1.6.1] - 2026-09-29 diff --git a/__tests__/steps-server-rendered-replies.test.ts b/__tests__/steps-server-rendered-replies.test.ts new file mode 100644 index 000000000..f985e6d15 --- /dev/null +++ b/__tests__/steps-server-rendered-replies.test.ts @@ -0,0 +1,233 @@ +/** + * How a server-rendered endpoint answers, in the Steps picture. + * + * A Spring MVC handler answers by what it RETURNS — a view name, a constant + * holding one, `"redirect:/owners/" + id` — which is no call at all, and a + * Laravel controller by `view(…)` / `redirect(…)`, bare function calls that + * name-matching used to bind to a same-named method or field somewhere in the + * project. Either way the page it renders and the redirect it sends were + * missing from the picture; only a 404 (a thrown exception) showed. + */ +import { describe, it, expect, beforeAll } from 'vitest'; +import { initGrammars } from '../src/extraction/grammars'; +import { returnsInSource } from '../src/graph/branch-guards'; + +beforeAll(async () => { + await initGrammars(); +}); + +const lineOf = (src: string, needle: string): number => src.split('\n').findIndex((l) => l.includes(needle)) + 1; + +describe('returnsInSource', () => { + const SRC = `@Controller +class OwnerController { + private static final String VIEWS_FORM = "owners/createOrUpdateOwnerForm"; + @PostMapping("/owners/new") + public String processCreationForm(@Valid Owner owner, BindingResult result) { + if (result.hasErrors()) { + return VIEWS_FORM; + } + list.forEach(x -> { return; }); + Runnable r = () -> { return "not-mine"; }; + Object o = new Object() { public String toString() { return "nested"; } }; + this.owners.save(owner); + return "redirect:/owners/" + owner.getId(); + } +} +`; + + it("lists a method's own returns, a constant's value included, and none from lambdas or nested classes", async () => { + const returns = await returnsInSource(SRC, 'java', lineOf(SRC, '@PostMapping')); + expect(returns.map((r) => [r.line, r.expression, r.value])).toEqual([ + [lineOf(SRC, 'return VIEWS_FORM'), 'VIEWS_FORM', 'owners/createOrUpdateOwnerForm'], + [lineOf(SRC, 'return "redirect'), '"redirect:/owners/" + owner.getId()', undefined], + ]); + }); +}); + +// --------------------------------------------------------------------------- +// End to end: the replies an endpoint's Steps picture draws. +// --------------------------------------------------------------------------- + +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { afterAll } from 'vitest'; +import { CodeGraph } from '../src'; +import { loadAllGrammars } from '../src/extraction/grammars'; +import { buildSteps } from '../src/ui-server/api/steps'; +import { classifyEffect, responseStatus, implicitResponseStatus } from '../src/ui-server/api/effects'; + +const projects: string[] = []; +afterAll(() => { + for (const p of projects.splice(0)) fs.rmSync(p, { recursive: true, force: true }); +}); + +async function project(files: Record): Promise<{ cg: CodeGraph; root: string }> { + await loadAllGrammars(); + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-steps-replies-')); + projects.push(root); + 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); + } + return { cg: await CodeGraph.init(root, { index: true }), root }; +} + +/** Each reply the route's picture draws, as `status · text · when`. */ +async function replies(cg: CodeGraph, root: string, routeName: string): Promise { + const route = cg.getNodesByKind('route').find((n) => n.name === routeName); + if (!route) throw new Error(`no route ${routeName}: ${cg.getNodesByKind('route').map((n) => n.name).join(', ')}`); + const payload = await buildSteps(cg, root, new URLSearchParams({ anchor: route.id })); + const replyIds = new Set(payload.steps.filter((s) => s.effect?.category === 'response').map((s) => s.id)); + return payload.links + .filter((l) => replyIds.has(l.to)) + .flatMap((l) => l.sites.map((site) => `${site.status ?? '—'} · ${site.text}${site.when ? ` · WHEN ${site.when}` : ''}`)) + .sort(); +} + +const SPRING_POM = 'spring-boot-starter-web'; + +describe('Spring MVC: a handler answers by what it returns', () => { + it('draws the view it renders and the redirect it sends, each under its condition', async () => { + const { cg, root } = await project({ + 'pom.xml': SPRING_POM, + 'src/main/java/app/owner/OwnerController.java': `package app.owner; +@Controller +class OwnerController { + private static final String VIEWS_FORM = "owners/createOrUpdateOwnerForm"; + @PostMapping("/owners/new") + public String processCreationForm(Owner owner, BindingResult result) { + if (result.hasErrors()) { + return VIEWS_FORM; + } + return "redirect:/owners/" + owner.getId(); + } + @GetMapping("/owners/{ownerId}") + public ModelAndView showOwner(@PathVariable int ownerId) { + ModelAndView mav = new ModelAndView("owners/ownerDetails"); + return mav; + } + @GetMapping("/owners/legacy") + public RedirectView legacy() { + return new RedirectView("/owners"); + } +} +`, + 'src/main/java/app/owner/StatusController.java': `package app.owner; +@RestController +class StatusController { + @GetMapping("/status") + public String status() { return "ok"; } +} +`, + }); + try { + expect(await replies(cg, root, 'POST /owners/new')).toEqual([ + '200 · view owners/createOrUpdateOwnerForm · WHEN result.hasErrors()', + '302 · redirect:/owners/${…} · WHEN !result.hasErrors()', + ]); + expect((await replies(cg, root, 'GET /owners/{ownerId}')).map((r) => r.split(' · ')[0])).toEqual(['200']); + expect((await replies(cg, root, 'GET /owners/legacy')).map((r) => r.split(' · ')[0])).toEqual(['302']); + // A @RestController's String is its body, not a view name: nothing is read into it. + expect(await replies(cg, root, 'GET /status')).toEqual([]); + } finally { + cg.close(); + } + }); +}); + +describe('Laravel: view() and redirect() are replies, not calls into the project', () => { + it("draws the page a controller renders and the redirect it sends — and binds neither to a same-named member", async () => { + const { cg, root } = await project({ + artisan: '#!/usr/bin/env php\n $book]); + } + public function store() { + return redirect()->back()->withErrors(['name' => 'required']); + } +} +`, + // Same-named members elsewhere: name matching used to bind the helpers to these. + 'app/Http/Controllers/DocsController.php': ' r.split(' · ')[0])).toEqual(['302']); + const show = cg.getNodesByKind('method').find((n) => n.name === 'show')!; + const bound = cg.getCallees(show.id).map((c) => `${c.node.kind} ${c.node.name}`); + expect(bound).not.toContain('method redirect'); + expect(bound).not.toContain('field view'); + expect(bound).not.toContain('class View'); + } finally { + cg.close(); + } + }); + + it('still binds $this->method() and a project helper function', async () => { + const { cg } = await project({ + artisan: '#!/usr/bin/env php\nsetTitle(format_title('x')); + } + protected function setTitle($t) {} +} +`, + }); + try { + const show = cg.getNodesByKind('method').find((n) => n.name === 'show')!; + expect(cg.getCallees(show.id).map((c) => `${c.node.kind} ${c.node.name}`).sort()).toEqual(['function format_title', 'method setTitle']); + } finally { + cg.close(); + } + }); +}); + +describe('reply classification', () => { + const php = (text: string) => classifyEffect({ text, kind: 'calls', language: 'php', project: 'api' })?.category ?? null; + const status = (text: string, args: string | null = null) => responseStatus(text, args) ?? implicitResponseStatus(text); + + it("reads Laravel's reply chains with their argument lists stripped, and their statuses", () => { + for (const t of ['view', 'redirect', 'redirect()->route', 'redirect()->back()->withErrors', 'back', 'to_route', 'response()->json', 'response()->noContent']) { + expect([t, php(t)]).toEqual([t, 'response']); + } + expect(['redirect()->route', 'redirect()->back()->withErrors', 'back()->with', 'to_route'].map((t) => status(t))).toEqual([302, 302, 302, 302]); + expect(status('view')).toBe(200); + expect(status('response()->json')).toBe(200); + expect(php('$this->view')).toBeNull(); + }); + + it("keeps PHP's programming-error exceptions out of the replies, namespaced or not, and namespaced HTTP ones in", () => { + const thrown = (text: string) => classifyEffect({ text, kind: 'instantiates', language: 'php', project: 'api' })?.category ?? null; + expect(thrown('InvalidArgumentException')).toBeNull(); + expect(thrown('\\InvalidArgumentException')).toBeNull(); + expect(thrown('\\LogicException')).toBeNull(); + expect(thrown('NotFoundException')).toBe('response'); + expect(thrown('\\Symfony\\Component\\HttpKernel\\Exception\\NotFoundHttpException')).toBe('response'); + }); + + it("reads Spring's ModelAndView and RedirectView", () => { + const jvm = (text: string) => classifyEffect({ text, kind: 'instantiates', language: 'java', project: 'api' })?.category ?? null; + expect(jvm('ModelAndView')).toBe('response'); + expect(jvm('RedirectView')).toBe('response'); + expect(status('ModelAndView', '"owners/ownerDetails"')).toBe(200); + expect(status('ModelAndView', '"redirect:/owners"')).toBe(302); + expect(status('RedirectView', '"/owners"')).toBe(302); + }); +}); diff --git a/docs/viewer-launch-changelog.md b/docs/viewer-launch-changelog.md index 910fb905e..4f9fab066 100644 --- a/docs/viewer-launch-changelog.md +++ b/docs/viewer-launch-changelog.md @@ -137,6 +137,8 @@ These describe `codegraph ui` and its screens. They were taken out of `## [Unrel ## Fixes — Steps — what a call does, and when +- **A server-rendered endpoint's Steps picture shows how it answers.** A Spring MVC handler answers by what it returns: a view name, a constant holding one, `"redirect:/owners/" + id`, a `ModelAndView` or a `RedirectView`. Each is now a reply, a render (`200`) or a redirect (`302`), under the condition it's returned in. petclinic's `POST /owners/new` draws the form re-rendered `WHEN result.hasErrors()` and the redirect otherwise. A Laravel controller's `view(…)`, `redirect(…)`, `redirect()->back()`, `back()`, `to_route(…)` and `response()->json(…)` are replies too, so BookStack's book page draws the page it renders beside its `404` and its old-slug redirect. PHP's own programming-error exceptions (`InvalidArgumentException`, `LogicException`, …) thrown deep in a helper are no longer drawn as one of the endpoint's answers. + - **A handler called from under a binding says what it passes.** A press that runs `tryCatchSync(onClosePress)` drew a box for the wrapper and stopped — leaving the one thing a reader asks ("what is being wrapped?") unsaid, even though every other call-shaped site already prints its arguments. The panel and tooltip now say `tryCatchSync(onClosePress)` — the argument is the answer. - **A step the walk stopped at keeps its whole name.** A boundary — another screen, or a cap the walk hit — ends its name with an ellipsis by design, but the box was not sized for it, so a longer name lost its last letters instead (`/scan-to-verif…` for `/scan-to-verify …`). The anchor's start mark clipped a long path the same way (`/sheets/forgot-passw…`). The box now makes room for both. diff --git a/src/graph/branch-guards.ts b/src/graph/branch-guards.ts index 2075697ae..76097b634 100644 --- a/src/graph/branch-guards.ts +++ b/src/graph/branch-guards.ts @@ -2069,6 +2069,102 @@ const RULES_BY_LANGUAGE: ReadonlyMap = new Map * capped. The index keeps no decorators, so they are read here at request * time like the guards. */ +/** + * A `return` a definition makes itself — never one inside a lambda, a local + * function or a nested / anonymous class — with what it returns, as written. + * `value` is the string a same-class constant holds, when the expression is + * a bare name that declares one: Spring MVC handlers return + * `VIEWS_OWNER_CREATE_OR_UPDATE_FORM`, and the view is its value. + */ +export interface ReturnSite { + line: number; + column: number; + expression: string; + value?: string; +} + +/** Scopes whose `return`s belong to something else. */ +const NESTED_SCOPE_TYPES: ReadonlySet = new Set([ + 'lambda_expression', + 'class_body', + 'lambda_literal', + 'anonymous_function', + 'object_literal', + 'arrow_function', + 'function_expression', + 'function_declaration', + 'function_definition', + 'method_declaration', + 'method_definition', + 'lambda', +]); + +export async function returnsForFile(absPath: string, language: Language, line: number): Promise { + if (!supportsBranchGuards(language)) return []; + const cached = await treeFor(absPath, language); + if (!cached) return []; + return returnsInTree(cached.tree.rootNode, line); +} + +/** {@link returnsForFile} over source text — the test surface. */ +export async function returnsInSource(source: string, language: Language, line: number): Promise { + if (!supportsBranchGuards(language)) return []; + const tree = await parse(source, language); + if (!tree) return []; + try { + return returnsInTree(tree.rootNode, line); + } finally { + tree.delete(); + } +} + +export function returnsInTree(root: SyntaxNode, line: number): ReturnSite[] { + const row = line - 1; + let node: SyntaxNode | null = innermostAt(root, row, firstNonBlankColumn(root.text, row)); + let definition: SyntaxNode | null = null; + for (let up = 0; node && up < 12; up++, node = node.parent) { + if (DEFINITION_TYPES.has(node.type)) { + definition = node; + break; + } + } + if (!definition) return []; + const out: ReturnSite[] = []; + const visit = (n: SyntaxNode): void => { + for (const c of namedChildren(n)) { + if (NESTED_SCOPE_TYPES.has(c.type)) continue; + const isReturn = c.type === 'return_statement' || (c.type === 'jump_expression' && /^return\b/.test(c.text)); + if (isReturn) { + const expr = namedChildren(c)[0]; + if (expr) out.push({ line: c.startPosition.row + 1, column: c.startPosition.column, expression: collapse(expr.text) }); + continue; + } + visit(c); + } + }; + visit(definition); + // A bare name: the string a same-class constant holds (`static final String X = "…"`). + let cls: SyntaxNode | null = definition.parent; + for (let up = 0; cls && up < 6 && !CLASS_TYPES.has(cls.type); up++) cls = cls.parent; + if (cls) { + const strings = new Map(); + const collect = (n: SyntaxNode): void => { + for (const c of namedChildren(n)) { + if (c.type === 'variable_declarator') { + const name = c.childForFieldName('name')?.text; + const value = c.childForFieldName('value'); + if (name && value?.type === 'string_literal') strings.set(name, value.text.replace(/^"|"$/g, '')); + } else if (c.type === 'field_declaration' || c.type === 'class_body') collect(c); + } + }; + collect(cls); + for (const r of out) { + if (/^[A-Za-z_$][\w$]*$/.test(r.expression) && strings.has(r.expression)) r.value = strings.get(r.expression)!; + } + } + return out; +} + export interface DefinitionDecorators { own: string[]; /** The enclosing class's, when the definition is a member. */ diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 1f1c29da1..d2675bddd 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -933,6 +933,35 @@ function isBareGoCall(ref: UnresolvedRef, context: ResolutionContext): boolean { return ref.language === 'go' && isReceiverLessCall(ref, context); } +/** + * Whether a PHP `calls` ref is a bare function call — `redirect($url)`, + * `view('books.show')` — rather than `$this->redirect()` / `$obj->view()` / + * `Foo::view()`. PHP has no implicit `$this`: a call written without a + * receiver can only be a function, so a method, field or property that shares + * the name is never what it calls. Name-matching used to bind BookStack's + * every `return redirect(…)` to ApiDocsController::redirect and every + * `return view(…)` to a `$view` field. + * + * PHP refs record the column of the call EXPRESSION — `$this->setPageTitle(` + * sits at `$this` — so a bare call is the one whose text at its column is the + * name itself (a leading `\` for a fully qualified function is allowed). + */ +function isBarePhpCall(ref: UnresolvedRef, context: ResolutionContext): boolean { + if (ref.language !== 'php' || ref.referenceKind !== 'calls') return false; + if (!/^\w+$/.test(ref.referenceName)) return false; + const line = context.getFileLines?.(ref.filePath)?.[ref.line - 1] + ?? context.readFile(ref.filePath)?.split('\n')[ref.line - 1]; + if (line === undefined) return false; + const at = line[ref.column] === '\\' ? ref.column + 1 : ref.column; + if (!line.startsWith(ref.referenceName, at)) return false; + CALL_OPENER.lastIndex = at + ref.referenceName.length; + if (!CALL_OPENER.test(line)) return false; + let end = at; + while (end > 0 && WHITESPACE.test(line[end - 1]!)) end--; + // `$obj->name(` / `Foo::name(` / `$obj?->name(`, should a column ever land on the name. + return !(end > 0 && (line[end - 1] === '>' || line[end - 1] === ':')); +} + function isReceiverLessCall(ref: UnresolvedRef, context: ResolutionContext): boolean { if (ref.referenceKind !== 'calls') return false; if (ref.referenceName.includes('.')) return false; @@ -1147,6 +1176,7 @@ export function matchByExactName( // large import-heavy (front-end + back-end) repos (#915). const bareJs = isBareJsCall(ref, context); const bareGo = isBareGoCall(ref, context); + const barePhp = isBarePhpCall(ref, context); if (bareJs) { const storeAction = matchJsStoreBindingCall(ref, context); if (storeAction) return storeAction; @@ -1174,6 +1204,8 @@ export function matchByExactName( n.kind !== 'import' && // A receiver-less JS/TS or Go call cannot reach a method (#1714, #1857). !((bareJs || bareGo) && n.kind === 'method') && + // A bare PHP call is a function call: nothing else is callable without a receiver. + !(barePhp && n.kind !== 'function') && // An `extends`/`implements` ref names a supertype, so anything that can't // BE one is not a candidate at all. This is eligibility, not // ranking: kind is only a scoring bonus below (and none is awarded for @@ -4162,6 +4194,9 @@ export function matchFuzzy( (finalCandidates[0]!.kind === 'method' || (finalCandidates[0]!.filePath !== ref.filePath && isLocallyBoundJsName(ref.referenceName, ref.filePath, context)))) && !(finalCandidates[0]!.kind === 'method' && isBareGoCall(ref, context)) && + // A bare PHP call is a function call (case-insensitive, so fuzzy may find + // one) — never the class `View` for `view(…)`, never a method. + !(finalCandidates[0]!.kind !== 'function' && isBarePhpCall(ref, context)) && isLexicallyReachable(finalCandidates[0]!, ref, context) ) { const isCrossLanguage = finalCandidates[0]!.language !== ref.language; diff --git a/src/ui-server/api/effects.ts b/src/ui-server/api/effects.ts index ac9aaa607..4aaa46ca6 100644 --- a/src/ui-server/api/effects.ts +++ b/src/ui-server/api/effects.ts @@ -92,6 +92,8 @@ export const EFFECT_RULES: ReadonlyArray = [ only: ['py'], }, { category: 'response', test: /^ResponseEntity(?:\.\w+)*$|^ResponseStatusException$|^(?:ServerResponse|Mono\.just\(ResponseEntity)/, only: ['jvm'] }, + // Spring MVC's other two answers: a view to render, and a redirect. + { category: 'response', test: /^(?:ModelAndView|RedirectView)$/, only: ['jvm'], instantiates: true }, { category: 'response', test: /^(?:Ok|NotFound|BadRequest|Created|CreatedAtAction|CreatedAtRoute|NoContent|Unauthorized|Forbid|Accepted|AcceptedAtAction|Problem|StatusCode|Conflict|UnprocessableEntity|Redirect|RedirectPermanent|RedirectToAction|RedirectToPage|RedirectToRoute|LocalRedirect|View|PartialView|Json|File|PhysicalFile|Content|Challenge|SignIn|SignOut|ValidationProblem|Page)$|^(?:Results|TypedResults)\.\w+$|^(?:Response|HttpContext\.Response)\.(?:WriteAsync|WriteAsJsonAsync|Redirect|StatusCode)$/, @@ -104,7 +106,9 @@ export const EFFECT_RULES: ReadonlyArray = [ }, { category: 'response', test: /^(?:Abort|Response|HTTPStatus\.\w+|req\.redirect|request\.redirect)$/, only: ['swift'] }, { category: 'response', test: /^(?:render|redirect_to|redirect_back|head|respond_to|respond_with|send_data|send_file|render_to_string)$/, only: ['rb'] }, - { category: 'response', test: /^(?:response|abort|abort_if|abort_unless|redirect|view|back|json)(?:\(\)->\w+)?$/, only: ['php'] }, + // Chains reach the rules without their argument lists: `response()->json(…)` + // is `response->json`, `redirect()->back()->withErrors(…)` is `redirect->back->withErrors`. + { category: 'response', test: /^(?:response|abort|abort_if|abort_unless|redirect|view|back|json|to_route)(?:->\w+)*$/, only: ['php'] }, { category: 'response', test: /^(?:HttpResponse|Json|StatusCode|Redirect|NamedFile|HttpResponseBuilder)(?:::\w+)*$/, only: ['rs'] }, // ---------------------------------------------------------------- database -- @@ -213,8 +217,8 @@ export const EFFECT_RULES: ReadonlyArray = [ * response. Only in a project with endpoints: in an app, `new * ValidationError` is an error, not a reply. */ -const EXCEPTION_RESPONSE = /(?:^|[.:])(?:\w+Exception|\w*HttpError|ApiError|\w+ApiError|HttpProblem|ProblemDetails|Abort|ResponseStatusException|ErrorResponse|\w+ErrorResponse|HTTPError|HTTPException|APIException|Http\d{3}|\w*(?:NotFound|BadRequest|Unauthorized|Unauthenticated|Forbidden|Conflict|Validation|Unprocessable|TooManyRequests|Gone|NotAllowed|MethodNotAllowed|Unsupported|RequestTimeout|InternalServer|ServiceUnavailable|PaymentRequired|PreconditionFailed|NotAcceptable|NotImplemented|BadGateway|RateLimit)Error)$/; -const NOT_A_RESPONSE = /^(?:Error|TypeError|RangeError|SyntaxError|ReferenceError|EvalError|URIError|AggregateError|Exception|RuntimeException|IllegalArgumentException|IllegalStateException|NullPointerException|IndexOutOfBoundsException|UnsupportedOperationException|ArgumentException|ArgumentNullException|ArgumentOutOfRangeException|InvalidOperationException|NotImplementedException|NotSupportedException|ValueError|TypeError|KeyError|IndexError|RuntimeError|NotImplementedError|AssertionError|StopIteration|InterruptedException|IOException|FileNotFoundException|ClassNotFoundException|NoSuchElementException|NumberFormatException|CloneNotSupportedException|ExecutionException|TimeoutException|OperationCanceledException|TaskCanceledException|ObjectDisposedException|FormatException|OverflowException|DivideByZeroException|JsonException|SerializationException|ParseException|DateTimeParseException|MalformedURLException|URISyntaxException|SQLException|DataAccessException|DbUpdateException|ConcurrencyException|EntityNotFoundException|NoResultException|OptimisticLockException)$/; +const EXCEPTION_RESPONSE = /(?:^|[.:\\])(?:\w+Exception|\w*HttpError|ApiError|\w+ApiError|HttpProblem|ProblemDetails|Abort|ResponseStatusException|ErrorResponse|\w+ErrorResponse|HTTPError|HTTPException|APIException|Http\d{3}|\w*(?:NotFound|BadRequest|Unauthorized|Unauthenticated|Forbidden|Conflict|Validation|Unprocessable|TooManyRequests|Gone|NotAllowed|MethodNotAllowed|Unsupported|RequestTimeout|InternalServer|ServiceUnavailable|PaymentRequired|PreconditionFailed|NotAcceptable|NotImplemented|BadGateway|RateLimit)Error)$/; +const NOT_A_RESPONSE = /^(?:Error|TypeError|RangeError|SyntaxError|ReferenceError|EvalError|URIError|AggregateError|Exception|RuntimeException|IllegalArgumentException|IllegalStateException|NullPointerException|IndexOutOfBoundsException|UnsupportedOperationException|ArgumentException|ArgumentNullException|ArgumentOutOfRangeException|InvalidOperationException|NotImplementedException|NotSupportedException|ValueError|TypeError|KeyError|IndexError|RuntimeError|NotImplementedError|AssertionError|StopIteration|InterruptedException|IOException|FileNotFoundException|ClassNotFoundException|NoSuchElementException|NumberFormatException|CloneNotSupportedException|ExecutionException|TimeoutException|OperationCanceledException|TaskCanceledException|ObjectDisposedException|FormatException|OverflowException|DivideByZeroException|JsonException|SerializationException|ParseException|DateTimeParseException|MalformedURLException|URISyntaxException|SQLException|DataAccessException|DbUpdateException|ConcurrencyException|EntityNotFoundException|NoResultException|OptimisticLockException|InvalidArgumentException|LogicException|DomainException|LengthException|OutOfRangeException|OutOfBoundsException|UnexpectedValueException|BadMethodCallException|BadFunctionCallException|RangeException|UnderflowException|ErrorException|JsonException|TypeError)$/; /** Receiver types that say what a call into them is, when the graph declared one. */ const RECEIVER_TYPE_RULES: ReadonlyArray<{ category: EffectCategory; test: RegExp }> = [ @@ -302,7 +306,8 @@ export function classifyEffect(input: EffectInput): Effect | null { // A thrown web exception is a response, in a project that has endpoints. if (project !== 'app' && (input.kind === 'instantiates' || families.includes('py') || families.includes('swift'))) { - const last = text.split(/[.:]/).pop() ?? text; + // `\\InvalidArgumentException`, `Foo\\BarException`: the class, not its namespace. + const last = (text.split(/[.:]/).pop() ?? text).replace(/^.*\\/, ''); if (EXCEPTION_RESPONSE.test(text) && !NOT_A_RESPONSE.test(last)) return { category: 'response' }; } return null; @@ -445,7 +450,11 @@ export function responseStatus(text: string, args: string | null | undefined, _k if (s !== null) return s; break; } - if (/^(?:redirect|redirect_to|RedirectResponse|HttpResponseRedirect|RedirectToAction|RedirectToPage|RedirectToRoute|LocalRedirect|permanentRedirect|Redirect)$/.test(last)) return /permanent/i.test(last) ? 308 : 302; + if (/^(?:redirect|redirect_to|RedirectResponse|HttpResponseRedirect|RedirectToAction|RedirectToPage|RedirectToRoute|LocalRedirect|permanentRedirect|Redirect|RedirectView)$/.test(last)) return /permanent/i.test(last) ? 308 : 302; + // `new ModelAndView("redirect:/owners")` redirects; any other view renders (implicitResponseStatus). + if (last === 'ModelAndView' && /^\s*"redirect:/.test(a)) return 302; + // Laravel: `redirect()->route('home')`, `redirect()->back()->withErrors(…)`, `back()`, `to_route('home')`. + if (/^(?:redirect->\w+|back|to_route)(?:->\w+)*$/.test(call.replace(/\([^()]*\)/g, ''))) return 302; if (/^(?:Created|CreatedAtAction|CreatedAtRoute|created)$/.test(last)) return 201; return null; } @@ -468,6 +477,7 @@ export function implicitResponseStatus(text: string): number | null { if (/^(?:NextResponse|Response)\.json$/.test(bare)) return 200; if (/^(?:JSONResponse|HTMLResponse|PlainTextResponse|ORJSONResponse|UJSONResponse|jsonify|render_template|render|make_response|HttpResponse|JsonResponse|send_file|send_from_directory)$/.test(bare)) return 200; if (/^(?:render|render_to_string|respond_with)$/.test(bare)) return 200; + if (/^ModelAndView$/.test(bare)) return 200; if (/^response\(\)->(?:json|view)$|^response->json$|^view$/.test(call.replace(/\s+/g, ''))) return 200; return null; } diff --git a/src/ui-server/api/steps.ts b/src/ui-server/api/steps.ts index d512d5494..f0aeff486 100644 --- a/src/ui-server/api/steps.ts +++ b/src/ui-server/api/steps.ts @@ -394,6 +394,33 @@ interface StepRecord extends WireStep { root: Node | null; } +/** + * The string a Java / Kotlin return expression builds, when it starts with a + * literal: `"owners/ownerDetails"`, `"redirect:/owners/" + owner.getId()` → + * `redirect:/owners/${…}`. Null for anything else — a variable, a call. + */ +function springViewString(expression: string): string | null { + let out = ''; + let rest = expression.trim(); + let first = true; + while (rest.length > 0) { + const lit = /^"((?:[^"\\]|\\.)*)"/.exec(rest); + if (lit) { + out += lit[1]!.replace(/\$\{[^}]*\}|\$\w+/g, '${…}'); + rest = rest.slice(lit[0].length).trim(); + } else { + if (first) return null; + const operand = /^[^+]+/.exec(rest); + out += '${…}'; + rest = rest.slice(operand ? operand[0].length : rest.length).trim(); + } + first = false; + if (rest.startsWith('+')) rest = rest.slice(1).trim(); + else break; + } + return first ? null : out; +} + export async function buildSteps(cg: CodeGraph, projectRoot: string, query: URLSearchParams): Promise { const started = Date.now(); const depthCap = intParam(query, 'depth', { min: 1, max: MAX_DEPTH, default: DEFAULT_DEPTH }); @@ -808,6 +835,35 @@ export async function buildSteps(cg: CodeGraph, projectRoot: string, query: URLS return record; }; + /** + * How a Spring MVC handler answers: each `return` of a view name — a string, + * or a same-class constant holding one (`VIEWS_OWNER_CREATE_OR_UPDATE_FORM`) + * — renders that view (200); `"redirect:/owners/" + id` sends a 302 and + * `"forward:/x"` hands the request on. Only for a mapped method of a class + * that renders views: a `@RestController` or `@ResponseBody` method returns + * its body, and a returned value that is not a literal is not guessed at. + */ + const springMvcReplies = async (node: Node): Promise> => { + if ((node.language !== 'java' && node.language !== 'kotlin') || node.kind !== 'method') return []; + const decs = await calls.decorators(node); + if (!decs || !decs.own.some((d) => /^(?:Get|Post|Put|Patch|Delete|Request)Mapping\b/.test(d))) return []; + if (decs.class.some((d) => /^RestController\b/.test(d)) || decs.own.some((d) => /^ResponseBody\b/.test(d))) return []; + const out: Array<{ text: string; status: number | null; line: number; column: number }> = []; + for (const r of await calls.returns(node)) { + const view = r.value ?? springViewString(r.expression); + if (view === null) continue; + const redirect = /^redirect:(.*)$/.exec(view); + const forward = /^forward:(.*)$/.exec(view); + out.push({ + text: redirect ? `redirect:${redirect[1]}` : forward ? `forward:${forward[1]}` : `view ${view}`, + status: redirect ? 302 : forward ? null : 200, + line: r.line, + column: r.column, + }); + } + return out; + }; + /** One effect site: the call as written, what it passes, when, what fires it, and — for a response — the status. */ const effectLink = async ( step: StepRecord, @@ -815,10 +871,12 @@ export async function buildSteps(cg: CodeGraph, projectRoot: string, query: URLS ref: { referenceName: string; referenceKind: 'calls' | 'instantiates'; line: number; column?: number }, trigger: WireStepTrigger | null, fallbackArgs: string | null = null, - requireReceiver = false + requireReceiver = false, + /** A site that is no call — a handler's `return "redirect:/x"` — arrives already classified. */ + preset: { effect: Effect; status: number | null } | null = null ): Promise => { const at = { line: ref.line, column: ref.column }; - const site = await callAt(fold.node, { ...at, callee: ref.referenceName }); + const site = preset ? null : await callAt(fold.node, { ...at, callee: ref.referenceName }); // The site read must be THIS call: its last segment is the reference's. const last = (n: string) => n.replace(/\([^()]*\)/g, '').split(/[.:]/).pop() ?? n; const usable = !!site && site.callee !== '' && last(site.callee) === last(ref.referenceName); @@ -829,24 +887,28 @@ export async function buildSteps(cg: CodeGraph, projectRoot: string, query: URLS // index through it: a library's `Repository`, or the project's own // `OwnerRepository` interface whose `save` comes from Spring Data — never // a project class that declares the method, which is a place to walk into. - const declared = await receiverTypeFor(fold.node, text); - const receiverType = declared && (await resolveByReceiver(fold.node, text)) === null ? declared : null; - const effect = classifyEffect({ - text, - kind: ref.referenceKind, - language: fold.node.language, - project, - receiverType, - args, - }); + let effect: Effect | null = preset?.effect ?? null; + if (!preset) { + const declared = await receiverTypeFor(fold.node, text); + const receiverType = declared && (await resolveByReceiver(fold.node, text)) === null ? declared : null; + effect = classifyEffect({ + text, + kind: ref.referenceKind, + language: fold.node.language, + project, + receiverType, + args, + }); + } if (effect === null) return false; // A reply's status, read before its box exists — the box is per outcome. // `NextResponse.json(user, { status: 201 })`: the code sits in an object // the abbreviation reduced to its keys; the site reader kept it. And a // body-sending reply that sets none is a 200, so a success has a box of // its own beside the 401's. - const status = - effect.category === 'response' + const status = preset + ? preset.status + : effect.category === 'response' ? (responseStatus(text, args, ref.referenceKind) ?? (usable && typeof site.status === 'number' ? site.status : null) ?? implicitResponseStatus(text)) : null; // What fires this call, read before its box is made: a call bound inside @@ -1091,6 +1153,11 @@ export async function buildSteps(cg: CodeGraph, projectRoot: string, query: URLS if (channelLines.has(ref.line)) continue; await effectLink(step, fold, { referenceName: ref.referenceName, referenceKind: ref.referenceKind, line: ref.line, column: ref.column }, null); } + // A Spring MVC handler answers by what it returns, which is no call. + for (const reply of await springMvcReplies(fold.node)) { + await effectLink(step, fold, { referenceName: reply.text, referenceKind: 'calls', line: reply.line, column: reply.column }, + null, null, false, { effect: { category: 'response' }, status: reply.status }); + } } let edges = (bySource.get(fold.node.id) ?? []).slice(); diff --git a/src/ui-server/api/when.ts b/src/ui-server/api/when.ts index 673edabd6..435fbf360 100644 --- a/src/ui-server/api/when.ts +++ b/src/ui-server/api/when.ts @@ -21,12 +21,14 @@ import { guardsForFile, loopsForFile, memberTypesForFile, + returnsForFile, siteKey, supportsBranchGuards, triggersForFile, type BranchGuard, type CallSiteText, type DefinitionDecorators, + type ReturnSite, type SiteLoop, type SiteTrigger, } from '../../graph/branch-guards'; @@ -120,6 +122,8 @@ export interface SiteReader { decorators(definition: { filePath: string; language: Language; startLine: number }): Promise; /** The declared types of the members of the class a definition belongs to, by member name; empty when unreadable. */ memberTypes(definition: { filePath: string; language: Language; startLine: number }): Promise>; + /** The `return`s a definition makes itself, with a same-class constant's value; empty when unreadable. */ + returns(definition: { filePath: string; language: Language; startLine: number }): Promise; /** * The `'use server'` / `'use client'` directive a JS-family file opens with, * and whether the definition itself opens with `'use server'` (a server @@ -226,6 +230,13 @@ export function createSiteReader(cg: CodeGraph, projectRoot: string, maxSites = if (!file) return new Map(); return memberTypesForFile(file.abs, file.language, definition.startLine); }, + async returns(definition) { + // Not counted: one read per handler, on a tree the walk has parsed anyway. + if (!definition.startLine || !supportsBranchGuards(definition.language)) return []; + const file = resolve(definition); + if (!file) return []; + return returnsForFile(file.abs, file.language, definition.startLine); + }, async directive(definition) { // Not counted: a text read, cached per file, no tree. const none = { file: null, own: false } as const;