desc/protoparse: report unused imports #403
Conversation
|
cc @bufdev |
| return md | ||
| } | ||
|
|
||
| // TODO: should this support public imports and be recursive? |
There was a problem hiding this comment.
just making sure nothing to do here
There was a problem hiding this comment.
I think this will involve a little more toying around with protoc to understand any nuances of its behavior and then mimic it exactly.
Also, FWIW, this is only for addressing #360 (using custom descriptor.proto), not for implementing the warning on unused imports. I addressed that issue at the same time just since my previous PR happened to already have test cases that cover the custom descriptor.proto case. This is a very rarely used feature of protoc (only useful if you are working on protoc or forking it and making changes to descriptor.proto). So this PR will at least get protoparse closer even if not yet perfect; so I don't think we need to do anything here right now.
|
LGTM |
This is a backport of #355 to master. That other pull request is based on a big refactor, for better interop with the newest protobuf runtime reflection stuff. There's enough risk (mostly in performance regressions) in that refactor (which is in #354), that I backported this change so we can land it without having to first land the refactor...
This also includes a change to support using a custom version of
descriptor.proto, which is something else that protoc supports and that was also added in that refactor PR.Resolves #281.
And also resolves #360.