Skip to content

Commit

Permalink
patterns: Explain what's going on with the relative patterns
Browse files Browse the repository at this point in the history
Summary:
It took me a minute to figure this out, make it more obvious in the type

Next diff will clean up some APIs too

Reviewed By: IanChilds

Differential Revision: D69713860

fbshipit-source-id: ff319131021535adfbdcabde4933d6172496f9da
  • Loading branch information
JakobDegen authored and facebook-github-bot committed Feb 18, 2025
1 parent 3c36960 commit 83ad155
Showing 1 changed file with 19 additions and 11 deletions.
30 changes: 19 additions & 11 deletions app/buck2_core/src/pattern/pattern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,10 @@ impl<T: PatternType> ParsedPattern<T> {
cell_alias_resolver,
None,
TargetParsingOptions {
relative: TargetParsingRel::RequireAbsolute(relative_dir),
relative: match relative_dir {
Some(d) => TargetParsingRel::AllowLimitedRelative(d),
None => TargetParsingRel::RequireAbsolute,
},
infer_target: false,
strip_package_trailing_slash: false,
},
Expand Down Expand Up @@ -773,26 +776,31 @@ fn normalize_package<'a>(
}

#[derive(Clone, Dupe)]
enum TargetParsingRel<'a> {
/// The dir this pattern should be interpreted relative to.
pub enum TargetParsingRel<'a> {
/// Parse the pattern relative to this package path
AllowRelative(CellPathRef<'a>),
/// The dir this pattern should be interpreted relative to.
/// This is only used for targets such as `:foo`.
RequireAbsolute(Option<CellPathRef<'a>>),
/// Allows relative patterns, but only if they're like `:foo`, not `bar:foo`
AllowLimitedRelative(CellPathRef<'a>),
/// Require the pattern to be absolute.
///
/// Still allows reference to the self-cell (`//foo:bar`)
RequireAbsolute,
}

impl<'a> TargetParsingRel<'a> {
fn dir(&self) -> Option<CellPathRef<'a>> {
match self {
TargetParsingRel::AllowRelative(dir) => Some(*dir),
TargetParsingRel::RequireAbsolute(dir) => *dir,
TargetParsingRel::AllowLimitedRelative(dir) => Some(*dir),
TargetParsingRel::RequireAbsolute => None,
}
}

fn allow_relative(&self) -> bool {
fn allow_full_relative(&self) -> bool {
match self {
TargetParsingRel::AllowRelative(_) => true,
TargetParsingRel::RequireAbsolute(_) => false,
TargetParsingRel::AllowLimitedRelative(_) => false,
TargetParsingRel::RequireAbsolute => false,
}
}
}
Expand All @@ -811,7 +819,7 @@ struct TargetParsingOptions<'a> {
impl<'a> TargetParsingOptions<'a> {
fn precise() -> TargetParsingOptions<'a> {
TargetParsingOptions {
relative: TargetParsingRel::RequireAbsolute(None),
relative: TargetParsingRel::RequireAbsolute,
infer_target: false,
strip_package_trailing_slash: false,
}
Expand Down Expand Up @@ -927,7 +935,7 @@ where
// just relative target). This is a bit of a wonky definition of "is_absolute" but we rely on
// it.
let is_absolute_or_adjacent = cell_alias.is_some() || pattern.is_adjacent_target();
if !relative.allow_relative() && !is_absolute_or_adjacent {
if !relative.allow_full_relative() && !is_absolute_or_adjacent {
return Err(TargetPatternParseError::AbsoluteRequired.into());
}

Expand Down

0 comments on commit 83ad155

Please sign in to comment.