Skip to content

Commit

Permalink
check for namespace shadowing
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@tardyp
tardyp committed Aug 26, 2024
1 parent 4a6dd64 commit ed27ba5
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 27 deletions.
76 changes: 53 additions & 23 deletions prost-reflect/src/descriptor/build/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ enum ResolveNameFilter {
FieldType,
}

enum ResolveNameResult<'a, 'b> {
enum ResolveNameResult<'a, 'b, 'c> {
Found {
name: Cow<'b, str>,
def: &'a Definition,
Expand All @@ -52,6 +52,10 @@ enum ResolveNameResult<'a, 'b> {
name: Cow<'b, str>,
file: FileIndex,
},
Shadowed {
name: Cow<'b, str>,
shadowed_name: Cow<'c, str>,
},
NotFound,
}

Expand Down Expand Up @@ -168,7 +172,7 @@ impl fmt::Display for ResolveNameFilter {
}
}

impl<'a, 'b> ResolveNameResult<'a, 'b> {
impl<'a, 'b, 'c> ResolveNameResult<'a, 'b, 'c> {
fn new(
dependencies: &HashSet<FileIndex>,
names: &'a HashMap<Box<str>, Definition>,
Expand All @@ -192,7 +196,7 @@ impl<'a, 'b> ResolveNameResult<'a, 'b> {
}
}

fn into_owned(self) -> ResolveNameResult<'a, 'static> {
fn into_owned(self) -> ResolveNameResult<'a, 'static, 'static> {
match self {
ResolveNameResult::Found { name, def } => ResolveNameResult::Found {
name: Cow::Owned(name.into_owned()),
Expand All @@ -209,6 +213,13 @@ impl<'a, 'b> ResolveNameResult<'a, 'b> {
name: Cow::Owned(name.into_owned()),
file,
},
ResolveNameResult::Shadowed {
name,
shadowed_name,
} => ResolveNameResult::Shadowed {
name: Cow::Owned(name.into_owned()),
shadowed_name: Cow::Owned(shadowed_name.into_owned()),
},
ResolveNameResult::NotFound => ResolveNameResult::NotFound,
}
}
Expand Down Expand Up @@ -268,6 +279,19 @@ impl<'a, 'b> ResolveNameResult<'a, 'b> {
),
help: None,
}),
ResolveNameResult::Shadowed { name, shadowed_name } => Err(DescriptorErrorKind::NameShadowed {
found: Label::new(
files,
"found here",
found_file,
join_path(found_path1, found_path2),
),
help: Some(format!(
"'{}' is is resolved to '{}', which is not defined. The innermost scope is searched first in name resolution. Consider using a leading '.'(i.e., '.{}') to start from the outermost scope.",
name, shadowed_name, name
)),
name: name.into_owned(),
}),
}
}
}
Expand All @@ -290,52 +314,58 @@ fn to_json_name(name: &str) -> String {
result
}

fn resolve_name<'a, 'b>(
fn resolve_name<'a, 'b, 'c>(
dependencies: &HashSet<FileIndex>,
names: &'a HashMap<Box<str>, Definition>,
scope: &str,
scope: &'c str,
name: &'b str,
filter: ResolveNameFilter,
) -> ResolveNameResult<'a, 'b> {
) -> ResolveNameResult<'a, 'b, 'c> {
match name.strip_prefix('.') {
Some(full_name) => ResolveNameResult::new(dependencies, names, full_name, filter),
None if scope.is_empty() => ResolveNameResult::new(dependencies, names, name, filter),
None => resolve_relative_name(dependencies, names, scope, name, filter),
}
}

fn resolve_relative_name<'a>(
fn resolve_relative_name<'a, 'b, 'c>(
dependencies: &HashSet<FileIndex>,
names: &'a HashMap<Box<str>, Definition>,
scope: &str,
relative_name: &str,
scope: &'c str,
relative_name: &'b str,
filter: ResolveNameFilter,
) -> ResolveNameResult<'a, 'static> {
) -> ResolveNameResult<'a, 'b, 'c> {
let mut err = ResolveNameResult::NotFound;

for candidate in resolve_relative_name_candidates(scope, relative_name) {
let relative_first_part = relative_name.split('.').next();
let mut last_candidate_last_part = None;
for candidate_parent in resolve_relative_candidate_parents(scope) {
let candidate = match candidate_parent {
"" => Cow::Borrowed(relative_name),
_ => Cow::Owned(format!("{}.{}", candidate_parent, relative_name)),
};
let res = ResolveNameResult::new(dependencies, names, candidate, filter);
if res.is_found() {
return res.into_owned();
} else if matches!(err, ResolveNameResult::NotFound) {
err = res;
}
if Some(relative_first_part) == Some(last_candidate_last_part) {
let shadowed_name = format!("{}.{}", candidate_parent, relative_name);
return ResolveNameResult::Shadowed {
name: Cow::Borrowed(relative_name),
shadowed_name: Cow::Owned(shadowed_name),
};
}
last_candidate_last_part = candidate_parent.rsplit('.').next();
}

err.into_owned()
}

