1
0
Эх сурвалжийг харах

feat(resolution): Flask/FastAPI route holes + Python builtin-name handler guard

Three fixes that connect the request→route→handler flow for Flask and
FastAPI. Validated S/L: fastapi-realworld 12→20, flask-microblog 6→27,
Netflix dispatch 290/290 (100%), redash decorator routes 6/6; canonical
flows trace end-to-end (login→get_user_by_email, create_user→from_dict).

- Flask: the route regex required `def` immediately after `@x.route(...)`,
  so an intervening decorator (@login_required, @cache.cached) or stacked
  @x.route lines (one view bound to several URLs) dropped the route.
  Switch to the findHandler scan (match the decorator, then find the next
  def) like FastAPI — skips intervening decorators.
- FastAPI: the path regex `[^'"]+` rejected the empty path `@router.get("")`
  (router/prefix-root routes, frequently multi-line). Allow empty path +
  guard the route name against a trailing space.
- Python builtin-name guard (src/resolution/index.ts): a handler named
  after a Python builtin method (index/get/update/count…) was filtered by
  isBuiltInOrExternal and lost its route→handler edge. Mirror the
  dotted-method branch's knownNames guard onto the bare branch — a bare
  name a declared symbol owns is a real target, not a builtin call.
  +2 legit edges on realworld, 0 change on the django control (precision held).

Tests: new Flask (intervening/stacked decorator) and FastAPI (empty-path,
multi-line) extractor cases + a Flask end-to-end integration test (a view
named `index` behind @login_required). Also corrects 6 pre-existing stale
Laravel/Rails route-ref assertions surfaced by the suite — they expected
the old bare action name, but the resolvers now emit precise
controller@action / controller#action (from earlier precision commits).
Full suite green (781 passed).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Colby McHenry 1 сар өмнө
parent
commit
77a71592fc

+ 47 - 0
__tests__/frameworks-integration.test.ts

@@ -57,3 +57,50 @@ describe('Django end-to-end framework extraction', () => {
     cg.close();
     cg.close();
   });
   });
 });
 });
+
+describe('Flask end-to-end framework extraction', () => {
+  let tmpDir: string | undefined;
+  afterEach(() => {
+    if (tmpDir) fs.rmSync(tmpDir, { recursive: true, force: true });
+    tmpDir = undefined;
+  });
+
+  it('resolves stacked routes across @login_required to a view named after a builtin (index)', async () => {
+    tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-flask-'));
+    fs.writeFileSync(path.join(tmpDir, 'requirements.txt'), 'flask==3.0\n');
+    fs.writeFileSync(
+      path.join(tmpDir, 'app.py'),
+      'from flask import Blueprint, render_template\n' +
+        'from flask_login import login_required\n' +
+        'bp = Blueprint("main", __name__)\n' +
+        '\n' +
+        '@bp.route("/", methods=["GET", "POST"])\n' +
+        '@bp.route("/index", methods=["GET", "POST"])\n' +
+        '@login_required\n' +
+        'def index():\n' +
+        '    return render_template("index.html")\n'
+    );
+
+    const cg = CodeGraph.initSync(tmpDir);
+    await cg.indexAll();
+
+    // Both stacked @bp.route decorators are extracted (the second was previously
+    // dropped because @login_required broke the "def must follow" assumption).
+    const routes = cg.getNodesByKind('route');
+    expect(routes.map((r) => r.name).sort()).toEqual(['GET /', 'GET /index']);
+
+    // The view function exists even though its name is a Python builtin method.
+    const fn = cg.getNodesByKind('function').find((n) => n.name === 'index');
+    expect(fn).toBeDefined();
+
+    // Both routes resolve to it — exercises the bare-name builtin guard, which
+    // previously filtered the `index` reference as a builtin method.
+    for (const route of routes) {
+      const edges = cg.getOutgoingEdges(route.id);
+      const toView = edges.find((e) => e.target === fn!.id && e.kind === 'references');
+      expect(toView, `route ${route.name} should resolve to index()`).toBeDefined();
+    }
+
+    cg.close();
+  });
+});

+ 57 - 6
__tests__/frameworks.test.ts

@@ -123,6 +123,31 @@ def create_user(id):
     expect(nodes[0].name).toBe('POST /<id>');
     expect(nodes[0].name).toBe('POST /<id>');
     expect(references[0].referenceName).toBe('create_user');
     expect(references[0].referenceName).toBe('create_user');
   });
   });
