Visitar URL original
[programgen] Guard references to conditionally-created resources (Python & NodeJS) by iwahbe · Pull Request #23634 · pulumi/pulumi · GitHub
Skip to content

[programgen] Guard references to conditionally-created resources (Python & NodeJS) - #23634

Merged
iwahbe merged 2 commits into
masterfrom
iwahbe/programgen-optional-resource-guards
Jun 23, 2026
Merged

iwahbe merged 2 commits into
masterfrom
iwahbe/programgen-optional-resource-guards

Conversation

@iwahbe

@iwahbe iwahbe commented Jun 18, 2026

Copy link
Copy Markdown
Member

Description

Stacked on #23632 (the l3-range-ref test split).

A resource declared with a boolean range option is created zero or one times, so its program variable is optional. Generated programs that dereferenced such a resource were not well-typed:

  • Python emitted bool_resource.name on a Target | None value → mypy union-attr error.
  • NodeJS emitted boolResource?.name, widening the result to Output<string> | undefined, which is not assignable to a required Input<string>.

This detects the dereference in a shared PCL helper, pcl.IsOptionalResource, and emits language-appropriate code:

  • Python (pkg/codegen/pcl/rewrite_optional_resource.go + pkg/codegen/python): wraps the access — and any apply over it — as expr if root is not None else None, run after applies are rewritten so the guard surrounds the apply rather than being hoisted inside it. Equality against null is now rendered with is/is not, which is idiomatic and lets the type checker narrow.
  • NodeJS (pkg/codegen/nodejs): emits a non-null assertion (boolResource!.name) at the optional-resource root, yielding Output<string>. Genuinely-optional schema properties keep optional chaining.

The two languages need different shapes because their generated inputs differ: Python resource inputs are Optional-typed (so the None fallback is assignable), whereas NodeJS inputs are required (so only a non-null assertion type-checks).

l3-range-bool-ref now type-checks under mypy and pyright and compiles under tsc, and is removed from the Python and NodeJS conformance expected-failure lists. Go has unrelated range program-generation bugs and is left failing. l3-range-map-ref (a separate collection-shape issue) is out of scope.

The is/is not rendering also updates a few existing program goldens (l2-resource-optional, l2-primitive-ref-optional, l1-config-types-object) from == None to is None.

Checklist

  • Conformance: l3-range-bool-ref passes for Python (Default/TOML/Classes) and NodeJS (TSC/TSNode/Bun)
  • Affected program goldens regenerated and pass
  • pkg/codegen program-gen golden batches unaffected
  • Changelog entry added

@iwahbe

iwahbe commented Jun 18, 2026 •

Copy link
Copy Markdown
Member Author

@iwahbe
iwahbe force-pushed the iwahbe/fix-python-bool-ref branch from ca82e44 to e98189c Compare June 19, 2026 16:46
@iwahbe
iwahbe force-pushed the iwahbe/programgen-optional-resource-guards branch from 7bea844 to 8955cbd Compare June 19, 2026 16:46

@julienp julienp 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.

nice

Base automatically changed from iwahbe/fix-python-bool-ref to master June 23, 2026 11:51
pull Bot pushed a commit to makesoftwaresafe/pulumi that referenced this pull request Jun 23, 2026
## Description

Splits the `l3-range-ref` language conformance test into three focused
tests so that each range-reference codegen path can be marked pass/fail
independently per language. Previously all four reference patterns lived
in one program, so a single broken pattern forced the whole test into a
language's `expectedFailures`, masking the others.

New tests:

