Skip to content

fix(lib): keep falsy default values in Fn.lookup - #584

Open
eduardomourar wants to merge 1 commit into
open-constructs:mainfrom
eduardomourar:fix/fn-lookup-falsy-default
Open

eduardomourar wants to merge 1 commit into
open-constructs:mainfrom
eduardomourar:fix/fn-lookup-falsy-default

Conversation

@eduardomourar

Copy link
Copy Markdown
Contributor

Related issue

Fixes #416

Description

Test for an undefined default instead of truthiness, so false, 0 and an empty string render a lookup() call. Update the fqn tests to use lookupNested and the edge integration expectations.

Checklist

  • I have updated the PR title to match CDKTN's style guide
  • I have run the linter on my code locally
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation if applicable
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works if applicable
  • New and existing unit tests pass locally with my changes

@jsteinich jsteinich left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for picking this up! I checked the fix on Terraform 1.14.3 and OpenTofu 1.12.6, in both JSON and HCL synth. false, 0 and "" now render lookup() and return the default for a missing key. Keys that exist evaluate the same as before, and the JSII signature is unchanged.

One gap: null. It now passes the !== undefined guard, but _lookup's variadic listOf() drops null elements (functions/helpers.ts:76). So Fn.lookup(m, "k", null) renders lookup(m, "k"), which still errors on a missing key. Terraform and OpenTofu both accept lookup(m, "k", null) and return null. Could you pass an explicit null token and add a null case to the test.each?

if (defaultValue !== undefined)
  return Fn._lookup(inputMap, key, [
    defaultValue === null ? Token.nullValue() : defaultValue,
  ]);

A nit: the inline snapshot in iterator.test.ts was re-indented by one space. Restoring the old indent keeps the diff to the boolean_map line.

For the release notes (no action needed here): a falsy default whose type differs from the map's element type (e.g. "" on a map(number)), or a list passed as inputMap, used to work by accident through the property-access path. These now fail at plan, as the same calls already do with a truthy default.

HCL synth has the same truthiness pattern for variable attributes and locals. That's out of scope here, so it's tracked separately in #589.

@eduardomourar
eduardomourar force-pushed the fix/fn-lookup-falsy-default branch from 3042d25 to b6d896c Compare November 9, 2026 16:43
@eduardomourar

Copy link
Copy Markdown
Contributor Author

All addressed now.

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.

lib: Fn.lookup silently drops a falsy default value, emitting a bare property access

2 participants