Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[2/n] no-context lint: migrate some crates #5640

Merged
merged 10 commits into from
Aug 3, 2023
19 changes: 18 additions & 1 deletion .config/ast-grep/rules/no-context.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
id: no-context
message: Don't name variables `context`.
note: Use a more specific name, such as chunking_context, asset_context, etc.
severity: warning
severity: error
language: Rust
rule:
regex: \bcontext\b
Expand All @@ -18,3 +18,20 @@ rule:
- kind: field_identifier
- inside:
kind: field_declaration
ignores:
- "./crates/turbopack-core/**"
- "./crates/turbopack-css/**"
- "./crates/turbopack-dev-server/**"
- "./crates/turbopack-dev/**"
- "./crates/turbopack-ecmascript-hmr-protocol/**"
- "./crates/turbopack-ecmascript-plugins/**"
- "./crates/turbopack-ecmascript-runtime/**"
- "./crates/turbopack-ecmascript/**"
- "./crates/turbopack-json/**"
- "./crates/turbopack-mdx/**"
- "./crates/turbopack-node/**"
- "./crates/turbopack-static/**"
- "./crates/turbopack-tests/**"
- "./crates/turbopack/**"
Comment on lines +22 to +35
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this just be ./crates/turbopack*/**?

Copy link
Member Author

Choose a reason for hiding this comment

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

Many crates are passing this, so ignoring them would remove value from this rule for now. This also acts as a list of crates to address.

- "./crates/turborepo-cache/**"
- "./crates/turborepo-scm/**"
58 changes: 29 additions & 29 deletions crates/node-file-trace/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,8 @@ impl Args {
}
}

async fn create_fs(name: &str, context: &str, watch: bool) -> Result<Vc<Box<dyn FileSystem>>> {
let fs = DiskFileSystem::new(name.to_string(), context.to_string());
async fn create_fs(name: &str, root: &str, watch: bool) -> Result<Vc<Box<dyn FileSystem>>> {
let fs = DiskFileSystem::new(name.to_string(), root.to_string());
if watch {
fs.await?.start_watching()?;
} else {
Expand All @@ -206,30 +206,30 @@ async fn create_fs(name: &str, context: &str, watch: bool) -> Result<Vc<Box<dyn
}

async fn add_glob_results(
context: Vc<Box<dyn AssetContext>>,
asset_context: Vc<Box<dyn AssetContext>>,
result: Vc<ReadGlobResult>,
list: &mut Vec<Vc<Box<dyn Module>>>,
) -> Result<()> {
let result = result.await?;
for entry in result.results.values() {
if let DirectoryEntry::File(path) = entry {
let source = Vc::upcast(FileSource::new(*path));
list.push(context.process(
list.push(asset_context.process(
source,
Value::new(turbopack_core::reference_type::ReferenceType::Undefined),
));
}
}
for result in result.inner.values() {
fn recurse<'a>(
context: Vc<Box<dyn AssetContext>>,
asset_context: Vc<Box<dyn AssetContext>>,
result: Vc<ReadGlobResult>,
list: &'a mut Vec<Vc<Box<dyn Module>>>,
) -> Pin<Box<dyn Future<Output = Result<()>> + Send + 'a>> {
Box::pin(add_glob_results(context, result, list))
Box::pin(add_glob_results(asset_context, result, list))
}
// Boxing for async recursion
recurse(context, *result, list).await?;
recurse(asset_context, *result, list).await?;
}
Ok(())
}
Expand All @@ -240,16 +240,16 @@ async fn input_to_modules(
input: Vec<String>,
exact: bool,
process_cwd: Option<String>,
context: String,
context_directory: String,
module_options: TransientInstance<ModuleOptionsContext>,
resolve_options: TransientInstance<ResolveOptionsContext>,
) -> Result<Vc<Modules>> {
let root = fs.root();
let process_cwd = process_cwd
.clone()
.map(|p| p.trim_start_matches(&context).to_owned());
.map(|p| p.trim_start_matches(&context_directory).to_owned());

let context: Vc<Box<dyn AssetContext>> = Vc::upcast(create_module_asset(
let asset_context: Vc<Box<dyn AssetContext>> = Vc::upcast(create_module_asset(
root,
process_cwd,
module_options,
Expand All @@ -260,42 +260,42 @@ async fn input_to_modules(
for input in input {
if exact {
let source = Vc::upcast(FileSource::new(root.join(input)));
list.push(context.process(
list.push(asset_context.process(
source,
Value::new(turbopack_core::reference_type::ReferenceType::Undefined),
));
} else {
let glob = Glob::new(input);
add_glob_results(context, root.read_glob(glob, false), &mut list).await?;
add_glob_results(asset_context, root.read_glob(glob, false), &mut list).await?;
};
}
Ok(Vc::cell(list))
}

fn process_context(dir: &Path, context_directory: Option<&String>) -> Result<String> {
let mut context = PathBuf::from(context_directory.map_or(".", |s| s));
if !context.is_absolute() {
context = dir.join(context);
let mut context_directory = PathBuf::from(context_directory.map_or(".", |s| s));
if !context_directory.is_absolute() {
context_directory = dir.join(context_directory);
}
// context = context.canonicalize().unwrap();
Ok(context
Ok(context_directory
.to_str()
.ok_or_else(|| anyhow!("context directory contains invalid characters"))
.unwrap()
.to_string())
}

fn make_relative_path(dir: &Path, context: &str, input: &str) -> Result<String> {
fn make_relative_path(dir: &Path, context_directory: &str, input: &str) -> Result<String> {
let mut input = PathBuf::from(input);
if !input.is_absolute() {
input = dir.join(input);
}
// input = input.canonicalize()?;
let input = input.strip_prefix(context).with_context(|| {
let input = input.strip_prefix(context_directory).with_context(|| {
anyhow!(
"{} is not part of the context directory {}",
input.display(),
context
context_directory
)
})?;
Ok(input
Expand All @@ -304,10 +304,10 @@ fn make_relative_path(dir: &Path, context: &str, input: &str) -> Result<String>
.replace('\\', "/"))
}

fn process_input(dir: &Path, context: &str, input: &[String]) -> Result<Vec<String>> {
fn process_input(dir: &Path, context_directory: &str, input: &[String]) -> Result<Vec<String>> {
input
.iter()
.map(|input| make_relative_path(dir, context, input))
.map(|input| make_relative_path(dir, context_directory, input))
.collect()
}

Expand Down Expand Up @@ -547,19 +547,19 @@ async fn main_operation(
ref process_cwd,
..
} = args.common();
let context = process_context(&dir, context_directory.as_ref()).unwrap();
let fs = create_fs("context directory", &context, watch).await?;
let context_directory = process_context(&dir, context_directory.as_ref()).unwrap();
let fs = create_fs("context directory", &context_directory, watch).await?;

match *args {
Args::Print { common: _ } => {
let input = process_input(&dir, &context, input).unwrap();
let input = process_input(&dir, &context_directory, input).unwrap();
let mut result = BTreeSet::new();
let modules = input_to_modules(
fs,
input,
exact,
process_cwd.clone(),
context,
context_directory,
module_options,
resolve_options,
)
Expand All @@ -577,15 +577,15 @@ async fn main_operation(
return Ok(Vc::cell(result.into_iter().collect::<Vec<_>>()));
}
Args::Annotate { common: _ } => {
let input = process_input(&dir, &context, input).unwrap();
let input = process_input(&dir, &context_directory, input).unwrap();
let mut output_nft_assets = Vec::new();
let mut emits = Vec::new();
for module in input_to_modules(
fs,
input,
exact,
process_cwd.clone(),
context,
context_directory,
module_options,
resolve_options,
)
Expand All @@ -608,7 +608,7 @@ async fn main_operation(
common: _,
} => {
let output = process_context(&dir, Some(output_directory)).unwrap();
let input = process_input(&dir, &context, input).unwrap();
let input = process_input(&dir, &context_directory, input).unwrap();
let out_fs = create_fs("output directory", &output, watch).await?;
let input_dir = fs.root();
let output_dir = out_fs.root();
Expand All @@ -618,7 +618,7 @@ async fn main_operation(
input,
exact,
process_cwd.clone(),
context,
context_directory,
module_options,
resolve_options,
)
Expand Down
6 changes: 3 additions & 3 deletions crates/node-file-trace/src/nft_json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,15 @@ impl OutputAsset for NftJsonAsset {
impl Asset for NftJsonAsset {
#[turbo_tasks::function]
async fn content(&self) -> Result<Vc<AssetContent>> {
let context = self.entry.ident().path().parent().await?;
let parent_dir = self.entry.ident().path().parent().await?;
// For clippy -- This explicit deref is necessary
let entry_path = &*self.entry.ident().path().await?;
let mut result = Vec::new();
if let Some(self_path) = context.get_relative_path_to(entry_path) {
if let Some(self_path) = parent_dir.get_relative_path_to(entry_path) {
let set = all_modules(self.entry);
for asset in set.await?.iter() {
let path = asset.ident().path().await?;
if let Some(rel_path) = context.get_relative_path_to(&path) {
if let Some(rel_path) = parent_dir.get_relative_path_to(&path) {
if rel_path != self_path {
result.push(rel_path);
}
Expand Down
6 changes: 3 additions & 3 deletions crates/turbo-tasks-build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,13 +267,13 @@ impl<'a> RegisterContext<'a> {
fn process_mod(&mut self, mod_item: ItemMod) -> Result<()> {
if mod_item.content.is_none() {
let name = mod_item.ident.to_string();
let context = self.file_path.parent().unwrap();
let direct = context.join(format!("{name}.rs"));
let parent_path = self.file_path.parent().unwrap();
let direct = parent_path.join(format!("{name}.rs"));
if direct.exists() {
self.queue
.push((format!("{}::{name}", self.mod_path), direct));
} else {
let nested = context.join(&name).join("mod.rs");
let nested = parent_path.join(&name).join("mod.rs");
if nested.exists() {
self.queue
.push((format!("{}::{name}", self.mod_path), nested));
Expand Down
8 changes: 4 additions & 4 deletions crates/turbo-tasks-fetch/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,11 +108,11 @@ impl FetchError {
pub async fn to_issue(
self: Vc<Self>,
severity: Vc<IssueSeverity>,
context: Vc<FileSystemPath>,
issue_context: Vc<FileSystemPath>,
) -> Result<Vc<FetchIssue>> {
let this = &*self.await?;
Ok(FetchIssue {
context,
issue_context,
severity,
url: this.url,
kind: this.kind,
Expand All @@ -124,7 +124,7 @@ impl FetchError {

#[turbo_tasks::value(shared)]
pub struct FetchIssue {
pub context: Vc<FileSystemPath>,
pub issue_context: Vc<FileSystemPath>,
pub severity: Vc<IssueSeverity>,
pub url: Vc<String>,
pub kind: Vc<FetchErrorKind>,
Expand All @@ -135,7 +135,7 @@ pub struct FetchIssue {
impl Issue for FetchIssue {
#[turbo_tasks::function]
fn context(&self) -> Vc<FileSystemPath> {
self.context
self.issue_context
}

#[turbo_tasks::function]
Expand Down
16 changes: 8 additions & 8 deletions crates/turbo-tasks-fs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -879,25 +879,25 @@ pub struct FileSystemPath {
}

impl FileSystemPath {
pub fn is_inside_ref(&self, context: &FileSystemPath) -> bool {
if self.fs == context.fs && self.path.starts_with(&context.path) {
if context.path.is_empty() {
pub fn is_inside_ref(&self, other: &FileSystemPath) -> bool {
if self.fs == other.fs && self.path.starts_with(&other.path) {
if other.path.is_empty() {
true
} else {
self.path.as_bytes().get(context.path.len()) == Some(&b'/')
self.path.as_bytes().get(other.path.len()) == Some(&b'/')
}
} else {
false
}
}

pub fn is_inside_or_equal_ref(&self, context: &FileSystemPath) -> bool {
if self.fs == context.fs && self.path.starts_with(&context.path) {
if context.path.is_empty() {
pub fn is_inside_or_equal_ref(&self, other: &FileSystemPath) -> bool {
if self.fs == other.fs && self.path.starts_with(&other.path) {
if other.path.is_empty() {
true
} else {
matches!(
self.path.as_bytes().get(context.path.len()),
self.path.as_bytes().get(other.path.len()),
Some(&b'/') | None
)
}
Expand Down
4 changes: 2 additions & 2 deletions crates/turbo-tasks/src/read_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ where
T: VcValueType,
<T::Read as VcRead<T>>::Target: TraceRawVcs,
{
fn trace_raw_vcs(&self, context: &mut TraceRawVcsContext) {
(**self).trace_raw_vcs(context);
fn trace_raw_vcs(&self, trace_context: &mut TraceRawVcsContext) {
(**self).trace_raw_vcs(trace_context);
}
}

Expand Down
4 changes: 2 additions & 2 deletions crates/turbo-tasks/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ impl<T: Debug> Debug for State<T> {
}

impl<T: TraceRawVcs> TraceRawVcs for State<T> {
fn trace_raw_vcs(&self, context: &mut crate::trace::TraceRawVcsContext) {
self.inner.lock().value.trace_raw_vcs(context);
fn trace_raw_vcs(&self, trace_context: &mut crate::trace::TraceRawVcsContext) {
self.inner.lock().value.trace_raw_vcs(trace_context);
}
}

Expand Down