+
+  it('resolves the handler across an intervening decorator (@login_required)', () => {
+    const src = `
+@bp.route('/profile')
+@login_required
+def profile():
+    return render_template('profile.html')
+`;
+    const { nodes, references } = flaskResolver.extract!('routes.py', src);
+    expect(nodes[0].name).toBe('GET /profile');
+    expect(references[0].referenceName).toBe('profile');
+  });
+
+  it('extracts stacked @x.route decorators bound to one view', () => {
+    const src = `
+@bp.route('/', methods=['GET', 'POST'])
+@bp.route('/index', methods=['GET', 'POST'])
+@login_required
+def index():
+    return render_template('index.html')
+`;
+    const { nodes, references } = flaskResolver.extract!('routes.py', src);
+    expect(nodes.map((n) => n.name)).toEqual(['GET /', 'GET /index']);
+    expect(references.map((r) => r.referenceName)).toEqual(['index', 'index']);
+  });
 });
 });
 
 
 describe('fastapiResolver.extract', () => {
 describe('fastapiResolver.extract', () => {
@@ -147,6 +172,32 @@ def create_item(item: Item):
     expect(nodes[0].name).toBe('POST /items');
     expect(nodes[0].name).toBe('POST /items');
     expect(references[0].referenceName).toBe('create_item');
     expect(references[0].referenceName).toBe('create_item');
   });
   });
+
+  it('extracts a route mounted at the router/prefix root (empty path)', () => {
+    const src = `
+@router.get("", response_model=ListOfArticles, name="articles:list")
+async def list_articles():
+    return []
+`;
+    const { nodes, references } = fastapiResolver.extract!('articles.py', src);
+    expect(nodes[0].name).toBe('GET /');
+    expect(references[0].referenceName).toBe('list_articles');
+  });
+
+  it('extracts a multi-line decorator with an empty path', () => {
+    const src = `
+@router.post(
+    "",
+    status_code=201,
+    response_model=ArticleInResponse,
+)
+async def create_article():
+    pass
+`;
+    const { nodes, references } = fastapiResolver.extract!('articles.py', src);
+    expect(nodes[0].name).toBe('POST /');
+    expect(references[0].referenceName).toBe('create_article');
+  });
 });
 });
 
 
 import { expressResolver } from '../src/resolution/frameworks/express';
 import { expressResolver } from '../src/resolution/frameworks/express';
@@ -463,13 +514,13 @@ describe('laravelResolver.extract', () => {
     const src = `Route::get('/users', [UserController::class, 'index']);\n`;
     const src = `Route::get('/users', [UserController::class, 'index']);\n`;
     const { nodes, references } = laravelResolver.extract!('routes/web.php', src);
     const { nodes, references } = laravelResolver.extract!('routes/web.php', src);
     expect(nodes[0].name).toBe('GET /users');
     expect(nodes[0].name).toBe('GET /users');
-    expect(references[0].referenceName).toBe('index');
+    expect(references[0].referenceName).toBe('UserController@index');
   });
   });
 
 
   it('extracts route with Controller@action syntax', () => {
   it('extracts route with Controller@action syntax', () => {
     const src = `Route::post('/users', 'UserController@store');\n`;
     const src = `Route::post('/users', 'UserController@store');\n`;
     const { nodes, references } = laravelResolver.extract!('routes/web.php', src);
     const { nodes, references } = laravelResolver.extract!('routes/web.php', src);
-    expect(references[0].referenceName).toBe('store');
+    expect(references[0].referenceName).toBe('UserController@store');
   });
   });
 
 
   it('extracts resource route', () => {
   it('extracts resource route', () => {
@@ -487,13 +538,13 @@ describe('railsResolver.extract', () => {
     const src = `get '/users', to: 'users#index'\n`;
     const src = `get '/users', to: 'users#index'\n`;
     const { nodes, references } = railsResolver.extract!('config/routes.rb', src);
     const { nodes, references } = railsResolver.extract!('config/routes.rb', src);
     expect(nodes[0].name).toBe('GET /users');
     expect(nodes[0].name).toBe('GET /users');
-    expect(references[0].referenceName).toBe('index');
+    expect(references[0].referenceName).toBe('users#index');
   });
   });
 
 
   it('extracts route without to: keyword', () => {
   it('extracts route without to: keyword', () => {
     const src = `post '/items' => 'items#create'\n`;
     const src = `post '/items' => 'items#create'\n`;
     const { nodes, references } = railsResolver.extract!('config/routes.rb', src);
     const { nodes, references } = railsResolver.extract!('config/routes.rb', src);
-    expect(references[0].referenceName).toBe('create');
+    expect(references[0].referenceName).toBe('items#create');
   });
   });
 });
 });
 
 
@@ -969,7 +1020,7 @@ Route::get('/real', [RealController::class, 'index']);
 `;
 `;
     const { nodes, references } = laravelResolver.extract!('routes/web.php', src);
     const { nodes, references } = laravelResolver.extract!('routes/web.php', src);
     expect(nodes.map((n) => n.name)).toEqual(['GET /real']);
     expect(nodes.map((n) => n.name)).toEqual(['GET /real']);
