fix duplicate logical id issue and made logical id give more information to user

Author: Deep Furiya

src/autocomplete/RelatedResourcesInlineCompletionProvider.ts

@@ -1,6 +1,6 @@
 import { InlineCompletionItem, InlineCompletionParams } from 'vscode-languageserver-protocol';
 import { Context } from '../context/Context';
-import { DocumentType } from '../document/Document';
+import { Document, DocumentType } from '../document/Document';
 import { DocumentManager } from '../document/DocumentManager';
 import { ResourceSchema } from '../schema/ResourceSchema';
 import { SchemaRetriever } from '../schema/SchemaRetriever';
@@ -10,6 +10,8 @@ import { applySnippetIndentation } from '../utils/IndentationUtils';
 import { CompletionFormatter } from './CompletionFormatter';
 import { InlineCompletionProvider } from './InlineCompletionProvider';
 
+type RelatedResource = { type: string; relatedTo: string };
+
 export class RelatedResourcesInlineCompletionProvider implements InlineCompletionProvider {
     private readonly log = LoggerFactory.getLogger(RelatedResourcesInlineCompletionProvider);
     private readonly MAX_SUGGESTIONS = 5;
@@ -54,29 +56,36 @@ export class RelatedResourcesInlineCompletionProvider implements InlineCompletio
                 return undefined;
             }
 
-            return this.generateInlineCompletionItems(relatedResourceTypes, params, context);
+            return this.generateInlineCompletionItems(relatedResourceTypes, params);
         } catch (error) {
             this.log.error({ error: String(error) }, 'Error generating related resources inline completion');
             return undefined;
         }
     }
 
-    private getRelatedResourceTypes(existingResourceTypes: string[]): string[] {
+    private getRelatedResourceTypes(existingResourceTypes: string[]): RelatedResource[] {
         const existingRelationships = new Map<string, Set<string>>();
         const allRelatedTypes = new Set<string>();
+        const typeToRelatedMap = new Map<string, string>();
 
         for (const resourceType of existingResourceTypes) {
             const relatedTypes = this.relationshipSchemaService.getAllRelatedResourceTypes(resourceType);
             existingRelationships.set(resourceType, relatedTypes);
             for (const type of relatedTypes) {
                 allRelatedTypes.add(type);
+                typeToRelatedMap.set(type, typeToRelatedMap.get(type) ?? resourceType);
             }
         }
 
         const existingTypesSet = new Set(existingResourceTypes);
         const suggestedTypes = [...allRelatedTypes].filter((type) => !existingTypesSet.has(type));
 
-        return this.rankSuggestionsByFrequency(suggestedTypes, existingRelationships);
+        const rankedTypes = this.rankSuggestionsByFrequency(suggestedTypes, existingRelationships);
+
+        return rankedTypes.map((type) => ({
+            type,
+            relatedTo: typeToRelatedMap.get(type) ?? type,
+        }));
     }
 
     private rankSuggestionsByFrequency(
@@ -108,36 +117,39 @@ export class RelatedResourcesInlineCompletionProvider implements InlineCompletio
     }
 
     private generateInlineCompletionItems(
-        relatedResourceTypes: string[],
+        relatedResourceTypes: RelatedResource[],
         params: InlineCompletionParams,
-        context: Context,
     ): InlineCompletionItem[] {
-        const completionItems: InlineCompletionItem[] = [];
-
+        const document = this.documentManager.get(params.textDocument.uri);
         const topSuggestions = relatedResourceTypes.slice(0, this.MAX_SUGGESTIONS);
 
-        for (const resourceType of topSuggestions) {
-            const insertText = this.generatePropertySnippet(resourceType, context.documentType, params);
+        return topSuggestions.map(({ type: resourceType, relatedTo }) => {
+            const insertText = this.generatePropertySnippet(resourceType, relatedTo, params, document);
 
-            completionItems.push({
+            return {
                 insertText,
                 range: {
                     start: params.position,
                     end: params.position,
                 },
-                filterText: `${resourceType}`,
-            });
-        }
-
-        return completionItems;
+                filterText: insertText,
+            };
+        });
     }
 
     private generatePropertySnippet(
         resourceType: string,
-        documentType: DocumentType,
+        relatedToType: string,
         params: InlineCompletionParams,
+        document: Document | undefined,
     ): string {
-        const logicalId = 'relatedResourceLogicalId';
+        const documentType = document?.documentType ?? DocumentType.YAML;
+        const baseLogicalId = `RelatedTo${relatedToType
+            .split('::')
+            .slice(1)
+            .join('')
+            .replaceAll(/[^a-zA-Z0-9]/g, '')}`;
+        const logicalId = this.getUniqueLogicalId(baseLogicalId, document);
         const indent0 = CompletionFormatter.getIndentPlaceholder(0);
         const indent1 = CompletionFormatter.getIndentPlaceholder(1);
 
@@ -243,4 +255,24 @@ export class RelatedResourcesInlineCompletionProvider implements InlineCompletio
             .replaceAll(/\n\s*{INDENT2}/g, `\n${propertyIndent}`)
             .replaceAll(/\n\s*{INDENT3}/g, `\n${' '.repeat(currentIndent + baseIndentSize * 3)}`);
     }
+
+    private getUniqueLogicalId(baseId: string, document: Document | undefined): string {
+        const logicalId = `${baseId}LogicalId`;
+
+        if (!document) {
+            return logicalId;
+        }
+
+        const templateText = document.getText();
+
+        let counter = 0;
+        let candidateId = logicalId;
+
+        while (templateText.includes(candidateId)) {
+            counter++;
+            candidateId = `${baseId}${counter}LogicalId`;
+        }
+
+        return candidateId;
+    }
 }

