Skip to content

Commit adc1835

Browse files
kddejonggemammercado
authored andcommitted
Update Entity provider to only suggest on keys (#27)
* Update Entity provider to only suggest on keys
1 parent c853172 commit adc1835

File tree

4 files changed

+68
-32
lines changed

4 files changed

+68
-32
lines changed

src/context/Context.ts

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -181,30 +181,31 @@ export class Context {
181181
return false;
182182
}
183183

184-
// Case 1: propertyPath.length === 3 (e.g., ['Resources', 'MyResource', 'Type'])
185-
// We're at a resource attribute level
186-
if (this.propertyPath.length === 3 && this.entitySection === this.text) {
187-
return true;
184+
// Case 1: If we are over 3 we know for sure we are beyond the entity level
185+
if (this.propertyPath.length > 3) {
186+
return false;
188187
}
189188

190-
// Case 2: propertyPath.length === 2 (e.g., ['Resources', 'MyResource'])
191-
// We need to distinguish between:
192-
// - Cursor in middle of resource name (should return false)
193-
// - Cursor after resource name, ready for attributes (should return true)
194-
if (this.propertyPath.length === 2) {
195-
// If the current text matches the logical ID (resource name),
196-
// it means the cursor is positioned within the resource name itself
197-
// In this case, we should NOT provide entity key completions
198-
if (this.text === this.logicalId) {
189+
// Case 2: Two situations exist that we need to account for:
190+
// isKey and isValue can be True when at the first key inside a value
191+
// when we are at level 2 this means we are at Entity/LogicalId as the first key
192+
// when we are at level 3 this means we are at Entity/LogicalId/Properties as the first key
193+
if (this.isKey() && this.isValue()) {
194+
if (this.propertyPath.length === 2) {
195+
return true;
196+
} else if (this.propertyPath.length === 3) {
199197
return false;
200198
}
199+
}
201200

202-
// If entitySection is undefined and text is not the resource name,
203-
// it means we're positioned after the resource name, ready for attributes
204-
return this.entitySection !== this.text;
201+
// Case 3 propertyPath.length === 2 (e.g., ['Resources', 'MyResource'])
202+
// We need to see if the cursor is in the resource logical id
203+
if (this.propertyPath.length === 2 && this.text === this.logicalId) {
204+
return false;
205205
}
206206

207-
return false;
207+
// Catch all at this point to say that the isKey is the most important thing
208+
return this.isKey();
208209
}
209210

210211
public getMappingKeys(): string[] {

tst/unit/autocomplete/EntityFieldCompletionProvider.test.ts

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,11 @@ describe('EntityFieldCompletionProvider', () => {
1414

1515
describe('Parameter', () => {
1616
test('should suggest Default, Description, ConstraintDescription with e as partial string', () => {
17-
const mockContext = createParameterContext('MyParameter', { text: 'e', data: { Type: 'String' } });
17+
const mockContext = createParameterContext('MyParameter', {
18+
text: 'e',
19+
data: { Type: 'String' },
20+
propertyPath: ['Parameters', 'MyParameter', 'e'],
21+
});
1822
const result = parameterFieldCompletionProvider.getCompletions(mockContext, mockParams);
1923
expect(result).toBeDefined();
2024
expect(result?.length).equal(9);
@@ -23,7 +27,11 @@ describe('EntityFieldCompletionProvider', () => {
2327
});
2428

2529
test('should be robust against typos and suggest Type when partial string is yp', () => {
26-
const mockContext = createParameterContext('MyParameter', { text: 'yp', data: { Type: undefined } });
30+
const mockContext = createParameterContext('MyParameter', {
31+
text: 'yp',
32+
data: { Type: undefined },
33+
propertyPath: ['Parameters', 'MyParameter', 'yp'],
34+
});
2735
const result = parameterFieldCompletionProvider.getCompletions(mockContext, mockParams);
2836
expect(result).toBeDefined();
2937
expect(result?.length).equal(1);
@@ -37,6 +45,7 @@ describe('EntityFieldCompletionProvider', () => {
3745
Type: 'string',
3846
Description: 'some description',
3947
},
48+
propertyPath: ['Parameters', 'MyParameter', 'e'],
4049
});
4150
const result = parameterFieldCompletionProvider.getCompletions(mockContext, mockParams);
4251
expect(result).toBeDefined();
@@ -47,7 +56,11 @@ describe('EntityFieldCompletionProvider', () => {
4756
});
4857

4958
test('should suggest all available fields starting with Type (required) when nothing typed yet', () => {
50-
const mockContext = createParameterContext('MyParameter', { text: '', data: { Type: undefined } });
59+
const mockContext = createParameterContext('MyParameter', {
60+
text: '',
61+
data: { Type: undefined },
62+
propertyPath: ['Parameters', 'MyParameter', ''],
63+
});
5164
const result = parameterFieldCompletionProvider.getCompletions(mockContext, mockParams);
5265
expect(result).toBeDefined();
5366
// All Parameter fields should be suggested when none are defined
@@ -80,6 +93,7 @@ describe('EntityFieldCompletionProvider', () => {
8093
Description: 'some description',
8194
Default: 'default value',
8295
},
96+
propertyPath: ['Parameters', 'MyParameter', ''],
8397
});
8498
const result = parameterFieldCompletionProvider.getCompletions(mockContext, mockParams);
8599
expect(result).toBeDefined();
@@ -103,7 +117,10 @@ describe('EntityFieldCompletionProvider', () => {
103117

104118
describe('Output', () => {
105119
test('should suggest export and description with e as partial string', () => {
106-
const mockContext = createOutputContext('MyOutput', { text: 'e' });
120+
const mockContext = createOutputContext('MyOutput', {
121+
text: 'e',
122+
propertyPath: ['Outputs', 'MyOutput', 'e'],
123+
});
107124
const result = outputFieldCompletionProvider.getCompletions(mockContext, mockParams);
108125
expect(result).toBeDefined();
109126
expect(result?.length).equal(2);
@@ -112,7 +129,10 @@ describe('EntityFieldCompletionProvider', () => {
112129
});
113130

114131
test('should be robust against typos and suggest Export when partial string is xpo', () => {
115-
const mockContext = createOutputContext('MyOutput', { text: 'xpo' });
132+
const mockContext = createOutputContext('MyOutput', {
133+
text: 'xpo',
134+
propertyPath: ['Outputs', 'MyOutput', 'xpo'],
135+
});
116136
const result = outputFieldCompletionProvider.getCompletions(mockContext, mockParams);
117137
expect(result).toBeDefined();
118138
expect(result?.length).equal(1);
@@ -125,6 +145,7 @@ describe('EntityFieldCompletionProvider', () => {
125145
data: {
126146
Description: 'some description',
127147
},
148+
propertyPath: ['Outputs', 'MyOutput', 'e'],
128149
});
129150
const result = outputFieldCompletionProvider.getCompletions(mockContext, mockParams);
130151
expect(result).toBeDefined();

tst/unit/autocomplete/ResourceEntityCompletionProvider.test.ts

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { describe, expect, test, beforeEach } from 'vitest';
22
import { CompletionParams, CompletionItemKind, CompletionItem, InsertTextFormat } from 'vscode-languageserver';
33
import { ResourceEntityCompletionProvider } from '../../../src/autocomplete/ResourceEntityCompletionProvider';
44
import { ResourceAttribute } from '../../../src/context/ContextType';
5+
import { YamlNodeTypes } from '../../../src/context/syntaxtree/utils/TreeSitterTypes';
56
import { CombinedSchemas } from '../../../src/schema/CombinedSchemas';
67
import { ResourceSchema } from '../../../src/schema/ResourceSchema';
78
import { ExtensionName } from '../../../src/utils/ExtensionConfig';
@@ -23,7 +24,11 @@ describe('ResourceEntityCompletionProvider', () => {
2324
});
2425

2526
test('should return resource heading completions when inside Resources section but not inside a resource type', () => {
26-
const mockContext = createResourceContext('MyResource', { text: '' });
27+
const mockContext = createResourceContext('MyResource', {
28+
text: '',
29+
nodeType: YamlNodeTypes.STRING_SCALAR,
30+
propertyPath: ['Resources', 'MyResource', ''],
31+
});
2732

2833
const result = provider.getCompletions(mockContext, mockParams);
2934

@@ -58,7 +63,10 @@ describe('ResourceEntityCompletionProvider', () => {
5863
});
5964

6065
test('should return filtered resource heading completions when text is provided', () => {
61-
const mockContext = createResourceContext('MyResource', { text: 'Prop' });
66+
const mockContext = createResourceContext('MyResource', {
67+
text: '',
68+
propertyPath: ['Resources', 'MyResource', ''],
69+
});
6270

6371
const result = provider.getCompletions(mockContext, mockParams);
6472

@@ -72,7 +80,10 @@ describe('ResourceEntityCompletionProvider', () => {
7280
});
7381

7482
test('should provide correct insert text for resource properties', () => {
75-
const mockContext = createResourceContext('MyResource', { text: '' });
83+
const mockContext = createResourceContext('MyResource', {
84+
text: '',
85+
propertyPath: ['Resources', 'MyResource', ''],
86+
});
7687
const completions = provider.getCompletions(mockContext, mockParams);
7788

7889
expect(completions).toBeDefined();
@@ -137,7 +148,7 @@ describe('ResourceEntityCompletionProvider', () => {
137148
// Setup context with a resource that has a Type
138149
const mockContext = createResourceContext('MyInstance', {
139150
text: '',
140-
propertyPath: ['Resources', 'MyInstance'],
151+
propertyPath: ['Resources', 'MyInstance', ''],
141152
data: {
142153
Type: 'AWS::EC2::Instance',
143154
},
@@ -166,7 +177,7 @@ describe('ResourceEntityCompletionProvider', () => {
166177
// Setup context with a resource that has a Type
167178
const mockContext = createResourceContext('MyInstance', {
168179
text: '',
169-
propertyPath: ['Resources', 'MyInstance'],
180+
propertyPath: ['Resources', 'MyInstance', ''],
170181
data: {
171182
Type: 'AWS::EC2::Instance',
172183
},
@@ -193,7 +204,7 @@ describe('ResourceEntityCompletionProvider', () => {
193204
// Setup context with a resource that has a Type
194205
const mockContext = createResourceContext('MyInstance', {
195206
text: '',
196-
propertyPath: ['Resources', 'MyInstance'],
207+
propertyPath: ['Resources', 'MyInstance', ''],
197208
data: {
198209
Type: 'AWS::EC2::Instance',
199210
},
@@ -218,7 +229,7 @@ describe('ResourceEntityCompletionProvider', () => {
218229
// Setup context with a resource that has no Type
219230
const mockContext = createResourceContext('MyResource', {
220231
text: '',
221-
propertyPath: ['Resources', 'MyResource'],
232+
propertyPath: ['Resources', 'MyResource', ''],
222233
data: {},
223234
});
224235

@@ -238,7 +249,7 @@ describe('ResourceEntityCompletionProvider', () => {
238249
// Setup context with a resource that has a Type
239250
const mockContext = createResourceContext('MyResource', {
240251
text: '',
241-
propertyPath: ['Resources', 'MyResource'],
252+
propertyPath: ['Resources', 'MyResource', ''],
242253
data: {
243254
Type: 'AWS::Unknown::Resource',
244255
},

tst/unit/autocomplete/ResourceSectionCompletionProvider.test.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,10 @@ describe('ResourceSectionCompletionProvider', () => {
108108
});
109109

110110
test('should delegate to entity provider when at entity key level', async () => {
111-
const mockContext = createResourceContext('MyResource', { text: '' });
111+
const mockContext = createResourceContext('MyResource', {
112+
text: '',
113+
propertyPath: ['Resources', 'MyResource', ''],
114+
});
112115
const entityProvider = resourceProviders.get('Entity' as any)!;
113116
const mockCompletions = [
114117
{ label: 'Type', kind: CompletionItemKind.Property },
@@ -147,7 +150,7 @@ describe('ResourceSectionCompletionProvider', () => {
147150
test('should delegate to property provider when at nested entity key level Properties', async () => {
148151
const mockContext = createResourceContext('MyBucket', {
149152
text: 'Bucket',
150-
propertyPath: ['Resources', 'MyBucket', 'Properties'],
153+
propertyPath: ['Resources', 'MyBucket', 'Properties', 'Bucket'],
151154
data: {
152155
Type: 'AWS::S3::Bucket',
153156
Properties: {},

0 commit comments

Comments
 (0)