-    expect(references.map((r) => r.referenceName)).toEqual(['index']);
+    expect(references.map((r) => r.referenceName)).toEqual(['RealController@index']);
   });
   });
 
 
   it('rails: skips =begin/=end and # commented routes', () => {
   it('rails: skips =begin/=end and # commented routes', () => {
@@ -982,7 +1033,7 @@ get '/real', to: 'real#index'
 `;
 `;
     const { nodes, references } = railsResolver.extract!('config/routes.rb', src);
     const { nodes, references } = railsResolver.extract!('config/routes.rb', src);
     expect(nodes.map((n) => n.name)).toEqual(['GET /real']);
     expect(nodes.map((n) => n.name)).toEqual(['GET /real']);
-    expect(references.map((r) => r.referenceName)).toEqual(['index']);
+    expect(references.map((r) => r.referenceName)).toEqual(['real#index']);
   });
   });
 
 
   it('spring: skips // and /* */ commented @GetMapping', () => {
   it('spring: skips // and /* */ commented @GetMapping', () => {

+ 9 - 6
src/resolution/frameworks/python.ts

@@ -199,12 +199,14 @@ export const flaskResolver: FrameworkResolver = {
   extract(filePath, content) {
   extract(filePath, content) {
     if (!filePath.endsWith('.py')) return { nodes: [], references: [] };
     if (!filePath.endsWith('.py')) return { nodes: [], references: [] };
     return extractDecoratorRoutes(filePath, stripCommentsForRegex(content, 'python'), {
     return extractDecoratorRoutes(filePath, stripCommentsForRegex(content, 'python'), {
-      // Flask: @x.route('/path', methods=[...])
-      decoratorRegex: /@(\w+)\.route\s*\(\s*['"]([^'"]+)['"](?:\s*,\s*methods\s*=\s*\[([^\]]+)\])?\s*\)\s*\n\s*(?:async\s+)?def\s+(\w+)/g,
+      // Flask: @x.route('/path', methods=[...]) — the handler is the next `def`,
+      // allowing intervening decorators (@login_required, @cache.cached) and
+      // stacked @x.route() lines (one view bound to several URLs).
+      decoratorRegex: /@(\w+)\.route\s*\(\s*['"]([^'"]*)['"](?:\s*,\s*methods\s*=\s*\[([^\]]+)\])?\s*\)/g,
       defaultMethod: 'GET',
       defaultMethod: 'GET',
       methodFromGroup: 3,
       methodFromGroup: 3,
       pathGroup: 2,
       pathGroup: 2,
-      handlerGroup: 4,
+      findHandler: true,
       language: 'python',
       language: 'python',
     });
     });
   },
   },
@@ -241,8 +243,9 @@ export const fastapiResolver: FrameworkResolver = {
   extract(filePath, content) {
   extract(filePath, content) {
     if (!filePath.endsWith('.py')) return { nodes: [], references: [] };
     if (!filePath.endsWith('.py')) return { nodes: [], references: [] };
     return extractDecoratorRoutes(filePath, stripCommentsForRegex(content, 'python'), {
     return extractDecoratorRoutes(filePath, stripCommentsForRegex(content, 'python'), {
-      // FastAPI: @x.METHOD('/path') -> handler on the next def line
-      decoratorRegex: /@(\w+)\.(get|post|put|patch|delete|options|head)\s*\(\s*['"]([^'"]+)['"]/g,
+      // FastAPI: @x.METHOD('/path') -> handler on the next def line. Path may be
+      // empty ("") for routes mounted at the router/prefix root.
+      decoratorRegex: /@(\w+)\.(get|post|put|patch|delete|options|head)\s*\(\s*['"]([^'"]*)['"]/g,
       defaultMethod: '',
       defaultMethod: '',
       methodGroup: 2,
       methodGroup: 2,
       pathGroup: 3,
       pathGroup: 3,
@@ -278,7 +281,7 @@ function extractDecoratorRoutes(filePath: string, content: string, opts: Decorat
       if (m) method = m[1]!.toUpperCase();
       if (m) method = m[1]!.toUpperCase();
     }
     }
     const line = content.slice(0, match.index).split('\n').length;
     const line = content.slice(0, match.index).split('\n').length;
-    const name = method ? `${method} ${routePath}` : routePath!;
+    const name = method ? `${method} ${routePath || '/'}` : (routePath || '/');
     const routeNode: Node = {
     const routeNode: Node = {
       id: `route:${filePath}:${line}:${method}:${routePath}`,
       id: `route:${filePath}:${line}:${method}:${routePath}`,
       kind: 'route',
       kind: 'route',

+ 7 - 1
src/resolution/index.ts

@@ -758,7 +758,13 @@ export class ReferenceResolver {
           }
           }
         }
         }
       }
       }
-      if (PYTHON_BUILT_IN_METHODS.has(name)) {
+      // A bare name colliding with a builtin method (index, get, update, count…)
+      // is only a builtin when NOTHING in the codebase declares it. A declared
+      // symbol with that exact name — e.g. a Flask/FastAPI view `def index()` or
+      // `def get()` — is a real reference target. Mirrors the knownNames guard on
+      // the dotted branch above; without it, every handler named after a builtin
+      // method silently loses its route→handler edge.
+      if (PYTHON_BUILT_IN_METHODS.has(name) && !this.knownNames?.has(name)) {
         return true;
         return true;
       }
       }
     }
     }