Skip to content

Add DependencyMap data structure for imports#268

Merged
LesterEvSe merged 0 commit intoBlockstreamResearch:dev/importsfrom
LesterEvSe:feature/resolution
Apr 2, 2026
Merged

Add DependencyMap data structure for imports#268
LesterEvSe merged 0 commit intoBlockstreamResearch:dev/importsfrom
LesterEvSe:feature/resolution

Conversation

@LesterEvSe
Copy link
Copy Markdown
Collaborator

  • Adds the DependencyMap and Remapping data structures.
  • Implements longest-prefix matching to correctly resolve paths.

@LesterEvSe LesterEvSe requested a review from KyrylR April 2, 2026 08:59
@LesterEvSe LesterEvSe self-assigned this Apr 2, 2026
@LesterEvSe LesterEvSe added the enhancement New feature or request label Apr 2, 2026
@LesterEvSe LesterEvSe requested a review from delta1 as a code owner April 2, 2026 08:59
@LesterEvSe LesterEvSe force-pushed the feature/resolution branch from 6b422d6 to ce70a8b Compare April 2, 2026 09:00
@apoelstra
Copy link
Copy Markdown
Contributor

ce70a8b needs rebase

Comment on lines +94 to +112
/// Inserts a dependency remapping without interacting with the physical file system.
///
/// **Warning:** This method completely bypasses OS path canonicalization (`std::fs::canonicalize`).
/// It is designed strictly for unit testing and virtual file environments where the
/// provided paths might not actually exist on the hard drive.
#[cfg(test)]
pub fn test_insert_without_canonicalize(
&mut self,
context: &Path,
dependency_root_path: String,
path: &Path,
) {
self.inner.push(Remapping {
context_prefix: context.to_path_buf(),
dependency_root_path,
target: path.to_path_buf(),
});
self.sort_mappings();
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not include methods meant for testing to impl blocks

// No matches found
Err(Error::UnknownLibrary(first_segment.to_string())).with_span(*use_decl.span())
}
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add some test cases

@LesterEvSe LesterEvSe merged commit ce70a8b into BlockstreamResearch:dev/imports Apr 2, 2026
11 checks passed
@LesterEvSe
Copy link
Copy Markdown
Collaborator Author

I rebase the dev/imports branch to bring it up to date with the master branch. After synchronising this branch with dev/imports, I accidentally moved the commit in this PR directly to the dev/imports branch.
This will be fixed in PR #269.

@LesterEvSe LesterEvSe deleted the feature/resolution branch April 3, 2026 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants