fix(50415): Language server debug failure - Did not expect GetAccessor to have an Identifier in its trivia

[Oleksandr Tarasiuk (a-tarasyuk)]

Fixes #50415

[Daniel Rosenwasser (DanielRosenwasser)]

Can you amend the test case to test both orders of accessors as in

20432a6

and also clone the nodes for the set-accessor even if it's technically unnecessary?

b1358db

[Oleksandr Tarasiuk (a-tarasyuk)]

and also clone the nodes for the set-accessor even if it's technically unnecessary?

Daniel Rosenwasser (@DanielRosenwasser) We need to clone nodes only for one of them - get or set. We have one copy of the properties applied to two methods simultaneously, which causes the error. Most of the props are new copies, so we just need to create new copies for one of them

TypeScript/src/services/codefixes/helpers.ts

Lines 82 to 84 in 0715791

	const name = getSynthesizedDeepClone(getNameOfDeclaration(declaration), /*includeTrivia*/ false) as PropertyName;
	const visibilityModifier = createVisibilityModifier(declaration ? getEffectiveModifierFlags(declaration) : ModifierFlags.None);
	const modifiers = visibilityModifier ? factory.createNodeArray([visibilityModifier]) : undefined;

[Daniel Rosenwasser (DanielRosenwasser)]

I understand that - I guess it doesn't entirely matter, but there are so many defensive copies all over the method other than in the updated section of code that I felt like it was probably better to be consistent than risk someone else glancing at the one place where we don't do a defensive copy.

If you're open to it, I was thinking of switching name, modifiers, and body to be local helper functions that generate unique deep clones for each usage. I'm not whether others would see it as an improvement.

[Gabriela Araujo Britto (gabritto)]

Looks good to me, thanks. I don't really have an opinion on what Daniel suggested, so it's up to you and him.

[Oleksandr Tarasiuk (a-tarasyuk)]

Gabriela Araujo Britto (@gabritto) I'll add updates