andrewhickman / prost-reflect

A protobuf library extending prost with reflection support and dynamic messages.
https://crates.io/crates/prost-reflect
Apache License 2.0
86 stars 19 forks source link

check for namespace shadowing #126

Closed tardyp closed 2 weeks ago

tardyp commented 3 weeks ago

protoc enforces that the name space resolution stops when it found a relative name which starts with a package fragment contained by the current package

e.g: if package is com.foo The type foo.Bar relative name resolution will never reach root path, it will stop searching at com.foo.Bar

The reasoning for this behavior I think is to avoid to have the resolution working at a certain point of time, and later somebody defines com.foo.Bar, and then your type is suddently pointing to somewhere else. This is why I called it shadowing.

a test in protox is created in https://github.com/andrewhickman/protox/pull/84

todo:

Fix: https://github.com/andrewhickman/protox/issues/82

andrewhickman commented 2 weeks ago

Awesome, thanks for the fix. Looks like a couple of tests are failing but the change looks good to me

codecov[bot] commented 2 weeks ago

Codecov Report

Attention: Patch coverage is 65.11628% with 15 lines in your changes missing coverage. Please review.

Project coverage is 76.24%. Comparing base (4a6dd64) to head (293acbb). Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
prost-reflect/src/descriptor/build/mod.rs 68.57% 11 Missing :warning:
prost-reflect/src/descriptor/error.rs 50.00% 4 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #126 +/- ## ========================================== - Coverage 76.59% 76.24% -0.35% ========================================== Files 31 31 Lines 5426 5456 +30 ========================================== + Hits 4156 4160 +4 - Misses 1270 1296 +26 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

tardyp commented 2 weeks ago

I think the workflow should pass entirely now. For some reason, github auto-approved a few of my attempts, but not the latest one.