Browse Source

fix(php): resolve static calls through import aliases (#1545) (#1795)

Co-authored-by: Colby McHenry <colbymchenry@users.noreply.github.com>
Colby Mchenry 34 minutes ago
parent
commit
040ba388da

+ 1 - 0
CHANGELOG.md

@@ -221,6 +221,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 #### Symbols, tests and the viewer
 
+- PHP static calls through imported class aliases now reach the correct class when services and repositories share method names, so callers and impact analysis show the right dependencies after re-indexing. (#1545)
 - TypeScript/JavaScript: a call through a field of the enclosing class — `this.mailer.send()` — now resolves on the field's declared type, so a delegating wrapper that shares the method's name no longer records itself as its own callee and `callers`, `impact` and trace stop lying on that shape. A field whose type is external or a builtin stays unresolved rather than guessed. Re-index after upgrading. (#1496)
 - TypeScript and JavaScript collection calls through local variables and their nested properties no longer link to unrelated project methods; re-index after upgrading. (#1566)
 

+ 9 - 0
__tests__/fixtures/php-import-alias-static/app/Http/Controllers/Backend/SettleController.php

@@ -0,0 +1,9 @@
+<?php
+namespace App\Http\Controllers\Backend;
+use App\Services\SettleService as Settle;
+
+class SettleController extends Controller {
+    public function excel($stores, $startDay = null, $endDay = null, $id = null) {
+        return Settle::getSettlesToExcel($stores, request()->_SELECTED, [$startDay, $endDay], $id);
+    }
+}

+ 8 - 0
__tests__/fixtures/php-import-alias-static/app/Repositories/SettleRepository.php

@@ -0,0 +1,8 @@
+<?php
+namespace App\Repositories;
+
+class SettleRepository {
+    public static function getSettlesToExcel($storeIds, $dayRange) {
+        return [];
+    }
+}

+ 8 - 0
__tests__/fixtures/php-import-alias-static/app/Services/SettleService.php

@@ -0,0 +1,8 @@
+<?php
+namespace App\Services;
+
+class SettleService {
+    public static function getSettlesToExcel($stores, $selected, $dayRange, $id) {
+        return [];
+    }
+}

+ 136 - 0
__tests__/php-import-alias-static-resolution.test.ts

@@ -0,0 +1,136 @@
+import { describe, it, expect, beforeEach, afterEach } from 'vitest';
+import * as fs from 'node:fs';
+import * as path from 'node:path';
+import * as os from 'node:os';
+import { CodeGraph } from '../src';
+
+const fixtureDir = path.join(__dirname, 'fixtures', 'php-import-alias-static');
+
+describe('PHP static calls through import aliases (#1545)', () => {
+  let dir: string;
+  let cg: CodeGraph | undefined;
+
+  beforeEach(() => {
+    dir = fs.mkdtempSync(path.join(os.tmpdir(), 'php-static-alias-'));
+    fs.cpSync(fixtureDir, dir, { recursive: true });
+  });
+
+  afterEach(() => {
+    cg?.close();
+    cg = undefined;
+    fs.rmSync(dir, { recursive: true, force: true });
+  });
+
+  it('attributes callers, callees and impact to SettleService instead of SettleRepository', async () => {
+    cg = await CodeGraph.init(dir, { silent: true });
+    await cg.indexAll();
+    const method = (qualifiedName: string) => {
+      const node = cg!.searchNodes(qualifiedName.split('::').pop()!)
+        .map((result) => result.node)
+        .find((n) => n.qualifiedName === qualifiedName);
+      expect(node, qualifiedName).toBeDefined();
+      return node!;
+    };
+    const excel = method('App\\Http\\Controllers\\Backend::SettleController::excel');
+    const service = method('App\\Services::SettleService::getSettlesToExcel');
+    const repository = method('App\\Repositories::SettleRepository::getSettlesToExcel');
+
+    expect(cg.getCallees(excel.id).map(({ node }) => node.id)).toContain(service.id);
+    expect(cg.getCallees(excel.id).map(({ node }) => node.id)).not.toContain(repository.id);
+    expect(cg.getCallers(service.id).map(({ node }) => node.id)).toContain(excel.id);
+    expect(cg.getCallers(repository.id).map(({ node }) => node.id)).not.toContain(excel.id);
+    expect([...cg.getImpactRadius(service.id).nodes.keys()]).toContain(excel.id);
+    expect([...cg.getImpactRadius(repository.id).nodes.keys()]).not.toContain(excel.id);
+  });
+
+  const write = (file: string, source: string) => {
+    const target = path.join(dir, file);
+    fs.mkdirSync(path.dirname(target), { recursive: true });
+    fs.writeFileSync(target, source);
+  };
+
+  const controllerPath = 'app/Http/Controllers/Backend/SettleController.php';
+  const servicePath = 'app/Services/SettleService.php';
+  const controllerSource = fs.readFileSync(path.join(fixtureDir, controllerPath), 'utf8');
+  const serviceSource = fs.readFileSync(path.join(fixtureDir, servicePath), 'utf8');
+  const serviceMethod = 'App\\Services::SettleService::getSettlesToExcel';
+
+  const callees = async () => {
+    cg = await CodeGraph.init(dir, { silent: true });
+    await cg.indexAll();
+    const excel = cg.searchNodes('excel').map(({ node }) => node)
+      .find((n) => n.kind === 'method' && n.filePath === controllerPath)!;
+    expect(excel).toBeDefined();
+    return cg.getCallees(excel.id).map(({ node }) => node.qualifiedName).sort();
+  };
+
+  it('uses the imported namespace even when another namespace declares SettleService', async () => {
+    write('app/Repositories/SettleService.php', serviceSource.replace('App\\Services', 'App\\Repositories'));
+    expect(await callees()).toEqual([serviceMethod]);
+  });
+
+  it('uses the import instead of a class whose actual name is the alias', async () => {
+    write('app/Http/Controllers/Backend/Settle.php', serviceSource
+      .replace('App\\Services', 'App\\Http\\Controllers\\Backend')
+      .replace('class SettleService', 'class Settle'));
+    expect(await callees()).toEqual([serviceMethod]);
+  });
+
+  it('constrains the method to its owner when another class shares the imported file', async () => {
+    write(servicePath, serviceSource.replace(
+      'class SettleService',
+      'class SettleServiceDecoy { public static function getSettlesToExcel() {} }\nclass SettleService',
+    ));
+    expect(await callees()).toEqual([serviceMethod]);
+  });
+
+  it('resolves by namespace even when the file is not named after the imported class', async () => {
+    fs.renameSync(path.join(dir, servicePath), path.join(dir, 'app/Services/exports.php'));
+    expect(await callees()).toEqual([serviceMethod]);
+  });
+
+  it('handles an import with a leading namespace separator', async () => {
+    write(controllerPath, controllerSource.replace('use App\\', 'use \\App\\'));
+    expect(await callees()).toEqual([serviceMethod]);
+  });
+
+  it('keeps an unaliased class import on its declared namespace', async () => {
+    write(controllerPath, controllerSource.replace(' as Settle;', ';').replace('Settle::', 'SettleService::'));
+    write('app/Repositories/SettleService.php', serviceSource.replace('App\\Services', 'App\\Repositories'));
+    expect(await callees()).toEqual([serviceMethod]);
+  });
+
+  it('supports an alias for a class in the global namespace', async () => {
+    write(controllerPath, controllerSource.replace('App\\Services\\SettleService', 'SettleService'));
+    write(servicePath, serviceSource.replace('namespace App\\Services;', ''));
+    expect(await callees()).toEqual(['SettleService::getSettlesToExcel']);
+  });
+
+  it.each(['method missing', 'class outside the index'])('leaves the call unresolved when the imported %s', async (scenario) => {
+    if (scenario === 'method missing') {
+      write(servicePath, serviceSource.replace('getSettlesToExcel', 'otherMethod'));
+    } else {
+      fs.rmSync(path.join(dir, servicePath));
+      // Even the right short name in the wrong namespace cannot donate a method.
+      write('app/Repositories/SettleService.php', serviceSource.replace('App\\Services', 'App\\Repositories'));
+    }
+    expect(await callees()).toEqual([]);
+  });
+
+  it('keeps a variable and a static receiver with the same spelling in separate namespaces', async () => {
+    write('app/Services/OtherService.php', String.raw`<?php
+namespace App\Services;
+class OtherService {
+    public function getSettlesToExcel() {}
+}
+`);
+    write(controllerPath, controllerSource.replace(
+      'return Settle::',
+      '$Settle = new OtherService();\n        $Settle->getSettlesToExcel();\n        return Settle::',
+    ));
+    expect((await callees()).filter((name) => name.endsWith('::getSettlesToExcel'))).toEqual([
+      'App\\Services::OtherService::getSettlesToExcel',
+      serviceMethod,
+    ]);
+  });
+});

+ 41 - 0
src/resolution/import-resolver.ts

@@ -1373,6 +1373,47 @@ function pickClosestJvmCandidate(candidates: Node[], fromPath: string): Node {
   return best;
 }
 
+/**
+ * PHP scoped calls are encoded as "Alias.method" by both extractors. A use
+ * mapping names a namespace, not a filesystem path, so resolve the receiver
+ * through its localName and look up the method on that exact imported type.
+ * undefined means this is not an imported static call; null means the import
+ * owns the call but its method is unavailable, so name fallbacks must not guess.
+ */
+export function resolvePhpImportedStaticCall(
+  ref: UnresolvedRef,
+  context: ResolutionContext,
+): ResolvedRef | null | undefined {
+  if (ref.language !== 'php' || ref.referenceKind !== 'calls') return undefined;
+  const call = /^(\w+)\.(\w+)$/.exec(ref.referenceName);
+  if (!call) return undefined;
+  const [, receiver, member] = call;
+  const imp = context.getImportMappings(ref.filePath, ref.language)
+    .find((i) => i.localName === receiver);
+  if (!imp) return undefined;
+
+  // PHP variables occupy a different namespace from class imports. Extraction
+  // strips the leading "$" from "$Alias->method()" too; leave that receiver to
+  // local type inference even when a class import has the same local name.
+  const lines = context.getFileLines?.(ref.filePath) ?? context.readFile(ref.filePath)?.split('\n');
+  const line = lines?.[ref.line - 1];
+  if (line?.slice(ref.column).startsWith('$')) return undefined;
+
+  const fqn = imp.source.replace(/^\\/, '');
+  const separator = fqn.lastIndexOf('\\');
+  const typeName = separator < 0
+    ? fqn
+    : `${fqn.slice(0, separator)}::${fqn.slice(separator + 1)}`;
+  const owners = context.getNodesByQualifiedName(typeName)
+    .filter((n) => n.language === 'php' && STATIC_MEMBER_CONTAINERS.has(n.kind));
+  if (owners.length !== 1) return null;
+  const owner = owners[0]!;
+  const methods = context.getNodesByQualifiedName(`${owner.qualifiedName}::${member}`)
+    .filter((n) => n.language === 'php' && n.kind === 'method' && n.filePath === owner.filePath);
+  if (methods.length !== 1) return null;
+  return { original: ref, targetNodeId: methods[0]!.id, confidence: 0.95, resolvedBy: 'import' };
+}
+
 export function resolveViaImport(
   ref: UnresolvedRef,
   context: ResolutionContext

+ 7 - 1
src/resolution/index.ts

@@ -17,7 +17,7 @@ import {
   ImportMapping,
 } from './types';
 import { isVisibleAcrossFiles, matchReference, matchFunctionRef, matchDottedCallChain, matchScopedCallChain, matchMethodCall, sameLanguageFamily, crossesKnownFamily, dumpNameMatcherProfile, clearNameMatcherMemos } from './name-matcher';
-import { resolveViaImport, resolveJvmImport, extractImportMappings, extractReExports, loadCppIncludeDirs, isPhpIncludePathRef, isCobolCopybookRef, isNixPathImportRef, clearImportResolverMemos, resolveImportPath } from './import-resolver';
+import { resolveViaImport, resolvePhpImportedStaticCall, resolveJvmImport, extractImportMappings, extractReExports, loadCppIncludeDirs, isPhpIncludePathRef, isCobolCopybookRef, isNixPathImportRef, clearImportResolverMemos, resolveImportPath } from './import-resolver';
 import { ResolverPool, minRefsForPool } from './resolver-pool';
 import { detectFrameworks } from './frameworks';
 import { synthesizeCallbackEdges } from './callback-synthesizer';
@@ -958,6 +958,12 @@ export class ReferenceResolver {
       if (razorResult) return razorResult;
     }
 
+    // An explicit PHP class import owns its static calls, including an
+    // unavailable method. Do not let same-name fallbacks change the receiver
+    // to an unrelated Service/Repository type (#1545).
+    const phpStaticImport = resolvePhpImportedStaticCall(ref, this.context);
+    if (phpStaticImport !== undefined) return this.gateLanguage(phpStaticImport, ref);
+
     const candidates: ResolvedRef[] = [];
 
     // Strategy 1: Try framework-specific resolution. Cross-language bridges