diff --git a/.release-please-manifest.json b/.release-please-manifest.json index 895bf0e..b725ab0 100644 --- a/.release-please-manifest.json +++ b/.release-please-manifest.json @@ -1,3 +1,3 @@ { - ".": "2.0.0" + ".": "2.0.1" } diff --git a/CHANGELOG.md b/CHANGELOG.md index 23de1e1..86c6eea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,19 @@ All notable changes to this project will be documented in this file. See [standard-version](https://github.com/conventional-changelog/standard-version) for commit guidelines. +## [2.0.1](https://github.com/rejmann/php-namespace-refactor/compare/php-namespace-refactor-v2.0.0...php-namespace-refactor-v2.0.1) (2026-07-31) + + +### 🐛 Bug Fixes + +* **property-rename:** default mismatched property renames to on ([c8bb98f](https://github.com/rejmann/php-namespace-refactor/commit/c8bb98f283d66cd0cf18df810af871ac5e312667)) + + +### ♻️ Code Refactoring + +* **property-rename:** fold property renames into the batch update pass ([8b7ef0a](https://github.com/rejmann/php-namespace-refactor/commit/8b7ef0a68e72ed190c83090422d366428624624e)) +* **workspace:** add polymorphic flag resolution for workspace settings ([fde3853](https://github.com/rejmann/php-namespace-refactor/commit/fde38531ab175cb018e12bd00f483dc7a1b8523a)) + ## [2.0.0](https://github.com/rejmann/php-namespace-refactor/compare/php-namespace-refactor-v1.9.2...php-namespace-refactor-v2.0.0) (2026-07-30) diff --git a/README.md b/README.md index 3ca1c70..2553eb1 100644 --- a/README.md +++ b/README.md @@ -26,6 +26,18 @@ Ideal for projects using PSR-4, making it easy to reorganize directories without - Rename Properties (off by default): When a class is renamed, also rename its class-typed constructor properties (promoted or not, readonly or not) and their `$this->x` usages to match the new class name. +### 🩺 Diagnostics and quick fixes + +Beyond the move/rename flow, the extension also watches files as you edit them and surfaces a few checks in the Problems panel, each individually toggleable: + +- Namespace Mismatch Diagnostics: Warns when a file's declared namespace doesn't match its PSR-4 location, with a quick fix to correct it in place (no move required). + +- Highlight Not Imported: Warns when a class is used in the file but not imported, whenever it resolves to exactly one class elsewhere in the workspace — with a quick fix to add the `use` statement. Ambiguous matches (the same class name found in more than one place) are left alone rather than guessed. + +- Highlight Not Used: Flags `use` imports that are never referenced in the file, with a quick fix to remove them. + +- Remove On Save / Sort On Save (off by default): Automatically remove unused imports and/or sort the remaining ones every time a PHP file is saved, independently of any move/rename operation. Sort order is configurable (natural, length, or alphabetical). + ## Requirements - PHP 7.4+ @@ -50,7 +62,13 @@ This extension contributes the following settings: ], "phpNamespaceRefactor.rename": true, "phpNamespaceRefactor.editFilesInBackground": true, - "phpNamespaceRefactor.renameProperties": false + "phpNamespaceRefactor.renameProperties": false, + "phpNamespaceRefactor.namespaceMismatchDiagnostics": true, + "phpNamespaceRefactor.highlightNotUsed": true, + "phpNamespaceRefactor.highlightNotImported": true, + "phpNamespaceRefactor.removeOnSave": false, + "phpNamespaceRefactor.sortOnSave": false, + "phpNamespaceRefactor.sortMode": "natural" } ``` @@ -111,6 +129,46 @@ This extension contributes the following settings: - Default: false. +**phpNamespaceRefactor.namespaceMismatchDiagnostics** + +- Shows a warning and a quick fix when a file's declared namespace doesn't match its PSR-4 location, without requiring a move/rename to fix it. + +- Default: true. + +**phpNamespaceRefactor.highlightNotUsed** + +- Shows a hint and a quick fix for `use` imports that are never referenced in the file. + +- Default: true. + +**phpNamespaceRefactor.highlightNotImported** + +- Shows a warning and a quick fix for classes used in the file that resolve to exactly one class elsewhere in the workspace but aren't imported yet. If the class name matches more than one location in the workspace, it's left alone rather than guessed. + +- Default: true. + +**phpNamespaceRefactor.removeOnSave** + +- Automatically removes unused `use` imports every time a PHP file is saved, independently of any move/rename operation. + +- Default: false. + +**phpNamespaceRefactor.sortOnSave** + +- Automatically sorts `use` imports every time a PHP file is saved, using the order configured in `phpNamespaceRefactor.sortMode`. +- When combined with `removeOnSave`, both happen together as a single edit. + +- Default: false. + +**phpNamespaceRefactor.sortMode** + +- Sort order used by `phpNamespaceRefactor.sortOnSave`. One of: + - `natural`: case-insensitive, numeric-aware order (e.g. `Item2` before `Item10`). + - `length`: shortest `use` statement first. + - `alphabetical`: strict character-by-character order. + +- Default: "natural". + ## Documentation For architecture, internals, and troubleshooting notes, see [./docs/](./docs/README.md). diff --git a/docs/README.md b/docs/README.md index 4bd9b37..6cb8d80 100644 --- a/docs/README.md +++ b/docs/README.md @@ -12,6 +12,7 @@ Documentation to support development and troubleshooting of the PHP Namespace Re - **[Namespace rename](./operations/namespace-rename.md)** — F2 on `namespace Foo\Bar;` - **[Class rename](./operations/class-rename.md)** — F2 on `class Foo`/`interface Foo`/`trait Foo` - **[File move](./operations/file-move.md)** — drag-and-drop in the Explorer (and the convergence point of the two flows above) +- **[Diagnostics and quick fixes](./diagnostics.md)** — namespace-mismatch/unused-import/missing-import checks in the Problems panel, and the `removeOnSave`/`sortOnSave` save-time edits ## Infrastructure diff --git a/docs/configuration.md b/docs/configuration.md index 8b62283..cb82d19 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -13,13 +13,21 @@ All keys are centralized in `ConfigKeys` (`src/domain/workspace/ConfigurationLoc | `phpNamespaceRefactor.rename` | `RENAME` | `boolean` | `true` | The `phpNamespaceRefactor.rename` command (`extension.ts`) and the F2 keybinding's `when` clause (`package.json`) | | `phpNamespaceRefactor.editFilesInBackground` | `EDIT_FILES_IN_BACKGROUND` | `boolean` | `true` | `FileEditApplier` | | `phpNamespaceRefactor.renameProperties` | `RENAME_PROPERTIES` | `boolean \| { renameMismatchedNames?: boolean }` | `false` | `PropertyRenameSettingsResolver`, consumed by `MultiFileReferenceUpdater`/`PropertyRenameOperation` | +| `phpNamespaceRefactor.namespaceMismatchDiagnostics` | `NAMESPACE_MISMATCH_DIAGNOSTICS` | `boolean` | `true` | `NamespaceDiagnosticsBuilder` | +| `phpNamespaceRefactor.highlightNotUsed` | `HIGHLIGHT_NOT_USED` | `boolean` | `true` | `UnusedImportDiagnosticsBuilder` | +| `phpNamespaceRefactor.highlightNotImported` | `HIGHLIGHT_NOT_IMPORTED` | `boolean` | `true` | `MissingImportDiagnosticsBuilder` | +| `phpNamespaceRefactor.removeOnSave` | `REMOVE_ON_SAVE` | `boolean` | `false` | `UseStatementBlockEditsBuilder` | +| `phpNamespaceRefactor.sortOnSave` | `SORT_ON_SAVE` | `boolean` | `false` | `UseStatementBlockEditsBuilder` | +| `phpNamespaceRefactor.sortMode` | `SORT_MODE` | `"natural" \| "length" \| "alphabetical"` | `"natural"` | `UseStatementBlockEditsBuilder`, sorting logic in `UseStatementSorter` | + +See [diagnostics.md](./diagnostics.md) for how the last six are wired together (the three diagnostics share one subscriber; the two save-time settings share one edit builder to avoid overlapping edits). ## How configuration is read Three classes access `workspace.getConfiguration('phpNamespaceRefactor')`, each with a distinct purpose: - **`ConfigurationLocator`** (`src/domain/workspace/ConfigurationLocator.ts`) — generic read, used for settings of any type (`ignoredDirectories`, `additionalExtensions`) -- **`FeatureFlagManager`** (`src/domain/workspace/FeatureFlagManager.ts`) — specialized `boolean` read, with `defaultValue = true`. Used for every plain on/off flag (`autoImportNamespace`, `removeUnusedImports`, `rename`, `editFilesInBackground`) +- **`FeatureFlagManager`** (`src/domain/workspace/FeatureFlagManager.ts`) — specialized `boolean` read, with `defaultValue = true`. Used for every plain on/off flag (`autoImportNamespace`, `removeUnusedImports`, `rename`, `editFilesInBackground`, `namespaceMismatchDiagnostics`, `highlightNotUsed`, `highlightNotImported`) — `removeOnSave` and `sortOnSave` also go through it, but pass `defaultValue: false` explicitly since they mutate the file on every save and shouldn't be on by default - **`PropertyRenameSettingsResolver`** (`src/domain/property/PropertyRenameSettingsResolver.ts`) — the one setting whose raw value isn't a plain boolean; see [`phpNamespaceRefactor.renameProperties`](#phpnamespacerefactorrenameproperties) below None of the three caches the `WorkspaceConfiguration` — `ConfigurationLocator`/`FeatureFlagManager` read `workspace.getConfiguration()` in their constructor, and `PropertyRenameSettingsResolver` reads through a fresh `ConfigurationLocator` on every `resolve()` call. All three are `@injectable()` (not singleton), so a fresh read happens on every `container.resolve()`. This means a change to the user's configuration is picked up on the next operation, with no need to reload the window. diff --git a/docs/diagnostics.md b/docs/diagnostics.md new file mode 100644 index 0000000..562806c --- /dev/null +++ b/docs/diagnostics.md @@ -0,0 +1,57 @@ +# Diagnostics and quick fixes + +**Files:** `src/app/services/NamespaceDiagnosticsBuilder.ts`, `src/app/services/UnusedImportDiagnosticsBuilder.ts`, `src/app/services/MissingImportDiagnosticsBuilder.ts`, `src/app/services/MissingImportResolver.ts`, `src/app/services/SingleImportInserter.ts`, `src/app/services/UseStatementBlockEditsBuilder.ts`, `src/app/subscribers/NamespaceDiagnosticsSubscriber.ts`, `src/app/commands/NamespaceCodeActionProvider.ts`, `src/app/commands/UnusedImportCodeActionProvider.ts`, `src/app/commands/MissingImportCodeActionProvider.ts`, `src/infra/vscode/NamespaceDiagnosticCollection.ts` + +## Responsibility + +Separate from the move/rename/F2 flows (see [architecture.md](./architecture.md#main-flows)), the extension also watches PHP documents as they're opened, saved, and closed, and surfaces three independent checks in the Problems panel. Each has its own feature flag (see [configuration.md](./configuration.md)) and its own `Diagnostic.code`, so a `CodeActionProvider` only ever reacts to the diagnostics it knows how to fix. + +| Check | Builder | Diagnostic code | Severity | Quick fix (`CodeActionProvider`) | +|---|---|---|---|---| +| Declared namespace doesn't match PSR-4 location | `NamespaceDiagnosticsBuilder` | `namespace-mismatch` | Warning | `NamespaceCodeActionProvider` — replaces the `namespace ...;` line in place | +| `use` import never referenced in the file | `UnusedImportDiagnosticsBuilder` | `unused-import` | Hint (`DiagnosticTag.Unnecessary`, fades the text) | `UnusedImportCodeActionProvider` — deletes the whole line | +| Class used but not imported, resolves to exactly one class in the workspace | `MissingImportDiagnosticsBuilder` | `missing-import` | Warning | `MissingImportCodeActionProvider` — inserts a `use` statement via the `phpNamespaceRefactor.insertMissingImport` command | + +## Event flow + +`NamespaceDiagnosticsSubscriber` (registered in `extension.ts`) is the single entry point that runs all three builders and merges their output into one `NamespaceDiagnosticCollection` (a thin wrapper around `languages.createDiagnosticCollection`): + +``` +workspace.onDidOpenTextDocument → subscriber.handle(document) +workspace.onDidSaveTextDocument → subscriber.handle(document) +workspace.onDidCloseTextDocument → subscriber.clear(document) +workspace.textDocuments (on activate) → subscriber.handle(document), so already-open files get diagnostics immediately +``` + +There's no `onDidChangeActiveTextEditor`/keystroke-level trigger on purpose — recomputing on every keystroke would be noisy and unnecessary; open + save covers the practical editing workflow. + +## Namespace mismatch: why the range is trimmed + +`NAMESPACE_DECLARATION_REGEX` (`src/domain/namespace/PhpPatterns.ts`) allows leading blank lines via `\s*` — other call sites (`MovedFileNamespaceUpdater`) rely on that to normalize spacing when replacing the line. `NamespaceDiagnosticsBuilder` trims that leading whitespace off before building the diagnostic's `Range`, otherwise the warning underline (and the quick fix's replace range) would start on the blank line above `namespace ...;` instead of the line itself. + +## Missing import: resolving a bare identifier to a class + +`MissingImportCandidateLocator` (domain, pure) scans the document text for bare capitalized identifiers that aren't already namespace-qualified, aren't right after `::`/`->` (a member access, not a class reference), aren't already imported/aliased, and aren't the file's own class name. It also blanks out the `namespace ...;` declaration itself before scanning (same-length space padding, so every other match's offset stays correct) — otherwise `namespace App;` would have its own `App` segment mistaken for a used-but-unimported identifier. + +`MissingImportResolver` (app) takes each candidate and looks it up via `NamespaceIndex.findClassLocations()` (new method — derives each indexed file's class name from its file name, the same convention as `WorkspacePathResolver.extractClassNameFromPath`). A candidate is only ever flagged when: + +- exactly one file in the workspace declares a class by that name (zero matches = unresolved, more than one = ambiguous — both are skipped rather than guessed, same philosophy as the ambiguous-property-rename skip in `PropertyRenameOperation`), and +- that one match isn't already in the file's own declared namespace (already in scope without an import, same rule `MissingClassImporter` applies for the move flow) + +This means built-in/global PHP classes (`Exception`, `Closure`, etc.) are never flagged — they're not in the workspace's own namespace index, so they simply don't resolve. + +The quick fix (`MissingImportCodeActionProvider`) re-runs `MissingImportResolver` against the document rather than stashing the resolved FQCN on the diagnostic, then triggers the `phpNamespaceRefactor.insertMissingImport` command, which delegates to `SingleImportInserter` — a thin wrapper around the same `UseStatementLocator`/`UseStatementCreator`/`UseStatementInjector` trio `MissingClassImporter` already uses for the move flow, so insertion point, duplicate-avoidance, and blank-line handling all stay consistent between the two features. + +## Save-time edits: `removeOnSave` and `sortOnSave` + +Unlike the three diagnostics above, `removeOnSave` and `sortOnSave` don't go through the Problems panel — they mutate the file directly on save, via `workspace.onWillSaveTextDocument` + `event.waitUntil(...)`. This is deliberate: contributing edits through `waitUntil` folds them into the save that's already happening, so the file doesn't get saved once, then edited again into a dirty state (which is what using `onDidSaveTextDocument` + `workspace.applyEdit` would cause). + +Both behaviors are computed by a single class, `UseStatementBlockEditsBuilder`, instead of two independent subscribers. They act on the same contiguous block of `use` lines, so two separate edits (one deleting unused lines, one reordering the rest) would overlap — VS Code rejects overlapping edits within one `TextEdit[]`. Instead, the builder: + +1. Locates every `use` line (`USE_STATEMENT_REGEX`) and checks they're on consecutive document lines — if anything is interleaved (a comment, a blank line), it bails out entirely rather than risk reordering across it. +2. If `removeOnSave` is on, drops the ones `UnusedUseStatementLocator` reports as unreferenced. +3. If `sortOnSave` is on, reorders what's left via `UseStatementSorter`, using the mode from `phpNamespaceRefactor.sortMode` (`natural` | `length` | `alphabetical`). +4. If the result is identical to the original block, returns no edit at all. +5. Otherwise, replaces the whole original block (first `use` line to last) with the final block in a single `TextEdit`. + +If every import ends up removed, this can leave one blank line behind where the block used to be — a minor cosmetic gap, not a correctness issue, consistent with the regex-based (not AST-based) approach used throughout this codebase. diff --git a/package-lock.json b/package-lock.json index 7efccb9..f7d50a6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "php-namespace-refactor", - "version": "2.0.0", + "version": "2.0.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "php-namespace-refactor", - "version": "2.0.0", + "version": "2.0.1", "dependencies": { "reflect-metadata": "^0.2.2", "tsyringe": "^4.10.0" diff --git a/package.json b/package.json index e017ad6..aa4617f 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "name": "php-namespace-refactor", "displayName": "PHP Namespace Refactor", "description": "PHP Namespace Refactor: Extension for Visual Studio Code that automatically refactors namespace and references when moving PHP files between directories.", - "version": "2.0.0", + "version": "2.0.1", "author": { "name": "rejman", "url": "https://github.com/rejmann" @@ -125,6 +125,46 @@ }, "additionalProperties": false, "description": "Rename constructor-typed properties (promoted or not, readonly or not) - and their $this->x usages - to match the class name when renaming a class. Set to true to enable everything, including renaming mismatched names, or to an object like { \"renameMismatchedNames\": false } to opt out of just that behavior." + }, + "phpNamespaceRefactor.namespaceMismatchDiagnostics": { + "type": "boolean", + "default": true, + "description": "Show a warning and a quick fix when a file's declared namespace doesn't match its PSR-4 location, without requiring a move/rename to fix it." + }, + "phpNamespaceRefactor.highlightNotUsed": { + "type": "boolean", + "default": true, + "description": "Show a hint and a quick fix for \"use\" imports that are never referenced in the file." + }, + "phpNamespaceRefactor.highlightNotImported": { + "type": "boolean", + "default": true, + "description": "Show a warning and a quick fix for classes used in the file that resolve to exactly one class elsewhere in the workspace but aren't imported yet." + }, + "phpNamespaceRefactor.removeOnSave": { + "type": "boolean", + "default": false, + "description": "Automatically remove unused \"use\" imports every time a PHP file is saved, independently of any move/rename operation." + }, + "phpNamespaceRefactor.sortOnSave": { + "type": "boolean", + "default": false, + "description": "Automatically sort \"use\" imports every time a PHP file is saved, using the order configured in phpNamespaceRefactor.sortMode." + }, + "phpNamespaceRefactor.sortMode": { + "type": "string", + "enum": [ + "natural", + "length", + "alphabetical" + ], + "enumDescriptions": [ + "Natural order (case-insensitive, numeric-aware), e.g. Item2 before Item10.", + "Shortest \"use\" statement first.", + "Strict character-by-character alphabetical order." + ], + "default": "natural", + "description": "Sort order used by phpNamespaceRefactor.sortOnSave." } } } diff --git a/src/app/commands/MissingImportCodeActionProvider.ts b/src/app/commands/MissingImportCodeActionProvider.ts new file mode 100644 index 0000000..a5f5956 --- /dev/null +++ b/src/app/commands/MissingImportCodeActionProvider.ts @@ -0,0 +1,43 @@ +import { MISSING_IMPORT_CODE } from '@app/services/MissingImportDiagnosticsBuilder'; +import { MissingImportResolver } from '@app/services/MissingImportResolver'; +import { inject, injectable } from 'tsyringe'; +import { CodeAction, CodeActionContext, CodeActionKind, CodeActionProvider, Range, TextDocument } from 'vscode'; + +export const INSERT_MISSING_IMPORT_COMMAND = 'phpNamespaceRefactor.insertMissingImport'; + +@injectable() +export class MissingImportCodeActionProvider implements CodeActionProvider { + public static readonly providedCodeActionKinds = [CodeActionKind.QuickFix]; + + constructor( + @inject(MissingImportResolver) private missingImportResolver: MissingImportResolver, + ) {} + + public provideCodeActions(document: TextDocument, _range: Range, context: CodeActionContext): CodeAction[] { + const diagnostics = context.diagnostics.filter(diagnostic => diagnostic.code === MISSING_IMPORT_CODE); + if (diagnostics.length === 0) { + return []; + } + + const resolved = this.missingImportResolver.resolve(document); + + return diagnostics.flatMap((diagnostic) => { + const identifier = document.getText(diagnostic.range); + const match = resolved.find(candidate => candidate.identifier === identifier); + if (!match) { + return []; + } + + const action = new CodeAction(`Import "${match.fullNamespace}"`, CodeActionKind.QuickFix); + action.diagnostics = [diagnostic]; + action.isPreferred = true; + action.command = { + command: INSERT_MISSING_IMPORT_COMMAND, + title: 'Import class', + arguments: [document.uri, match.fullNamespace], + }; + + return [action]; + }); + } +} diff --git a/src/app/commands/NamespaceCodeActionProvider.ts b/src/app/commands/NamespaceCodeActionProvider.ts new file mode 100644 index 0000000..97a973b --- /dev/null +++ b/src/app/commands/NamespaceCodeActionProvider.ts @@ -0,0 +1,43 @@ +import { NAMESPACE_MISMATCH_CODE } from '@app/services/NamespaceDiagnosticsBuilder'; +import { NamespaceCreator } from '@domain/namespace/NamespaceCreator'; +import { inject, injectable } from 'tsyringe'; +import { + CodeAction, + CodeActionContext, + CodeActionKind, + CodeActionProvider, + Range, + TextDocument, + WorkspaceEdit, +} from 'vscode'; + +@injectable() +export class NamespaceCodeActionProvider implements CodeActionProvider { + public static readonly providedCodeActionKinds = [CodeActionKind.QuickFix]; + + constructor( + @inject(NamespaceCreator) private namespaceCreator: NamespaceCreator, + ) {} + + public async provideCodeActions(document: TextDocument, _range: Range, context: CodeActionContext): Promise { + const diagnostic = context.diagnostics.find(d => d.code === NAMESPACE_MISMATCH_CODE); + if (!diagnostic) { + return []; + } + + const { namespace: expectedNamespace } = await this.namespaceCreator.execute({ uri: document.uri }); + if (!expectedNamespace) { + return []; + } + + const action = new CodeAction('Fix namespace to match file location', CodeActionKind.QuickFix); + action.diagnostics = [diagnostic]; + action.isPreferred = true; + + const edit = new WorkspaceEdit(); + edit.replace(document.uri, diagnostic.range, `namespace ${expectedNamespace};`); + action.edit = edit; + + return [action]; + } +} diff --git a/src/app/commands/UnusedImportCodeActionProvider.ts b/src/app/commands/UnusedImportCodeActionProvider.ts new file mode 100644 index 0000000..0cac906 --- /dev/null +++ b/src/app/commands/UnusedImportCodeActionProvider.ts @@ -0,0 +1,36 @@ +import { UNUSED_IMPORT_CODE } from '@app/services/UnusedImportDiagnosticsBuilder'; +import { injectable } from 'tsyringe'; +import { + CodeAction, + CodeActionContext, + CodeActionKind, + CodeActionProvider, + Diagnostic, + Range, + TextDocument, + WorkspaceEdit, +} from 'vscode'; + +@injectable() +export class UnusedImportCodeActionProvider implements CodeActionProvider { + public static readonly providedCodeActionKinds = [CodeActionKind.QuickFix]; + + public provideCodeActions(document: TextDocument, _range: Range, context: CodeActionContext): CodeAction[] { + return context.diagnostics + .filter(diagnostic => diagnostic.code === UNUSED_IMPORT_CODE) + .map(diagnostic => this.buildRemoveAction(document, diagnostic)); + } + + private buildRemoveAction(document: TextDocument, diagnostic: Diagnostic): CodeAction { + const action = new CodeAction('Remove unused import', CodeActionKind.QuickFix); + action.diagnostics = [diagnostic]; + action.isPreferred = true; + + const line = document.lineAt(diagnostic.range.start.line); + const edit = new WorkspaceEdit(); + edit.delete(document.uri, line.rangeIncludingLineBreak); + action.edit = edit; + + return action; + } +} diff --git a/src/app/services/MissingImportDiagnosticsBuilder.ts b/src/app/services/MissingImportDiagnosticsBuilder.ts new file mode 100644 index 0000000..2439dae --- /dev/null +++ b/src/app/services/MissingImportDiagnosticsBuilder.ts @@ -0,0 +1,40 @@ +import { MissingImportResolver } from '@app/services/MissingImportResolver'; +import { Config, ConfigKeys } from '@domain/workspace/ConfigurationLocator'; +import { FeatureFlagManager } from '@domain/workspace/FeatureFlagManager'; +import { inject, injectable } from 'tsyringe'; +import { Diagnostic, DiagnosticSeverity, Range, TextDocument } from 'vscode'; + +export const MISSING_IMPORT_CODE = 'missing-import'; + +@injectable() +export class MissingImportDiagnosticsBuilder { + constructor( + @inject(FeatureFlagManager) private featureFlagManager: FeatureFlagManager, + @inject(MissingImportResolver) private missingImportResolver: MissingImportResolver, + ) {} + + public execute(document: TextDocument): Diagnostic[] { + if (!this.featureFlagManager.isActive({ key: ConfigKeys.HIGHLIGHT_NOT_IMPORTED })) { + return []; + } + + const resolved = this.missingImportResolver.resolve(document); + + return resolved.map(({ identifier, fullNamespace, index, length }) => { + const range = new Range( + document.positionAt(index), + document.positionAt(index + length), + ); + + const diagnostic = new Diagnostic( + range, + `Class "${identifier}" is not imported (found at "${fullNamespace}").`, + DiagnosticSeverity.Warning, + ); + diagnostic.code = MISSING_IMPORT_CODE; + diagnostic.source = Config; + + return diagnostic; + }); + } +} diff --git a/src/app/services/MissingImportResolver.ts b/src/app/services/MissingImportResolver.ts new file mode 100644 index 0000000..105ac63 --- /dev/null +++ b/src/app/services/MissingImportResolver.ts @@ -0,0 +1,62 @@ +import { MissingImportCandidateLocator } from '@domain/namespace/MissingImportCandidateLocator'; +import { NAMESPACE_DECLARATION_REGEX } from '@domain/namespace/PhpPatterns'; +import { WorkspacePathResolver } from '@domain/workspace/WorkspacePathResolver'; +import { NamespaceIndex } from '@infra/index/NamespaceIndex'; +import { inject, injectable } from 'tsyringe'; +import { TextDocument } from 'vscode'; + +export interface ResolvedMissingImport { + identifier: string + fullNamespace: string + index: number + length: number +} + +@injectable() +export class MissingImportResolver { + constructor( + @inject(WorkspacePathResolver) private workspacePathResolver: WorkspacePathResolver, + @inject(MissingImportCandidateLocator) private missingImportCandidateLocator: MissingImportCandidateLocator, + @inject(NamespaceIndex) private namespaceIndex: NamespaceIndex, + ) {} + + /** + * Only resolves a candidate identifier when the workspace index locates + * exactly one class by that name outside of the current file and outside + * the file's own declared namespace (already in scope, no import needed). + * Zero matches (unresolved) or more than one (ambiguous) are skipped + * rather than guessed - same "skip rather than guess" rule this extension + * already applies to ambiguous property renames. + */ + public resolve(document: TextDocument): ResolvedMissingImport[] { + const text = document.getText(); + const ownClassName = this.workspacePathResolver.extractClassNameFromPath(document.uri.fsPath); + const declaredNamespace = text.match(NAMESPACE_DECLARATION_REGEX)?.[1] ?? null; + + const candidates = this.missingImportCandidateLocator.execute(text, ownClassName); + + const resolved: ResolvedMissingImport[] = []; + for (const candidate of candidates) { + const locations = this.namespaceIndex.findClassLocations(candidate.identifier) + .filter(location => location.fsPath !== document.uri.fsPath); + + if (locations.length !== 1) { + continue; + } + + const [location] = locations; + if (location.namespace === declaredNamespace) { + continue; + } + + resolved.push({ + identifier: candidate.identifier, + fullNamespace: `${location.namespace}\\${candidate.identifier}`, + index: candidate.index, + length: candidate.length, + }); + } + + return resolved; + } +} diff --git a/src/app/services/NamespaceDiagnosticsBuilder.ts b/src/app/services/NamespaceDiagnosticsBuilder.ts new file mode 100644 index 0000000..b280534 --- /dev/null +++ b/src/app/services/NamespaceDiagnosticsBuilder.ts @@ -0,0 +1,58 @@ +import { NamespaceCreator } from '@domain/namespace/NamespaceCreator'; +import { NamespaceMismatchDetector } from '@domain/namespace/NamespaceMismatchDetector'; +import { NAMESPACE_DECLARATION_REGEX } from '@domain/namespace/PhpPatterns'; +import { Config, ConfigKeys } from '@domain/workspace/ConfigurationLocator'; +import { FeatureFlagManager } from '@domain/workspace/FeatureFlagManager'; +import { inject, injectable } from 'tsyringe'; +import { Diagnostic, DiagnosticSeverity, Range, TextDocument } from 'vscode'; + +export const NAMESPACE_MISMATCH_CODE = 'namespace-mismatch'; + +@injectable() +export class NamespaceDiagnosticsBuilder { + constructor( + @inject(FeatureFlagManager) private featureFlagManager: FeatureFlagManager, + @inject(NamespaceCreator) private namespaceCreator: NamespaceCreator, + @inject(NamespaceMismatchDetector) private namespaceMismatchDetector: NamespaceMismatchDetector, + ) {} + + public async execute(document: TextDocument): Promise { + if (!this.featureFlagManager.isActive({ key: ConfigKeys.NAMESPACE_MISMATCH_DIAGNOSTICS })) { + return []; + } + + const text = document.getText(); + const match = text.match(NAMESPACE_DECLARATION_REGEX); + if (!match) { + return []; + } + + const declaredNamespace = match[1]; + const { namespace: expectedNamespace } = await this.namespaceCreator.execute({ uri: document.uri }); + + const isMismatched = this.namespaceMismatchDetector.execute({ declaredNamespace, expectedNamespace }); + if (!isMismatched) { + return []; + } + + // NAMESPACE_DECLARATION_REGEX allows leading blank lines via `\s*` (other + // call sites rely on that to normalize spacing on replace) - trim it here + // so the diagnostic/quick-fix range covers only the "namespace ...;" line. + const leadingWhitespaceLength = match[0].match(/^\s*/)![0].length; + const startIndex = match.index! + leadingWhitespaceLength; + const range = new Range( + document.positionAt(startIndex), + document.positionAt(match.index! + match[0].length), + ); + + const diagnostic = new Diagnostic( + range, + `Namespace does not match the file's location. Expected "${expectedNamespace}".`, + DiagnosticSeverity.Warning, + ); + diagnostic.code = NAMESPACE_MISMATCH_CODE; + diagnostic.source = Config; + + return [diagnostic]; + } +} diff --git a/src/app/services/SingleImportInserter.ts b/src/app/services/SingleImportInserter.ts new file mode 100644 index 0000000..dcdc039 --- /dev/null +++ b/src/app/services/SingleImportInserter.ts @@ -0,0 +1,34 @@ +import { UseStatementCreator } from '@domain/namespace/UseStatementCreator'; +import { UseStatementInjector } from '@domain/namespace/UseStatementInjector'; +import { UseStatementLocator } from '@domain/namespace/UseStatementLocator'; +import { inject, injectable } from 'tsyringe'; +import { TextDocument, WorkspaceEdit } from 'vscode'; + +interface Props { + document: TextDocument + fullNamespace: string +} + +@injectable() +export class SingleImportInserter { + constructor( + @inject(UseStatementLocator) private useStatementLocator: UseStatementLocator, + @inject(UseStatementCreator) private useStatementCreator: UseStatementCreator, + @inject(UseStatementInjector) private useStatementInjector: UseStatementInjector, + ) {} + + public async execute({ document, fullNamespace }: Props): Promise { + const location = this.useStatementLocator.execute({ document }); + const useNamespace = this.useStatementCreator.single({ fullNamespace }); + + await this.useStatementInjector.save({ + document, + workspaceEdit: new WorkspaceEdit(), + uri: document.uri, + useNamespace, + lastUseEndIndex: location.index, + isFirstUse: location.isFirstUse, + flush: true, + }); + } +} diff --git a/src/app/services/UnusedImportDiagnosticsBuilder.ts b/src/app/services/UnusedImportDiagnosticsBuilder.ts new file mode 100644 index 0000000..7bff225 --- /dev/null +++ b/src/app/services/UnusedImportDiagnosticsBuilder.ts @@ -0,0 +1,41 @@ +import { UnusedUseStatementLocator } from '@domain/namespace/UnusedUseStatementLocator'; +import { Config, ConfigKeys } from '@domain/workspace/ConfigurationLocator'; +import { FeatureFlagManager } from '@domain/workspace/FeatureFlagManager'; +import { inject, injectable } from 'tsyringe'; +import { Diagnostic, DiagnosticSeverity, DiagnosticTag, Range, TextDocument } from 'vscode'; + +export const UNUSED_IMPORT_CODE = 'unused-import'; + +@injectable() +export class UnusedImportDiagnosticsBuilder { + constructor( + @inject(FeatureFlagManager) private featureFlagManager: FeatureFlagManager, + @inject(UnusedUseStatementLocator) private unusedUseStatementLocator: UnusedUseStatementLocator, + ) {} + + public execute(document: TextDocument): Diagnostic[] { + if (!this.featureFlagManager.isActive({ key: ConfigKeys.HIGHLIGHT_NOT_USED })) { + return []; + } + + const unusedImports = this.unusedUseStatementLocator.execute(document.getText()); + + return unusedImports.map(({ fullNamespace, index, length }) => { + const range = new Range( + document.positionAt(index), + document.positionAt(index + length), + ); + + const diagnostic = new Diagnostic( + range, + `Unused import: "${fullNamespace}".`, + DiagnosticSeverity.Hint, + ); + diagnostic.code = UNUSED_IMPORT_CODE; + diagnostic.source = Config; + diagnostic.tags = [DiagnosticTag.Unnecessary]; + + return diagnostic; + }); + } +} diff --git a/src/app/services/UseStatementBlockEditsBuilder.ts b/src/app/services/UseStatementBlockEditsBuilder.ts new file mode 100644 index 0000000..048b034 --- /dev/null +++ b/src/app/services/UseStatementBlockEditsBuilder.ts @@ -0,0 +1,86 @@ +import { USE_STATEMENT_REGEX } from '@domain/namespace/PhpPatterns'; +import { UnusedUseStatementLocator } from '@domain/namespace/UnusedUseStatementLocator'; +import { UseStatementSorter, UseStatementSortMode } from '@domain/namespace/UseStatementSorter'; +import { ConfigKeys, ConfigurationLocator } from '@domain/workspace/ConfigurationLocator'; +import { FeatureFlagManager } from '@domain/workspace/FeatureFlagManager'; +import { FILE_EXTENSION } from '@infra/utils/constants'; +import { inject, injectable } from 'tsyringe'; +import { Position, Range, TextDocument, TextEdit } from 'vscode'; + +interface Statement { + fullNamespace: string + line: string +} + +@injectable() +export class UseStatementBlockEditsBuilder { + constructor( + @inject(FeatureFlagManager) private featureFlagManager: FeatureFlagManager, + @inject(ConfigurationLocator) private configurationLocator: ConfigurationLocator, + @inject(UnusedUseStatementLocator) private unusedUseStatementLocator: UnusedUseStatementLocator, + @inject(UseStatementSorter) private useStatementSorter: UseStatementSorter, + ) {} + + /** + * Removing unused imports and sorting the remaining ones both act on the + * same contiguous block of `use` lines, so they're combined into a single + * replace edit here instead of two independent subscribers - two separate + * edits touching the same lines in the same onWillSaveTextDocument pass + * would overlap and VS Code rejects overlapping edits. + */ + public execute(document: TextDocument): TextEdit[] { + if (!document.fileName.endsWith(FILE_EXTENSION)) { + return []; + } + + const removeUnused = this.featureFlagManager.isActive({ key: ConfigKeys.REMOVE_ON_SAVE, defaultValue: false }); + const sort = this.featureFlagManager.isActive({ key: ConfigKeys.SORT_ON_SAVE, defaultValue: false }); + + if (!removeUnused && !sort) { + return []; + } + + const text = document.getText(); + const matches = [...text.matchAll(USE_STATEMENT_REGEX)]; + if (matches.length === 0) { + return []; + } + + const lineNumbers = matches.map(match => document.positionAt(match.index!).line); + const isContiguous = lineNumbers.every((line, i) => i === 0 || line === lineNumbers[i - 1] + 1); + if (!isContiguous) { + // Something else (a comment, blank line, etc.) sits between two `use` + // lines - skip rather than risk reordering across it and corrupting it. + return []; + } + + let statements: Statement[] = matches.map(match => ({ fullNamespace: match[1], line: match[0] })); + + if (removeUnused) { + const unusedFullNamespaces = new Set( + this.unusedUseStatementLocator.execute(text).map(unused => unused.fullNamespace), + ); + statements = statements.filter(statement => !unusedFullNamespaces.has(statement.fullNamespace)); + } + + if (sort) { + const mode = this.configurationLocator.get({ key: ConfigKeys.SORT_MODE, defaultValue: 'natural' }); + statements = this.useStatementSorter.sort(statements, mode); + } + + const originalLines = matches.map(match => match[0]); + const finalLines = statements.map(statement => statement.line); + if (finalLines.join('\n') === originalLines.join('\n')) { + return []; + } + + const startLine = lineNumbers[0]; + const endLine = lineNumbers[lineNumbers.length - 1]; + const range = new Range( + new Position(startLine, 0), + new Position(endLine, document.lineAt(endLine).text.length), + ); + + return [TextEdit.replace(range, finalLines.join('\n'))]; + } +} diff --git a/src/app/services/update/MovedFileNamespaceUpdater.ts b/src/app/services/update/MovedFileNamespaceUpdater.ts index 58af018..fc6d84a 100644 --- a/src/app/services/update/MovedFileNamespaceUpdater.ts +++ b/src/app/services/update/MovedFileNamespaceUpdater.ts @@ -1,3 +1,4 @@ +import { NAMESPACE_DECLARATION_REGEX } from '@domain/namespace/PhpPatterns'; import { FileEditApplier } from '@infra/vscode/FileEditApplier'; import { TextDocumentOpener } from '@infra/vscode/TextDocumentOpener'; import { inject, injectable } from 'tsyringe'; @@ -18,8 +19,7 @@ export class MovedFileNamespaceUpdater { public async execute({ newNamespace, newUri }: Props) { const { document, text } = await this.textDocumentOpener.execute({ uri: newUri }); - const namespaceRegex = /^\s*namespace\s+[\w\\]+;/m; - const match = text.match(namespaceRegex); + const match = text.match(NAMESPACE_DECLARATION_REGEX); if (!match) { return false; diff --git a/src/app/subscribers/NamespaceDiagnosticsSubscriber.ts b/src/app/subscribers/NamespaceDiagnosticsSubscriber.ts new file mode 100644 index 0000000..9af9ad1 --- /dev/null +++ b/src/app/subscribers/NamespaceDiagnosticsSubscriber.ts @@ -0,0 +1,34 @@ +import { MissingImportDiagnosticsBuilder } from '@app/services/MissingImportDiagnosticsBuilder'; +import { NamespaceDiagnosticsBuilder } from '@app/services/NamespaceDiagnosticsBuilder'; +import { UnusedImportDiagnosticsBuilder } from '@app/services/UnusedImportDiagnosticsBuilder'; +import { FILE_EXTENSION } from '@infra/utils/constants'; +import { NamespaceDiagnosticCollection } from '@infra/vscode/NamespaceDiagnosticCollection'; +import { inject, injectable } from 'tsyringe'; +import { TextDocument } from 'vscode'; + +@injectable() +export class NamespaceDiagnosticsSubscriber { + constructor( + @inject(NamespaceDiagnosticsBuilder) private namespaceDiagnosticsBuilder: NamespaceDiagnosticsBuilder, + @inject(UnusedImportDiagnosticsBuilder) private unusedImportDiagnosticsBuilder: UnusedImportDiagnosticsBuilder, + @inject(MissingImportDiagnosticsBuilder) private missingImportDiagnosticsBuilder: MissingImportDiagnosticsBuilder, + @inject(NamespaceDiagnosticCollection) private namespaceDiagnosticCollection: NamespaceDiagnosticCollection, + ) {} + + public clear(document: TextDocument): void { + this.namespaceDiagnosticCollection.delete(document.uri); + } + + public async handle(document: TextDocument): Promise { + if (!document.fileName.endsWith(FILE_EXTENSION)) { + return; + } + + const diagnostics = [ + ...await this.namespaceDiagnosticsBuilder.execute(document), + ...this.unusedImportDiagnosticsBuilder.execute(document), + ...this.missingImportDiagnosticsBuilder.execute(document), + ]; + this.namespaceDiagnosticCollection.set(document.uri, diagnostics); + } +} diff --git a/src/domain/namespace/MissingImportCandidateLocator.ts b/src/domain/namespace/MissingImportCandidateLocator.ts new file mode 100644 index 0000000..cfe9e07 --- /dev/null +++ b/src/domain/namespace/MissingImportCandidateLocator.ts @@ -0,0 +1,76 @@ +import { injectable } from 'tsyringe'; + +import { + NAMESPACE_DECLARATION_REGEX, + NOT_FOLLOWED_BY_NAMESPACE_CHAR, + NOT_PRECEDED_BY_NAMESPACE_CHAR, + USE_STATEMENT_REGEX, +} from './PhpPatterns'; + +export interface MissingImportCandidate { + identifier: string + index: number + length: number +} + +// A bare capitalized identifier, not already namespace-qualified on either +// side (so multi-segment FQCNs and already-imported paths are left alone), +// and not immediately after "::" or "->" (a static/instance member access, +// never itself a class name to import). +const CANDIDATE_IDENTIFIER_REGEX = new RegExp( + `(?)${NOT_PRECEDED_BY_NAMESPACE_CHAR}[A-Z]\\w*${NOT_FOLLOWED_BY_NAMESPACE_CHAR}`, + 'g', +); + +@injectable() +export class MissingImportCandidateLocator { + /** + * Returns one entry per distinct capitalized identifier used in the file + * that isn't already imported, aliased, or the file's own class name. + * Whether it actually resolves to a real class anywhere in the workspace + * is decided later (see MissingImportResolver) - this only narrows down + * candidates from raw text. + */ + public execute(contentDocument: string, ownClassName: string): MissingImportCandidate[] { + const importedIdentifiers = this.extractImportedIdentifiers(contentDocument); + + // Blanked out (same length, so every other match's index stays correct) + // rather than excluded by position - "namespace App;" would otherwise + // have its own bare segment ("App") mistaken for a used identifier. + const contentWithoutNamespaceDeclaration = contentDocument.replace( + NAMESPACE_DECLARATION_REGEX, + match => ' '.repeat(match.length), + ); + + const seen = new Set(); + const candidates: MissingImportCandidate[] = []; + + for (const match of contentWithoutNamespaceDeclaration.matchAll(CANDIDATE_IDENTIFIER_REGEX)) { + const identifier = match[0]; + + if (identifier === ownClassName || importedIdentifiers.has(identifier) || seen.has(identifier)) { + continue; + } + + seen.add(identifier); + candidates.push({ + identifier, + index: match.index!, + length: identifier.length, + }); + } + + return candidates; + } + + private extractImportedIdentifiers(contentDocument: string): Set { + const identifiers = new Set(); + + for (const match of contentDocument.matchAll(USE_STATEMENT_REGEX)) { + const [, fullNamespace, alias] = match; + identifiers.add(alias ?? fullNamespace.split('\\').pop()!); + } + + return identifiers; + } +} diff --git a/src/domain/namespace/NamespaceMismatchDetector.ts b/src/domain/namespace/NamespaceMismatchDetector.ts new file mode 100644 index 0000000..9e4f883 --- /dev/null +++ b/src/domain/namespace/NamespaceMismatchDetector.ts @@ -0,0 +1,23 @@ +import { injectable } from 'tsyringe'; + +interface Props { + declaredNamespace: string | null + expectedNamespace?: string +} + +@injectable() +export class NamespaceMismatchDetector { + /** + * Only flags a mismatch when both sides are known: a declared namespace + * line must exist (no guessing where to insert one) and PSR-4 must resolve + * an expected namespace for the file's path (otherwise there's nothing to + * compare against, e.g. files outside any autoload/autoload-dev prefix). + */ + public execute({ declaredNamespace, expectedNamespace }: Props): boolean { + if (!expectedNamespace || declaredNamespace === null) { + return false; + } + + return declaredNamespace !== expectedNamespace; + } +} diff --git a/src/domain/namespace/PhpPatterns.ts b/src/domain/namespace/PhpPatterns.ts index 8b5ed01..99645f6 100644 --- a/src/domain/namespace/PhpPatterns.ts +++ b/src/domain/namespace/PhpPatterns.ts @@ -28,3 +28,14 @@ export const PHP_CLASS_DECLARATION_REGEX = new RegExp( // inside "Foo\Bar\Baz" (a sub-namespace that merely starts with "Foo"). export const NOT_PRECEDED_BY_NAMESPACE_CHAR = '(?(statements: T[], mode: UseStatementSortMode): T[] { + const sorted = [...statements]; + + switch (mode) { + case 'length': + sorted.sort((a, b) => a.line.length - b.line.length || a.fullNamespace.localeCompare(b.fullNamespace)); + break; + case 'alphabetical': + sorted.sort((a, b) => a.fullNamespace.localeCompare(b.fullNamespace)); + break; + case 'natural': + default: + sorted.sort((a, b) => a.fullNamespace.localeCompare(b.fullNamespace, undefined, { numeric: true, sensitivity: 'base' })); + break; + } + + return sorted; + } +} diff --git a/src/domain/rename/ExtractNameFromCursor.ts b/src/domain/rename/ExtractNameFromCursor.ts index df51967..c1c82db 100644 --- a/src/domain/rename/ExtractNameFromCursor.ts +++ b/src/domain/rename/ExtractNameFromCursor.ts @@ -1,4 +1,4 @@ -import { PHP_CLASS_DECLARATION_REGEX } from '@domain/namespace/PhpPatterns'; +import { NAMESPACE_DECLARATION_REGEX, PHP_CLASS_DECLARATION_REGEX } from '@domain/namespace/PhpPatterns'; import { injectable } from 'tsyringe'; import { Position, TextDocument } from 'vscode'; @@ -7,8 +7,6 @@ interface Props { position: Position } -const NAMESPACE_REGEX = /^\s*namespace\s+([\w\\]+);/; - @injectable() export class ExtractNameFromCursor { public async execute({ document, position }: Props): Promise { @@ -20,7 +18,7 @@ export class ExtractNameFromCursor { return null; } - const namespaceMatch = currentLine.match(NAMESPACE_REGEX); + const namespaceMatch = currentLine.match(NAMESPACE_DECLARATION_REGEX); if (namespaceMatch) { return namespaceMatch[1] ?? null; } diff --git a/src/domain/workspace/ConfigurationLocator.ts b/src/domain/workspace/ConfigurationLocator.ts index 17183be..5dc81b2 100644 --- a/src/domain/workspace/ConfigurationLocator.ts +++ b/src/domain/workspace/ConfigurationLocator.ts @@ -14,6 +14,12 @@ export const ConfigKeys = { // boolean - a single polymorphic key avoids VS Code's settings schema conflict that // comes from one key being both a leaf boolean and the parent of another setting. RENAME_PROPERTIES: 'renameProperties', + NAMESPACE_MISMATCH_DIAGNOSTICS: 'namespaceMismatchDiagnostics', + HIGHLIGHT_NOT_USED: 'highlightNotUsed', + HIGHLIGHT_NOT_IMPORTED: 'highlightNotImported', + REMOVE_ON_SAVE: 'removeOnSave', + SORT_ON_SAVE: 'sortOnSave', + SORT_MODE: 'sortMode', } as const; export type Props = { diff --git a/src/extension.ts b/src/extension.ts index 994fa5b..31fd357 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -1,16 +1,22 @@ import 'reflect-metadata'; import { FileRenameHandler } from '@app/commands/FileRenameHandler'; +import { INSERT_MISSING_IMPORT_COMMAND, MissingImportCodeActionProvider } from '@app/commands/MissingImportCodeActionProvider'; +import { NamespaceCodeActionProvider } from '@app/commands/NamespaceCodeActionProvider'; import { RenameHandler } from '@app/commands/RenameHandler'; +import { UnusedImportCodeActionProvider } from '@app/commands/UnusedImportCodeActionProvider'; +import { SingleImportInserter } from '@app/services/SingleImportInserter'; +import { UseStatementBlockEditsBuilder } from '@app/services/UseStatementBlockEditsBuilder'; import { FileCreatedSubscriber } from '@app/subscribers/FileCreatedSubscriber'; import { FileDeletedSubscriber } from '@app/subscribers/FileDeletedSubscriber'; import { FileSavedSubscriber } from '@app/subscribers/FileSavedSubscriber'; +import { NamespaceDiagnosticsSubscriber } from '@app/subscribers/NamespaceDiagnosticsSubscriber'; import { ConfigKeys, ConfigurationLocator } from '@domain/workspace/ConfigurationLocator'; import { FeatureFlagManager } from '@domain/workspace/FeatureFlagManager'; import { NamespaceIndexBuilder } from '@infra/index/NamespaceIndexBuilder'; import * as fs from 'fs'; import { container } from 'tsyringe'; -import { commands, ExtensionContext, FileRenameEvent, window, workspace } from 'vscode'; +import { commands, ExtensionContext, FileRenameEvent, languages, Uri, window, workspace } from 'vscode'; export async function activate(context: ExtensionContext) { await fs.promises.mkdir(context.storageUri!.fsPath, { recursive: true }); @@ -32,6 +38,41 @@ export async function activate(context: ExtensionContext) { const fileRenameHandler = container.resolve(FileRenameHandler); workspace.onDidRenameFiles(event => fileRenameHandler.handle(event)); + const namespaceDiagnosticsSubscriber = container.resolve(NamespaceDiagnosticsSubscriber); + workspace.onDidOpenTextDocument(document => namespaceDiagnosticsSubscriber.handle(document)); + workspace.onDidSaveTextDocument(document => namespaceDiagnosticsSubscriber.handle(document)); + workspace.onDidCloseTextDocument(document => namespaceDiagnosticsSubscriber.clear(document)); + workspace.textDocuments.forEach(document => namespaceDiagnosticsSubscriber.handle(document)); + + const useStatementBlockEditsBuilder = container.resolve(UseStatementBlockEditsBuilder); + workspace.onWillSaveTextDocument((event) => { + event.waitUntil(Promise.resolve(useStatementBlockEditsBuilder.execute(event.document))); + }); + + languages.registerCodeActionsProvider( + { language: 'php' }, + container.resolve(NamespaceCodeActionProvider), + { providedCodeActionKinds: NamespaceCodeActionProvider.providedCodeActionKinds }, + ); + + languages.registerCodeActionsProvider( + { language: 'php' }, + container.resolve(UnusedImportCodeActionProvider), + { providedCodeActionKinds: UnusedImportCodeActionProvider.providedCodeActionKinds }, + ); + + languages.registerCodeActionsProvider( + { language: 'php' }, + container.resolve(MissingImportCodeActionProvider), + { providedCodeActionKinds: MissingImportCodeActionProvider.providedCodeActionKinds }, + ); + + commands.registerCommand(INSERT_MISSING_IMPORT_COMMAND, async (uri: Uri, fullNamespace: string) => { + const document = await workspace.openTextDocument(uri); + const singleImportInserter = container.resolve(SingleImportInserter); + await singleImportInserter.execute({ document, fullNamespace }); + }); + const command = ConfigurationLocator.getConfigKey(ConfigKeys.RENAME); commands.registerCommand(command, () => { const configuration = container.resolve(FeatureFlagManager); diff --git a/src/infra/index/NamespaceIndex.ts b/src/infra/index/NamespaceIndex.ts index 48d31f0..aeee8f6 100644 --- a/src/infra/index/NamespaceIndex.ts +++ b/src/infra/index/NamespaceIndex.ts @@ -1,3 +1,5 @@ +import { NAMESPACE_DECLARATION_REGEX, USE_STATEMENT_REGEX } from '@domain/namespace/PhpPatterns'; +import { WorkspacePathResolver } from '@domain/workspace/WorkspacePathResolver'; import * as fs from 'fs'; import * as path from 'path'; import { inject, singleton } from 'tsyringe'; @@ -12,6 +14,11 @@ interface IndexData { usages: Record } +export interface ClassLocation { + fsPath: string + namespace: string +} + const INDEX_FILENAME = 'namespace-index.json'; @singleton() @@ -21,10 +28,37 @@ export class NamespaceIndex { constructor( @inject('StorageUri') storagePath: string, + @inject(WorkspacePathResolver) private workspacePathResolver: WorkspacePathResolver, ) { this.indexPath = path.join(storagePath, INDEX_FILENAME); } + /** + * Every indexed file whose class name (derived from its file name, same + * convention as WorkspacePathResolver.extractClassNameFromPath) matches + * `className`. Used to resolve a bare identifier used in a file to the + * one place in the workspace that declares it - callers decide what to + * do when this returns zero (unresolved) or more than one (ambiguous) + * result, rather than guessing. + */ + public findClassLocations(className: string): ClassLocation[] { + const locations: ClassLocation[] = []; + + for (const [fsPath, entry] of Object.entries(this.data.files)) { + if (!entry.declares) { + continue; + } + + if (this.workspacePathResolver.extractClassNameFromPath(fsPath) !== className) { + continue; + } + + locations.push({ fsPath, namespace: entry.declares }); + } + + return locations; + } + public getFilesUsing(namespace: string): string[] { return this.data.usages[namespace] ?? []; } @@ -71,12 +105,12 @@ export class NamespaceIndex { } private extractImports(content: string): string[] { - const matches = [...content.matchAll(/^use\s+([\w\\]+)(?:\s+as\s+\w+)?;/gm)]; + const matches = [...content.matchAll(USE_STATEMENT_REGEX)]; return matches.map(m => m[1]); } private extractNamespace(content: string): string | null { - const match = content.match(/^\s*namespace\s+([\w\\]+);/m); + const match = content.match(NAMESPACE_DECLARATION_REGEX); return match ? match[1] : null; } } diff --git a/src/infra/vscode/NamespaceDiagnosticCollection.ts b/src/infra/vscode/NamespaceDiagnosticCollection.ts new file mode 100644 index 0000000..5bd0d03 --- /dev/null +++ b/src/infra/vscode/NamespaceDiagnosticCollection.ts @@ -0,0 +1,19 @@ +import { singleton } from 'tsyringe'; +import { Diagnostic, languages, Uri } from 'vscode'; + +@singleton() +export class NamespaceDiagnosticCollection { + private readonly collection = languages.createDiagnosticCollection('phpNamespaceRefactor'); + + public delete(uri: Uri): void { + this.collection.delete(uri); + } + + public dispose(): void { + this.collection.dispose(); + } + + public set(uri: Uri, diagnostics: Diagnostic[]): void { + this.collection.set(uri, diagnostics); + } +} diff --git a/src/test/MissingImportCandidateLocator.test.ts b/src/test/MissingImportCandidateLocator.test.ts new file mode 100644 index 0000000..af41c61 --- /dev/null +++ b/src/test/MissingImportCandidateLocator.test.ts @@ -0,0 +1,57 @@ +import 'reflect-metadata'; + +import * as assert from 'assert'; + +import { MissingImportCandidateLocator } from '../domain/namespace/MissingImportCandidateLocator'; + +suite('MissingImportCandidateLocator', () => { + const locator = new MissingImportCandidateLocator(); + + test('flags a bare capitalized identifier used as a type hint', () => { + const content = ' c.identifier), ['AuthService']); + }); + + test('does not flag an identifier that is already imported', () => { + const content = ' { + const content = ' { + const content = ' { + const content = ' { + const content = ' c.identifier), ['Status']); + }); + + test('deduplicates repeated usages of the same missing identifier', () => { + const content = ' isActive } as unknown as import('../domain/workspace/FeatureFlagManager').FeatureFlagManager; +} + +function fakeMissingImportResolver(resolved: ResolvedMissingImport[]) { + return { resolve: () => resolved } as unknown as import('../app/services/MissingImportResolver').MissingImportResolver; +} + +async function openDocument(content: string): Promise { + return vscode.workspace.openTextDocument({ content, language: 'php' }); +} + +suite('MissingImportDiagnosticsBuilder', () => { + test('returns a warning diagnostic for each resolved missing import', async () => { + const content = ' { + const document = await openDocument('({ defaultValue }: Props): T => defaultValue as T, + } as ConfigurationLocator; +} + +function buildWorkspacePathResolver(): WorkspacePathResolver { + return new WorkspacePathResolver( + new ComposerAutoloadManager(), + new FileExtensionResolver(fakeConfigurationLocator()), + ); +} + +function buildResolver(namespaceIndex: NamespaceIndex): MissingImportResolver { + return new MissingImportResolver( + buildWorkspacePathResolver(), + new MissingImportCandidateLocator(), + namespaceIndex, + ); +} + +async function openOrderDocument(dir: string): Promise { + const filePath = path.join(dir, 'Order.php'); + await fs.writeFile( + filePath, + ' { + test('resolves a candidate that matches exactly one class in the workspace index', async () => { + const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'php-namespace-refactor-')); + const document = await openOrderDocument(dir); + + const namespaceIndex = new NamespaceIndex(dir, buildWorkspacePathResolver()); + namespaceIndex.parseAndAdd(path.join(dir, 'AuthService.php'), 'namespace App\\Services;\nclass AuthService {}'); + + const resolved = buildResolver(namespaceIndex).resolve(document); + + assert.strictEqual(resolved.length, 1); + assert.strictEqual(resolved[0].identifier, 'AuthService'); + assert.strictEqual(resolved[0].fullNamespace, 'App\\Services\\AuthService'); + }); + + test('skips a candidate that resolves to more than one class (ambiguous)', async () => { + const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'php-namespace-refactor-')); + const document = await openOrderDocument(dir); + + const namespaceIndex = new NamespaceIndex(dir, buildWorkspacePathResolver()); + namespaceIndex.parseAndAdd(path.join(dir, 'One', 'AuthService.php'), 'namespace App\\One;\nclass AuthService {}'); + namespaceIndex.parseAndAdd(path.join(dir, 'Two', 'AuthService.php'), 'namespace App\\Two;\nclass AuthService {}'); + + assert.deepStrictEqual(buildResolver(namespaceIndex).resolve(document), []); + }); + + test('skips a candidate whose only match is already in the same declared namespace', async () => { + const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'php-namespace-refactor-')); + const document = await openOrderDocument(dir); + + const namespaceIndex = new NamespaceIndex(dir, buildWorkspacePathResolver()); + namespaceIndex.parseAndAdd(path.join(dir, 'AuthService.php'), 'namespace App\\Domain;\nclass AuthService {}'); + + assert.deepStrictEqual(buildResolver(namespaceIndex).resolve(document), []); + }); + + test('returns nothing when the identifier does not resolve to any indexed class', async () => { + const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'php-namespace-refactor-')); + const document = await openOrderDocument(dir); + + const namespaceIndex = new NamespaceIndex(dir, buildWorkspacePathResolver()); + + assert.deepStrictEqual(buildResolver(namespaceIndex).resolve(document), []); + }); +}); diff --git a/src/test/MultiFileReferenceUpdater.test.ts b/src/test/MultiFileReferenceUpdater.test.ts index 3e59270..7def0bc 100644 --- a/src/test/MultiFileReferenceUpdater.test.ts +++ b/src/test/MultiFileReferenceUpdater.test.ts @@ -39,6 +39,15 @@ function fakeConfigurationLocator(): ConfigurationLocator { } as ConfigurationLocator; } +function buildNamespaceIndex(storagePath: string): NamespaceIndex { + const workspacePathResolver = new WorkspacePathResolver( + new ComposerAutoloadManager(), + new FileExtensionResolver(fakeConfigurationLocator()), + ); + + return new NamespaceIndex(storagePath, workspacePathResolver); +} + function fakeFeatureFlagManager(editFilesInBackground = true): FeatureFlagManager { return { isActive: ({ defaultValue = true }) => defaultValue && editFilesInBackground, @@ -117,7 +126,7 @@ suite('MultiFileReferenceUpdater', () => { const consumerContent = ' { const consumerContent = ' { const consumerContent = ' { const consumerContent = ' { const consumerContent = 'order->run();\n }\n}\n'; const consumerUri = await writeTempPhpFile(dir, 'OrderController.php', consumerContent); - const namespaceIndex = new NamespaceIndex(os.tmpdir()); + const namespaceIndex = buildNamespaceIndex(os.tmpdir()); namespaceIndex.parseAndAdd(consumerUri.fsPath, consumerContent); const updater = buildUpdater(namespaceIndex, true, true); @@ -312,7 +321,7 @@ suite('MultiFileReferenceUpdater', () => { const consumerContent = 'order = $order;\n }\n}\n'; const consumerUri = await writeTempPhpFile(dir, 'OrderController.php', consumerContent); - const namespaceIndex = new NamespaceIndex(os.tmpdir()); + const namespaceIndex = buildNamespaceIndex(os.tmpdir()); namespaceIndex.parseAndAdd(consumerUri.fsPath, consumerContent); const updater = buildUpdater(namespaceIndex, true, true); @@ -354,7 +363,7 @@ suite('MultiFileReferenceUpdater', () => { const consumerContent = ' { ].join('\n'); const consumerUri = await writeTempPhpFile(dir, 'RenamedClassController.php', consumerContent); - const namespaceIndex = new NamespaceIndex(os.tmpdir()); + const namespaceIndex = buildNamespaceIndex(os.tmpdir()); namespaceIndex.parseAndAdd(consumerUri.fsPath, consumerContent); const updater = buildUpdater(namespaceIndex); diff --git a/src/test/NamespaceDiagnosticsBuilder.test.ts b/src/test/NamespaceDiagnosticsBuilder.test.ts new file mode 100644 index 0000000..2b1a335 --- /dev/null +++ b/src/test/NamespaceDiagnosticsBuilder.test.ts @@ -0,0 +1,69 @@ +import 'reflect-metadata'; + +import * as assert from 'assert'; +import * as vscode from 'vscode'; + +import { NAMESPACE_MISMATCH_CODE, NamespaceDiagnosticsBuilder } from '../app/services/NamespaceDiagnosticsBuilder'; +import { Namespace } from '../domain/namespace/NamespaceCreator'; +import { NamespaceMismatchDetector } from '../domain/namespace/NamespaceMismatchDetector'; + +function fakeFeatureFlagManager(isActive: boolean) { + return { isActive: () => isActive } as unknown as import('../domain/workspace/FeatureFlagManager').FeatureFlagManager; +} + +function fakeNamespaceCreator(namespace?: string) { + return { + execute: async (): Promise => ({ + namespace, + className: 'Order', + fullNamespace: namespace ? `${namespace}\\Order` : 'Order', + }), + } as unknown as import('../domain/namespace/NamespaceCreator').NamespaceCreator; +} + +async function openDocument(content: string): Promise { + return vscode.workspace.openTextDocument({ content, language: 'php' }); +} + +function buildBuilder(isActive: boolean, expectedNamespace?: string): NamespaceDiagnosticsBuilder { + return new NamespaceDiagnosticsBuilder( + fakeFeatureFlagManager(isActive), + fakeNamespaceCreator(expectedNamespace), + new NamespaceMismatchDetector(), + ); +} + +suite('NamespaceDiagnosticsBuilder', () => { + test('returns a warning diagnostic on the namespace line when it does not match PSR-4', async () => { + const document = await openDocument(' { + const document = await openDocument(' { + const document = await openDocument(' { + const document = await openDocument('({ defaultValue }: Props): T => defaultValue as T, + } as ConfigurationLocator; +} + +function buildNamespaceIndex(storagePath: string): NamespaceIndex { + const workspacePathResolver = new WorkspacePathResolver( + new ComposerAutoloadManager(), + new FileExtensionResolver(fakeConfigurationLocator()), + ); + + return new NamespaceIndex(storagePath, workspacePathResolver); +} + suite('NamespaceIndex', () => { let index: NamespaceIndex; setup(() => { - index = new NamespaceIndex(os.tmpdir()); + index = buildNamespaceIndex(os.tmpdir()); }); /** @@ -134,4 +153,35 @@ suite('NamespaceIndex', () => { }); }); }); + + suite('findClassLocations', () => { + test('finds the single file declaring a class by name', () => { + index.parseAndAdd('/src/Services/AuthService.php', 'namespace App\\Services;\nclass AuthService {}'); + + const locations = index.findClassLocations('AuthService'); + + assert.deepStrictEqual(locations, [ + { fsPath: '/src/Services/AuthService.php', namespace: 'App\\Services' }, + ]); + }); + + test('returns an empty array when no indexed file declares that class name', () => { + assert.deepStrictEqual(index.findClassLocations('Unknown'), []); + }); + + test('returns every location when more than one file declares the same class name', () => { + index.parseAndAdd('/src/Models/User.php', 'namespace App\\Models;\nclass User {}'); + index.parseAndAdd('/src/Legacy/User.php', 'namespace App\\Legacy;\nclass User {}'); + + const locations = index.findClassLocations('User'); + + assert.strictEqual(locations.length, 2); + }); + + test('ignores an indexed file with no declared namespace', () => { + index.parseAndAdd('/src/helpers.php', ' { + const detector = new NamespaceMismatchDetector(); + + test('flags a declared namespace that differs from the expected one', () => { + const result = detector.execute({ + declaredNamespace: 'App\\Old', + expectedNamespace: 'App\\New', + }); + assert.strictEqual(result, true); + }); + + test('does not flag a declared namespace that matches the expected one', () => { + const result = detector.execute({ + declaredNamespace: 'App\\Services', + expectedNamespace: 'App\\Services', + }); + assert.strictEqual(result, false); + }); + + test('does not flag when there is no declared namespace to compare (nothing to insert into)', () => { + const result = detector.execute({ + declaredNamespace: null, + expectedNamespace: 'App\\Services', + }); + assert.strictEqual(result, false); + }); + + test('does not flag when PSR-4 resolves no expected namespace (file outside any autoload prefix)', () => { + const result = detector.execute({ + declaredNamespace: 'App\\Services', + expectedNamespace: undefined, + }); + assert.strictEqual(result, false); + }); +}); diff --git a/src/test/UnusedImportDiagnosticsBuilder.test.ts b/src/test/UnusedImportDiagnosticsBuilder.test.ts new file mode 100644 index 0000000..b1d7f44 --- /dev/null +++ b/src/test/UnusedImportDiagnosticsBuilder.test.ts @@ -0,0 +1,52 @@ +import 'reflect-metadata'; + +import * as assert from 'assert'; +import * as vscode from 'vscode'; + +import { UNUSED_IMPORT_CODE, UnusedImportDiagnosticsBuilder } from '../app/services/UnusedImportDiagnosticsBuilder'; +import { ClassNameBoundaryRegexBuilder } from '../domain/namespace/ClassNameBoundaryRegexBuilder'; +import { UnusedUseStatementLocator } from '../domain/namespace/UnusedUseStatementLocator'; + +function fakeFeatureFlagManager(isActive: boolean) { + return { isActive: () => isActive } as unknown as import('../domain/workspace/FeatureFlagManager').FeatureFlagManager; +} + +function buildBuilder(isActive: boolean): UnusedImportDiagnosticsBuilder { + return new UnusedImportDiagnosticsBuilder( + fakeFeatureFlagManager(isActive), + new UnusedUseStatementLocator(new ClassNameBoundaryRegexBuilder()), + ); +} + +async function openDocument(content: string): Promise { + return vscode.workspace.openTextDocument({ content, language: 'php' }); +} + +suite('UnusedImportDiagnosticsBuilder', () => { + test('returns a hint diagnostic on an unused import', async () => { + const document = await openDocument(' { + const content = ' { + const document = await openDocument(' { + test('flags an import that is never referenced in the body', () => { + const content = [ + ' { + const content = [ + ' { + const content = [ + ' { + const content = [ + ' { + const content = '): FeatureFlagManager { + return { + isActive: ({ key, defaultValue }: { key: string, defaultValue?: boolean }) => flags[key] ?? defaultValue ?? true, + } as unknown as FeatureFlagManager; +} + +function fakeConfigurationLocator(sortMode: string): ConfigurationLocator { + return { + get: ({ defaultValue }: Props): T => (sortMode as unknown as T) ?? defaultValue as T, + } as ConfigurationLocator; +} + +function buildBuilder(flags: Record, sortMode = 'natural'): UseStatementBlockEditsBuilder { + return new UseStatementBlockEditsBuilder( + fakeFeatureFlagManager(flags), + fakeConfigurationLocator(sortMode), + new UnusedUseStatementLocator(new ClassNameBoundaryRegexBuilder()), + new UseStatementSorter(), + ); +} + +async function openPhpDocument(content: string): Promise { + const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'php-namespace-refactor-')); + const filePath = path.join(dir, 'Order.php'); + await fs.writeFile(filePath, content, 'utf8'); + return vscode.workspace.openTextDocument(vscode.Uri.file(filePath)); +} + +suite('UseStatementBlockEditsBuilder', () => { + test('removeOnSave alone deletes only the unused import, keeping the others in place', async () => { + const content = ' { + const content = ' { + const content = ' { + const content = ' { + const content = ' { + const content = ' { + const sorter = new UseStatementSorter(); + + test('alphabetical sorts strictly by character order', () => { + const statements = [ + { fullNamespace: 'App\\Item10', line: 'use App\\Item10;' }, + { fullNamespace: 'App\\Item2', line: 'use App\\Item2;' }, + { fullNamespace: 'App\\Apple', line: 'use App\\Apple;' }, + ]; + + const result = sorter.sort(statements, 'alphabetical').map(s => s.fullNamespace); + + assert.deepStrictEqual(result, ['App\\Apple', 'App\\Item10', 'App\\Item2']); + }); + + test('natural sorts numeric segments in numeric order', () => { + const statements = [ + { fullNamespace: 'App\\Item10', line: 'use App\\Item10;' }, + { fullNamespace: 'App\\Item2', line: 'use App\\Item2;' }, + { fullNamespace: 'App\\Apple', line: 'use App\\Apple;' }, + ]; + + const result = sorter.sort(statements, 'natural').map(s => s.fullNamespace); + + assert.deepStrictEqual(result, ['App\\Apple', 'App\\Item2', 'App\\Item10']); + }); + + test('length sorts the shortest "use" statement first', () => { + const statements = [ + { fullNamespace: 'App\\Services\\AuthService', line: 'use App\\Services\\AuthService;' }, + { fullNamespace: 'App\\User', line: 'use App\\User;' }, + ]; + + const result = sorter.sort(statements, 'length').map(s => s.fullNamespace); + + assert.deepStrictEqual(result, ['App\\User', 'App\\Services\\AuthService']); + }); + + test('does not mutate the input array', () => { + const statements = [ + { fullNamespace: 'App\\B', line: 'use App\\B;' }, + { fullNamespace: 'App\\A', line: 'use App\\A;' }, + ]; + + sorter.sort(statements, 'alphabetical'); + + assert.deepStrictEqual(statements.map(s => s.fullNamespace), ['App\\B', 'App\\A']); + }); +});