diff --git a/.quality-baseline.json b/.quality-baseline.json index 958dd78..dda4559 100644 --- a/.quality-baseline.json +++ b/.quality-baseline.json @@ -219,12 +219,6 @@ "singleUseVariable" ] }, - "src/core/commands/rename-command.ts": { - "lastCommitId": "bf89440ad4d87ed9c05abccbdce0a584a374bedf", - "violations": [ - "singleUseVariable" - ] - }, "src/core/commands/select-command.ts": { "lastCommitId": "876773659621d2929ad9c16187ad30ab7dcc177b", "violations": [ diff --git a/src/core/commands/rename-command.ts b/src/core/commands/rename-command.ts index 6ee54a0..a4584c0 100644 --- a/src/core/commands/rename-command.ts +++ b/src/core/commands/rename-command.ts @@ -6,6 +6,8 @@ import { VariableLocator, VariableNodeResult } from '../locators/variable-locato import { RenameVariableTransformation } from '../transformations/rename-variable-transformation'; import { LocationRange } from '../ast/location-range'; import { NodeAnalyzer } from '../services/node-analyzer'; +import { VariableNameValidator } from '../services/variable-name-validator'; +import { ScopeAnalyzer } from '../services/scope-analyzer'; export class RenameCommand implements RefactoringCommand { readonly name = 'rename'; @@ -15,15 +17,15 @@ export class RenameCommand implements RefactoringCommand { private consoleOutput!: ConsoleOutput; private astService!: ASTService; private variableLocator!: VariableLocator; + private nameValidator!: VariableNameValidator; async execute(file: string, options: CommandOptions): Promise { this.validateOptions(options); this.astService = ASTService.createForFile(file); this.variableLocator = new VariableLocator(this.astService.getProject()); - const sourceFile = this.astService.loadSourceFile(file); - const node = this.findTargetNode(options); - await this.performRename(node, options.to as string); - await this.astService.saveSourceFile(sourceFile); + this.nameValidator = new VariableNameValidator(); + await this.performRename(this.findTargetNode(options), options.to as string); + await this.astService.saveSourceFile(this.astService.loadSourceFile(file)); } private findTargetNode(options: CommandOptions): Node { @@ -47,8 +49,9 @@ export class RenameCommand implements RefactoringCommand { NodeAnalyzer.validateIdentifierNode(node); const sourceFile = node.getSourceFile(); const nodeResult = this.findVariableNodesAtPosition(node, sourceFile); - const transformation = this.createRenameTransformation(nodeResult, newName); - await transformation.transform(sourceFile); + + this.validateNewName(nodeResult.declaration, newName); + await this.createRenameTransformation(nodeResult, newName).transform(sourceFile); } private findVariableNodesAtPosition(node: Node, sourceFile: SourceFile) { @@ -71,4 +74,8 @@ export class RenameCommand implements RefactoringCommand { setConsoleOutput(consoleOutput: ConsoleOutput): void { this.consoleOutput = consoleOutput; } + + private validateNewName(declarationNode: Node, newName: string): void { + this.nameValidator.generateUniqueName(newName, ScopeAnalyzer.getNodeScope(declarationNode)); + } } \ No newline at end of file diff --git a/tests/fixtures/commands/rename/parameter-conflict.expected.err b/tests/fixtures/commands/rename/parameter-conflict.expected.err new file mode 100644 index 0000000..89aa6a0 --- /dev/null +++ b/tests/fixtures/commands/rename/parameter-conflict.expected.err @@ -0,0 +1 @@ +Variable name 'param' already exists in this scope. Please choose a different name. \ No newline at end of file diff --git a/tests/fixtures/commands/rename/parameter-conflict.expected.ts b/tests/fixtures/commands/rename/parameter-conflict.expected.ts new file mode 100644 index 0000000..9cd4c36 --- /dev/null +++ b/tests/fixtures/commands/rename/parameter-conflict.expected.ts @@ -0,0 +1,9 @@ +/** + * @description Test rename with parameter conflict + * @command refakts rename "[parameter-conflict.input.ts 7:11-7:12]" --to "param" + * @expect-error true + */ +function f(param: number) { + const x = 1; + return x + param; +} \ No newline at end of file diff --git a/tests/fixtures/commands/rename/parameter-conflict.input.ts b/tests/fixtures/commands/rename/parameter-conflict.input.ts new file mode 100644 index 0000000..9cd4c36 --- /dev/null +++ b/tests/fixtures/commands/rename/parameter-conflict.input.ts @@ -0,0 +1,9 @@ +/** + * @description Test rename with parameter conflict + * @command refakts rename "[parameter-conflict.input.ts 7:11-7:12]" --to "param" + * @expect-error true + */ +function f(param: number) { + const x = 1; + return x + param; +} \ No newline at end of file diff --git a/tests/fixtures/commands/rename/parent-scope-conflict.expected.err b/tests/fixtures/commands/rename/parent-scope-conflict.expected.err new file mode 100644 index 0000000..46e3abd --- /dev/null +++ b/tests/fixtures/commands/rename/parent-scope-conflict.expected.err @@ -0,0 +1 @@ +Variable name 'y' already exists in this scope. Please choose a different name. \ No newline at end of file diff --git a/tests/fixtures/commands/rename/parent-scope-conflict.expected.ts b/tests/fixtures/commands/rename/parent-scope-conflict.expected.ts new file mode 100644 index 0000000..6cad157 --- /dev/null +++ b/tests/fixtures/commands/rename/parent-scope-conflict.expected.ts @@ -0,0 +1,14 @@ +/** + * @description Test rename with parent scope conflict + * @command refakts rename "[parent-scope-conflict.input.ts 7:11-7:12]" --to "y" + * @expect-error true + */ +function f() { + const x = 1; + { + const y = 2; + { + return x; + } + } +} \ No newline at end of file diff --git a/tests/fixtures/commands/rename/parent-scope-conflict.input.ts b/tests/fixtures/commands/rename/parent-scope-conflict.input.ts new file mode 100644 index 0000000..6cad157 --- /dev/null +++ b/tests/fixtures/commands/rename/parent-scope-conflict.input.ts @@ -0,0 +1,14 @@ +/** + * @description Test rename with parent scope conflict + * @command refakts rename "[parent-scope-conflict.input.ts 7:11-7:12]" --to "y" + * @expect-error true + */ +function f() { + const x = 1; + { + const y = 2; + { + return x; + } + } +} \ No newline at end of file diff --git a/tests/fixtures/commands/rename/same-scope-conflict.expected.err b/tests/fixtures/commands/rename/same-scope-conflict.expected.err new file mode 100644 index 0000000..46e3abd --- /dev/null +++ b/tests/fixtures/commands/rename/same-scope-conflict.expected.err @@ -0,0 +1 @@ +Variable name 'y' already exists in this scope. Please choose a different name. \ No newline at end of file diff --git a/tests/fixtures/commands/rename/same-scope-conflict.expected.ts b/tests/fixtures/commands/rename/same-scope-conflict.expected.ts new file mode 100644 index 0000000..0bfa9ea --- /dev/null +++ b/tests/fixtures/commands/rename/same-scope-conflict.expected.ts @@ -0,0 +1,10 @@ +/** + * @description Test rename with same scope conflict + * @command refakts rename "[same-scope-conflict.input.ts 7:11-7:12]" --to "y" + * @expect-error true + */ +function f() { + const x = 1; + const y = 2; + return x + y; +} \ No newline at end of file diff --git a/tests/fixtures/commands/rename/same-scope-conflict.input.ts b/tests/fixtures/commands/rename/same-scope-conflict.input.ts new file mode 100644 index 0000000..0bfa9ea --- /dev/null +++ b/tests/fixtures/commands/rename/same-scope-conflict.input.ts @@ -0,0 +1,10 @@ +/** + * @description Test rename with same scope conflict + * @command refakts rename "[same-scope-conflict.input.ts 7:11-7:12]" --to "y" + * @expect-error true + */ +function f() { + const x = 1; + const y = 2; + return x + y; +} \ No newline at end of file