Skip to content

Commit 7f553f4

Browse files
author
Andy
authored
refactorConvertToGetAccessAndSetAccess: Don't trigger on leading trivia (#25054)
* refactorConvertToGetAccessAndSetAccess: Don't trigger on leading trivia * Update API (#24966)
1 parent db85f37 commit 7f553f4

File tree

6 files changed

+37
-16
lines changed

6 files changed

+37
-16
lines changed

src/harness/fourslash.ts

+9-1
Original file line numberDiff line numberDiff line change
@@ -3045,6 +3045,10 @@ Actual: ${stringify(fullActual)}`);
30453045
}
30463046
}
30473047

3048+
public verifyRefactorsAvailable(names: ReadonlyArray<string>): void {
3049+
assert.deepEqual(unique(this.getApplicableRefactors(this.getSelection()), r => r.name), names);
3050+
}
3051+
30483052
public verifyRefactor({ name, actionName, refactors }: FourSlashInterface.VerifyRefactorOptions) {
30493053
const actualRefactors = this.getApplicableRefactors(this.getSelection()).filter(r => r.name === name && r.actions.some(a => a.name === actionName));
30503054
this.assertObjectsEqual(actualRefactors, refactors);
@@ -3815,7 +3819,7 @@ ${code}
38153819
}
38163820

38173821
/** Collects an array of unique outputs. */
3818-
function unique<T>(inputs: T[], getOutput: (t: T) => string): string[] {
3822+
function unique<T>(inputs: ReadonlyArray<T>, getOutput: (t: T) => string): string[] {
38193823
const set = ts.createMap<true>();
38203824
for (const input of inputs) {
38213825
const out = getOutput(input);
@@ -4106,6 +4110,10 @@ namespace FourSlashInterface {
41064110
this.state.verifyApplicableRefactorAvailableForRange(this.negative);
41074111
}
41084112

4113+
public refactorsAvailable(names: ReadonlyArray<string>): void {
4114+
this.state.verifyRefactorsAvailable(names);
4115+
}
4116+
41094117
public refactor(options: VerifyRefactorOptions) {
41104118
this.state.verifyRefactor(options);
41114119
}

src/services/refactors/generateGetAccessorAndSetAccessor.ts

+14-15
Original file line numberDiff line numberDiff line change
@@ -9,20 +9,19 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor {
99
type ContainerDeclaration = ClassLikeDeclaration | ObjectLiteralExpression;
1010

1111
interface Info {
12-
container: ContainerDeclaration;
13-
isStatic: boolean;
14-
isReadonly: boolean;
15-
type: TypeNode | undefined;
16-
declaration: AcceptedDeclaration;
17-
fieldName: AcceptedNameType;
18-
accessorName: AcceptedNameType;
19-
originalName: AcceptedNameType;
20-
renameAccessor: boolean;
12+
readonly container: ContainerDeclaration;
13+
readonly isStatic: boolean;
14+
readonly isReadonly: boolean;
15+
readonly type: TypeNode | undefined;
16+
readonly declaration: AcceptedDeclaration;
17+
readonly fieldName: AcceptedNameType;
18+
readonly accessorName: AcceptedNameType;
19+
readonly originalName: AcceptedNameType;
20+
readonly renameAccessor: boolean;
2121
}
2222

2323
function getAvailableActions(context: RefactorContext): ApplicableRefactorInfo[] | undefined {
24-
const { file } = context;
25-
if (!getConvertibleFieldAtPosition(context, file)) return undefined;
24+
if (!getConvertibleFieldAtPosition(context)) return undefined;
2625

2726
return [{
2827
name: actionName,
@@ -39,7 +38,7 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor {
3938
function getEditsForAction(context: RefactorContext, _actionName: string): RefactorEditInfo | undefined {
4039
const { file } = context;
4140

42-
const fieldInfo = getConvertibleFieldAtPosition(context, file);
41+
const fieldInfo = getConvertibleFieldAtPosition(context);
4342
if (!fieldInfo) return undefined;
4443

4544
const isJS = isSourceFileJavaScript(file);
@@ -117,14 +116,14 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor {
117116
return name.charCodeAt(0) === CharacterCodes._;
118117
}
119118

120-
function getConvertibleFieldAtPosition(context: RefactorContext, file: SourceFile): Info | undefined {
121-
const { startPosition, endPosition } = context;
119+
function getConvertibleFieldAtPosition(context: RefactorContext): Info | undefined {
120+
const { file, startPosition, endPosition } = context;
122121

123122
const node = getTokenAtPosition(file, startPosition, /*includeJsDocComment*/ false);
124123
const declaration = findAncestor(node.parent, isAcceptedDeclaration);
125124
// make sure declaration have AccessibilityModifier or Static Modifier or Readonly Modifier
126125
const meaning = ModifierFlags.AccessibilityModifier | ModifierFlags.Static | ModifierFlags.Readonly;
127-
if (!declaration || !rangeOverlapsWithStartEnd(declaration.name, startPosition, endPosition!) // TODO: GH#18217
126+
if (!declaration || !nodeOverlapsWithStartEnd(declaration.name, file, startPosition, endPosition!) // TODO: GH#18217
128127
|| !isConvertibleName(declaration.name) || (getModifierFlags(declaration) | meaning) !== meaning) return undefined;
129128

130129
const name = declaration.name.text;

src/services/utilities.ts

+4
Original file line numberDiff line numberDiff line change
@@ -439,6 +439,10 @@ namespace ts {
439439
return startEndOverlapsWithStartEnd(r1.pos, r1.end, start, end);
440440
}
441441

442+
export function nodeOverlapsWithStartEnd(node: Node, sourceFile: SourceFile, start: number, end: number) {
443+
return startEndOverlapsWithStartEnd(node.getStart(sourceFile), node.end, start, end);
444+
}
445+
442446
export function startEndOverlapsWithStartEnd(start1: number, end1: number, start2: number, end2: number) {
443447
const start = Math.max(start1, start2);
444448
const end = Math.min(end1, end2);

tests/baselines/reference/api/tsserverlibrary.d.ts

+1
Original file line numberDiff line numberDiff line change
@@ -10670,6 +10670,7 @@ declare namespace ts {
1067010670
function startEndContainsRange(start: number, end: number, range: TextRange): boolean;
1067110671
function rangeContainsStartEnd(range: TextRange, start: number, end: number): boolean;
1067210672
function rangeOverlapsWithStartEnd(r1: TextRange, start: number, end: number): boolean;
10673+
function nodeOverlapsWithStartEnd(node: Node, sourceFile: SourceFile, start: number, end: number): boolean;
1067310674
function startEndOverlapsWithStartEnd(start1: number, end1: number, start2: number, end2: number): boolean;
1067410675
/**
1067510676
* Assumes `candidate.start <= position` holds.

tests/cases/fourslash/fourslash.ts

+1
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ declare namespace FourSlashInterface {
190190
applicableRefactorAvailableForRange(): void;
191191

192192
refactorAvailable(name: string, actionName?: string): void;
193+
refactorsAvailable(names: ReadonlyArray<string>): void;
193194
refactor(options: {
194195
name: string;
195196
actionName: string;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
////class A {
4+
/////*a*/ /*b*/p = 0;
5+
////}
6+
7+
goTo.select("a", "b");
8+
verify.refactorsAvailable([]);

0 commit comments

Comments
 (0)