mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-17 05:07:37 +08:00
fix(laravel): a string controller resolves without a Controller suffix (#2271)
Only `XController@action` was claimed and resolved, and the written namespace was stripped, so akaunting's 'Common\\Uploads@inline' (and a Route::resource with a string controller plus options) bound 304 of 313 routes to nothing. Any `Class@action` is claimed; the namespace path under App\\Http\\Controllers is kept and picks between same-named controllers; a resource reads its first argument and resolves its controller class in the Laravel resolver (no `use` brings a string-named class into scope). Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -14,6 +14,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
||||
|
||||
### Fixes
|
||||
|
||||
- In Laravel apps, a route whose controller is named by string now links to its handler, without needing a `Controller` suffix and keeping the namespace it's written with: `'Common\Uploads@inline'`, and `Route::resource('companies', 'Common\Companies', [...])` with an options array. Two same-named controllers in different namespaces are told apart by that namespace. Before, akaunting linked 304 of its 313 routes to nothing.
|
||||
- In Nuxt and Astro projects, a file-routed page is now linked to the component its file defines. An Astro endpoint is linked to the HTTP-verb handlers it exports, like `export const GET`. Before, a page route like Nuxt's `pages/admin.vue` or Astro's `src/pages/about.astro` linked to nothing, so callers, impact and flows stopped at the route.
|
||||
- In Rails apps, a `resources` line's `only:` and `except:` are now read in every form Rails accepts: `%i[new create index]`, `%w(index show)`, a quoted name, and the older `:only => [...]`. Before, only a bracketed list was read, so maybe's `resources :family_exports, only: %i[new create index]` became all seven routes, four of them to actions the controller doesn't have.
|
||||
- In Vue, Svelte and Astro files, a component now owns its script, and what a `<script setup>`, a Svelte instance script or Astro frontmatter does at its top level is the component's doing. That covers a Nuxt page's `useAsyncData(…)`, a Svelte page's `onMount(…)` and Astro's `await getCollection(…)`. Imports and a module-level script stay with the file. Before, those calls belonged to the file, so callers and flows from a component found nothing. The component was also recorded as containing its own file, and a file with no script had no file node at all.
|
||||
|
||||
@@ -0,0 +1,74 @@
|
||||
/**
|
||||
* A Laravel route names its controller however the app writes it: akaunting
|
||||
* writes `'Common\Uploads@inline'` — no `Controller` suffix, a namespace below
|
||||
* App\Http\Controllers — and `Route::resource('companies', 'Common\Companies',
|
||||
* [...options])`. Only `XController@action` was read, so 304 of akaunting's
|
||||
* 313 routes bound to nothing. Same-named controllers in two namespaces are
|
||||
* told apart by the written path.
|
||||
*/
|
||||
import { describe, it, expect, afterAll, beforeAll } from 'vitest';
|
||||
import * as fs from 'fs';
|
||||
import * as os from 'os';
|
||||
import * as path from 'path';
|
||||
import { CodeGraph } from '../src';
|
||||
|
||||
let root = '';
|
||||
let cg: CodeGraph;
|
||||
|
||||
const controller = (ns: string, name: string, methods: string[]) => `<?php
|
||||
|
||||
namespace App\\Http\\Controllers\\${ns};
|
||||
|
||||
class ${name}
|
||||
{
|
||||
${methods.map((m) => ` public function ${m}() { return 1; }`).join('\n')}
|
||||
}
|
||||
`;
|
||||
|
||||
beforeAll(async () => {
|
||||
root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-laravel-strings-'));
|
||||
const files: Record<string, string> = {
|
||||
'artisan': '#!/usr/bin/env php\n',
|
||||
'composer.json': JSON.stringify({ require: { 'laravel/framework': '^10.0' } }),
|
||||
'routes/admin.php': `<?php
|
||||
|
||||
use Illuminate\\Support\\Facades\\Route;
|
||||
|
||||
Route::get('uploads/{id}/inline', 'Common\\Uploads@inline')->name('inline');
|
||||
Route::delete('uploads/{id}', 'Common\\Uploads@destroy');
|
||||
Route::get('portal/uploads/{id}', 'Portal\\Uploads@inline');
|
||||
Route::resource('companies', 'Common\\Companies', ['middleware' => ['dropzone']]);
|
||||
`,
|
||||
'app/Http/Controllers/Common/Uploads.php': controller('Common', 'Uploads', ['inline', 'destroy']),
|
||||
'app/Http/Controllers/Portal/Uploads.php': controller('Portal', 'Uploads', ['inline']),
|
||||
'app/Http/Controllers/Common/Companies.php': controller('Common', 'Companies', ['index', 'store']),
|
||||
};
|
||||
for (const [rel, content] of Object.entries(files)) {
|
||||
fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true });
|
||||
fs.writeFileSync(path.join(root, rel), content);
|
||||
}
|
||||
cg = await CodeGraph.init(root, { index: true });
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
cg?.close();
|
||||
if (root) fs.rmSync(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
const servedBy = (routeName: string) => {
|
||||
const route = cg.getNodesByKind('route').find((r) => r.name === routeName);
|
||||
expect(route, routeName).toBeDefined();
|
||||
return cg.getOutgoingEdgesFrom([route!.id]).map((e) => cg.getNode(e.target)!).map((n) => `${n.name} ${n.filePath}`);
|
||||
};
|
||||
|
||||
describe('Laravel string controllers', () => {
|
||||
it('resolve without a Controller suffix, by their namespace path', () => {
|
||||
expect(servedBy('GET /uploads/{id}/inline')).toEqual(['inline app/Http/Controllers/Common/Uploads.php']);
|
||||
expect(servedBy('DELETE /uploads/{id}')).toEqual(['destroy app/Http/Controllers/Common/Uploads.php']);
|
||||
expect(servedBy('GET /portal/uploads/{id}')).toEqual(['inline app/Http/Controllers/Portal/Uploads.php']);
|
||||
});
|
||||
|
||||
it('read a resource controller named by string, past its options', () => {
|
||||
expect(servedBy('resource:companies')).toEqual(['Companies app/Http/Controllers/Common/Companies.php']);
|
||||
});
|
||||
});
|
||||
@@ -49,7 +49,7 @@ export const laravelResolver: FrameworkResolver = {
|
||||
// pre-filter would drop them before resolve() runs (Pattern 4). Claim them —
|
||||
// same hook the django ORM / Rails routing work needed.
|
||||
claimsReference(name: string): boolean {
|
||||
return /^[A-Za-z_][A-Za-z0-9_]*Controller@\w+$/.test(name);
|
||||
return CONTROLLER_ACTION.test(name) || NAMESPACED_CLASS.test(name);
|
||||
},
|
||||
|
||||
resolve(ref: UnresolvedRef, context: ResolutionContext): ResolvedRef | null {
|
||||
@@ -82,8 +82,16 @@ export const laravelResolver: FrameworkResolver = {
|
||||
return null;
|
||||
}
|
||||
|
||||
// Pattern 5: a resource route's controller, named by string or `::class`.
|
||||
if (ref.referenceKind === 'imports' && ref.fromNodeId.includes(':RESOURCE:')) {
|
||||
const result = resolveControllerClass(ref.referenceName, context);
|
||||
if (result) {
|
||||
return { original: ref, targetNodeId: result, confidence: 0.9, resolvedBy: 'framework' };
|
||||
}
|
||||
}
|
||||
|
||||
// Pattern 4: Controller method references
|
||||
const controllerMatch = ref.referenceName.match(/^([A-Z][a-zA-Z]+Controller)@(\w+)$/);
|
||||
const controllerMatch = ref.referenceName.match(CONTROLLER_ACTION);
|
||||
if (controllerMatch) {
|
||||
const [, controller, method] = controllerMatch;
|
||||
const result = resolveControllerMethod(controller!, method!, context);
|
||||
@@ -172,7 +180,8 @@ export const laravelResolver: FrameworkResolver = {
|
||||
nodes.push(routeNode);
|
||||
|
||||
if (handlerExpr) {
|
||||
const controllerName = extractLaravelHandler(handlerExpr);
|
||||
// The controller is the first argument; an options array may follow.
|
||||
const controllerName = extractLaravelHandler(splitArgs(handlerExpr)[0] ?? handlerExpr);
|
||||
if (controllerName) {
|
||||
references.push({
|
||||
fromNodeId: routeNode.id,
|
||||
@@ -478,14 +487,22 @@ function extractLaravelHandler(expr: string): string | null {
|
||||
const tupleMatch = trimmed.match(/^\[\s*([A-Za-z_\\][\w\\]*)::class\s*,\s*['"]([^'"]+)['"]\s*\]/);
|
||||
if (tupleMatch) return `${short(tupleMatch[1]!)}@${tupleMatch[2]!}`;
|
||||
|
||||
// 'Controller@method' (possibly namespaced) → `Controller@method`
|
||||
// 'Controller@method' → `Controller@method`, keeping the namespace below
|
||||
// the controllers root: akaunting writes 'Common\Uploads@inline' for
|
||||
// App\Http\Controllers\Common\Uploads, and a `Portal\Uploads` beside it is a
|
||||
// different controller.
|
||||
const atMatch = trimmed.match(/^['"]([^'"@]+)@([^'"]+)['"]$/);
|
||||
if (atMatch) return `${short(atMatch[1]!)}@${atMatch[2]!}`;
|
||||
if (atMatch) return `${controllerPath(atMatch[1]!)}@${atMatch[2]!}`;
|
||||
|
||||
// Class::class (Route::resource controller) → `Class`
|
||||
const classMatch = trimmed.match(/^([A-Za-z_\\][\w\\]*)::class/);
|
||||
if (classMatch) return short(classMatch[1]!);
|
||||
|
||||
// A controller named by string — no `use` brings it in, so its namespace
|
||||
// path below the controllers root is what finds it.
|
||||
const stringClass = trimmed.match(/^['"]([A-Za-z_\\][\w\\]*)['"]$/);
|
||||
if (stringClass) return controllerPath(stringClass[1]!);
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -539,38 +556,71 @@ function resolveModelCall(
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve a Controller@method reference
|
||||
* A route's `Controller@action`: a class name, or a namespace path under the
|
||||
* controllers root (`Common\\Uploads`), or a fully qualified one
|
||||
* (`Modules\\OfflinePayments\\Http\\Controllers\\Settings`).
|
||||
*/
|
||||
const CONTROLLER_ACTION = /^([A-Za-z_][\w\\]*)@(\w+)$/;
|
||||
|
||||
/** A string handler's class as a path under the controllers root, a leading `\\` and `App\\Http\\Controllers\\` off. */
|
||||
function controllerPath(written: string): string {
|
||||
return written.replace(/^\\+/, '').replace(/^App\\Http\\Controllers\\/i, '');
|
||||
}
|
||||
|
||||
/** A class written with its namespace path (`Common\\Companies`). */
|
||||
const NAMESPACED_CLASS = /^[A-Za-z_]\w*(?:\\\w+)+$/;
|
||||
|
||||
/** The controller class a resource route names: by its path under the controllers root, else by name. */
|
||||
function resolveControllerClass(controller: string, context: ResolutionContext): string | null {
|
||||
const rel = controller.replace(/\\/g, '/');
|
||||
const className = rel.split('/').pop()!;
|
||||
const classes = context
|
||||
.getNodesByName(className)
|
||||
.filter((n) => n.kind === 'class' && n.filePath.includes('Controllers'));
|
||||
const suffix = `/${rel.toLowerCase()}.php`;
|
||||
const exact = classes.find((n) => `/${n.filePath.toLowerCase()}`.endsWith(suffix));
|
||||
if (exact) return exact.id;
|
||||
return classes.length === 1 ? classes[0]!.id : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve a Controller@method reference: the file the path names under
|
||||
* `app/Http/Controllers/`, else a class of that name in a controllers
|
||||
* directory — the one whose path ends with the written namespace path when
|
||||
* several share the name.
|
||||
*/
|
||||
function resolveControllerMethod(
|
||||
controller: string,
|
||||
method: string,
|
||||
context: ResolutionContext
|
||||
): string | null {
|
||||
// Try app/Http/Controllers/
|
||||
const controllerPath = `app/Http/Controllers/${controller}.php`;
|
||||
const rel = controller.replace(/\\/g, '/');
|
||||
const methodIn = (filePath: string): string | null =>
|
||||
context.getNodesInFile(filePath).find((n) => n.kind === 'method' && n.name === method)?.id ?? null;
|
||||
|
||||
const controllerPath = `app/Http/Controllers/${rel}.php`;
|
||||
if (context.fileExists(controllerPath)) {
|
||||
const nodes = context.getNodesInFile(controllerPath);
|
||||
const methodNode = nodes.find(
|
||||
(n) => n.kind === 'method' && n.name === method
|
||||
);
|
||||
if (methodNode) {
|
||||
return methodNode.id;
|
||||
}
|
||||
const found = methodIn(controllerPath);
|
||||
if (found) return found;
|
||||
}
|
||||
|
||||
// Try name-based lookup for namespaced controllers
|
||||
const controllerCandidates = context.getNodesByName(controller);
|
||||
for (const ctrl of controllerCandidates) {
|
||||
if (ctrl.kind === 'class' && ctrl.filePath.includes('Controllers')) {
|
||||
const nodesInFile = context.getNodesInFile(ctrl.filePath);
|
||||
const methodNode = nodesInFile.find(
|
||||
(n) => n.kind === 'method' && n.name === method
|
||||
);
|
||||
if (methodNode) {
|
||||
return methodNode.id;
|
||||
}
|
||||
const className = rel.split('/').pop()!;
|
||||
const classes = context
|
||||
.getNodesByName(className)
|
||||
.filter((n) => n.kind === 'class' && n.filePath.includes('Controllers'));
|
||||
if (rel.includes('/')) {
|
||||
const suffix = `/${rel.toLowerCase()}.php`;
|
||||
const exact = classes.filter((n) => `/${n.filePath.toLowerCase()}`.endsWith(suffix));
|
||||
for (const ctrl of exact) {
|
||||
const found = methodIn(ctrl.filePath);
|
||||
if (found) return found;
|
||||
}
|
||||
// A namespace path that names no file decides nothing by name alone.
|
||||
if (classes.length > 1) return null;
|
||||
}
|
||||
for (const ctrl of classes) {
|
||||
const found = methodIn(ctrl.filePath);
|
||||
if (found) return found;
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user