Handle non-string parameter values in hover (#536)

Author: chrisqm-dev

src/context/semantic/Entity.ts

@@ -1,8 +1,8 @@
-import { stringToBoolean } from '../../utils/String';
+import { stringToBoolean, toStringOrUndefined } from '../../utils/String';
 import { toNumber } from '../../utils/TypeConverters';
 import { EntityType } from '../CloudFormationEnums';
 import { CfnIntrinsicFunction, CfnValue, MappingValueType } from './CloudFormationTypes';
-import { coerceParameterToTypedValues, ParameterType, ParameterValueType } from './ParameterType';
+import { coerceParameterToTypedValues, ParameterType, ParameterValueType, PARAMETER_TYPES } from './ParameterType';
 
 export abstract class Entity {
     private _keys!: ReadonlyArray<string>;
@@ -114,12 +114,14 @@ export class Parameter extends Entity {
 
         return new Parameter(
             logicalId,
-            object['Type'] as ParameterType | undefined,
+            typeof object['Type'] === 'string' && PARAMETER_TYPES.includes(object['Type'] as ParameterType)
+                ? (object['Type'] as ParameterType)
+                : undefined,
             Default,
-            object['AllowedPattern'] as string | undefined,
+            toStringOrUndefined(object['AllowedPattern']),
             AllowedValues,
-            object['ConstraintDescription'] as string | undefined,
-            object['Description'] as string | undefined,
+            toStringOrUndefined(object['ConstraintDescription']),
+            toStringOrUndefined(object['Description']),
             object['MaxLength'] === undefined ? undefined : toNumber(object['MaxLength']),
             object['MaxValue'] === undefined ? undefined : toNumber(object['MaxValue']),
             object['MinLength'] === undefined ? undefined : toNumber(object['MinLength']),

src/utils/String.ts

@@ -38,3 +38,9 @@ export function byteSize(str: string) {
 export function formatNumber(value: number, decimals: number = 2) {
     return value.toFixed(decimals);
 }
+
+export function toStringOrUndefined(value: unknown): string | undefined {
+    if (typeof value === 'string') return value;
+    if (typeof value === 'number' || typeof value === 'boolean') return String(value);
+    return undefined;
+}

tst/unit/autocomplete/EntityFieldCompletionProvider.test.ts

@@ -42,7 +42,7 @@ describe('EntityFieldCompletionProvider', () => {
             const mockContext = createParameterContext('MyParameter', {
                 text: 'e',
                 data: {
-                    Type: 'string',
+                    Type: 'String',
                     Description: 'some description',
                 },
                 propertyPath: ['Parameters', 'MyParameter', 'e'],

tst/unit/context/semantic/Entity.test.ts

@@ -100,5 +100,58 @@ describe('Entity', () => {
             expect(parameter.name).toBe('TestParam');
             expect(parameter.NoEcho).toBe(true);
         });
+
+        it('should handle intrinsic functions in string fields by setting them to undefined', () => {
+            const data = {
+                Type: { 'Fn::Sub': 'String' },
+                Description: { 'Fn::Sub': 'Repository for ${AWS::StackName}' },
+                ConstraintDescription: { 'Fn::If': ['Condition', 'Valid', 'Invalid'] },
+                AllowedPattern: { 'Fn::Sub': '^${AWS::StackName}.*' },
+                Default: 'valid-default',
+            } as any;
+
+            const parameter = Parameter.from('TestParam', data);
+
+            expect(parameter.name).toBe('TestParam');
+            expect(parameter.Type).toBeUndefined();
+            expect(parameter.Default).toBe('valid-default');
+            expect(parameter.Description).toBeUndefined();
+            expect(parameter.ConstraintDescription).toBeUndefined();
+            expect(parameter.AllowedPattern).toBeUndefined();
+        });
+
+        it('should handle invalid string values in Type field', () => {
+            const data = {
+                Type: 'InvalidParameterType',
+                Description: 'Valid description',
+                Default: 'valid-default',
+            } as any;
+
+            const parameter = Parameter.from('TestParam', data);
+
+            expect(parameter.name).toBe('TestParam');
+            expect(parameter.Type).toBeUndefined();
+            expect(parameter.Description).toBe('Valid description');
+            expect(parameter.Default).toBe('valid-default');
+        });
+
+        it('should coerce numbers and booleans to strings', () => {
+            const data = {
+                Type: ParameterType.String,
+                Description: 123,
+                ConstraintDescription: true,
+                AllowedPattern: false,
+                Default: 'valid-default',
+            } as any;
+
+            const parameter = Parameter.from('TestParam', data);
+
+            expect(parameter.name).toBe('TestParam');
+            expect(parameter.Type).toBe(ParameterType.String);
+            expect(parameter.Description).toBe('123');
+            expect(parameter.ConstraintDescription).toBe('true');
+            expect(parameter.AllowedPattern).toBe('false');
+            expect(parameter.Default).toBe('valid-default');
+        });
     });
 });

tst/unit/hover/ParameterHoverProvider.test.ts

@@ -1,4 +1,5 @@
 import { describe, it, expect } from 'vitest';
+import { ParameterType } from '../../../src/context/semantic/ParameterType';
 import { ParameterHoverProvider } from '../../../src/hover/ParameterHoverProvider';
 import { createParameterContext } from '../../utils/MockContext';
 
@@ -8,7 +9,7 @@ describe('ParameterHoverProvider', () => {
         it('should return parameter information from template', () => {
             const mockContext = createParameterContext('EnvironmentType', {
                 data: {
-                    Type: 'String' as any,
+                    Type: ParameterType.String,
                     Default: 'dev',
                     Description: 'Environment type',
                     AllowedValues: ['dev', 'test', 'prod'],
@@ -27,5 +28,24 @@ describe('ParameterHoverProvider', () => {
             expect(result).toContain('- prod');
             expect(result).toContain('**Constraint Description:** Must be dev, test, or prod');
         });
+
+        it('should handle parameter with intrinsic function in Description without crashing', () => {
+            const mockContext = createParameterContext('EcrRepoName', {
+                data: {
+                    Type: 'String' as any,
+                    Default: 'my-repo',
+                    Description: { 'Fn::Sub': 'Repository for ${AWS::StackName}' },
+                },
+            });
+
+            // Should not throw an error
+            const result = parameterHoverProvider.getInformation(mockContext);
+
+            expect(result).toContain('(parameter) EcrRepoName: string');
+            expect(result).toContain('**Type:** String');
+            expect(result).toContain('**Default Value:** "my-repo"');
+            expect(result).not.toContain('Description');
+            expect(result).not.toContain('Fn::Sub');
+        });
     });
 });

tst/unit/utils/String.test.ts

@@ -1,5 +1,5 @@
 import { describe, it, expect } from 'vitest';
-import { dashesToUnderscores } from '../../../src/utils/String';
+import { dashesToUnderscores, toStringOrUndefined } from '../../../src/utils/String';
 
 describe('String', () => {
     describe('dashesToUnderscores', () => {
@@ -12,4 +12,26 @@ describe('String', () => {
             expect(dashesToUnderscores('test-123_abc-xyz')).toBe('test_123_abc_xyz');
         });
     });
+
+    describe('toStringOrUndefined', () => {
+        it('should return string values unchanged', () => {
+            expect(toStringOrUndefined('hello')).toBe('hello');
+            expect(toStringOrUndefined('')).toBe('');
+            expect(toStringOrUndefined('123')).toBe('123');
+        });
+
+        it('should convert numbers and booleans to strings', () => {
+            expect(toStringOrUndefined(123)).toBe('123');
+            expect(toStringOrUndefined(true)).toBe('true');
+            expect(toStringOrUndefined(false)).toBe('false');
+        });
+
+        it('should return undefined for objects, null, undefined, arrays', () => {
+            expect(toStringOrUndefined(null)).toBeUndefined();
+            expect(toStringOrUndefined(undefined)).toBeUndefined();
+            expect(toStringOrUndefined({})).toBeUndefined();
+            expect(toStringOrUndefined([])).toBeUndefined();
+            expect(toStringOrUndefined({ 'Fn::Sub': 'test' })).toBeUndefined();
+        });
+    });
 });