Skip to content

Commit 14188bc

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 angular#63018
1 parent e34776a commit 14188bc

2 files changed

Lines changed: 115 additions & 1 deletion

File tree

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

Lines changed: 67 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,26 @@ export function migrateStandardTsReference(
123123
replacements.push(
124124
...createNewBlockToInsertVariable(parent, filePath, temporaryVariableStr),
125125
);
126+
} else if (
127+
ts.isBlock(recommendedNode) &&
128+
shouldInsertAtMethodStart(reference, recommendedNode, referenceNodeInBlock)
129+
) {
130+
const blockStart = recommendedNode.getStart() + 1;
131+
const firstStatement = recommendedNode.statements[0];
132+
const leadingSpace = firstStatement
133+
? ts.getLineAndCharacterOfPosition(sf, firstStatement.getStart())
134+
: ts.getLineAndCharacterOfPosition(sf, referenceNodeInBlock.getStart());
135+
136+
replacements.push(
137+
new Replacement(
138+
filePath,
139+
new TextUpdate({
140+
position: blockStart,
141+
end: blockStart,
142+
toInsert: `${' '.repeat(leadingSpace.character)}${temporaryVariableStr}\n${' '.repeat(leadingSpace.character)}`,
143+
}),
144+
),
145+
);
126146
} else {
127147
const leadingSpace = ts.getLineAndCharacterOfPosition(sf, referenceNodeInBlock.getStart());
128148

@@ -151,3 +171,49 @@ export function migrateStandardTsReference(
151171
}
152172
}
153173
}
174+
175+
/**
176+
* Determines if a temporary variable should be inserted at the start of a method.
177+
*
178+
* This function performs several checks to ensure it's safe to insert a temporary variable:
179+
* 1. Verifies the recommended node is a method declaration block
180+
* 2. Ensures all references are contained within the method body
181+
* 3. Confirms the reference node is the first statement in the method
182+
* 4. Validates the reference node is an expression statement with an assignment operation
183+
*
184+
* @param references - Object containing the references to be checked
185+
* @param recommendedNode - The node where insertion is being considered (must be a method body block)
186+
* @param referenceNodeInBlock - The specific node within the block where the reference occurs
187+
* @returns `true` if all conditions are met for safe insertion at method start,
188+
* `false` if any condition fails
189+
*/
190+
function shouldInsertAtMethodStart(
191+
references: NarrowableTsReferences,
192+
recommendedNode: ts.Node,
193+
referenceNodeInBlock: ts.Node,
194+
): boolean {
195+
if (!ts.isBlock(recommendedNode) || !ts.isMethodDeclaration(recommendedNode.parent)) {
196+
return false;
197+
}
198+
199+
const methodBody = recommendedNode;
200+
const allReferencesInMethod = references.accesses.every((access) => {
201+
let current: ts.Node | undefined = access;
202+
while (current && current !== methodBody) {
203+
current = current.parent;
204+
}
205+
return current === methodBody;
206+
});
207+
208+
if (!allReferencesInMethod) {
209+
return false;
210+
}
211+
212+
return (
213+
methodBody.statements.length > 0 &&
214+
ts.isExpressionStatement(referenceNodeInBlock) &&
215+
methodBody.statements[0] === referenceNodeInBlock &&
216+
ts.isBinaryExpression(referenceNodeInBlock.expression) &&
217+
referenceNodeInBlock.expression.operatorToken.kind === ts.SyntaxKind.EqualsToken
218+
);
219+
}

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)