Skip to content

Commit 2322485

Browse files
authored
FindInMap Bug Fix (#60)
* FindInMap bug fix, removed .todo from a now passing test * Removed unecessary debug logs * FindInMap bug fix, removed .todo from a now passing test * Removed unecessary debug logs * Move getSecondLevelKeysDynamic into getSecondLevelKeys * fix lint errors
1 parent b887ae4 commit 2322485

File tree

3 files changed

+86
-20
lines changed

3 files changed

+86
-20
lines changed

src/autocomplete/IntrinsicFunctionArgumentCompletionProvider.ts

Lines changed: 71 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ export class IntrinsicFunctionArgumentCompletionProvider implements CompletionPr
7171
provider: 'IntrinsicFunctionArgument',
7272
context: context.record(),
7373
intrinsicFunction: intrinsicFunction.type,
74+
args: intrinsicFunction.args,
7475
},
7576
'Processing intrinsic function argument completion request',
7677
);
@@ -530,38 +531,98 @@ export class IntrinsicFunctionArgumentCompletionProvider implements CompletionPr
530531
context: Context,
531532
): CompletionItem[] | undefined {
532533
// Validate arguments structure for second-level keys
533-
if (!Array.isArray(args) || args.length < 2 || typeof args[0] !== 'string' || typeof args[1] !== 'string') {
534+
if (!this.isValidSecondLevelKeyArgs(args)) {
534535
log.debug('Invalid arguments for second-level key completions');
535536
return undefined;
536537
}
537538

538539
try {
539540
const mappingName = args[0];
540-
const topLevelKey = args[1];
541+
const topLevelKey = args[1] as string | { Ref: unknown } | { '!Ref': unknown };
541542

542543
const mappingEntity = this.getMappingEntity(mappingsEntities, mappingName);
543544
if (!mappingEntity) {
544545
log.debug(`Mapping entity not found: ${mappingName}`);
545546
return undefined;
546547
}
547548

548-
const secondLevelKeys = mappingEntity.getSecondLevelKeys(topLevelKey);
549+
const secondLevelKeys = this.getSecondLevelKeysForTopLevelKey(mappingEntity, topLevelKey);
549550
if (secondLevelKeys.length === 0) {
550-
log.debug(`No second-level keys found for mapping: ${mappingName}, top-level key: ${topLevelKey}`);
551+
log.debug(`No second-level keys found for mapping: ${mappingName}`);
551552
return undefined;
552553
}
553554

554555
const items = secondLevelKeys.map((key) =>
555556
createCompletionItem(key, CompletionItemKind.EnumMember, { context }),
556557
);
557558

558-
return context.text.length > 0 ? this.fuzzySearch(items, context.text) : items;
559+
return this.filterSecondLevelKeyItems(items, context, topLevelKey);
559560
} catch (error) {
560-
log.error({ error }, 'Error creating second-level key completions');
561+
log.debug({ error }, 'Error creating second-level key completions');
561562
return undefined;
562563
}
563564
}
564565

566+
private isValidSecondLevelKeyArgs(args: unknown): args is [string, string | object] {
567+
if (!Array.isArray(args) || args.length < 2 || typeof args[0] !== 'string') {
568+
return false;
569+
}
570+
571+
// Second argument valid if it is a string i.e. 'us-east-1' or object '{Ref: AWS::Region}' or '{!Ref: AWS::Region}'
572+
return typeof args[1] === 'string' || this.isRefObject(args[1]);
573+
}
574+
575+
private getSecondLevelKeysForTopLevelKey(
576+
mappingEntity: Mapping,
577+
topLevelKey: string | { Ref: unknown } | { '!Ref': unknown },
578+
): string[] {
579+
if (typeof topLevelKey === 'string') {
580+
return mappingEntity.getSecondLevelKeys(topLevelKey);
581+
} else {
582+
// For dynamic references, get all possible keys
583+
return mappingEntity.getSecondLevelKeys();
584+
}
585+
}
586+
587+
private filterSecondLevelKeyItems(
588+
items: CompletionItem[],
589+
context: Context,
590+
topLevelKey: string | { Ref: unknown } | { '!Ref': unknown },
591+
): CompletionItem[] {
592+
// Check if context.text contains the full FindInMap syntax (empty third argument case)
593+
if (context.text.startsWith('[') && context.text.endsWith(']')) {
594+
return items;
595+
}
596+
597+
// If no text typed, return all items
598+
if (context.text.length === 0) {
599+
return items;
600+
}
601+
602+
return this.applySecondLevelKeyFiltering(items, context, topLevelKey);
603+
}
604+
605+
private applySecondLevelKeyFiltering(
606+
items: CompletionItem[],
607+
context: Context,
608+
topLevelKey: string | { Ref: unknown } | { '!Ref': unknown },
609+
): CompletionItem[] {
610+
// For dynamic keys, try prefix matching first, then fall back to fuzzy search
611+
if (typeof topLevelKey === 'object') {
612+
const prefixMatches = items.filter((item) =>
613+
item.label.toLowerCase().startsWith(context.text.toLowerCase()),
614+
);
615+
616+
if (prefixMatches.length === 0) {
617+
return this.fuzzySearch(items, context.text);
618+
}
619+
620+
return prefixMatches;
621+
}
622+
623+
return this.fuzzySearch(items, context.text);
624+
}
625+
565626
private getMappingEntity(mappingsEntities: Map<string, Context>, mappingName: string): Mapping | undefined {
566627
try {
567628
const mappingContext = mappingsEntities.get(mappingName);
@@ -741,4 +802,8 @@ export class IntrinsicFunctionArgumentCompletionProvider implements CompletionPr
741802

742803
return undefined;
743804
}
805+
806+
private isRefObject(value: unknown): value is { Ref: unknown } | { '!Ref': unknown } {
807+
return typeof value === 'object' && value !== null && ('Ref' in value || '!Ref' in value);
808+
}
744809
}

src/context/semantic/Entity.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -140,9 +140,20 @@ export class Mapping extends Entity {
140140
return Object.keys(this.value);
141141
}
142142

143-
public getSecondLevelKeys(topLevelKey: string): string[] {
144-
if (this.value[topLevelKey]) {
145-
return Object.keys(this.value[topLevelKey]);
143+
public getSecondLevelKeys(topLevelKey?: string): string[] {
144+
if (topLevelKey === undefined) {
145+
const allKeys = new Set<string>();
146+
const topLevelKeys = this.getTopLevelKeys();
147+
148+
for (const tlKey of topLevelKeys) {
149+
const keys = this.getSecondLevelKeys(tlKey);
150+
for (const key of keys) allKeys.add(key);
151+
}
152+
return [...allKeys];
153+
} else {
154+
if (this.value[topLevelKey]) {
155+
return Object.keys(this.value[topLevelKey]);
156+
}
146157
}
147158
return [];
148159
}

tst/e2e/autocomplete/Autocomplete.test.ts

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -407,10 +407,6 @@ Rules:
407407
position: { line: 114, character: 54 },
408408
expectation: CompletionExpectationBuilder.create()
409409
.expectContainsItems(['InstanceType'])
410-
.todo(
411-
`second level of Mapping not being suggested when using !Ref for first level key
412-
works using 'us-east-1'`,
413-
)
414410
.build(),
415411
},
416412
},
@@ -1438,13 +1434,7 @@ O`,
14381434
description: 'suggest Mapping second level key in deeply nested intrinsic function',
14391435
verification: {
14401436
position: { line: 471, character: 61 },
1441-
expectation: CompletionExpectationBuilder.create()
1442-
.expectItems(['AMI'])
1443-
.todo(
1444-
`fix bug in FindInMap completion where using intrinsic in second arg breaks
1445-
suggestion for third arg`,
1446-
)
1447-
.build(),
1437+
expectation: CompletionExpectationBuilder.create().expectItems(['AMI']).build(),
14481438
},
14491439
},
14501440
{

0 commit comments

Comments
 (0)