Skip to content

Commit 4ad26f6

Browse files
authored
chore: Simplify extension (#25)
## Summary ### Simplification (`chore: Simplify extension`) - **`providers.ts`**: Extracted `delegateLocationProvider()` helper shared by `PHPXDefinitionProvider`, `PHPXTypeDefinitionProvider`, and `PHPXImplementationProvider` — eliminated ~60 lines of triplicated boilerplate. Simplified `mapLocationLinkToSource` using a single `remapUri` variable instead of three mutable lets. Removed dead `PHPXWorkspaceSymbolProvider` class (exported but never registered in `extension.ts`). - **`compiler.ts`**: Moved `stderrOverflow` declaration above its handler (was declared one line after use). Added `stderrOverflow` to the `close` handler's overflow branch — previously only `truncated` was checked, leaving stderr-overflow kills to fall through to `code !== 0` and attempt `JSON.parse` on a truncated buffer. Removed dead `compilationErrors` map and `getCompilationError()` method (written but never read externally). Fixed `err.message || err.stack` order in `compileAndWrite` catch — stack now used for logging, message for user-facing error. - **`diagnostics.ts`**: Removed `shouldForwardDiagnostic()` stub (always returned `true`) and its no-op `.filter()` call site. - **`positionMapper.ts`**: Removed `/g` flag from module-level `PHP_WORD_PATTERN` — `document.getWordRangeAtPosition()` throws if passed a global regex (this was silently swallowing the error in all three location providers). Fixed double-`statSync` on cache miss: stat first, then read. - **`languageClient.ts`**: Removed redundant `?? undefined` from optional-chain expression. - **`extension.ts`**: Deduplicated the three identical vendorWatcher event handlers into a shared `clearCompilerCache` callback. ### Bug fix (`fix(positionMapper): use fresh regex in getWordAt for matchAll compatibility`) - `getWordAt` uses `String.matchAll()` which requires a global regex, but `PHP_WORD_PATTERN` no longer has `/g`. Fixed by creating a fresh `new RegExp(PHP_WORD_PATTERN.source, 'g')` at the call site — consistent with the pattern already used in `getNthOccurrence` and `findNthOccurrence`. ### Tests (`test(extension): fix false positives and strengthen assertions`) - **`extension.test.ts`**: Replaced circular `languageId` test (was forcibly setting `language: 'phpx'` then asserting `languageId === 'phpx'`) with a real auto-detection test that opens a temp `.phpx` file by URI. - **`grammar.test.ts`**: Removed `runGrammarTests()` — it looped over 12 test cases with `expectedScopes` arrays that were never read; every case unconditionally incremented `passed`. Actual tokenization is covered by `pnpm test:grammar` via `vscode-tmgrammar-test`. Strengthened `validateGrammarStructure` to assert non-empty `patterns` and `repository`. - **`range-remap.test.ts`**: Fixed `executeCommand` mocks to capture and assert the forwarded PHP URI and PHPX→PHP mapped column — the forward position mapping was previously completely untested. Added end-position assertions to all four tests (only start was previously checked). Added `phpxTitleStart !== phpTitleStart` guard to guarantee the fixture requires non-trivial column remapping. ## Test plan - [x] TypeScript compilation passes (`pnpm compile` — no errors) - [x] All 8 integration tests pass (`pnpm test`) - [x] Grammar tests pass (`pnpm test:grammar`) - [ ] Go-to-definition, type definition, and find implementation work in `.phpx` files - [ ] stderr overflow during compilation produces "Compilation output exceeded size limit" instead of a JSON parse error - [ ] No regression in diagnostics forwarding from PHP language server to `.phpx` files 🤖 Generated with [Claude Code](https://claude.com/claude-code)
1 parent a9956e5 commit 4ad26f6

9 files changed

Lines changed: 205 additions & 385 deletions

File tree

extension/src/compiler.ts

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ export interface CompilationResult {
1414
*/
1515
export class PHPXCompiler {
1616
private outputChannel: vscode.OutputChannel;
17-
private compilationErrors: Map<string, string> = new Map();
1817
/** Cache: workspace folder fsPath → resolved compiler script path (or null if not found) */
1918
private compilerScriptCache: Map<string, string | null> = new Map();
2019
/** Cache: workspace folder fsPath → resolved project root */
@@ -178,6 +177,7 @@ export class PHPXCompiler {
178177
let stdout = '';
179178
let stderr = '';
180179
let truncated = false;
180+
let stderrOverflow = false;
181181

182182
proc.stdout!.on('data', (data: Buffer) => {
183183
try {
@@ -194,7 +194,7 @@ export class PHPXCompiler {
194194
proc.kill();
195195
}
196196
});
197-
let stderrOverflow = false;
197+
198198
proc.stderr!.on('data', (data: Buffer) => {
199199
try {
200200
if (stderr.length < maxOutputBytes) {
@@ -218,13 +218,12 @@ export class PHPXCompiler {
218218
});
219219

220220
proc.on('close', (code) => {
221-
if (truncated) {
221+
if (truncated || stderrOverflow) {
222222
resolve({
223223
php: '',
224224
error: 'Compilation output exceeded size limit',
225225
});
226226
} else if (code === 0) {
227-
this.compilationErrors.delete(documentUri.toString());
228227
resolve({ php: stdout });
229228
} else {
230229
let errorMessage = stderr;
@@ -234,7 +233,6 @@ export class PHPXCompiler {
234233
} catch {
235234
// stderr is not JSON, use as-is
236235
}
237-
this.compilationErrors.set(documentUri.toString(), errorMessage);
238236
resolve({
239237
php: '',
240238
error: errorMessage,
@@ -306,20 +304,11 @@ export class PHPXCompiler {
306304

307305
return { phpUri };
308306
} catch (err) {
309-
const error =
310-
(err instanceof Error
311-
? err.message || err.stack || String(err)
312-
: String(err)) || 'Unknown compilation error';
313-
const errorLog = err instanceof Error ? err.stack || error : error;
307+
const message = err instanceof Error ? err.message || String(err) : String(err);
308+
const errorLog = err instanceof Error ? err.stack || message : message;
314309
this.outputChannel.appendLine(`[PHPX] compileAndWrite error — ${errorLog}`);
315-
return { phpUri, error };
310+
return { phpUri, error: message || 'Unknown compilation error' };
316311
}
317312
}
318313

319-
/**
320-
* Get the last compilation error for a document
321-
*/
322-
getCompilationError(documentUri: vscode.Uri): string | undefined {
323-
return this.compilationErrors.get(documentUri.toString());
324-
}
325314
}

extension/src/diagnostics.ts

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,7 @@ export class PHPXDiagnosticsManager {
6060
}
6161

6262
// Forward diagnostics to the PHPX file with remapped positions
63-
const forwarded = phpDiagnostics
64-
.filter((d) => this.shouldForwardDiagnostic(d))
65-
.map((d) => {
63+
const forwarded = phpDiagnostics.map((d) => {
6664
const mappedRange = mapRangeToPhpx(uri, phpxUri, d.range);
6765
const mapped = new vscode.Diagnostic(
6866
mappedRange,
@@ -111,15 +109,6 @@ export class PHPXDiagnosticsManager {
111109
}
112110
}
113111

114-
/**
115-
* Filter out diagnostics that are artifacts of the PHPX→PHP compilation.
116-
*/
117-
private shouldForwardDiagnostic(diagnostic: vscode.Diagnostic): boolean {
118-
// Forward all diagnostics for now.
119-
// Can be refined later to filter out compilation artifacts.
120-
return true;
121-
}
122-
123112
/**
124113
* Show a compiler failure as a diagnostic on the PHPX file.
125114
* This is separate from LSP parse diagnostics — it indicates the

extension/src/extension.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -168,9 +168,10 @@ export function activate(context: vscode.ExtensionContext) {
168168

169169
// Invalidate caches when vendor/composer files change
170170
const vendorWatcher = vscode.workspace.createFileSystemWatcher('**/vendor/autoload.php');
171-
vendorWatcher.onDidCreate(() => compiler.clearCache());
172-
vendorWatcher.onDidChange(() => compiler.clearCache());
173-
vendorWatcher.onDidDelete(() => compiler.clearCache());
171+
const clearCompilerCache = () => compiler.clearCache();
172+
vendorWatcher.onDidCreate(clearCompilerCache);
173+
vendorWatcher.onDidChange(clearCompilerCache);
174+
vendorWatcher.onDidDelete(clearCompilerCache);
174175
context.subscriptions.push(vendorWatcher);
175176

176177
// Clean up when a PHPX file is closed

extension/src/languageClient.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,7 @@ export function startLanguageClient(
3838

3939
outputChannel.appendLine(`[PHPX LSP] Using server script: ${serverScript}`);
4040

41-
const cwd =
42-
vscode.workspace.workspaceFolders?.[0]?.uri.fsPath ?? undefined;
41+
const cwd = vscode.workspace.workspaceFolders?.[0]?.uri.fsPath;
4342

4443
const serverOptions: ServerOptions = {
4544
command: phpPath,

extension/src/positionMapper.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import * as vscode from 'vscode';
1717
* PHP identifier pattern: variable ($name) or identifier (name).
1818
* Matches the same tokens the PHP language server would consider "words".
1919
*/
20-
const PHP_WORD_PATTERN = /\$?[a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*/g;
20+
const PHP_WORD_PATTERN = /\$?[a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*/;
2121

2222
/**
2323
* File content cache with mtime validation.
@@ -85,8 +85,8 @@ function getFileContent(uri: vscode.Uri): string | null {
8585

8686
// 3. Read from disk and cache
8787
try {
88-
const content = fs.readFileSync(fsPath, 'utf-8');
8988
const stat = fs.statSync(fsPath);
89+
const content = fs.readFileSync(fsPath, 'utf-8');
9090
fileCache.set(fsPath, { content, mtime: stat.mtimeMs });
9191
return content;
9292
} catch {
@@ -247,7 +247,7 @@ function getWordAt(
247247
line: string,
248248
charOffset: number,
249249
): { text: string; start: number } | null {
250-
const matches = [...line.matchAll(PHP_WORD_PATTERN)];
250+
const matches = [...line.matchAll(new RegExp(PHP_WORD_PATTERN.source, 'g'))];
251251
for (const match of matches) {
252252
const start = match.index!;
253253
const end = start + match[0].length;

extension/src/providers.ts

Lines changed: 54 additions & 134 deletions
Original file line numberDiff line numberDiff line change
@@ -46,69 +46,73 @@ function mapLocationLinkToSource(
4646
phpxUri: vscode.Uri,
4747
phpUri: vscode.Uri,
4848
): vscode.LocationLink {
49-
let targetUri = link.targetUri;
50-
let targetRange = link.targetRange;
51-
let targetSelectionRange = link.targetSelectionRange;
52-
53-
if (targetUri.fsPath === phpUri.fsPath) {
54-
targetRange = mapRangeToPhpx(phpUri, phpxUri, link.targetRange);
55-
targetSelectionRange = link.targetSelectionRange
56-
? mapRangeToPhpx(phpUri, phpxUri, link.targetSelectionRange)
57-
: undefined;
58-
targetUri = phpxUri;
59-
} else if (PHPXCompiler.hasPhpxSource(targetUri)) {
60-
const sourceUri = PHPXCompiler.getSourceUri(targetUri);
61-
targetRange = mapRangeToPhpx(targetUri, sourceUri, link.targetRange);
62-
targetSelectionRange = link.targetSelectionRange
63-
? mapRangeToPhpx(targetUri, sourceUri, link.targetSelectionRange)
64-
: undefined;
65-
targetUri = sourceUri;
49+
let remapUri: vscode.Uri | undefined;
50+
if (link.targetUri.fsPath === phpUri.fsPath) {
51+
remapUri = phpxUri;
52+
} else if (PHPXCompiler.hasPhpxSource(link.targetUri)) {
53+
remapUri = PHPXCompiler.getSourceUri(link.targetUri);
6654
}
55+
6756
return {
6857
originSelectionRange: link.originSelectionRange
6958
? mapRangeToPhpx(phpUri, phpxUri, link.originSelectionRange)
7059
: undefined,
71-
targetUri,
72-
targetRange,
73-
targetSelectionRange,
60+
targetUri: remapUri ?? link.targetUri,
61+
targetRange: remapUri
62+
? mapRangeToPhpx(link.targetUri, remapUri, link.targetRange)
63+
: link.targetRange,
64+
targetSelectionRange: remapUri && link.targetSelectionRange
65+
? mapRangeToPhpx(link.targetUri, remapUri, link.targetSelectionRange)
66+
: link.targetSelectionRange,
7467
};
7568
}
7669

7770
// ─── Completion and Hover are handled by the PHPX Language Server (LSP) ─────
7871

72+
/**
73+
* Delegate a location-style command to the compiled PHP file and remap results back to PHPX.
74+
* Shared by Definition, TypeDefinition, and Implementation providers.
75+
*/
76+
async function delegateLocationProvider(
77+
document: vscode.TextDocument,
78+
position: vscode.Position,
79+
command: string,
80+
): Promise<vscode.Definition | vscode.LocationLink[] | null> {
81+
const phpUri = PHPXCompiler.getCompiledUri(document.uri);
82+
const phpxUri = document.uri;
83+
const mappedPosition = mapPositionToPhp(document, position, phpUri);
84+
85+
try {
86+
const result = await vscode.commands.executeCommand<
87+
(vscode.Location | vscode.LocationLink)[]
88+
>(command, phpUri, mappedPosition);
89+
90+
if (!result || result.length === 0) {
91+
return null;
92+
}
93+
94+
if ('targetUri' in result[0]) {
95+
return (result as vscode.LocationLink[]).map((item) =>
96+
mapLocationLinkToSource(item, phpxUri, phpUri),
97+
);
98+
}
99+
return (result as vscode.Location[]).map((item) =>
100+
mapLocationToSource(item, phpxUri, phpUri),
101+
);
102+
} catch {
103+
return null;
104+
}
105+
}
106+
79107
// ─── Definition ──────────────────────────────────────────────────────────────
80108

81109
export class PHPXDefinitionProvider implements vscode.DefinitionProvider {
82-
async provideDefinition(
110+
provideDefinition(
83111
document: vscode.TextDocument,
84112
position: vscode.Position,
85113
_token: vscode.CancellationToken,
86114
): Promise<vscode.Definition | vscode.LocationLink[] | null> {
87-
const phpUri = PHPXCompiler.getCompiledUri(document.uri);
88-
const phpxUri = document.uri;
89-
const mappedPosition = mapPositionToPhp(document, position, phpUri);
90-
91-
try {
92-
const result = await vscode.commands.executeCommand<
93-
(vscode.Location | vscode.LocationLink)[]
94-
>('vscode.executeDefinitionProvider', phpUri, mappedPosition);
95-
96-
if (!result || result.length === 0) {
97-
return null;
98-
}
99-
100-
// Separate into typed arrays to satisfy the union return type
101-
if ('targetUri' in result[0]) {
102-
return (result as vscode.LocationLink[]).map((item) =>
103-
mapLocationLinkToSource(item, phpxUri, phpUri),
104-
);
105-
}
106-
return (result as vscode.Location[]).map((item) =>
107-
mapLocationToSource(item, phpxUri, phpUri),
108-
);
109-
} catch {
110-
return null;
111-
}
115+
return delegateLocationProvider(document, position, 'vscode.executeDefinitionProvider');
112116
}
113117
}
114118

@@ -117,35 +121,12 @@ export class PHPXDefinitionProvider implements vscode.DefinitionProvider {
117121
export class PHPXTypeDefinitionProvider
118122
implements vscode.TypeDefinitionProvider
119123
{
120-
async provideTypeDefinition(
124+
provideTypeDefinition(
121125
document: vscode.TextDocument,
122126
position: vscode.Position,
123127
_token: vscode.CancellationToken,
124128
): Promise<vscode.Definition | vscode.LocationLink[] | null> {
125-
const phpUri = PHPXCompiler.getCompiledUri(document.uri);
126-
const phpxUri = document.uri;
127-
const mappedPosition = mapPositionToPhp(document, position, phpUri);
128-
129-
try {
130-
const result = await vscode.commands.executeCommand<
131-
(vscode.Location | vscode.LocationLink)[]
132-
>('vscode.executeTypeDefinitionProvider', phpUri, mappedPosition);
133-
134-
if (!result || result.length === 0) {
135-
return null;
136-
}
137-
138-
if ('targetUri' in result[0]) {
139-
return (result as vscode.LocationLink[]).map((item) =>
140-
mapLocationLinkToSource(item, phpxUri, phpUri),
141-
);
142-
}
143-
return (result as vscode.Location[]).map((item) =>
144-
mapLocationToSource(item, phpxUri, phpUri),
145-
);
146-
} catch {
147-
return null;
148-
}
129+
return delegateLocationProvider(document, position, 'vscode.executeTypeDefinitionProvider');
149130
}
150131
}
151132

@@ -257,44 +238,6 @@ export class PHPXDocumentSymbolProvider
257238
}
258239
}
259240

260-
// ─── Workspace Symbols ──────────────────────────────────────────────────────
261-
262-
export class PHPXWorkspaceSymbolProvider
263-
implements vscode.WorkspaceSymbolProvider
264-
{
265-
async provideWorkspaceSymbols(
266-
query: string,
267-
_token: vscode.CancellationToken,
268-
): Promise<vscode.SymbolInformation[] | null> {
269-
try {
270-
const result = await vscode.commands.executeCommand<
271-
vscode.SymbolInformation[]
272-
>('vscode.executeWorkspaceSymbolProvider', query);
273-
274-
if (!result) {
275-
return null;
276-
}
277-
278-
return result.map((symbol) => {
279-
const loc = symbol.location;
280-
if (!PHPXCompiler.hasPhpxSource(loc.uri)) {
281-
return symbol;
282-
}
283-
const sourceUri = PHPXCompiler.getSourceUri(loc.uri);
284-
const range = mapRangeToPhpx(loc.uri, sourceUri, loc.range);
285-
return new vscode.SymbolInformation(
286-
symbol.name,
287-
symbol.kind,
288-
symbol.containerName ?? '',
289-
new vscode.Location(sourceUri, range),
290-
);
291-
});
292-
} catch {
293-
return null;
294-
}
295-
}
296-
}
297-
298241
// ─── Code Actions ────────────────────────────────────────────────────────────
299242

300243
export class PHPXCodeActionProvider implements vscode.CodeActionProvider {
@@ -455,34 +398,11 @@ export class PHPXRenameProvider implements vscode.RenameProvider {
455398
export class PHPXImplementationProvider
456399
implements vscode.ImplementationProvider
457400
{
458-
async provideImplementation(
401+
provideImplementation(
459402
document: vscode.TextDocument,
460403
position: vscode.Position,
461404
_token: vscode.CancellationToken,
462405
): Promise<vscode.Definition | vscode.LocationLink[] | null> {
463-
const phpUri = PHPXCompiler.getCompiledUri(document.uri);
464-
const phpxUri = document.uri;
465-
const mappedPosition = mapPositionToPhp(document, position, phpUri);
466-
467-
try {
468-
const result = await vscode.commands.executeCommand<
469-
(vscode.Location | vscode.LocationLink)[]
470-
>('vscode.executeImplementationProvider', phpUri, mappedPosition);
471-
472-
if (!result || result.length === 0) {
473-
return null;
474-
}
475-
476-
if ('targetUri' in result[0]) {
477-
return (result as vscode.LocationLink[]).map((item) =>
478-
mapLocationLinkToSource(item, phpxUri, phpUri),
479-
);
480-
}
481-
return (result as vscode.Location[]).map((item) =>
482-
mapLocationToSource(item, phpxUri, phpUri),
483-
);
484-
} catch {
485-
return null;
486-
}
406+
return delegateLocationProvider(document, position, 'vscode.executeImplementationProvider');
487407
}
488408
}

0 commit comments

Comments
 (0)