fn resolve_relative_name_candidates<'b: 'c, 'c>(
scope: &'c str,
relative_name: &'b str,
) -> impl Iterator<Item = Cow<'b, str>> + 'c {
iter::once(Cow::Owned(format!("{scope}.{relative_name}")))
.chain(
scope
.rmatch_indices('.')
.map(move |(i, _)| Cow::Owned(format!("{}.{relative_name}", &scope[..i]))),
)
.chain(iter::once(Cow::Borrowed(relative_name)))
fn resolve_relative_candidate_parents<'c>(scope: &'c str) -> impl Iterator<Item = &'c str> {
iter::once(scope)
.chain(scope.rmatch_indices('.').map(move |(i, _)| &scope[..i]))
.chain(iter::once(""))
}

fn join_path(path1: &[i32], path2: &[i32]) -> Box<[i32]> {
Expand Down
19 changes: 17 additions & 2 deletions prost-reflect/src/descriptor/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,12 @@ pub(super) enum DescriptorErrorKind {
#[cfg_attr(not(feature = "miette"), allow(dead_code))]
help: Option<String>,
},
NameShadowed {
name: String,
found: Label,
#[cfg_attr(not(feature = "miette"), allow(dead_code))]
help: Option<String>,
},
InvalidType {
name: String,
expected: String,
Expand Down Expand Up @@ -316,6 +322,7 @@ impl DescriptorErrorKind {
DescriptorErrorKind::FieldNumberInExtensionRange { found, .. } => Some(found),
DescriptorErrorKind::ExtensionNumberOutOfRange { found, .. } => Some(found),
DescriptorErrorKind::NameNotFound { found, .. } => Some(found),
DescriptorErrorKind::NameShadowed { found, .. } => Some(found),
DescriptorErrorKind::InvalidType { found, .. } => Some(found),
DescriptorErrorKind::InvalidFieldDefault { found, .. } => Some(found),
DescriptorErrorKind::EmptyEnum { found } => Some(found),
Expand Down Expand Up @@ -380,6 +387,9 @@ impl DescriptorErrorKind {
DescriptorErrorKind::NameNotFound { found, .. } => {
found.resolve_span(file, source);
}
DescriptorErrorKind::NameShadowed { found, .. } => {
found.resolve_span(file, source);
}
DescriptorErrorKind::InvalidType { found, defined, .. } => {
found.resolve_span(file, source);
defined.resolve_span(file, source);
Expand Down Expand Up @@ -522,6 +532,9 @@ impl fmt::Display for DescriptorErrorKind {
DescriptorErrorKind::NameNotFound { name, .. } => {
write!(f, "name '{}' is not defined", name)
}
DescriptorErrorKind::NameShadowed { name, .. } => {
write!(f, "name '{}' is shadowed", name)
}
DescriptorErrorKind::InvalidType { name, expected, .. } => {
write!(f, "'{}' is not {}", name, expected)
}
Expand Down Expand Up @@ -634,7 +647,8 @@ impl miette::Diagnostic for DescriptorErrorKind {
DescriptorErrorKind::FieldNumberInExtensionRange { .. } => None,
DescriptorErrorKind::DuplicateFieldJsonName { .. } => None,
DescriptorErrorKind::DuplicateFieldCamelCaseName { .. } => None,
DescriptorErrorKind::NameNotFound { help, .. } => help
DescriptorErrorKind::NameNotFound { help, .. } |
DescriptorErrorKind::NameShadowed { help, .. } => help
.as_ref()
.map(|h| -> Box<dyn fmt::Display> { Box::new(h.clone()) }),
DescriptorErrorKind::InvalidType { .. } => None,
Expand Down Expand Up @@ -685,7 +699,8 @@ impl miette::Diagnostic for DescriptorErrorKind {
spans.extend(first.to_span());
spans.extend(second.to_span());
}
DescriptorErrorKind::NameNotFound { found, .. } => {
DescriptorErrorKind::NameNotFound { found, .. } |
DescriptorErrorKind::NameShadowed{ found, .. } => {
spans.extend(found.to_span());
}
DescriptorErrorKind::InvalidFieldNumber { found, .. } => {
Expand Down
4 changes: 2 additions & 2 deletions prost-reflect/src/descriptor/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,14 +154,14 @@ struct Identity {
name_index: usize,
}

#[derive(Clone)]
#[derive(Clone, Debug)]
struct Definition {
file: FileIndex,
path: Box<[i32]>,
kind: DefinitionKind,
}

#[derive(Copy, Clone)]
#[derive(Copy, Clone, Debug)]
enum DefinitionKind {
Package,
Message(MessageIndex),
Expand Down

0 comments on commit ed27ba5

Please sign in to comment.