mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-18 13:53:01 +08:00
fix(rails): only:/except: are read as %i[], %w(), a string or :only => (#2269)
Only a bracketed list (and the scoped reader's bare :sym) was read, so maybe's `resources :family_exports, only: %i[new create index]` drew all seven RESTful routes — four to actions the controller lacks, listed by the viewer as routes bound to nothing. Both route readers now share one option reader for every form Rails accepts. 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 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.
|
||||
- In JavaScript and TypeScript, a CommonJS `require('./application')` now counts as an import of that file, the way an ESM `import` does. A module path also resolves the way Node and bundlers find it: extensions, `index` files and path aliases. Before, a CommonJS file depended on nothing at file level, so Express's own `lib/express.js` showed no imports and impact analysis missed the files it loads. An extensionless side-effect import like `import './polyfills'` also stopped at its own import statement instead of reaching the file.
|
||||
- An Express route whose handler is written inline as `function (req, res) {…}`, the form Express's own examples use, now links to what that handler calls, just like an arrow handler does. The handler is the route's last argument, so an inline middleware before a named handler no longer stands in for it, and a wrapper like `asyncHandler(async function (req, res) {…})` and a trailing comma both work. Before, a route such as `POST /login` linked to nothing.
|
||||
|
||||
@@ -0,0 +1,57 @@
|
||||
/**
|
||||
* A `resources` line's `only:` / `except:` is read however Rails lets it be
|
||||
* written. maybe's `resources :family_exports, only: %i[new create index]` was
|
||||
* not read, so it drew all seven RESTful routes — four of them to actions the
|
||||
* controller does not have, which the viewer listed as routes bound to nothing.
|
||||
*/
|
||||
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;
|
||||
|
||||
beforeAll(async () => {
|
||||
root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-rails-only-'));
|
||||
const files: Record<string, string> = {
|
||||
'Gemfile': 'source "https://rubygems.org"\ngem "rails", "~> 7.1"\n',
|
||||
'config/routes.rb': `Rails.application.routes.draw do
|
||||
resources :family_exports, only: %i[new create index] do
|
||||
member do
|
||||
get :download
|
||||
end
|
||||
end
|
||||
resources :tags, except: %w(destroy edit update)
|
||||
resources :imports, :only => [:index, :show]
|
||||
resource :session, only: "show"
|
||||
end
|
||||
`,
|
||||
};
|
||||
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 routes = (controller: string) =>
|
||||
cg.getNodesByKind('route').map((r) => r.qualifiedName).filter((q) => q.includes(`${controller}#`)).map((q) => q.slice(q.lastIndexOf('#') + 1)).sort();
|
||||
|
||||
describe('Rails resources only: / except:', () => {
|
||||
it('reads %i[] and %w() lists', () => {
|
||||
expect(routes('family_exports')).toEqual(['create', 'download', 'index', 'new']);
|
||||
expect(routes('tags')).toEqual(['create', 'index', 'new', 'show']);
|
||||
});
|
||||
|
||||
it('reads the hash-rocket and string forms', () => {
|
||||
expect(routes('imports')).toEqual(['index', 'show']);
|
||||
expect(routes('sessions')).toEqual(['show']);
|
||||
});
|
||||
});
|
||||
@@ -8,6 +8,25 @@ import { Node } from '../../types';
|
||||
import { FrameworkResolver, FrameworkExtractionResult, UnresolvedRef, ResolvedRef, ResolutionContext } from '../types';
|
||||
import { stripCommentsForRegex } from '../strip-comments';
|
||||
|
||||
/**
|
||||
* A `resources` line's `only:` / `except:` action list, written any way Rails
|
||||
* accepts it: `[:index, :show]`, `:show`, `%i[new create index]`, `%w(index
|
||||
* show)`, `"show"`, or the older `:only => [...]`. Null when the option is
|
||||
* absent. maybe's `only: %i[new create index]` was not read, so every
|
||||
* resource drew all seven routes, four of them to actions that do not exist.
|
||||
*/
|
||||
function railsActionOption(tail: string, key: 'only' | 'except'): Set<string> | null {
|
||||
const m = new RegExp(
|
||||
String.raw`(?:\b${key}:|:${key}\s*=>)\s*(?:\[([^\]]*)\]|%[iIwW]\s*[\[(]([^\])]*)[\])]|:(\w+)|["'](\w+)["'])`
|
||||
).exec(tail);
|
||||
if (!m) return null;
|
||||
if (m[1] !== undefined) {
|
||||
return new Set(m[1].split(',').map((v) => v.trim().replace(/^:/, '').replace(/^["']|["']$/g, '')).filter(Boolean));
|
||||
}
|
||||
if (m[2] !== undefined) return new Set(m[2].trim().split(/\s+/).filter(Boolean));
|
||||
return new Set([m[3] ?? m[4]!]);
|
||||
}
|
||||
|
||||
export const railsResolver: FrameworkResolver = {
|
||||
name: 'rails',
|
||||
languages: ['ruby'],
|
||||
@@ -165,11 +184,10 @@ export const railsResolver: FrameworkResolver = {
|
||||
const resName = match[2]!;
|
||||
const tail = match[3] || '';
|
||||
let actions = plural ? PLURAL_ACTIONS : SINGULAR_ACTIONS;
|
||||
const only = tail.match(/only:\s*\[([^\]]*)\]/);
|
||||
const except = tail.match(/except:\s*\[([^\]]*)\]/);
|
||||
const symList = (s: string) => new Set(s.split(',').map((x) => x.trim().replace(/^:/, '')));
|
||||
if (only) { const s = symList(only[1]!); actions = actions.filter((a) => s.has(a)); }
|
||||
else if (except) { const s = symList(except[1]!); actions = actions.filter((a) => !s.has(a)); }
|
||||
const only = railsActionOption(tail, 'only');
|
||||
const except = railsActionOption(tail, 'except');
|
||||
if (only) actions = actions.filter((a) => only.has(a));
|
||||
else if (except) actions = actions.filter((a) => !except.has(a));
|
||||
// `resources :articles` → ArticlesController; `resource :user` → UsersController.
|
||||
const ctrl = plural ? resName : pluralize(resName);
|
||||
const line = safe.slice(0, match.index).split('\n').length;
|
||||
@@ -276,11 +294,10 @@ function extractScopedRailsRoutes(filePath: string, safe: string): FrameworkExtr
|
||||
const collectionPath = join(frame.path, segment);
|
||||
const memberPath = plural ? join(collectionPath, ':id') : collectionPath;
|
||||
let actions = plural ? PLURAL_ACTIONS : SINGULAR_ACTIONS;
|
||||
const only = /only:\s*(?:\[([^\]]*)\]|(:\w+))/.exec(tail);
|
||||
const except = /except:\s*(?:\[([^\]]*)\]|(:\w+))/.exec(tail);
|
||||
const symList = (x: RegExpExecArray) => new Set((x[1] ?? x[2] ?? '').split(',').map((v) => sym(v.trim())));
|
||||
if (only) { const keep = symList(only); actions = actions.filter((a) => keep.has(a)); }
|
||||
else if (except) { const drop = symList(except); actions = actions.filter((a) => !drop.has(a)); }
|
||||
const only = railsActionOption(tail, 'only');
|
||||
const except = railsActionOption(tail, 'except');
|
||||
if (only) actions = actions.filter((a) => only.has(a));
|
||||
else if (except) actions = actions.filter((a) => !except.has(a));
|
||||
for (const action of actions) {
|
||||
const spec = RESTFUL_ROUTES[action]!;
|
||||
const path = action === 'index' || action === 'create' ? collectionPath
|
||||
|
||||
Reference in New Issue
Block a user