tst/unit/autocomplete/RelatedResourcesInlineCompletionProvider.test.ts

@@ -125,7 +125,7 @@ Resources:
             // Verify that suggested resources are from the expected set
             for (const item of result) {
                 expect(item.insertText).toBeDefined();
-                expect(item.insertText).toContain('relatedResourceLogicalId:');
+                expect(item.insertText).toContain('RelatedToS3BucketLogicalId:');
                 expect(item.insertText).toContain('Type:');
                 expect(item.range).toBeDefined();
                 expect(item.range.start).toEqual(mockParams.position);
@@ -164,7 +164,7 @@ Resources:
             expect(result).toBeDefined();
             expect(Array.isArray(result)).toBe(true);
             // Should only include AWS::IAM::Role (not the existing S3 and Lambda)
-            expect(result).toEqual(['AWS::IAM::Role']);
+            expect(result).toEqual([{ type: 'AWS::IAM::Role', relatedTo: 'AWS::S3::Bucket' }]);
         });
 
         test('should rank suggestions by frequency', () => {
@@ -181,21 +181,21 @@ Resources:
             expect(result).toBeDefined();
             expect(Array.isArray(result)).toBe(true);
             // AWS::IAM::Role should be first (appears in all 3), others alphabetically
-            expect(result[0]).toBe('AWS::IAM::Role');
+            expect(result[0].type).toBe('AWS::IAM::Role');
         });
     });
 
     describe('generateInlineCompletionItems', () => {
         test('should limit suggestions to top 5', () => {
             const manyResourceTypes = [
-                'AWS::IAM::Role',
-                'AWS::CloudFront::Distribution',
-                'AWS::API::Gateway',
-                'AWS::EC2::SecurityGroup',
-                'AWS::RDS::DBInstance',
-                'AWS::DynamoDB::Table',
-                'AWS::SNS::Topic',
-                'AWS::SQS::Queue',
+                { type: 'AWS::IAM::Role', relatedTo: 'AWS::S3::Bucket' },
+                { type: 'AWS::CloudFront::Distribution', relatedTo: 'AWS::S3::Bucket' },
+                { type: 'AWS::API::Gateway', relatedTo: 'AWS::Lambda::Function' },
+                { type: 'AWS::EC2::SecurityGroup', relatedTo: 'AWS::EC2::Instance' },
+                { type: 'AWS::RDS::DBInstance', relatedTo: 'AWS::EC2::Instance' },
+                { type: 'AWS::DynamoDB::Table', relatedTo: 'AWS::Lambda::Function' },
+                { type: 'AWS::SNS::Topic', relatedTo: 'AWS::Lambda::Function' },
+                { type: 'AWS::SQS::Queue', relatedTo: 'AWS::Lambda::Function' },
             ];
 
             const result = (provider as any).generateInlineCompletionItems(manyResourceTypes, mockParams, mockContext);
@@ -206,7 +206,10 @@ Resources:
         });
 
         test('should create proper completion items structure', () => {
-            const resourceTypes = ['AWS::IAM::Role', 'AWS::CloudFront::Distribution'];
+            const resourceTypes = [
+                { type: 'AWS::IAM::Role', relatedTo: 'AWS::S3::Bucket' },
+                { type: 'AWS::CloudFront::Distribution', relatedTo: 'AWS::S3::Bucket' },
+            ];
 
             const result = (provider as any).generateInlineCompletionItems(resourceTypes, mockParams, mockContext);
 
@@ -215,7 +218,7 @@ Resources:
 
             for (const item of result) {
                 expect(item.insertText).toBeDefined();
-                expect(item.insertText).toContain('relatedResourceLogicalId:');
+                expect(item.insertText).toContain('RelatedToS3BucketLogicalId:');
                 expect(item.range).toBeDefined();
                 expect(item.range.start).toEqual(mockParams.position);
                 expect(item.range.end).toEqual(mockParams.position);
@@ -235,34 +238,48 @@ Resources:
     describe('generatePropertySnippet', () => {
         test('should generate basic property snippet', () => {
             const resourceType = 'AWS::S3::Bucket';
+            const relatedToType = 'AWS::Lambda::Function';
 
             const result = (provider as any).generatePropertySnippet(
                 resourceType,
-                mockContext.documentType,
+                relatedToType,
                 mockParams,
+                undefined,
             );
 
             expect(result).toBeDefined();
-            expect(result).toContain('relatedResourceLogicalId:');
+            expect(result).toContain('RelatedToLambdaFunctionLogicalId:');
             expect(result).toContain('Type: AWS::S3::Bucket');
         });
 
         test('should handle different resource types', () => {
-            const testCases = ['AWS::Lambda::Function', 'AWS::IAM::Role', 'AWS::EC2::Instance', 'AWS::RDS::DBInstance'];
+            const testCases = [
+                { resourceType: 'AWS::Lambda::Function', relatedTo: 'AWS::S3::Bucket' },
+                { resourceType: 'AWS::IAM::Role', relatedTo: 'AWS::Lambda::Function' },
+                { resourceType: 'AWS::EC2::Instance', relatedTo: 'AWS::IAM::Role' },
+                { resourceType: 'AWS::RDS::DBInstance', relatedTo: 'AWS::EC2::Instance' },
+            ];
 
-            for (const resourceType of testCases) {
+            for (const { resourceType, relatedTo } of testCases) {
                 const result = (provider as any).generatePropertySnippet(
                     resourceType,
-                    mockContext.documentType,
+                    relatedTo,
                     mockParams,
+                    undefined,
                 );
-                expect(result).toContain('relatedResourceLogicalId:');
+                const expectedLogicalId = `RelatedTo${relatedTo
+                    .split('::')
+                    .slice(1)
+                    .join('')
+                    .replaceAll(/[^a-zA-Z0-9]/g, '')}LogicalId`;
+                expect(result).toContain(`${expectedLogicalId}:`);
                 expect(result).toContain(`Type: ${resourceType}`);
             }
         });
 
         test('should include required properties when schema has required fields', () => {
             const resourceType = 'AWS::S3::Bucket';
+            const relatedToType = 'AWS::Lambda::Function';
 
             // Mock schema with required properties
             const mockSchema = {
@@ -275,11 +292,12 @@ Resources:
 
             const result = (provider as any).generatePropertySnippet(
                 resourceType,
-                mockContext.documentType,
+                relatedToType,
                 mockParams,
+                undefined,
             );
 
-            expect(result).toContain('relatedResourceLogicalId:');
+            expect(result).toContain('RelatedToLambdaFunctionLogicalId:');
             expect(result).toContain(`Type: ${resourceType}`);
             expect(result).toContain('Properties:');
             expect(result).toContain('BucketName: ');
@@ -290,6 +308,7 @@ Resources:
 
         test('should generate properly indented YAML snippets with cursor context', () => {
             const resourceType = 'AWS::IAM::Role';
+            const relatedToType = 'AWS::S3::Bucket';
             const mockSchema = {
                 required: ['AssumeRolePolicyDocument'],
             };
@@ -316,25 +335,28 @@ Resources:
                     '      BucketName: test',
                     '  ',
                 ],
+                documentType: 'YAML',
             };
             mockDocumentManager.get.returns(mockDocument as any);
             mockDocumentManager.getEditorSettingsForDocument.returns({ tabSize: 2, insertSpaces: true } as any);
 
             const result = (provider as any).generatePropertySnippet(
                 resourceType,
-                mockContext.documentType,
+                relatedToType,
                 mockParams,
+                mockDocument,
             );
 
             // Verify proper YAML structure with cursor context indentation
-            expect(result).toContain('relatedResourceLogicalId:');
+            expect(result).toContain('RelatedToS3BucketLogicalId:');
             expect(result).toContain('    Type: AWS::IAM::Role'); // 4 spaces (2 existing + 2 more)
             expect(result).toContain('    Properties:'); // 4 spaces (2 existing + 2 more)
             expect(result).toContain('      AssumeRolePolicyDocument: '); // 6 spaces (2 existing + 4 more)
         });
 
         test('should generate properly indented JSON snippets with cursor context', () => {
             const resourceType = 'AWS::IAM::Role';
+            const relatedToType = 'AWS::S3::Bucket';
             const mockSchema = {
                 required: ['AssumeRolePolicyDocument'],
             };
@@ -361,20 +383,20 @@ Resources:
                     '    },',
                     '  ',
                 ],
+                documentType: 'JSON',
             };
             mockDocumentManager.get.returns(mockDocument as any);
             mockDocumentManager.getEditorSettingsForDocument.returns({ tabSize: 2, insertSpaces: true } as any);
 
-            const jsonContext = { ...mockContext, documentType: 'JSON' };
-
             const result = (provider as any).generatePropertySnippet(
                 resourceType,
-                jsonContext.documentType,
+                relatedToType,
                 mockParams,
+                mockDocument,
             );
 
             // Verify proper JSON structure with cursor context indentation
-            expect(result).toContain('"relatedResourceLogicalId": {');
+            expect(result).toContain('"RelatedToS3BucketLogicalId": {');
             expect(result).toContain('    "Type": "AWS::IAM::Role"'); // 4 spaces (2 existing + 2 more)
             expect(result).toContain('    "Properties": {'); // 4 spaces (2 existing + 2 more)
             expect(result).toContain('      "AssumeRolePolicyDocument": ""'); // 6 spaces (2 existing + 4 more)