- **`l3-range-list-ref`** — numeric and list ranges, indexed references
(`numResource[0]`, `listResource[1]`), and dynamic indexing
(`listResource[range.key]`, the pulumi#14037 quote/index-rewrite path).
- **`l3-range-map-ref`** — a map range indexed by a string key
(`mapResource["k1"]`).
- **`l3-range-bool-ref`** — dereferencing a conditionally-created
(boolean `range`) resource (`boolResource.name`).

This PR is a pure test refactor — **no program-generation changes**.
Per-language status is recorded exactly as it stands today:

| Test | PCL | Python | NodeJS | Go |
|------|-----|--------|--------|----|
| `l3-range-list-ref` | ✅ pass | ✅ pass | ✅ pass | ❌ expected-fail
(syntax errors) |
| `l3-range-map-ref` | ✅ pass | ❌ expected-fail (`__getitem__` overload)
| ❌ expected-fail (`Property 'k1'`) | ❌ expected-fail |
| `l3-range-bool-ref` | ✅ pass | ❌ expected-fail (`union-attr`) | ❌
expected-fail (`Output<string> \| undefined`) | ❌ expected-fail |

Follow-up pulumi#23634 (stacked on this PR) fixes the Python and NodeJS
program generation so `l3-range-bool-ref` passes for both, removing it
from their expected-failure lists.

## Checklist

- [x] Conformance suites run locally for PCL, Python
(Default/TOML/Classes), NodeJS (TSC/TSNode/Bun), and Go
- [x] `expectedFailures` updated per language; `l3-range-list-ref`
goldens committed
iwahbe added 2 commits June 23, 2026 20:28
A resource that uses a boolean `range` option is created zero or one
times, so its program variable is optional. Generated programs that
dereferenced such a resource were not well-typed: Python emitted
`bool_resource.name` on a `Target | None` value (mypy union-attr error)
and NodeJS emitted `boolResource?.name`, widening the result to
`Output<string> | undefined`, which is not assignable to a required
input.

Detect these dereferences in a shared PCL helper, `pcl.IsOptionalResource`,
and emit language-appropriate code:

  - Python wraps the access (and any apply over it) as
    `expr if root is not None else None`, run after applies are rewritten
    so the guard surrounds the apply rather than being hoisted inside it.
    Equality against `null` is now rendered with `is`/`is not`, which is
    idiomatic and lets the type checker narrow.
  - NodeJS emits a non-null assertion (`boolResource\!.name`) at the
    optional-resource root, yielding `Output<string>`. Genuinely-optional
    schema properties continue to use optional chaining.

l3-range-bool-ref now type-checks under mypy and pyright and compiles
under tsc, so it is removed from the Python and NodeJS conformance
expected-failure lists. Go has unrelated range program-generation bugs
and is left failing.
@iwahbe
iwahbe force-pushed the iwahbe/programgen-optional-resource-guards branch from 8955cbd to 3cb4817 Compare June 23, 2026 18:35
@iwahbe
iwahbe enabled auto-merge June 23, 2026 18:36
@iwahbe
iwahbe added this pull request to the merge queue Jun 23, 2026
Merged via the queue into master with commit e2cca27 Jun 23, 2026
235 of 238 checks passed
@iwahbe
iwahbe deleted the iwahbe/programgen-optional-resource-guards branch June 23, 2026 21:05
pull Bot pushed a commit to Mattlk13/pulumi that referenced this pull request Jun 24, 2026
… NodeJS) (pulumi#23639)

## Description

Stacked on pulumi#23634.

A resource with a map `range` option was collected into an ordered list
(Python `list`, NodeJS array) and then referenced by its map key
(`mapResource["k1"]`), which does not type-check — a list/array can't be
indexed by a string. Python failed mypy `__getitem__` overload; NodeJS
failed `Property 'k1' does not exist on type 'Target[]'`.

Generate a **key-indexed, strongly-typed** collection for map ranges:

- **Python:** `dict[str, T]` populated by `m[range['key']] = ...`
- **NodeJS:** `{[key: string]: T}` populated by `m[range.key] = ...`

Detection reuses the existing inline map check at the range site
(`pcl.UnwrapOption(rangeExpr.Type()).(*model.MapType)`) — no new helper.

### Also in this PR

- **Tighten list typing:** numeric/list ranges are now `list[T]` in
Python instead of `list[Any]` (NodeJS already used `T[]`).
- **Unique Python loop variable per range:** each range statement binds
`<resource>_range` instead of the shared `range` name. Reusing `range`
type-locked its element type across loops — a numeric range fixed it to
`dict[str, int]`, which then rejected a later map range's string
`range['key']` as a dict key. Unique names remove the coupling (and stop
shadowing the `range` builtin). NodeJS already block-scopes each loop's
`range` with `const`, so it needs no change.

`l3-range-map-ref` now type-checks under mypy and pyright and compiles
under tsc, and is removed from the Python and NodeJS expected-failure
lists. Go has unrelated range program-generation bugs and is left
failing.

## Checklist

- [x] Conformance: full `l3-range*` family passes for Python
(Default/TOML/Classes) and NodeJS (TSC/TSNode/Bun)
- [x] `pkg/codegen` program-gen goldens regenerated (components-pp,
python-regress-14037-pp, unknown-resource-pp) and pass
- [x] Changelog entry added
@pulumi-bot

Copy link
Copy Markdown
Contributor

This PR has been shipped in release v3.248.0.

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