diff --git a/README.md b/README.md index 07ecd38..3ca1c70 100644 --- a/README.md +++ b/README.md @@ -96,7 +96,7 @@ This extension contributes the following settings: **phpNamespaceRefactor.renameProperties** -- When a class is renamed, also renames its class-typed constructor properties (promoted or not, readonly or not) and every `$this->x` usage to match the new class name — e.g. `private Test $teste` becomes `private NewTest $newTest` when `Teste` is renamed to `Novo`. +- When a class is renamed, also renames its class-typed constructor properties (promoted or not, readonly or not) and every `$this->x` usage to match the new class name — e.g. `private Test $test` becomes `private NewTest $newTest` when `Test` is renamed to `NewTest`. - If more than one property shares the same type in a constructor, the file is skipped rather than guessing which one to rename. - Accepts either a boolean or an object: ```json @@ -104,10 +104,10 @@ This extension contributes the following settings: ``` ```json "phpNamespaceRefactor.renameProperties": { - "renameMismatchedNames": true + "renameMismatchedNames": false } ``` - Setting an object automatically enables the feature. `renameMismatchedNames` (default `false`) additionally renames properties whose current name doesn't already match the class name on purpose (e.g. `private Test $service` becomes `private NewTest $newTest`); without it, only properties already named after the old class are renamed. + Setting `true` (or an empty object) enables the feature with every child behavior on by default, including `renameMismatchedNames` — properties whose current name doesn't already match the class name (e.g. `private Test $service`) get renamed too. Use the object form only to dial a specific child back to `false`. - Default: false. diff --git a/docs/configuration.md b/docs/configuration.md index 1c2ab71..8b62283 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -12,7 +12,7 @@ All keys are centralized in `ConfigKeys` (`src/domain/workspace/ConfigurationLoc | `phpNamespaceRefactor.additionalExtensions` | `ADDITIONAL_EXTENSIONS` | `string[]` | `["php"]` | `FileExtensionResolver` and `WorkspaceIndex` | | `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 `FileMoveOperation`/`PropertyRenameOperation` | +| `phpNamespaceRefactor.renameProperties` | `RENAME_PROPERTIES` | `boolean \| { renameMismatchedNames?: boolean }` | `false` | `PropertyRenameSettingsResolver`, consumed by `MultiFileReferenceUpdater`/`PropertyRenameOperation` | ## How configuration is read @@ -43,7 +43,7 @@ Normalized by `normalizeExtensions()` (`src/infra/utils/extensions.ts`): strips ## `phpNamespaceRefactor.renameProperties` -Master switch for `PropertyRenameOperation` — off by default, unlike the extension's other flags, because it renames identifiers (constructor parameters, promoted/non-promoted properties, `$this->x` usages), not just type hints. Two conditions gate it: the class itself must have actually been renamed (`oldClassName !== newClassName` — a plain directory move is a no-op), and only files `NamespaceBatchUpdater`/`MultiFileReferenceUpdater` already determined were affected by that specific rename are ever touched, never an independent workspace-wide scan — so an unrelated class in another namespace that happens to share a short name is never at risk. +Master switch for `PropertyRenameOperation` — off by default, unlike the extension's other flags, because it renames identifiers (constructor parameters, promoted/non-promoted properties, `$this->x` usages), not just type hints. Two conditions gate it: the class itself must have actually been renamed (`oldClassName !== newClassName` — a plain directory move is a no-op), and only files `MultiFileReferenceUpdater` already determined were affected by that specific rename are ever touched, never an independent workspace-wide scan — so an unrelated class in another namespace that happens to share a short name is never at risk. The rename is folded into the very same per-file `WorkspaceEdit` as the class-name replacement itself, not a separate later pass. For each candidate file, `ClassTypedPropertyLocator` looks for the single constructor property typed as the renamed class (promoted or not, readonly or not); if more than one property shares that type, the file is skipped entirely rather than guessing which one to rename — see [file-move.md](./operations/file-move.md#3-property-rename-propertyrenameoperation-optional). @@ -53,16 +53,16 @@ This one setting doubles as its own sub-option, via `PropertyRenameSettingsResol ```jsonc "phpNamespaceRefactor.renameProperties": false // disabled (default) -"phpNamespaceRefactor.renameProperties": true // enabled, mismatched names left alone -"phpNamespaceRefactor.renameProperties": {} // enabled, mismatched names left alone -"phpNamespaceRefactor.renameProperties": { "renameMismatchedNames": true } // enabled, mismatched names renamed too +"phpNamespaceRefactor.renameProperties": true // enabled, mismatched names renamed too (default child behavior) +"phpNamespaceRefactor.renameProperties": {} // enabled, mismatched names renamed too (default child behavior) +"phpNamespaceRefactor.renameProperties": { "renameMismatchedNames": false } // enabled, but mismatched names left alone ``` `renameMismatchedNames` controls what happens when a property's current name doesn't already follow the class-name convention: -- unset/`false` (default) — a property is only renamed if its name already matches the *old* class name (e.g. `$teste` for `Teste`); a property named something else on purpose (e.g. `$service`) is left untouched -- `true` — mismatched names are renamed too, to match the *new* class name (e.g. `$service` → `$novo`) +- unset/`true` (default whenever the feature is enabled) — mismatched names are renamed too, to match the *new* class name (e.g. `$service` → `$newTest`) +- `false` — a property is only renamed if its name already matches the *old* class name (e.g. `$test` for `Test`); a property named something else on purpose (e.g. `$service`) is left untouched -Any object value implies the feature is enabled — `false` is the only way to turn it off; there's no `{ "enabled": false }` form. +Turning the feature on — via a bare `true` or an object — defaults every child behavior to `true` as well; the object form exists only to dial a specific child back to `false`. Any object value implies the feature is enabled — `false` is the only way to turn it off; there's no `{ "enabled": false }` form. **Why one polymorphic setting instead of two plain booleans:** VS Code's settings schema doesn't allow a key to be both a leaf value and the parent of another key. An earlier version declared `phpNamespaceRefactor.renameProperties` (boolean) alongside `phpNamespaceRefactor.renameProperties.renameMismatchedNames` (boolean) as two separate keys — VS Code detected the conflict, logged `Ignoring phpNamespaceRefactor.renameProperties.renameMismatchedNames as phpNamespaceRefactor.renameProperties is false` in the console, and silently resolved **both** settings to `false` regardless of what the user configured. Collapsing them into a single `boolean | object` setting sidesteps the conflict entirely, since there's only ever one registered key. diff --git a/docs/operations/class-rename.md b/docs/operations/class-rename.md index d0b3afa..d09d765 100644 --- a/docs/operations/class-rename.md +++ b/docs/operations/class-rename.md @@ -31,7 +31,7 @@ It then delegates the rename to `FileRenameHandler.create()`, which triggers VS - Update the `namespace` declaration in the file - Update the class name inside the file (via `ClassNameUpdater`) - Update every `use` statement referencing the class throughout the project -- Optionally, rename class-typed constructor properties (and their `$this->x` usages) in the affected files to match the new class name — only when `renameProperties` is enabled, see `PropertyRenameOperation` in [file-move.md](./file-move.md#3-property-rename-propertyrenameoperation-optional) +- Optionally, rename class-typed constructor properties (and their `$this->x` usages) in the affected files to match the new class name — only when `renameProperties` is enabled, folded into the same pass as the reference update; see `PropertyRenameOperation` in [file-move.md](./file-move.md#2-namespace-and-reference-update-namespacebatchupdater) ## Difference from a direct Explorer rename diff --git a/docs/operations/file-move.md b/docs/operations/file-move.md index b139f11..e50de25 100644 --- a/docs/operations/file-move.md +++ b/docs/operations/file-move.md @@ -14,10 +14,10 @@ onDidRenameFiles (VS Code event) → FileMoveOperation.execute(files) → DirectoryMovedFilesResolver.execute() 1. expands directory moves into per-file moves → for each .php file: - → NamespaceBatchUpdater.execute() 2. updates namespace/class + references - → PropertyRenameOperation.execute() 3. (optional) renames class-typed properties to match the new class name - → MissingClassImporter.execute() 4. (optional) auto-imports classes from the old directory - → ImportRemover.execute() 5. (optional, checked internally) removes stale imports + → NamespaceBatchUpdater.execute() 2. updates namespace/class + references, folding in + (optional) class-typed property renaming + → MissingClassImporter.execute() 3. (optional) auto-imports classes from the old directory + → ImportRemover.execute() 4. (optional, checked internally) removes stale imports ``` ### Serialized queue @@ -47,7 +47,7 @@ For each moved file, `src/app/services/NamespaceBatchUpdater.ts`: 5. If the namespace declaration wasn't found/changed (step 4 returns `false`), the operation stops here — there's nothing to propagate for a file with no `namespace` declaration 6. Otherwise, calls `MultiFileReferenceUpdater` to propagate the change across the rest of the project -`NamespaceBatchUpdater.execute()` returns the list of files `MultiFileReferenceUpdater` determined were affected (empty when it never ran, e.g. no `namespace` declaration to key off). `FileMoveOperation` forwards that same list into step 3 below, so property renaming only ever touches files this specific rename actually reached — never an independent, broader scan. +`NamespaceBatchUpdater.execute()` returns the list of files `MultiFileReferenceUpdater` determined were affected (empty when it never ran, e.g. no `namespace` declaration to key off). #### `MultiFileReferenceUpdater` — how affected files are found @@ -61,28 +61,31 @@ The union of both lists is the final set. This means even an empty/stale index n For each affected file, it replaces via regex: - every occurrence of the old fully-qualified namespace with the new one - if the **class name** also changed, every occurrence of the old name with the new one (excluding spans already covered by the namespace substitution, to avoid overlapping ranges) +- if `renameProperties` resolves to enabled (off by default — see [configuration.md](../configuration.md#phpnamespacerefactorrenameproperties)) and the class was actually renamed, folds in a class-typed constructor property rename for that same file (see below) — into the *same* `WorkspaceEdit`, not a separate later pass - if the file had no occurrence of the old namespace at all (i.e. didn't match either case above) but sits in the same directory as the moved file, it tries to insert a new `use` statement -Every edit across every affected file is accumulated into a **single `WorkspaceEdit`** before being applied (`FileEditApplier.apply`), so the whole refactor shows up as one undo-stack entry in VS Code instead of one edit per file — see [issue #72](https://github.com/rejmann/php-namespace-refactor/issues/72). +The renamed file itself (`newUri`) is excluded from the above (it's the source of the rename, not a consumer), but still gets its own property-rename check afterward — covers the rare case of a class holding a property typed as itself (e.g. a linked-list node). -At the end, `MultiFileReferenceUpdater` always calls `ImportRemover.execute({ uri: newUri })` for the moved file itself (in addition to the call `FileMoveOperation` already makes in step 5 below — both are no-ops if the flag is disabled or there's nothing to remove). +Every edit across every affected file — namespace, class name, **and** property renaming — is accumulated into a **single `WorkspaceEdit`** before being applied once (`FileEditApplier.apply`), so the whole refactor (including property renaming) shows up as one undo-stack entry in VS Code instead of one edit per file, and each file is only opened/saved once — see [issue #72](https://github.com/rejmann/php-namespace-refactor/issues/72). -### 3. Property rename (`PropertyRenameOperation`, optional) +At the end, `MultiFileReferenceUpdater` always calls `ImportRemover.execute({ uri: newUri })` for the moved file itself (in addition to the call `FileMoveOperation` already makes in step 4 below — both are no-ops if the flag is disabled or there's nothing to remove). -Only runs if `renameProperties` resolves to enabled (off by default — see [configuration.md](../configuration.md#phpnamespacerefactorrenameproperties)). `PropertyRenameSettingsResolver` reads the raw setting (`boolean | { renameMismatchedNames?: boolean }`) once in `FileMoveOperation` and turns it into `{ enabled, renameMismatchedNames }`; only `enabled` gates whether this step runs at all. Also only acts when the class itself was actually renamed (`oldClassName !== newClassName`); a plain move to a different directory with the same class name is a no-op. +##### Property renaming (`PropertyRenameOperation`, optional, folded in above) -1. Derives the expected old/new property names from the class names (`PropertyNameResolver`, e.g. `Teste` → `teste`) -2. Builds the candidate file list from the `affectedFiles` `FileMoveOperation` received back from `NamespaceBatchUpdater` (step 2's output), plus the moved file itself — deliberately **not** an independent workspace-wide scan, so a differently-namespaced class that happens to share a short name is never touched -3. For each candidate file whose text contains the new class name, `ClassTypedPropertyLocator` looks for the constructor property typed as that class — promoted or not, readonly or not, confirming a non-promoted parameter by checking for a `$this->x = $x;` assignment in the constructor body (see `ConstructorSpanFinder` for how the constructor's parameter list and body are located via brace/paren matching). The property's own declaration line doesn't need a type hint to be found this way — a legacy `private $x;` typed only via a `@var ClassName` docblock is still matched and renamed, via `PropertyDeclarationPattern`, once the constructor param already confirmed what class it holds +`PropertyRenameSettingsResolver` reads the raw setting (`boolean | { renameMismatchedNames?: boolean }`) once per `MultiFileReferenceUpdater.execute()` call and turns it into `{ enabled, renameMismatchedNames }`; only `enabled` gates whether this runs at all. Also only acts when the class itself was actually renamed (`oldClassName !== newClassName`); a plain move to a different directory with the same class name is a no-op. + +1. Derives the expected old/new property names from the class names (`PropertyNameResolver`, e.g. `Test` → `test`) +2. Runs for every file `MultiFileReferenceUpdater` already determined was affected (plus the moved file itself) — deliberately **not** an independent workspace-wide scan, so a differently-namespaced class that happens to share a short name is never touched +3. For each candidate file, `ClassTypedPropertyLocator` looks for the constructor property typed as the renamed class — checking whichever of the old/new class name is actually present in that file's text at this point in the pass (still the old name for files whose class-name replacement is only queued, not yet applied) — promoted or not, readonly or not, confirming a non-promoted parameter by checking for a `$this->x = $x;` assignment in the constructor body (see `ConstructorSpanFinder` for how the constructor's parameter list and body are located via brace/paren matching). The property's own declaration line doesn't need a type hint to be found this way — a legacy `private $x;` typed only via a `@var ClassName` docblock is still matched and renamed, via `PropertyDeclarationPattern`, once the constructor param already confirmed what class it holds 4. If more than one property shares that type in the same file, it's ambiguous — the file is skipped entirely rather than guessing 5. If exactly one property is found: it's renamed when its current name already matches the *old* class-name convention, or — only when `renameMismatchedNames` also resolved to `true` — regardless of what it was named before -6. Renaming rewrites the constructor parameter/property declaration and every `$this->x` usage in that file, accumulated into a single `WorkspaceEdit` (its own undo stop, separate from `MultiFileReferenceUpdater`'s) +6. Renaming rewrites the constructor parameter/property declaration and every `$this->x` usage in that file, added to the same shared `WorkspaceEdit` as the class-name replacement -### 4. Auto-import of classes from the source directory (`MissingClassImporter`, optional) +### 3. Auto-import of classes from the source directory (`MissingClassImporter`, optional) Only runs if the `autoImportNamespace` flag is enabled. Lists the `.php` files still left in the source directory, checks which classes from those files are used in the moved file's text but not imported, and inserts the corresponding `use` statements. -### 5. Removing stale imports (`ImportRemover`) +### 4. Removing stale imports (`ImportRemover`) Unlike the other flags, the `removeUnusedImports` check happens **inside** `ImportRemover` itself (not in `FileMoveOperation`) — so it's always called, but returns immediately if the flag is disabled. @@ -92,10 +95,10 @@ When enabled: it collects the class names declared in the other files of the mov | Flag | Behavior | |---|---| -| `renameProperties` (`true`/`{}`) | Enables step 3 (renaming class-typed constructor properties to match the new class name) | -| `renameProperties: { renameMismatchedNames: true }` | Extends step 3 to also rename properties whose name doesn't already match the class-name convention | -| `autoImportNamespace` | Enables step 4 (auto-import of classes from the old directory) | -| `removeUnusedImports` | Enables the import removal in step 5 (checked inside `ImportRemover`) | +| `renameProperties` (`true`/`{}`) | Enables property renaming, folded into step 2 (renaming class-typed constructor properties to match the new class name); `true`/`{}` also renames mismatched names by default (see below) | +| `renameProperties: { renameMismatchedNames: false }` | Opts out of renaming properties whose name doesn't already match the class-name convention (on by default whenever the feature is enabled) | +| `autoImportNamespace` | Enables step 3 (auto-import of classes from the old directory) | +| `removeUnusedImports` | Enables the import removal in step 4 (checked inside `ImportRemover`) | | `editFilesInBackground` | Doesn't change what's edited, only whether touched files open a tab in the editor or are saved silently — see [configuration.md](../configuration.md) | ## Error handling diff --git a/package.json b/package.json index 2bd9ab9..e017ad6 100644 --- a/package.json +++ b/package.json @@ -119,12 +119,12 @@ "properties": { "renameMismatchedNames": { "type": "boolean", - "default": false, - "description": "Also rename properties whose current name doesn't match the class name (e.g. $service for a Teste type)." + "default": true, + "description": "Also rename properties whose current name doesn't match the class name (e.g. $service for a Test type). Defaults to true whenever renaming is enabled; set to false here to opt out just of this behavior." } }, "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/false to toggle, or to an object like { \"renameMismatchedNames\": true } to also rename properties whose current name doesn't match the class." + "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." } } } diff --git a/src/app/operations/FileMoveOperation.ts b/src/app/operations/FileMoveOperation.ts index 6418aaa..cf56987 100644 --- a/src/app/operations/FileMoveOperation.ts +++ b/src/app/operations/FileMoveOperation.ts @@ -2,13 +2,11 @@ import { DirectoryMovedFilesResolver } from '@app/services/DirectoryMovedFilesRe import { MissingClassImporter } from '@app/services/MissingClassImporter'; import { NamespaceBatchUpdater } from '@app/services/NamespaceBatchUpdater'; import { ImportRemover } from '@app/services/remove/ImportRemover'; -import { PropertyRenameSettingsResolver } from '@domain/property/PropertyRenameSettingsResolver'; import { ConfigKeys } from '@domain/workspace/ConfigurationLocator'; import { FeatureFlagManager } from '@domain/workspace/FeatureFlagManager'; import { inject, injectable } from 'tsyringe'; import type { FileMove } from './FileMove'; -import { PropertyRenameOperation } from './PropertyRenameOperation'; @injectable() export class FileMoveOperation { @@ -18,8 +16,6 @@ export class FileMoveOperation { @inject(MissingClassImporter) private missingClassImporter: MissingClassImporter, @inject(ImportRemover) private importRemover: ImportRemover, @inject(FeatureFlagManager) private featureFlagManager: FeatureFlagManager, - @inject(PropertyRenameOperation) private propertyRenameOperation: PropertyRenameOperation, - @inject(PropertyRenameSettingsResolver) private propertyRenameSettingsResolver: PropertyRenameSettingsResolver, ) {} public async execute(files: ReadonlyArray): Promise { @@ -31,17 +27,11 @@ export class FileMoveOperation { } try { - const affectedFiles = await this.namespaceBatchUpdater.execute({ newUri, oldUri }); - - const propertyRenameSettings = this.propertyRenameSettingsResolver.resolve(); - if (propertyRenameSettings.enabled) { - await this.propertyRenameOperation.execute({ - oldUri, - newUri, - affectedFiles, - renameMismatchedNames: propertyRenameSettings.renameMismatchedNames, - }); - } + // Property renaming for affected files now happens inside + // NamespaceBatchUpdater/MultiFileReferenceUpdater, folded into the + // same per-file WorkspaceEdit as the class rename itself, rather + // than as a separate pass here that re-opened every file again. + await this.namespaceBatchUpdater.execute({ newUri, oldUri }); if (this.featureFlagManager.isActive({ key: ConfigKeys.AUTO_IMPORT_NAMESPACE })) { await this.missingClassImporter.execute({ oldUri, newUri }); diff --git a/src/app/operations/PropertyRenameOperation.ts b/src/app/operations/PropertyRenameOperation.ts index 654baa6..5b66319 100644 --- a/src/app/operations/PropertyRenameOperation.ts +++ b/src/app/operations/PropertyRenameOperation.ts @@ -15,6 +15,13 @@ interface Props { renameMismatchedNames: boolean } +export interface PropertyRenameNames { + oldClassName: string + newClassName: string + expectedOldName: string + expectedNewName: string +} + @injectable() export class PropertyRenameOperation { constructor( @@ -26,16 +33,46 @@ export class PropertyRenameOperation { @inject(ConstructorSpanFinder) private constructorSpanFinder: ConstructorSpanFinder, ) {} - public async execute({ oldUri, newUri, affectedFiles, renameMismatchedNames }: Props): Promise { - const oldClassName = this.workspacePathResolver.extractClassNameFromPath(oldUri.fsPath); - const newClassName = this.workspacePathResolver.extractClassNameFromPath(newUri.fsPath); + public collectEdits( + edit: WorkspaceEdit, + uri: Uri, + document: TextDocument, + text: string, + names: PropertyRenameNames, + renameMismatchedNames: boolean, + ): void { + const { oldClassName, newClassName, expectedOldName, expectedNewName } = names; + + // The property's declared type still reads as whichever class name is + // actually present in this text: the new one when called after that + // rename has already landed in the document (the standalone execute() + // path below, and this class's own test suite), or still the old one + // when called from MultiFileReferenceUpdater's per-file loop, where the + // class-name replacement has only been queued into the shared edit, not + // yet applied to the buffer. + const searchClassName = text.includes(newClassName) ? newClassName : oldClassName; + if (!text.includes(searchClassName)) { + return; + } - if (!oldClassName || !newClassName || oldClassName === newClassName) { + const match = this.classTypedPropertyLocator.execute({ text, className: searchClassName }); + if (!match || match.propertyName === expectedNewName) { return; } - const expectedOldName = this.propertyNameResolver.resolve(oldClassName); - const expectedNewName = this.propertyNameResolver.resolve(newClassName); + const matchesOldConvention = match.propertyName === expectedOldName; + if (!matchesOldConvention && !renameMismatchedNames) { + return; + } + + this.addPropertyRenameEdits(edit, uri, document, text, searchClassName, match, expectedNewName); + } + + public async execute({ oldUri, newUri, affectedFiles, renameMismatchedNames }: Props): Promise { + const names = this.resolveNames(oldUri, newUri); + if (!names) { + return; + } const files = this.getCandidateFiles(newUri, affectedFiles); const edit = new WorkspaceEdit(); @@ -43,21 +80,7 @@ export class PropertyRenameOperation { await Promise.all(files.map(async (file) => { try { const { document, text } = await this.textDocumentOpener.execute({ uri: file }); - if (!text.includes(newClassName)) { - return; - } - - const match = this.classTypedPropertyLocator.execute({ text, className: newClassName }); - if (!match || match.propertyName === expectedNewName) { - return; - } - - const matchesOldConvention = match.propertyName === expectedOldName; - if (!matchesOldConvention && !renameMismatchedNames) { - return; - } - - this.addPropertyRenameEdits(edit, file, document, text, newClassName, match, expectedNewName); + this.collectEdits(edit, file, document, text, names, renameMismatchedNames); } catch (_) { return; } @@ -66,6 +89,29 @@ export class PropertyRenameOperation { await this.fileEditApplier.apply(edit); } + /** + * Resolves the old/new property-name convention for a class rename, so a + * caller that already has a file's document open (e.g. MultiFileReferenceUpdater, + * mid class-rename) can fold property renaming into that same pass and + * WorkspaceEdit instead of re-opening every affected file in a second, + * later one. + */ + public resolveNames(oldUri: Uri, newUri: Uri): PropertyRenameNames | null { + const oldClassName = this.workspacePathResolver.extractClassNameFromPath(oldUri.fsPath); + const newClassName = this.workspacePathResolver.extractClassNameFromPath(newUri.fsPath); + + if (!oldClassName || !newClassName || oldClassName === newClassName) { + return null; + } + + return { + oldClassName, + newClassName, + expectedOldName: this.propertyNameResolver.resolve(oldClassName), + expectedNewName: this.propertyNameResolver.resolve(newClassName), + }; + } + private addPropertyRenameEdits( edit: WorkspaceEdit, uri: Uri, diff --git a/src/app/services/update/MultiFileReferenceUpdater.ts b/src/app/services/update/MultiFileReferenceUpdater.ts index 6ae94a2..847dc02 100644 --- a/src/app/services/update/MultiFileReferenceUpdater.ts +++ b/src/app/services/update/MultiFileReferenceUpdater.ts @@ -1,9 +1,11 @@ +import { PropertyRenameNames,PropertyRenameOperation } from '@app/operations/PropertyRenameOperation'; import { ImportRemover } from '@app/services/remove/ImportRemover'; import { ClassNameBoundaryRegexBuilder } from '@domain/namespace/ClassNameBoundaryRegexBuilder'; import { NOT_FOLLOWED_BY_NAMESPACE_CHAR } from '@domain/namespace/PhpPatterns'; import { UseStatementCreator } from '@domain/namespace/UseStatementCreator'; import { UseStatementInjector } from '@domain/namespace/UseStatementInjector'; import { UseStatementLocator } from '@domain/namespace/UseStatementLocator'; +import { PropertyRenameSettingsResolver } from '@domain/property/PropertyRenameSettingsResolver'; import { WorkspacePathResolver } from '@domain/workspace/WorkspacePathResolver'; import { NamespaceIndex } from '@infra/index/NamespaceIndex'; import { WorkspaceIndex } from '@infra/index/WorkspaceIndex'; @@ -37,6 +39,8 @@ export class MultiFileReferenceUpdater { @inject(UseStatementInjector) private useStatementInjector: UseStatementInjector, @inject(FileEditApplier) private fileEditApplier: FileEditApplier, @inject(ClassNameBoundaryRegexBuilder) private classNameBoundaryRegexBuilder: ClassNameBoundaryRegexBuilder, + @inject(PropertyRenameOperation) private propertyRenameOperation: PropertyRenameOperation, + @inject(PropertyRenameSettingsResolver) private propertyRenameSettingsResolver: PropertyRenameSettingsResolver, ) {} public async execute({ @@ -56,6 +60,15 @@ export class MultiFileReferenceUpdater { ? this.classNameBoundaryRegexBuilder.execute({ className }) : null; + // Resolved once so property renaming can be folded into the very same + // per-file loop (and WorkspaceEdit) as the class-name replacement below, + // instead of a second pass that reopens every affected file after this + // one has already applied and saved. + const propertyRenameSettings = this.propertyRenameSettingsResolver.resolve(); + const propertyNames = propertyRenameSettings.enabled + ? this.propertyRenameOperation.resolveNames(oldUri, newUri) + : null; + // Files that import/use the old namespace. const indexedPaths = this.namespaceIndex .getFilesUsing(useOldNamespace) @@ -84,6 +97,12 @@ export class MultiFileReferenceUpdater { this.addRegexReplacements(edit, file, document, text, classNameRegex, newClassName, namespaceMatches); } + if (propertyNames) { + this.propertyRenameOperation.collectEdits( + edit, file, document, text, propertyNames, propertyRenameSettings.renameMismatchedNames, + ); + } + // A file affected here already references useOldNamespace (that's how it // landed in affectedPaths), so the substitution above already turns its // own `use` line into the new one. Appending another would duplicate it. @@ -113,11 +132,31 @@ export class MultiFileReferenceUpdater { } else { await this.appendUseStatement(edit, file, document, text, directoryPath, useImport, className); } + + if (propertyNames) { + this.propertyRenameOperation.collectEdits( + edit, file, document, text, propertyNames, propertyRenameSettings.renameMismatchedNames, + ); + } } catch (_) { return; } })); + if (propertyNames) { + // The renamed class's own file: covers a class that holds a + // self-typed property (e.g. a linked-list node), which is never part + // of affectedPaths/sameDirectoryFiles since newUri is always excluded there. + try { + const { document, text } = await this.textDocumentOpener.execute({ uri: newUri }); + this.propertyRenameOperation.collectEdits( + edit, newUri, document, text, propertyNames, propertyRenameSettings.renameMismatchedNames, + ); + } catch (_) { + // ignore + } + } + await this.fileEditApplier.apply(edit); await this.importRemover.execute({ uri: newUri }); diff --git a/src/domain/property/PropertyDeclarationPattern.ts b/src/domain/property/PropertyDeclarationPattern.ts index f03454d..5b4a1a7 100644 --- a/src/domain/property/PropertyDeclarationPattern.ts +++ b/src/domain/property/PropertyDeclarationPattern.ts @@ -2,8 +2,8 @@ const VISIBILITY = 'public|protected|private'; /** * Matches a class-body property declaration for `varName` - e.g. - * `private Test $teste;` or, since the type hint is optional in PHP, - * a legacy `private $teste;` typed only via a `@var Teste` docblock. + * `private Test $test;` or, since the type hint is optional in PHP, + * a legacy `private $test;` typed only via a `@var Test` docblock. * `className` is accepted but not required, so a property whose type was * never declared in code (only documented) is still found once the caller * has already confirmed by other means (e.g. a constructor assignment) diff --git a/src/domain/property/PropertyRenameSettingsResolver.ts b/src/domain/property/PropertyRenameSettingsResolver.ts index 3d04c79..6f63120 100644 --- a/src/domain/property/PropertyRenameSettingsResolver.ts +++ b/src/domain/property/PropertyRenameSettingsResolver.ts @@ -6,13 +6,13 @@ export interface PropertyRenameSettings { renameMismatchedNames: boolean } -type RenamePropertiesValue = boolean | { renameMismatchedNames?: boolean }; - /** * `phpNamespaceRefactor.renameProperties` is a single setting that accepts * either a boolean or an object (`{ renameMismatchedNames: boolean }`) - - * any object value implies the feature is enabled, since a bare boolean - * `false` is the only way to turn it off. + * see `ConfigurationLocator.getPolymorphicFlag` for how turning the feature + * on (bare `true` or any object) defaults every child behavior to `true` + * too, with the object form only used to dial a specific child back to + * `false`. */ @injectable() export class PropertyRenameSettingsResolver { @@ -21,15 +21,9 @@ export class PropertyRenameSettingsResolver { ) {} public resolve(): PropertyRenameSettings { - const value = this.configurationLocator.get({ + return this.configurationLocator.getPolymorphicFlag({ key: ConfigKeys.RENAME_PROPERTIES, - defaultValue: false, + childKeys: ['renameMismatchedNames'] as const, }); - - if (typeof value === 'object' && value !== null) { - return { enabled: true, renameMismatchedNames: value.renameMismatchedNames === true }; - } - - return { enabled: value === true, renameMismatchedNames: false }; } } diff --git a/src/domain/workspace/ConfigurationLocator.ts b/src/domain/workspace/ConfigurationLocator.ts index 78010c3..17183be 100644 --- a/src/domain/workspace/ConfigurationLocator.ts +++ b/src/domain/workspace/ConfigurationLocator.ts @@ -21,6 +21,13 @@ export type Props = { defaultValue?: T } +export type PolymorphicFlag = { enabled: boolean } & Record; + +interface PolymorphicFlagProps { + key: string + childKeys: readonly ChildKey[] +} + @injectable() export class ConfigurationLocator { private config: WorkspaceConfiguration; @@ -36,4 +43,31 @@ export class ConfigurationLocator { public static getConfigKey(key: string): string { return `${Config}.${key}`; } + + /** + * Resolves a setting that accepts either a boolean or an object of child + * flags (e.g. `phpNamespaceRefactor.renameProperties`) - a single + * polymorphic key, since VS Code's settings schema doesn't allow one key + * to be both a leaf value and the parent of another setting. + * + * A bare `true` (or any object, even `{}`) enables the feature and + * defaults every child flag to `true` as well; a bare `false` disables + * everything. The object form only exists to dial a specific child back + * to `false` - there's no way to enable a child while leaving the parent + * disabled. + */ + public getPolymorphicFlag( + { key, childKeys }: PolymorphicFlagProps, + ): PolymorphicFlag { + const value = this.get> | undefined>({ key, defaultValue: false }); + const isObject = typeof value === 'object' && value !== null; + const enabled = isObject || value === true; + + const children = {} as Record; + for (const childKey of childKeys) { + children[childKey] = isObject ? value[childKey] !== false : enabled; + } + + return { enabled, ...children }; + } } diff --git a/src/test/ClassTypedPropertyLocator.test.ts b/src/test/ClassTypedPropertyLocator.test.ts index 2da4d42..c8ceeec 100644 --- a/src/test/ClassTypedPropertyLocator.test.ts +++ b/src/test/ClassTypedPropertyLocator.test.ts @@ -15,43 +15,43 @@ suite('ClassTypedPropertyLocator', () => { const text = [ 'class UserController', '{', - ' public function __construct(private Test $teste)', + ' public function __construct(private Test $test)', ' {', ' }', '}', ].join('\n'); - const match = locate(text, 'Teste'); + const match = locate(text, 'Test'); assert.ok(match); - assert.strictEqual(match!.propertyName, 'teste'); + assert.strictEqual(match!.propertyName, 'test'); assert.strictEqual(match!.isPromoted, true); assert.strictEqual(match!.hasSeparateDeclaration, false); }); test('finds a promoted readonly property regardless of modifier order', () => { - const first = locate('function __construct(private readonly Teste $teste) {}', 'Teste'); - const second = locate('function __construct(readonly private Test $teste) {}', 'Teste'); + const first = locate('function __construct(private readonly Test $test) {}', 'Test'); + const second = locate('function __construct(readonly private Test $test) {}', 'Test'); - assert.strictEqual(first!.propertyName, 'teste'); - assert.strictEqual(second!.propertyName, 'teste'); + assert.strictEqual(first!.propertyName, 'test'); + assert.strictEqual(second!.propertyName, 'test'); }); test('finds a non-promoted property confirmed by a constructor assignment', () => { const text = [ 'class UserController', '{', - ' private Test $teste;', + ' private Test $test;', '', - ' public function __construct(Teste $teste)', + ' public function __construct(Test $test)', ' {', - ' $this->teste = $teste;', + ' $this->test = $test;', ' }', '}', ].join('\n'); - const match = locate(text, 'Teste'); + const match = locate(text, 'Test'); assert.ok(match); - assert.strictEqual(match!.propertyName, 'teste'); + assert.strictEqual(match!.propertyName, 'test'); assert.strictEqual(match!.isPromoted, false); assert.strictEqual(match!.hasSeparateDeclaration, true); }); @@ -83,43 +83,43 @@ suite('ClassTypedPropertyLocator', () => { const text = [ 'class Validator', '{', - ' public function __construct(Teste $teste)', + ' public function __construct(Test $test)', ' {', - ' $teste->validate();', + ' $test->validate();', ' }', '}', ].join('\n'); - assert.strictEqual(locate(text, 'Teste'), null); + assert.strictEqual(locate(text, 'Test'), null); }); test('matches a property with a mismatched name', () => { const text = 'function __construct(private Test $service) {}'; - const match = locate(text, 'Teste'); + const match = locate(text, 'Test'); assert.ok(match); assert.strictEqual(match!.propertyName, 'service'); }); test('matches a nullable type hint', () => { - const text = 'function __construct(private ?Teste $teste) {}'; - const match = locate(text, 'Teste'); + const text = 'function __construct(private ?Test $test) {}'; + const match = locate(text, 'Test'); assert.ok(match); - assert.strictEqual(match!.propertyName, 'teste'); + assert.strictEqual(match!.propertyName, 'test'); }); test('returns null when there is no constructor', () => { - assert.strictEqual(locate('class Teste {}', 'Teste'), null); + assert.strictEqual(locate('class Test {}', 'Test'), null); }); test('returns null when the class type does not appear in the constructor', () => { const text = 'function __construct(private Other $other) {}'; - assert.strictEqual(locate(text, 'Teste'), null); + assert.strictEqual(locate(text, 'Test'), null); }); test('returns null when two parameters share the same type (ambiguous)', () => { const text = 'function __construct(private Test $a, private Test $b) {}'; - assert.strictEqual(locate(text, 'Teste'), null); + assert.strictEqual(locate(text, 'Test'), null); }); }); diff --git a/src/test/MultiFileReferenceUpdater.test.ts b/src/test/MultiFileReferenceUpdater.test.ts index a1c4be0..3e59270 100644 --- a/src/test/MultiFileReferenceUpdater.test.ts +++ b/src/test/MultiFileReferenceUpdater.test.ts @@ -6,12 +6,17 @@ import * as os from 'os'; import * as path from 'path'; import * as vscode from 'vscode'; +import { PropertyRenameOperation } from '../app/operations/PropertyRenameOperation'; import { ImportRemover } from '../app/services/remove/ImportRemover'; import { MultiFileReferenceUpdater } from '../app/services/update/MultiFileReferenceUpdater'; import { ClassNameBoundaryRegexBuilder } from '../domain/namespace/ClassNameBoundaryRegexBuilder'; import { UseStatementCreator } from '../domain/namespace/UseStatementCreator'; import { UseStatementInjector } from '../domain/namespace/UseStatementInjector'; import { UseStatementLocator } from '../domain/namespace/UseStatementLocator'; +import { ClassTypedPropertyLocator } from '../domain/property/ClassTypedPropertyLocator'; +import { ConstructorSpanFinder } from '../domain/property/ConstructorSpanFinder'; +import { PropertyNameResolver } from '../domain/property/PropertyNameResolver'; +import { PropertyRenameSettingsResolver } from '../domain/property/PropertyRenameSettingsResolver'; import { ConfigurationLocator, Props } from '../domain/workspace/ConfigurationLocator'; import { FeatureFlagManager } from '../domain/workspace/FeatureFlagManager'; import { FileExtensionResolver } from '../domain/workspace/FileExtensionResolver'; @@ -22,6 +27,12 @@ import { WorkspaceIndex } from '../infra/index/WorkspaceIndex'; import { FileEditApplier } from '../infra/vscode/FileEditApplier'; import { TextDocumentOpener } from '../infra/vscode/TextDocumentOpener'; +function fakePropertyRenameSettingsResolver(enabled = false): PropertyRenameSettingsResolver { + return { + resolve: () => ({ enabled, renameMismatchedNames: false }), + } as PropertyRenameSettingsResolver; +} + function fakeConfigurationLocator(): ConfigurationLocator { return { get: ({ defaultValue }: Props): T => defaultValue as T, @@ -34,12 +45,26 @@ function fakeFeatureFlagManager(editFilesInBackground = true): FeatureFlagManage } as FeatureFlagManager; } -function buildUpdater(namespaceIndex: NamespaceIndex, editFilesInBackground = true): MultiFileReferenceUpdater { +function buildUpdater( + namespaceIndex: NamespaceIndex, + editFilesInBackground = true, + propertyRenameEnabled = false, +): MultiFileReferenceUpdater { const workspacePathResolver = new WorkspacePathResolver( new ComposerAutoloadManager(), new FileExtensionResolver(fakeConfigurationLocator()), ); const fileEditApplier = new FileEditApplier(fakeFeatureFlagManager(editFilesInBackground)); + const constructorSpanFinder = new ConstructorSpanFinder(); + + const propertyRenameOperation = new PropertyRenameOperation( + workspacePathResolver, + new TextDocumentOpener(), + fileEditApplier, + new ClassTypedPropertyLocator(constructorSpanFinder), + new PropertyNameResolver(), + constructorSpanFinder, + ); return new MultiFileReferenceUpdater( workspacePathResolver, @@ -52,6 +77,8 @@ function buildUpdater(namespaceIndex: NamespaceIndex, editFilesInBackground = tr new UseStatementInjector(fileEditApplier), fileEditApplier, new ClassNameBoundaryRegexBuilder(), + propertyRenameOperation, + fakePropertyRenameSettingsResolver(propertyRenameEnabled), ); } @@ -242,6 +269,113 @@ suite('MultiFileReferenceUpdater', () => { assert.ok(!/\bprivate Order \$order\b/.test(text), `old bare class name should be gone, got:\n${text}`); }); + test('with property renaming enabled, renames the constructor property in the same pass as the class rename', async () => { + const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'php-namespace-refactor-')); + + const oldUri = vscode.Uri.file(path.join(dir, 'Order.php')); + const newUri = vscode.Uri.file(path.join(dir, 'PurchaseOrder.php')); + + const consumerContent = 'order->run();\n }\n}\n'; + const consumerUri = await writeTempPhpFile(dir, 'OrderController.php', consumerContent); + + const namespaceIndex = new NamespaceIndex(os.tmpdir()); + namespaceIndex.parseAndAdd(consumerUri.fsPath, consumerContent); + + const updater = buildUpdater(namespaceIndex, true, true); + + await updater.execute({ + useOldNamespace: 'App\\Domain\\Order', + useNewNamespace: 'App\\Domain\\PurchaseOrder', + newUri, + oldUri, + }); + + const text = (await vscode.workspace.openTextDocument(consumerUri)).getText(); + + assert.ok( + text.includes('private PurchaseOrder $purchaseOrder'), + `expected the constructor property to be renamed alongside the class, got:\n${text}`, + ); + assert.ok( + text.includes('$this->purchaseOrder->run();'), + `expected $this-> usages to be renamed, got:\n${text}`, + ); + assert.ok(!/\$order\b/.test(text), `expected no leftover old property name, got:\n${text}`); + }); + + test('with property renaming enabled, renames a non-promoted property in the same pass as the class rename', async () => { + const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'php-namespace-refactor-')); + + const oldUri = vscode.Uri.file(path.join(dir, 'Order.php')); + const newUri = vscode.Uri.file(path.join(dir, 'PurchaseOrder.php')); + + const consumerContent = 'order = $order;\n }\n}\n'; + const consumerUri = await writeTempPhpFile(dir, 'OrderController.php', consumerContent); + + const namespaceIndex = new NamespaceIndex(os.tmpdir()); + namespaceIndex.parseAndAdd(consumerUri.fsPath, consumerContent); + + const updater = buildUpdater(namespaceIndex, true, true); + + await updater.execute({ + useOldNamespace: 'App\\Domain\\Order', + useNewNamespace: 'App\\Domain\\PurchaseOrder', + newUri, + oldUri, + }); + + const text = (await vscode.workspace.openTextDocument(consumerUri)).getText(); + + assert.ok( + text.includes('private PurchaseOrder $purchaseOrder;'), + `expected the declared property to be renamed alongside the class, got:\n${text}`, + ); + assert.ok( + text.includes('__construct(PurchaseOrder $purchaseOrder)'), + `expected the constructor parameter to be renamed, got:\n${text}`, + ); + assert.ok( + text.includes('$this->purchaseOrder = $purchaseOrder;'), + `expected the assignment to be renamed, got:\n${text}`, + ); + assert.ok(!/\$order\b/.test(text), `expected no leftover old property name, got:\n${text}`); + }); + + test('with renameMismatchedNames off, still renames the class name and import even when the property name is left untouched', async () => { + const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'php-namespace-refactor-')); + + const oldUri = vscode.Uri.file(path.join(dir, 'Order.php')); + const newUri = vscode.Uri.file(path.join(dir, 'PurchaseOrder.php')); + + // "$service" doesn't follow the old class-name convention ("order"), so + // with renameMismatchedNames off the property itself must stay + // untouched - but that must not stop the class name and import from + // being renamed everywhere else in this same file. + const consumerContent = ' { '', 'class RenamedClassController', '{', - ' private RenamedClass $RenamedClass;', + ' private RenamedClass $instance;', '}', '', ].join('\n'); @@ -292,7 +426,7 @@ suite('MultiFileReferenceUpdater', () => { `the exact FQCN import should be renamed, got:\n${text}`, ); assert.ok( - text.includes('private RenamedClassTest $RenamedClass;'), + text.includes('private RenamedClassTest $instance;'), `the bare class name usage should be renamed, got:\n${text}`, ); assert.ok( diff --git a/src/test/PhpPatterns.test.ts b/src/test/PhpPatterns.test.ts index b9377d8..ca092e8 100644 --- a/src/test/PhpPatterns.test.ts +++ b/src/test/PhpPatterns.test.ts @@ -129,13 +129,13 @@ suite('NOT_PRECEDED_BY_NAMESPACE_CHAR / NOT_FOLLOWED_BY_NAMESPACE_CHAR', () => { test('still replaces the bare identifier when it is not part of a namespace path', () => { const regex = buildGuardedRegex('RenamedClass'); - const content = 'private RenamedClass $RenamedClass;\n\nnew RenamedClass();'; + const content = 'private RenamedClass $instance;\n\nnew RenamedClass();'; const result = content.replace(regex, 'RenamedClassTeste'); assert.strictEqual( result, - 'private RenamedClassTeste $RenamedClass;\n\nnew RenamedClassTeste();', + 'private RenamedClassTeste $instance;\n\nnew RenamedClassTeste();', ); }); diff --git a/src/test/PropertyRenameOperation.test.ts b/src/test/PropertyRenameOperation.test.ts index 4507bdb..bc6f00d 100644 --- a/src/test/PropertyRenameOperation.test.ts +++ b/src/test/PropertyRenameOperation.test.ts @@ -59,10 +59,10 @@ async function writeTempPhpFile(dir: string, fileName: string, content: string): suite('PropertyRenameOperation', () => { test('renames a promoted property that matches the old class-name convention', async () => { const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'php-namespace-refactor-')); - const oldUri = vscode.Uri.file(path.join(dir, 'Teste.php')); - const newUri = vscode.Uri.file(path.join(dir, 'Novo.php')); + const oldUri = vscode.Uri.file(path.join(dir, 'Test.php')); + const newUri = vscode.Uri.file(path.join(dir, 'NewTest.php')); - const consumerContent = 'teste->run();\n }\n}\n'; + const consumerContent = 'test->run();\n }\n}\n'; const consumerUri = await writeTempPhpFile(dir, 'UserController.php', consumerContent); const operation = buildOperation(); @@ -70,16 +70,16 @@ suite('PropertyRenameOperation', () => { const text = (await vscode.workspace.openTextDocument(consumerUri)).getText(); assert.ok(text.includes('private NewTest $newTest'), `expected the promoted property to be renamed, got:\n${text}`); - assert.ok(text.includes('$this->novo->run();'), `expected $this-> usages to be renamed, got:\n${text}`); - assert.ok(!text.includes('teste'), `expected no leftover old property name, got:\n${text}`); + assert.ok(text.includes('$this->newTest->run();'), `expected $this-> usages to be renamed, got:\n${text}`); + assert.ok(!text.includes('$test'), `expected no leftover old property name, got:\n${text}`); }); test('renames a non-promoted property confirmed by its constructor assignment', async () => { const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'php-namespace-refactor-')); - const oldUri = vscode.Uri.file(path.join(dir, 'Teste.php')); - const newUri = vscode.Uri.file(path.join(dir, 'Novo.php')); + const oldUri = vscode.Uri.file(path.join(dir, 'Test.php')); + const newUri = vscode.Uri.file(path.join(dir, 'NewTest.php')); - const consumerContent = 'teste = $teste;\n }\n}\n'; + const consumerContent = 'test = $test;\n }\n}\n'; const consumerUri = await writeTempPhpFile(dir, 'UserController.php', consumerContent); const operation = buildOperation(); @@ -87,34 +87,34 @@ suite('PropertyRenameOperation', () => { const text = (await vscode.workspace.openTextDocument(consumerUri)).getText(); assert.ok(text.includes('private NewTest $newTest;'), `expected the declared property to be renamed, got:\n${text}`); - assert.ok(text.includes('__construct(Novo $novo)'), `expected the constructor parameter to be renamed, got:\n${text}`); - assert.ok(text.includes('$this->novo = $novo;'), `expected the assignment to be renamed, got:\n${text}`); + assert.ok(text.includes('__construct(NewTest $newTest)'), `expected the constructor parameter to be renamed, got:\n${text}`); + assert.ok(text.includes('$this->newTest = $newTest;'), `expected the assignment to be renamed, got:\n${text}`); }); test('renames an untyped property declared only via a @var docblock', async () => { const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'php-namespace-refactor-')); const oldUri = vscode.Uri.file(path.join(dir, 'UserRepository.php')); - const newUri = vscode.Uri.file(path.join(dir, 'ClienteRepository.php')); + const newUri = vscode.Uri.file(path.join(dir, 'ClientRepository.php')); - const consumerContent = 'repository = $repository;\n }\n}\n'; + const consumerContent = 'repository = $repository;\n }\n}\n'; const consumerUri = await writeTempPhpFile(dir, 'UserService.php', consumerContent); const operation = buildOperation(); await operation.execute({ oldUri, newUri, affectedFiles: [consumerUri], renameMismatchedNames: true }); const text = (await vscode.workspace.openTextDocument(consumerUri)).getText(); - assert.ok(text.includes('private $clienteRepository;'), `expected the untyped declaration to be renamed, got:\n${text}`); - assert.ok(text.includes('__construct(ClienteRepository $clienteRepository)'), `expected the constructor parameter to be renamed, got:\n${text}`); - assert.ok(text.includes('$this->clienteRepository = $clienteRepository;'), `expected the assignment to be renamed, got:\n${text}`); + assert.ok(text.includes('private $clientRepository;'), `expected the untyped declaration to be renamed, got:\n${text}`); + assert.ok(text.includes('__construct(ClientRepository $clientRepository)'), `expected the constructor parameter to be renamed, got:\n${text}`); + assert.ok(text.includes('$this->clientRepository = $clientRepository;'), `expected the assignment to be renamed, got:\n${text}`); assert.ok(!text.includes('$repository'), `expected no leftover old property name, got:\n${text}`); }); test('leaves a mismatched property name untouched when renameMismatchedNames is false', async () => { const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'php-namespace-refactor-')); - const oldUri = vscode.Uri.file(path.join(dir, 'Teste.php')); - const newUri = vscode.Uri.file(path.join(dir, 'Novo.php')); + const oldUri = vscode.Uri.file(path.join(dir, 'Test.php')); + const newUri = vscode.Uri.file(path.join(dir, 'NewTest.php')); - const consumerContent = ' { test('renames a mismatched property name when renameMismatchedNames is true', async () => { const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'php-namespace-refactor-')); - const oldUri = vscode.Uri.file(path.join(dir, 'Teste.php')); - const newUri = vscode.Uri.file(path.join(dir, 'Novo.php')); + const oldUri = vscode.Uri.file(path.join(dir, 'Test.php')); + const newUri = vscode.Uri.file(path.join(dir, 'NewTest.php')); - const consumerContent = 'service->run();\n }\n}\n'; + const consumerContent = 'service->run();\n }\n}\n'; const consumerUri = await writeTempPhpFile(dir, 'UserController.php', consumerContent); const operation = buildOperation(); @@ -137,15 +137,15 @@ suite('PropertyRenameOperation', () => { const text = (await vscode.workspace.openTextDocument(consumerUri)).getText(); assert.ok(text.includes('private NewTest $newTest'), `expected the mismatched property to be renamed, got:\n${text}`); - assert.ok(text.includes('$this->novo->run();'), `expected $this-> usages to be renamed, got:\n${text}`); + assert.ok(text.includes('$this->newTest->run();'), `expected $this-> usages to be renamed, got:\n${text}`); }); test('skips a file when two properties share the renamed class type (ambiguous)', async () => { const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'php-namespace-refactor-')); - const oldUri = vscode.Uri.file(path.join(dir, 'Teste.php')); - const newUri = vscode.Uri.file(path.join(dir, 'Novo.php')); + const oldUri = vscode.Uri.file(path.join(dir, 'Test.php')); + const newUri = vscode.Uri.file(path.join(dir, 'NewTest.php')); - const consumerContent = ' { test('does nothing when the class name did not actually change', async () => { const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'php-namespace-refactor-')); - const sameUri = vscode.Uri.file(path.join(dir, 'Teste.php')); + const sameUri = vscode.Uri.file(path.join(dir, 'Test.php')); - const consumerContent = ' { */ test('ignores a file that matches by text but was not part of the affected-files set', async () => { const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'php-namespace-refactor-')); - const oldUri = vscode.Uri.file(path.join(dir, 'Teste.php')); - const newUri = vscode.Uri.file(path.join(dir, 'Novo.php')); + const oldUri = vscode.Uri.file(path.join(dir, 'Test.php')); + const newUri = vscode.Uri.file(path.join(dir, 'NewTest.php')); - const unrelatedContent = ' rawValue, - } as unknown as ConfigurationLocator; + // A real ConfigurationLocator instance (so the resolver exercises the + // actual getPolymorphicFlag implementation) with only the underlying + // raw-value read stubbed out. + const configurationLocator = Object.assign( + Object.create(ConfigurationLocator.prototype) as ConfigurationLocator, + { get: () => rawValue }, + ); return new PropertyRenameSettingsResolver(configurationLocator); } @@ -28,17 +32,17 @@ suite('PropertyRenameSettingsResolver', () => { ); }); - test('resolves true to enabled, without the mismatch behavior', () => { + test('resolves true to enabled, with the mismatch behavior also on by default', () => { assert.deepStrictEqual( buildResolver(true).resolve(), - { enabled: true, renameMismatchedNames: false }, + { enabled: true, renameMismatchedNames: true }, ); }); - test('resolves an empty object to enabled, without the mismatch behavior', () => { + test('resolves an empty object to enabled, with the mismatch behavior also on by default', () => { assert.deepStrictEqual( buildResolver({}).resolve(), - { enabled: true, renameMismatchedNames: false }, + { enabled: true, renameMismatchedNames: true }, ); });