Skip to content

Commit d0010e7

Browse files
committed
fix(migrations): migrating input with more than 1 usage in a method
When the migration command was run for inputs, if the input had more than one reference in a method the migration would generate incorrect code Fixes #63018
1 parent e34776a commit d0010e7

File tree

2 files changed

+107
-1
lines changed

2 files changed

+107
-1
lines changed

packages/core/schematics/migrations/signal-migration/src/passes/reference_migration/helpers/standard_reference.ts

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import {analyzeControlFlow, ControlFlowAnalysisNode} from '../../../flow_analysi
1111
import {ProgramInfo, projectFile, Replacement, TextUpdate} from '../../../../../../utils/tsurge';
1212
import {traverseAccess} from '../../../utils/traverse_access';
1313
import {UniqueNamesGenerator} from '../../../utils/unique_names';
14-
import {createNewBlockToInsertVariable} from '../helpers/create_block_arrow_function';
14+
import {createNewBlockToInsertVariable} from './create_block_arrow_function';
1515
import assert from 'assert';
1616

1717
export interface NarrowableTsReferences {
@@ -123,6 +123,24 @@ export function migrateStandardTsReference(
123123
replacements.push(
124124
...createNewBlockToInsertVariable(parent, filePath, temporaryVariableStr),
125125
);
126+
} else if (shouldInsertAtMethodStart(reference, recommendedNode, referenceNodeInBlock)) {
127+
const blockNode = recommendedNode as ts.Block;
128+
const blockStart = blockNode.getStart() + 1;
129+
const firstStatement = blockNode.statements[0];
130+
const leadingSpace = firstStatement
131+
? ts.getLineAndCharacterOfPosition(sf, firstStatement.getStart())
132+
: ts.getLineAndCharacterOfPosition(sf, referenceNodeInBlock.getStart());
133+
134+
replacements.push(
135+
new Replacement(
136+
filePath,
137+
new TextUpdate({
138+
position: blockStart,
139+
end: blockStart,
140+
toInsert: `${' '.repeat(leadingSpace.character)}${temporaryVariableStr}\n${' '.repeat(leadingSpace.character)}`,
141+
}),
142+
),
143+
);
126144
} else {
127145
const leadingSpace = ts.getLineAndCharacterOfPosition(sf, referenceNodeInBlock.getStart());
128146

@@ -151,3 +169,43 @@ export function migrateStandardTsReference(
151169
}
152170
}
153171
}
172+
173+
/**
174+
* Determines if a temporary variable should be inserted at the start of a method.
175+
*
176+
* This function performs several checks to ensure it's safe to insert a temporary variable:
177+
* 1. Verifies the recommended node is a method declaration block
178+
* 2. Ensures all references are contained within the method body
179+
* 3. Confirms the reference node is the first statement in the method
180+
* 4. Validates the reference node is an expression statement with an assignment operation
181+
*/
182+
function shouldInsertAtMethodStart(
183+
references: NarrowableTsReferences,
184+
recommendedNode: ts.Node,
185+
referenceNodeInBlock: ts.Node,
186+
): boolean {
187+
if (!ts.isBlock(recommendedNode) || !ts.isMethodDeclaration(recommendedNode.parent)) {
188+
return false;
189+
}
190+
191+
const methodBody = recommendedNode;
192+
const allReferencesInMethod = references.accesses.every((access) => {
193+
let current: ts.Node | undefined = access;
194+
while (current && current !== methodBody) {
195+
current = current.parent;
196+
}
197+
return current === methodBody;
198+
});
199+
200+
if (!allReferencesInMethod) {
201+
return false;
202+
}
203+
204+
return (
205+
methodBody.statements.length > 0 &&
206+
ts.isExpressionStatement(referenceNodeInBlock) &&
207+
methodBody.statements[0] === referenceNodeInBlock &&
208+
ts.isBinaryExpression(referenceNodeInBlock.expression) &&
209+
referenceNodeInBlock.expression.operatorToken.kind === ts.SyntaxKind.EqualsToken
210+
);
211+
}

packages/core/schematics/test/signals_migration_spec.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,4 +122,52 @@ describe('combined signals migration', () => {
122122
`),
123123
);
124124
});
125+
126+
it('should be able to migrate input with more than 1 usage in a method', async () => {
127+
writeFile(
128+
'/index.ts',
129+
`
130+
import {Component, Input} from '@angular/core';
131+
132+
@Component({
133+
template: 'it works'
134+
})
135+
export class TestMigrationComponent {
136+
@Input() public model: any;
137+
138+
public onSaveClick(): void {
139+
this.model.requisitionId = 145;
140+
this.model.comment = 'value';
141+
this.model.status = 8;
142+
this.model.finilizeReasonId = 4
143+
}
144+
}`,
145+
);
146+
147+
await runMigration(['inputs']);
148+
149+
expect(stripWhitespace(tree.readContent('/index.ts'))).toBe(
150+
stripWhitespace(
151+
`
152+
import {Component, input} from '@angular/core';
153+
154+
@Component({
155+
template: 'it works'
156+
})
157+
export class TestMigrationComponent {
158+
public readonly model = input<any>(undefined);
159+
160+
public onSaveClick(): void {
161+
const model = this.model();
162+
163+
model.requisitionId = 145;
164+
model.comment = 'value';
165+
model.status = 8;
166+
model.finilizeReasonId = 4
167+
}
168+
}
169+
`,
170+
),
171+
);
172+
});
125173
});

0 commit comments

Comments
 (0)