Skip to content

Commit f5901e9

Browse files
satyakighsatyakigh
authored
Fix intrinsic parsing issues (#369)
1 parent fef1515 commit f5901e9

File tree

9 files changed

+144
-51
lines changed

9 files changed

+144
-51
lines changed

‎src/autocomplete/CompletionRouter.ts‎

Lines changed: 55 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
1+
import { SyntaxNode } from 'tree-sitter';
12
import { CompletionItem, CompletionParams } from 'vscode-languageserver';
23
import { Context } from '../context/Context';
34
import { ContextManager } from '../context/ContextManager';
45
import {
6+
EntitySection,
57
IntrinsicFunction,
8+
IntrinsicShortForms,
69
IntrinsicsUsingConditionKeyword,
710
ResourceAttribute,
811
TopLevelSection,
@@ -38,7 +41,6 @@ export type CompletionProviderType =
3841
| 'IntrinsicFunctionArgument'
3942
| 'ParameterTypeValue'
4043
| EntityType;
41-
const Condition = 'Condition';
4244

4345
export class CompletionRouter implements SettingsConfigurable, Closeable {
4446
private completionSettings: CompletionSettings = DefaultSettings.completion;
@@ -122,13 +124,34 @@ export class CompletionRouter implements SettingsConfigurable, Closeable {
122124
}
123125

124126
private shouldUseConditionCompletionProvider(context: Context): boolean {
127+
// Check for YAML short form !Condition - tree-sitter may place cursor node inside the argument,
128+
// so we check parent nodes to find the containing !Condition expression
129+
if (this.isInsideConditionShortForm(context.syntaxNode)) {
130+
return true;
131+
}
125132
return (
126-
context.entitySection === Condition ||
133+
context.entitySection === EntitySection.Condition ||
127134
this.isAtConditionKey(context) ||
128135
this.conditionUsageWithinIntrinsic(context)
129136
);
130137
}
131138

139+
private isInsideConditionShortForm(node?: SyntaxNode): boolean {
140+
if (!node) {
141+
return false;
142+
}
143+
144+
const MAX_PARENT_DEPTH = 3;
145+
let current: SyntaxNode | undefined | null = node;
146+
for (let i = 0; i < MAX_PARENT_DEPTH && current; i++) {
147+
if (current.text.startsWith(IntrinsicShortForms.Condition)) {
148+
return true;
149+
}
150+
current = current?.parent;
151+
}
152+
return false;
153+
}
154+
132155
private isAtConditionKey(context: Context): boolean {
133156
const propertyPath = context.propertyPath;
134157
if (propertyPath.length === 0) {
@@ -137,31 +160,43 @@ export class CompletionRouter implements SettingsConfigurable, Closeable {
137160

138161
const lastPathElement = propertyPath[propertyPath.length - 1];
139162

140-
if (lastPathElement === Condition) {
163+
if (lastPathElement === EntitySection.Condition) {
141164
return this.isInConditionUsageContext(context);
142165
}
143166

144167
return false;
145168
}
146169

147170
private isInConditionUsageContext(context: Context): boolean {
148-
// Resource Condition attribute: ['Resources', 'LogicalId', this.CONDITION]
149-
if (context.matchPathWithLogicalId(TopLevelSection.Resources, Condition)) {
171+
// Resource Condition attribute: ['Resources', 'LogicalId', 'Condition']
172+
if (context.matchPathWithLogicalId(TopLevelSection.Resources, EntitySection.Condition)) {
150173
return true;
151174
}
152175

153-
// Resource UpdatePolicy Condition: ['Resources', 'LogicalId', 'UpdatePolicy', this.CONDITION]
154-
if (context.matchPathWithLogicalId(TopLevelSection.Resources, ResourceAttribute.UpdatePolicy, Condition)) {
176+
// Resource UpdatePolicy Condition: ['Resources', 'LogicalId', 'UpdatePolicy', 'Condition']
177+
if (
178+
context.matchPathWithLogicalId(
179+
TopLevelSection.Resources,
180+
ResourceAttribute.UpdatePolicy,
181+
EntitySection.Condition,
182+
)
183+
) {
155184
return true;
156185
}
157186

158-
// Resource Metadata Condition: ['Resources', 'LogicalId', 'Metadata', this.CONDITION]
159-
if (context.matchPathWithLogicalId(TopLevelSection.Resources, ResourceAttribute.Metadata, Condition)) {
187+
// Resource Metadata Condition: ['Resources', 'LogicalId', 'Metadata', 'Condition']
188+
if (
189+
context.matchPathWithLogicalId(
190+
TopLevelSection.Resources,
191+
ResourceAttribute.Metadata,
192+
EntitySection.Condition,
193+
)
194+
) {
160195
return true;
161196
}
162197

163-
// Output Condition attribute: ['Outputs', 'LogicalId', this.CONDITION]
164-
return context.matchPathWithLogicalId(TopLevelSection.Outputs, Condition);
198+
// Output Condition attribute: ['Outputs', 'LogicalId', 'Condition']
199+
return context.matchPathWithLogicalId(TopLevelSection.Outputs, EntitySection.Condition);
165200
}
166201

167202
private conditionUsageWithinIntrinsic(context: Context): boolean {
@@ -252,9 +287,16 @@ export class CompletionRouter implements SettingsConfigurable, Closeable {
252287
}
253288

254289
private shouldUseIntrinsicFunctionProvider(context: Context): boolean {
255-
// YAML short form
290+
// YAML short form - check if user is typing a function name (starts with !)
256291
if (context.documentType !== DocumentType.JSON && context.text.startsWith('!')) {
257-
return true;
292+
// Check the last token - if it starts with !, user is typing a function name
293+
const lastSpaceIndex = context.text.lastIndexOf(' ');
294+
if (lastSpaceIndex === -1) {
295+
return true; // No space, just "!Ref" or "!Su"
296+
}
297+
// Check if part after last space starts with ! (nested function like "!Base64 !Re")
298+
const afterSpace = context.text.slice(Math.max(0, lastSpaceIndex + 1));
299+
return afterSpace.startsWith('!');
258300
}
259301

260302
// Typing "Fn:" for function name completion

‎src/autocomplete/ConditionCompletionProvider.ts‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { CompletionItem, CompletionItemKind, CompletionParams } from 'vscode-languageserver';
22
import { Context } from '../context/Context';
3-
import { TopLevelSection } from '../context/ContextType';
3+
import { IntrinsicShortForms, TopLevelSection } from '../context/ContextType';
44
import { getEntityMap } from '../context/SectionContextBuilder';
55
import { SyntaxTreeManager } from '../context/syntaxtree/SyntaxTreeManager';
66
import { LoggerFactory } from '../telemetry/LoggerFactory';
@@ -40,8 +40,14 @@ export class ConditionCompletionProvider implements CompletionProvider {
4040
[...conditionMap.keys()].filter((k) => k !== conditionLogicalNameToOmit),
4141
);
4242

43-
if (context.text.length > 0) {
44-
return this.conditionFuzzySearch(items, context.text);
43+
// Extract search text, handling !Condition prefix
44+
let searchText = context.text;
45+
if (searchText.startsWith(IntrinsicShortForms.Condition)) {
46+
searchText = searchText.slice(IntrinsicShortForms.Condition.length).trim();
47+
}
48+
49+
if (searchText.length > 0) {
50+
return this.conditionFuzzySearch(items, searchText);
4551
}
4652

4753
return items;

‎src/autocomplete/IntrinsicFunctionArgumentCompletionProvider.ts‎

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,13 @@
11
import { CompletionItem, CompletionItemKind, CompletionParams, Position, TextEdit } from 'vscode-languageserver';
22
import { pseudoParameterDocsMap } from '../artifacts/PseudoParameterDocs';
33
import { Context } from '../context/Context';
4-
import { IntrinsicFunction, PseudoParameter, PseudoParametersSet, TopLevelSection } from '../context/ContextType';
4+
import {
5+
IntrinsicFunction,
6+
IntrinsicShortForms,
7+
PseudoParameter,
8+
PseudoParametersSet,
9+
TopLevelSection,
10+
} from '../context/ContextType';
511
import { getEntityMap } from '../context/SectionContextBuilder';
612
import { Constant, Mapping, Parameter, Resource } from '../context/semantic/Entity';
713
import { EntityType } from '../context/semantic/SemanticTypes';
@@ -70,6 +76,16 @@ export class IntrinsicFunctionArgumentCompletionProvider implements CompletionPr
7076
return undefined;
7177
}
7278

79+
// Handle nested !Ref expressions (e.g., !Equals [!Ref AWS::, ...])
80+
// The intrinsicContext reports the outer function (Equals), but context.text contains the nested !Ref
81+
// Skip if user is typing another nested function (e.g., !Ref !Su)
82+
if (
83+
context.text.startsWith(IntrinsicShortForms.Ref) &&
84+
!context.text.slice(IntrinsicShortForms.Ref.length).trimStart().startsWith('!')
85+
) {
86+
return this.handleRefArguments(context, params, syntaxTree);
87+
}
88+
7389
const intrinsicFunction = context.intrinsicContext.intrinsicFunction();
7490
if (!intrinsicFunction) {
7591
return undefined;
@@ -114,11 +130,17 @@ export class IntrinsicFunctionArgumentCompletionProvider implements CompletionPr
114130
...constantsCompletions,
115131
];
116132

133+
// Extract just the argument part if context.text includes the !Ref prefix
134+
let searchText = context.text;
135+
if (searchText.startsWith(IntrinsicShortForms.Ref)) {
136+
searchText = searchText.slice(IntrinsicShortForms.Ref.length).trim();
137+
}
138+
117139
if (allCompletions.length === this.pseudoParameterCompletionItems.length) {
118-
return this.applyFuzzySearch(this.pseudoParameterCompletionItems, context.text);
140+
return this.applyFuzzySearch(this.pseudoParameterCompletionItems, searchText);
119141
}
120142

121-
return this.applyFuzzySearch(allCompletions, context.text);
143+
return this.applyFuzzySearch(allCompletions, searchText);
122144
}
123145

124146
private handleSubArguments(

‎src/context/ContextType.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,15 @@ export enum IntrinsicFunction {
4545
Implies = 'Fn::Implies',
4646
}
4747

48+
export enum IntrinsicShortForms {
49+
Condition = '!Condition',
50+
Ref = '!Ref',
51+
}
52+
53+
export enum EntitySection {
54+
Condition = 'Condition',
55+
}
56+
4857
export const IntrinsicsUsingConditionKeyword: ReadonlyArray<IntrinsicFunction> = [
4958
IntrinsicFunction.And,
5059
IntrinsicFunction.Or,

‎src/context/semantic/LogicalIdReferenceFinder.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -216,13 +216,13 @@ const YamlIfColon = /['"]?Fn::If['"]?\s*:\s*\[\s*['"]?([A-Za-z][A-Za-z0-9]*)['"]
216216
const YamlCondition = /(?<![A-Za-z])['"]?Condition['"]?\s*:\s*['"]?([A-Za-z][A-Za-z0-9]*)['"]?/g; // Matches Condition:, 'Condition':, "Condition": ConditionName with optional quoted values
217217
const YamlSingleDep = /(?<![A-Za-z])['"]?DependsOn['"]?\s*:\s*['"]?([A-Za-z][A-Za-z0-9]*)['"]?/g; // Matches DependsOn: LogicalId with optional quotes
218218
const YamlInlineDeps = /(?<![A-Za-z])['"]?DependsOn['"]?\s*:\s*\[([^\]]+)]/g; // Matches DependsOn: [Id1, Id2] with optional quotes
219-
const YamlListItem = /-\s*([A-Za-z][A-Za-z0-9]*)/g; // Matches - LogicalId in YAML list format
219+
const YamlListItem = /(?:^|\s)-\s+([A-Za-z][A-Za-z0-9]*)/gm; // Matches - LogicalId in YAML list format (requires whitespace before hyphen)
220220
const YamlInlineItemPattern = /([A-Za-z][A-Za-z0-9]*)/g; // Matches LogicalId within the inline array
221221
const YamlValueOfShort = /!ValueOf\s+\[\s*['"]?([A-Za-z][A-Za-z0-9]*)['"]?/g; // Matches !ValueOf [ParamName, Attr] - YAML short form
222222
const YamlValueOf = /['"]?Fn::ValueOf['"]?\s*:\s*\[\s*['"]?([A-Za-z][A-Za-z0-9]*)['"]?/g; // Matches Fn::ValueOf: [ParamName, Attr] with optional quotes
223223

224224
// Shared pattern for ${} variables - used by both JSON and YAML
225-
const SubVariables = /\$\{([A-Za-z][A-Za-z0-9]*)(?:[.:]|(?=\}))/g; // Matches ${LogicalId} or ${Resource.Attr} or ${AWS::Region} - captures first segment only
225+
const SubVariables = /\$\{([A-Za-z][A-Za-z0-9]*)(?:[^A-Za-z0-9]|$)/g; // Matches ${LogicalId} followed by any non-alphanumeric char or end
226226

227227
const ValidLogicalId = /^[A-Za-z][A-Za-z0-9.]+$/;
228228

‎src/definition/DefinitionProvider.ts‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,16 @@ export class DefinitionProvider {
1818
const locations = [];
1919
for (const section of context.relatedEntities.values()) {
2020
// For GetAtt expressions like "Vpc.VpcId", extract just the resource name "Vpc"
21-
const searchText = context.text.includes('.') ? context.text.split('.')[0] : context.text;
21+
let searchText = context.text.includes('.') ? context.text.split('.')[0] : context.text;
22+
23+
// When cursor is inside a Sub string like "${Variable}", context.text may be empty
24+
// or contain the full "${...}" pattern - extract the variable name from parent nodes
25+
if (!searchText || searchText.includes('${')) {
26+
const subVar = this.extractSubVariable(context.entityRootNode?.text);
27+
if (subVar) {
28+
searchText = subVar;
29+
}
30+
}
2231

2332
const relatedContext = section.get(searchText);
2433
if (relatedContext) {
@@ -38,4 +47,15 @@ export class DefinitionProvider {
3847
}
3948
return locations;
4049
}
50+
51+
private extractSubVariable(text: string | undefined): string | undefined {
52+
if (!text) {
53+
return undefined;
54+
}
55+
56+
const match = /\$\{([^}]+)}/.exec(text);
57+
if (match) {
58+
return match[1].split('.')[0]; // Return base name for GetAtt-style refs like ${Resource.Attr}
59+
}
60+
}
4161
}

‎tst/integration/autocomplete/Autocomplete.test.ts‎

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -310,21 +310,16 @@ Conditions:
310310
verification: {
311311
position: { line: 103, character: 25 },
312312
expectation: CompletionExpectationBuilder.create()
313-
.expectContainsItems(['AWS::Region'])
314-
.todo(
315-
`intrinsic functions are being suggested incorrectly!
316-
[
317-
"!RefAll",
318-
"!GetAZs",
319-
"!Ref",
320-
"!Equals",
321-
"!Base64",
322-
"!GetAtt",
323-
"!Transform",
324-
"!EachMemberEquals",
325-
"!EachMemberIn",
326-
]`,
327-
)
313+
.expectItems([
314+
'AWS::AccountId',
315+
'AWS::NoValue',
316+
'AWS::NotificationARNs',
317+
'AWS::Partition',
318+
'AWS::Region',
319+
'AWS::StackId',
320+
'AWS::StackName',
321+
'AWS::URLSuffix',
322+
])
328323
.build(),
329324
},
330325
},
@@ -393,8 +388,16 @@ Rules:
393388
verification: {
394389
position: { line: 114, character: 45 },
395390
expectation: CompletionExpectationBuilder.create()
396-
.expectContainsItems(['AWS::Region'])
397-
.todo(`no suggestion of pseudo-parameter after AWS:: is typed; needs the R`)
391+
.expectItems([
392+
'AWS::AccountId',
393+
'AWS::NoValue',
394+
'AWS::NotificationARNs',
395+
'AWS::Partition',
396+
'AWS::Region',
397+
'AWS::StackId',
398+
'AWS::StackName',
399+
'AWS::URLSuffix',
400+
])
398401
.build(),
399402
},
400403
},
@@ -1072,7 +1075,6 @@ Resources:
10721075
'IsProductionOrStaging',
10731076
])
10741077
.expectExcludesItems(['ComplexCondition', 'HasMultipleAZs'])
1075-
.todo(`not working even when testing not on last line of YAML`)
10761078
.build(),
10771079
},
10781080
},

‎tst/integration/goto/Goto.test.ts‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,6 @@ Resources:
125125
position: { line: 65, character: 32 },
126126
expectation: GotoExpectationBuilder.create()
127127
.expectDefinition('EnvironmentName')
128-
.todo('No logic for goto in formatted string')
129128
.expectDefinitionPosition({ line: 4, character: 2 })
130129
.build(),
131130
},
@@ -492,7 +491,6 @@ Resources:
492491
position: { line: 196, character: 35 },
493492
expectation: GotoExpectationBuilder.create()
494493
.expectDefinition('EnvironmentName')
495-
.todo('No logic for goto in formatted string')
496494
.expectDefinitionPosition({ line: 4, character: 2 })
497495
.build(),
498496
},
@@ -791,7 +789,6 @@ Outputs:
791789
position: { line: 96, character: 46 },
792790
expectation: GotoExpectationBuilder.create()
793791
.expectDefinition('EnvironmentName')
794-
.todo('No logic for goto in formatted string')
795792
.expectDefinitionPosition({ line: 4, character: 2 })
796793
.build(),
797794
},
@@ -1172,10 +1169,7 @@ Outputs:
11721169
description: 'Click on EnvironmentName in !Sub formatted string',
11731170
verification: {
11741171
position: { line: 319, character: 46 },
1175-
expectation: GotoExpectationBuilder.create()
1176-
.expectDefinition('EnvironmentName')
1177-
.todo('No logic for goto in formatted string')
1178-
.build(),
1172+
expectation: GotoExpectationBuilder.create().expectDefinition('EnvironmentName').build(),
11791173
},
11801174
},
11811175
{

‎tst/integration/hover/Hover.test.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -509,8 +509,7 @@ Conditions:`,
509509
verification: {
510510
position: { line: 101, character: 25 },
511511
expectation: HoverExpectationBuilder.create()
512-
.expectContainsText(['**Condition**, IsProductionOrStaging'])
513-
.todo(`hover on condition name reference for !Condition and not just Condition:`)
512+
.expectContainsText(['**Condition:** IsProductionOrStaging'])
514513
.build(),
515514
},
516515
},
@@ -2572,8 +2571,7 @@ Resources:
25722571
verification: {
25732572
position: { line: 114, character: 28 },
25742573
expectation: HoverExpectationBuilder.create()
2575-
.expectContainsText(['**Condition**, IsProductionOrStaging'])
2576-
.todo('hover on condition name reference for !Condition and not just Condition:')
2574+
.expectContainsText(['**Condition:** IsProductionOrStaging'])
25772575
.build(),
25782576
},
25792577
},

0 commit comments

Comments
 (0)