Skip to content

Commit 20b43f7

Browse files
authored
fix: JSON resource type value completion formatted as key (#520)
* fix: JSON resource type value completion formatted as key CompletionFormatter.formatForJson treated all completions as key insertions — wrapping in quotes with trailing colon and replacing from column 0. This broke resource type value completion in JSON (e.g. typing AWS::S3::B inside "Type": "") because: 1. newText was '"AWS::S3::Bucket":' (colon appended to value) 2. textEdit range started at column 0, wiping out the "Type" key 3. filterText was '"AWS::S3::Bu"' which editors couldn't match Fix: detect value completions via CompletionItemKind.Class (used exclusively for resource types) and replace only the quoted value token with the correct text. Also fixes debug_tree.ts crash on undefined cfnFileType. Adds e2e and unit tests for JSON resource type value completion. * refactor: use syntax tree to detect JSON value position Replace item.kind === CompletionItemKind.Class check with proper syntax tree inspection. A node is a value completion when it is the value child of a pair node in the JSON tree-sitter AST: node.parent?.type === 'pair' && parent.childForFieldName('value') === node This is more precise than checking item.kind since TopLevelSectionCompletionProvider also uses CompletionItemKind.Class for key completions. * test: add e2e test for JSON enum value completion Adds test for DeletionPolicy enum values in JSON templates. This was also affected by the value formatting bug. * refactor: move JSON value detection to Context.isJsonPairValue() Add isJsonPairValue() to Context that uses the tree-sitter pair node structure to reliably detect value positions in JSON. A node is a value when it is the value child of a pair node. This is consistent with how YAML key/value detection works in Context (using synthetic nodes) and avoids relying on item.kind or the unreliable propertyPath heuristic for JSON.
1 parent 802e844 commit 20b43f7

5 files changed

Lines changed: 176 additions & 5 deletions

File tree

src/autocomplete/CompletionFormatter.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,10 @@ export class CompletionFormatter {
9595

9696
// Set filterText for ALL items (including snippets) when in JSON with quotes
9797
const isInJsonString = documentType === DocumentType.JSON && context.syntaxNode.type === 'string';
98-
if (isInJsonString) {
98+
const isJsonValueNode = isInJsonString && context.isJsonPairValue();
99+
if (isJsonValueNode) {
100+
formattedItem.filterText = `"${item.label}"`;
101+
} else if (isInJsonString) {
99102
formattedItem.filterText = `"${context.text}"`;
100103
}
101104

@@ -171,6 +174,20 @@ export class CompletionFormatter {
171174
const indentation = ' '.repeat(context.startPosition.column);
172175
const indentString = getIndentationString(editorSettings, DocumentType.JSON);
173176

177+
// When completing a value (e.g. resource type "AWS::S3::Bucket"), just replace the value text
178+
// Check the syntax tree: if the node is the value child of a JSON pair, it's a value completion
179+
const isValueCompletion = context.isJsonPairValue();
180+
if (isValueCompletion) {
181+
// Include surrounding quotes in the range so VS Code matches the full token
182+
const startCol = Math.max(0, context.startPosition.column - 1);
183+
const endCol = context.endPosition.column + 1;
184+
const range = Range.create(
185+
Position.create(context.startPosition.row, startCol),
186+
Position.create(context.endPosition.row, endCol),
187+
);
188+
return { text: `"${label}"`, range, isSnippet: false };
189+
}
190+
174191
let replacementText = `${indentation}"${label}":`;
175192
let isSnippet = false;
176193

src/context/Context.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import { entityTypeFromSection, nodeToEntity } from './semantic/EntityBuilder';
1919
import { normalizeIntrinsicFunction } from './semantic/Intrinsics';
2020
import { PropertyPath } from './syntaxtree/SyntaxTree';
2121
import { NodeType } from './syntaxtree/utils/NodeType';
22-
import { YamlNodeTypes, CommonNodeTypes } from './syntaxtree/utils/TreeSitterTypes';
22+
import { YamlNodeTypes, CommonNodeTypes, JsonNodeTypes, FieldNames } from './syntaxtree/utils/TreeSitterTypes';
2323
import { TransformContext } from './TransformContext';
2424

2525
type QuoteCharacter = '"' | "'";
@@ -179,6 +179,18 @@ export class Context {
179179
return !(this.propertyPath.at(-1) === this.text);
180180
}
181181

182+
/**
183+
* Check if the cursor is at a JSON value position using the syntax tree.
184+
* Uses the tree-sitter pair node structure: a node is a value when it is
185+
* the value child of a pair node.
186+
*/
187+
public isJsonPairValue(): boolean {
188+
return (
189+
this.node.parent?.type === JsonNodeTypes.PAIR &&
190+
this.node.parent.childForFieldName(FieldNames.VALUE) === this.node
191+
);
192+
}
193+
182194
public isResourceAttributeProperty(): boolean {
183195
if (this.section !== TopLevelSection.Resources || !this.hasLogicalId) {
184196
return false;

tools/debug_tree.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ class DebugTreeTool {
143143
path: filePath,
144144
extension: fileExtension,
145145
size: content.length,
146-
cloudFormationFileType: cloudFormationFileType!.toString(),
146+
cloudFormationFileType: cloudFormationFileType?.toString() ?? 'unknown',
147147
},
148148
syntaxTree: {
149149
rootNodeType: '',

tst/e2e/Completion.test.ts

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2508,6 +2508,75 @@ Resources:
25082508
});
25092509
});
25102510

2511+
describe('Resource Type Value', () => {
2512+
it('should provide resource type value completions when typing in Type field', async () => {
2513+
const template = getSimpleJsonTemplateText();
2514+
const updatedTemplate = `{
2515+
"AWSTemplateFormatVersion": "2010-09-09",
2516+
"Resources": {
2517+
"MyBucket": {
2518+
"Type": "AWS::S3::B"
2519+
}
2520+
}
2521+
}`;
2522+
const uri = await client.openJsonTemplate(template);
2523+
2524+
await client.changeDocument({
2525+
textDocument: { uri, version: 2 },
2526+
contentChanges: [{ text: updatedTemplate }],
2527+
});
2528+
2529+
const completions: any = await client.completion({
2530+
textDocument: { uri },
2531+
position: { line: 4, character: 23 },
2532+
});
2533+
2534+
expect(completions).toBeDefined();
2535+
expect(completions?.items).toBeDefined();
2536+
expect(completions.items.length).toBeGreaterThan(0);
2537+
2538+
const labels = completions.items.map((item: any) => item.label);
2539+
expect(labels).toContain('AWS::S3::Bucket');
2540+
2541+
await client.closeDocument({ textDocument: { uri } });
2542+
});
2543+
});
2544+
2545+
describe('Enum Value Completions', () => {
2546+
it('should provide enum value completions for DeletionPolicy in JSON', async () => {
2547+
const template = getSimpleJsonTemplateText();
2548+
const updatedTemplate = `{
2549+
"AWSTemplateFormatVersion": "2010-09-09",
2550+
"Resources": {
2551+
"MyBucket": {
2552+
"Type": "AWS::S3::Bucket",
2553+
"DeletionPolicy": ""
2554+
}
2555+
}
2556+
}`;
2557+
const uri = await client.openJsonTemplate(template);
2558+
2559+
await client.changeDocument({
2560+
textDocument: { uri, version: 2 },
2561+
contentChanges: [{ text: updatedTemplate }],
2562+
});
2563+
2564+
const completions: any = await client.completion({
2565+
textDocument: { uri },
2566+
position: { line: 5, character: 24 },
2567+
});
2568+
2569+
expect(completions).toBeDefined();
2570+
expect(completions?.items).toBeDefined();
2571+
expect(completions.items.length).toBeGreaterThan(0);
2572+
2573+
const labels = completions.items.map((item: any) => item.label);
2574+
expect(labels).toContain('Retain');
2575+
expect(labels).toContain('Delete');
2576+
2577+
await client.closeDocument({ textDocument: { uri } });
2578+
});
2579+
});
25112580
describe('Resource Attributes', () => {
25122581
it('should provide Type attribute completion', async () => {
25132582
const template = getSimpleJsonTemplateText();

tst/unit/autocomplete/CompletionFormatter.test.ts

Lines changed: 75 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -542,7 +542,7 @@ describe('CompletionFormatAdapter', () => {
542542
const mockContext = createResourceContext('MyBucket', {
543543
type: DocumentType.JSON,
544544
text: 'BucketEncryption',
545-
propertyPath: ['Resources', 'MyBucket', 'Properties'],
545+
propertyPath: ['Resources', 'MyBucket', 'Properties', 'BucketEncryption'],
546546
data: { Type: 'AWS::S3::Bucket' },
547547
nodeType: 'string',
548548
});
@@ -779,10 +779,11 @@ describe('CompletionFormatAdapter', () => {
779779

780780
describe('JSON filterText handling', () => {
781781
test('should set filterText with quotes when in JSON string context', () => {
782-
const mockContext = createTopLevelContext('Resources', {
782+
const mockContext = createTopLevelContext('Unknown', {
783783
type: DocumentType.JSON,
784784
nodeType: 'string',
785785
text: 'Res',
786+
propertyPath: ['Res'],
786787
});
787788
const completions: CompletionList = {
788789
isIncomplete: false,
@@ -869,4 +870,76 @@ describe('CompletionFormatAdapter', () => {
869870
expect(result.items[0].textEdit?.newText).toContain('"MultiTypeProperty": "$0"');
870871
});
871872
});
873+
874+
describe('JSON value completions', () => {
875+
function mockValueNode(context: any) {
876+
const node = context.syntaxNode;
877+
const pairParent = {
878+
type: 'pair',
879+
childForFieldName: (name: string) => (name === 'value' ? node : null),
880+
};
881+
Object.defineProperty(node, 'parent', { value: pairParent, writable: true });
882+
}
883+
884+
test('should format resource type value completion without colon', () => {
885+
const mockContext = createResourceContext('MyBucket', {
886+
type: DocumentType.JSON,
887+
text: 'AWS::S3::B',
888+
propertyPath: ['Resources', 'MyBucket', 'Type'],
889+
data: { Type: 'AWS::S3::B' },
890+
nodeType: 'string',
891+
});
892+
mockValueNode(mockContext);
893+
894+
const completions: CompletionList = {
895+
isIncomplete: false,
896+
items: [
897+
{
898+
label: 'AWS::S3::Bucket',
899+
kind: CompletionItemKind.Class,
900+
},
901+
],
902+
};
903+
904+
const lineContent = ' "Type": "AWS::S3::B"';
905+
const result = formatter.format(completions, mockContext, defaultEditorSettings, lineContent);
906+
907+
expect(result.items[0].textEdit).toBeDefined();
908+
// Value completions should not append a colon after the value
909+
expect(result.items[0].textEdit?.newText).not.toMatch(/:$/);
910+
expect(result.items[0].textEdit?.newText).not.toMatch(/:\s*$/);
911+
// Should include quotes around the value
912+
expect(result.items[0].textEdit?.newText).toContain('"AWS::S3::Bucket"');
913+
// filterText should include quotes for VS Code matching
914+
expect(result.items[0].filterText).toBe('"AWS::S3::Bucket"');
915+
});
916+
917+
test('should not replace the key when completing a value', () => {
918+
const mockContext = createResourceContext('MyBucket', {
919+
type: DocumentType.JSON,
920+
text: 'AWS::S3::B',
921+
propertyPath: ['Resources', 'MyBucket', 'Type'],
922+
data: { Type: 'AWS::S3::B' },
923+
nodeType: 'string',
924+
});
925+
mockValueNode(mockContext);
926+
927+
const completions: CompletionList = {
928+
isIncomplete: false,
929+
items: [
930+
{
931+
label: 'AWS::S3::Bucket',
932+
kind: CompletionItemKind.Class,
933+
},
934+
],
935+
};
936+
937+
const lineContent = ' "Type": "AWS::S3::B"';
938+
const result = formatter.format(completions, mockContext, defaultEditorSettings, lineContent);
939+
940+
// Range should start near the value, not at column 0
941+
const textEdit = result.items[0].textEdit as any;
942+
expect(textEdit.range.start.character).toBeGreaterThan(0);
943+
});
944+
});
872945
});

0 commit comments

Comments
 (0)