Skip to content

fix(jsii-pacmak): inheritance between nested types fails#4993

Merged
mergify[bot] merged 2 commits intomainfrom
huijbers/nested-inheritance
Nov 27, 2025
Merged

fix(jsii-pacmak): inheritance between nested types fails#4993
mergify[bot] merged 2 commits intomainfrom
huijbers/nested-inheritance

Conversation

@rix0rrr
Copy link
Copy Markdown
Contributor

@rix0rrr rix0rrr commented Nov 27, 2025

In Python, two types that are nested inside the same other class couldn't inherit from each other; the type references were rendered incorrectly.

Specifically, the following:

class A { }

namespace A {
  interface B { }
  interface C extends B { }
}

Would produce could that doesn't compile correctly. It turns out that the referencing rules are different during the initialization of classes:

class A:
    class B: pass
    class C(B):                         # <-- short
       def some_method(self, b: "A.B"): # <-- long (in strings)
          return A.B()                  # <-- long

This PR fixes that, and introduces some simplifications and refactoring of the type handling code along the way:

  • Reversed the order of 2 code blocks in the UserType.pythonType() function, to consistently structure the code as "early exit" paths (if (weird_condition) { return ...; }). This makes the diff bigger, sorry, but the long-term maintainability of this code better (imo).
  • Moved the implicit typeAnnotation?: boolean property of the naming context to an explicit parameter of the rendering function pythonType().
  • We now distinguish 3 ways of rendering types: as type annotations, as values in methods, as values during class initialization.
  • Strictly render all type annotations using quotes. This always works and is simpler: it used to allow us to get rid of emittedTypes which tracks types we should emit quotes for and types which we can reference directly... but there are no benefits to direct references, so why would we?

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

In Python, two types that are nested inside the same other class
couldn't inherit from each other; the type references were rendered
incorrectly.

Specifically, the following:

```ts
class A { }

namespace A {
  interface B { }
  interface C extends B { }
}
```

Would produce could that doesn't compile correctly. It turns out that
the referencing rules are different during the initialization of
classes:

```py
class A:
    class B: pass
    class C(B):                         # <-- short
       def some_method(self, b: "A.B"): # <-- long (in strings)
          return A.B()                  # <-- long
```

This PR fixes that, and introduces some simplifications and refactoring
of the type handling code along the way:

- Reversed the order of 2 code blocks in the `UserType.pythonType()` function, to
  consistently structure the code as "early exit" paths (`if
  (weird_condition) { return ...; }`). This makes the diff bigger,
  sorry, but the long-term maintainability of this code better (imo).
- Moved the implicit `typeAnnotation?: boolean` property of the naming
  context to an explicit parameter of the rendering function
  `pythonType()`.
- We now distinguish 3 ways of rendering types: as type annotations,
  as values in methods, as values during class initialization.
- Strictly render all type annotations using quotes. This always works
  and is simpler: it used to allow us to get rid of `emittedTypes` which
  tracks types we should emit quotes for and types which we can
  reference directly... but there are no benefits to direct references,
  so why would we?
@rix0rrr rix0rrr requested review from a team November 27, 2025 10:06
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Nov 27, 2025
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Nov 27, 2025

Thank you for contributing! ❤️ I will now look into making sure the PR is up-to-date, then proceed to try and merge it!

@mergify mergify bot added the pr/ready-to-merge This PR is ready to be merged. label Nov 27, 2025
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Nov 27, 2025

Merge Queue Status Beta

✅ The pull request has been merged

This pull request spent 33 minutes 58 seconds in the queue, including 33 minutes 47 seconds waiting for CI.
The checks were run in-place.

Required conditions to merge
  • #approved-reviews-by >= 1 [🛡 GitHub branch protection]
  • #changes-requested-reviews-by = 0 [🛡 GitHub branch protection]
  • status-success=Integration test (jsii-pacmak)
  • status-success=Unit Tests
  • any of [🛡 GitHub branch protection]:
    • check-success = Integration test (jsii-pacmak)
    • check-neutral = Integration test (jsii-pacmak)
    • check-skipped = Integration test (jsii-pacmak)
  • any of [🛡 GitHub branch protection]:
    • check-success = Build
    • check-neutral = Build
    • check-skipped = Build
  • any of [🛡 GitHub branch protection]:
    • check-success = Unit Tests
    • check-neutral = Unit Tests
    • check-skipped = Unit Tests

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Nov 27, 2025

Merging (with squash)...

@mergify mergify bot added the queued label Nov 27, 2025
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Nov 27, 2025

Merging (with squash)...

@mergify mergify bot merged commit d5009c5 into main Nov 27, 2025
35 checks passed
@mergify mergify bot deleted the huijbers/nested-inheritance branch November 27, 2025 11:22
@mergify mergify bot removed pr/ready-to-merge This PR is ready to be merged. queued labels Nov 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contribution/core This is a PR that came from AWS.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants