Skip to content

Handle non-string parameter values in hover#536

Merged
chrisqm-dev merged 5 commits intomainfrom
fix/hover-parameter-casting
Apr 24, 2026
Merged

Handle non-string parameter values in hover#536
chrisqm-dev merged 5 commits intomainfrom
fix/hover-parameter-casting

Conversation

@chrisqm-dev
Copy link
Copy Markdown
Contributor

@chrisqm-dev chrisqm-dev commented Apr 23, 2026

Description of changes:

  • If a parameter field (such as description) is not a string, formatParameterHover will throw a TypeError due to item.trim() requiring a string
  • Added string type checking before item.trim() is called
    • Note: All parameter fields must be a string for a CloudFormation Template to be valid, CFN-Lint properly detects non-string fields and produces diagnostics for it

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@chrisqm-dev chrisqm-dev requested a review from a team as a code owner April 23, 2026 19:34
Comment thread src/hover/HoverFormatter.ts Outdated
Comment thread tst/unit/autocomplete/EntityFieldCompletionProvider.test.ts
@chrisqm-dev chrisqm-dev reopened this Apr 23, 2026
Copy link
Copy Markdown
Collaborator

@kddejong kddejong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix — the crash on non-string values is a real issue worth addressing.

One suggestion on the approach: instead of silently dropping non-string scalars to undefined, consider coercing them to strings. For example, if a user writes Description: 12345 (a number) or Description: true (a boolean), the current PR would show no description at all in hover. Converting scalars to strings would preserve the user's intent — they'd see "12345" or "true" which is likely what they meant.

This is already the pattern used for NoEcho on the very next line: String(object['NoEcho']).

A small reusable helper would keep this clean and could be used across the codebase wherever we parse template values:

/** Convert scalar values (string, number, boolean) to string. Returns undefined for objects/arrays/undefined. */
function toStringOrUndefined(value: unknown): string | undefined {
    if (typeof value === 'string') return value;
    if (typeof value === 'number' || typeof value === 'boolean') return String(value);
    return undefined;
}

Then Parameter.from becomes:

toStringOrUndefined(object['Description'])
toStringOrUndefined(object['AllowedPattern'])
toStringOrUndefined(object['ConstraintDescription'])

This way numeric/boolean descriptions render in hover instead of disappearing, objects (like intrinsic functions) are still correctly filtered out, and we have a utility that's reusable anywhere we need to safely extract string values from parsed template data.

Comment thread src/context/semantic/Entity.ts Outdated
Comment thread tst/unit/context/semantic/Entity.test.ts Outdated
@chrisqm-dev
Copy link
Copy Markdown
Contributor Author

Thanks for the fix — the crash on non-string values is a real issue worth addressing.

One suggestion on the approach: instead of silently dropping non-string scalars to undefined, consider coercing them to strings. For example, if a user writes Description: 12345 (a number) or Description: true (a boolean), the current PR would show no description at all in hover. Converting scalars to strings would preserve the user's intent — they'd see "12345" or "true" which is likely what they meant.

This is already the pattern used for NoEcho on the very next line: String(object['NoEcho']).

A small reusable helper would keep this clean and could be used across the codebase wherever we parse template values:

/** Convert scalar values (string, number, boolean) to string. Returns undefined for objects/arrays/undefined. */
function toStringOrUndefined(value: unknown): string | undefined {
    if (typeof value === 'string') return value;
    if (typeof value === 'number' || typeof value === 'boolean') return String(value);
    return undefined;
}

Then Parameter.from becomes:

toStringOrUndefined(object['Description'])
toStringOrUndefined(object['AllowedPattern'])
toStringOrUndefined(object['ConstraintDescription'])

This way numeric/boolean descriptions render in hover instead of disappearing, objects (like intrinsic functions) are still correctly filtered out, and we have a utility that's reusable anywhere we need to safely extract string values from parsed template data.

Created toStringOrUndefined method and updated Entity.ts

@chrisqm-dev chrisqm-dev merged commit ac7561c into main Apr 24, 2026
15 checks passed
@chrisqm-dev chrisqm-dev deleted the fix/hover-parameter-casting branch April 24, 2026 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants