fix(rosetta): structs starting with I are incorrectly interpreted as non-structs
#3040
Conversation
Prettier is adding a lot of line breaks that are cluttering up the code. Set the line width hint to 120, just for the Rosetta subproject.
Not all interfaces that start with the name `I` are structs: only if the second letter is also uppercase. Also clean up type handling a bit, we were doing some things that the TypeScript compiler has built-ins for.
|
The title of this Pull Request does not conform with [Conventional Commits] guidelines. It will need to be adjusted before the PR can be merged. |
| // Start with an I and another uppercase character | ||
| return !/^I[A-Z]/.test(name); |
There was a problem hiding this comment.
This is a better heuristic perhaps but still some can slip in, no?
Is the intention to just make this better but not catch all? Would be useful to call that out in the PR description.
| @@ -4,7 +4,8 @@ import { AstRenderer } from '../renderer'; | |||
| import { typeContainsUndefined } from '../typescript/types'; | |||
|
|
|||
| export function isStructInterface(name: string) { | |||
| return !name.startsWith('I'); | |||
| // Start with an I and another uppercase character | |||
| return !/^I[A-Z]/.test(name); | |||
There was a problem hiding this comment.
What about honouring the @struct annotation that was recently introduced?
There was a problem hiding this comment.
At this point we don't have any access to the jsii assembly, because there's no mapping between TypeScript symbols and jsii FQNs.
We will have that eventually, but not just yet.
|
Thank you for contributing! ❤️ I will now look into making sure the PR is up-to-date, then proceed to try and merge it! |
|
Merging (with squash)... |
|
The title of this Pull Request does not conform with [Conventional Commits] guidelines. It will need to be adjusted before the PR can be merged. |
|
Merging (with squash)... |
Not all interfaces that start with the name
Iare structs: only ifthe second letter is also uppercase.
Also clean up type handling a bit, we were doing some things that the
TypeScript compiler has built-ins for.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.