Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

struct unwrap fails TypeScript's noImplicitReturns check #432

Closed
absoludity opened this issue Dec 6, 2021 · 3 comments · Fixed by #436
Closed

struct unwrap fails TypeScript's noImplicitReturns check #432

absoludity opened this issue Dec 6, 2021 · 3 comments · Fixed by #436
Labels

Comments

@absoludity
Copy link

Since 1.88.0 was released, the code to unwrap a struct has failed TypeScript's noImplicitReturns check, which is normally a sane check to have on.

In 1.88.0, the function unwrapAnyValue was added, without an explicit return (even though you may know one of the conditionals is guaranteed to be true), resulting in:

src/gen/google/protobuf/struct.ts:415:4 - error TS7030: Not all code paths return a value.

415 ): string | number | boolean | Object | null | Array<any> | undefined {
       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This is still the case, though in a slightly different form in the latest 1.91, still resulting in the lint error:

src/gen/google/protobuf/struct.ts:397:6 - error TS7030: Not all code paths return a value.

397   ): string | number | boolean | Object | null | Array<any> | undefined {
         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@stephenh
Copy link
Owner

stephenh commented Dec 7, 2021

@boukeversteegh hm, wdyt about this issue? Should Value.unwrap have a final return undefined or a throw new Error("Unhandled message ${message}");.

@boukeversteegh
Copy link
Collaborator

boukeversteegh commented Dec 8, 2021

@stephenh I think undefined should be returned at the end. This matches the signature, and it's not really an error for a Value object to be empty. It internally contains a one-of field, and afaik there's no guarantee that exactly one field is set, so it may fall through the if/else branches.

I also see that value is not checked for null or empty. I suppose someone could construct a protobuf message that decoded into an object with a Value field which is unset, so it is technically possible to trigger an error.

As I'm on holiday now, I don't have a good setup to work on ts-proto, but I'll try a PR from the github editor to fix these two issues.

boukeversteegh added a commit that referenced this issue Dec 8, 2021
stephenh pushed a commit that referenced this issue Dec 8, 2021
* fix noImplicitReturns error in Value.unwrap

Fixes #432

* Force shell script LF line-endings

* Update generated struct.ts files

* Ensure parameters.txt are checked out with LF line endings

* Ensure protoc-gen-dump is checked out with LF line endings
stephenh pushed a commit that referenced this issue Dec 8, 2021
## [1.92.2](v1.92.1...v1.92.2) (2021-12-08)

### Bug Fixes

* noImplicitReturns error in Value.unwrap ([#436](#436)) ([2d7a5d0](2d7a5d0)), closes [#432](#432)
@stephenh
Copy link
Owner

stephenh commented Dec 8, 2021

🎉 This issue has been resolved in version 1.92.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants