codemod(turbopack): Rewrite self: Vc<Self> as &self in trivial ca…

…ses (#70412)

Noticed this pattern while touching `turbo-tasks-fs`.

Functions that take `self: Vc<Self>` and immediately await it without ever using the `Vc` version of the argument should just take `&self` as an argument.

There are some remaining cleanup opportunities here, which I have some still-WIP codemods for:

- [x] If we have a trivial `let this = self` or `let this = &self.0`, we should rewrite references to `this` to use `self`. *Done in #70431*
- [ ] If we can be sure that a function never returns anything other than `Ok(...)`, we should remove `Result` from the return type.

Those changes need to be performed as a separate codemod pass due to limitations with `ast-grep`.

## How?

ast-grep: https://ast-grep.github.io/

Using the ast-grep config: https://gist.github.com/bgw/b7bc0a921cf3e3447acaf8feda60b518

Ran it with:

```
sg scan -U -r ../codemod_rewrite_vc_self.yml .
```

## Should this be a lint?

**Maybe.** I've managed to get the false-positive rate down to zero on our existing codebase (at the cost of missing some opportunities for cleanup).

ast-grep supports running as a lint rule, and this can be autofixed. However, if we fix the `let this = ...` and `Result<...>` cases as well with additional lint rules, **this might be a bit annoying as it'll trigger cascading lint rules**. You *might* need to run the linter multiple times for it to eventually settle.

It does not appear to be possible to do all these changes in a single lint rule, as we require modifying overlapping ranges of code (which ast-grep sadly doesn't seem to support with the `rewriters` rules).

We should also consider `dynlint` before heavily investing into `ast-grep` as a linter: https://github.com/trailofbits/dylint

crates/next-api/src/middleware.rs

@@ -255,8 +255,8 @@ impl MiddlewareEndpoint {
     }
 
     #[turbo_tasks::function]
-    async fn userland_module(self: Vc<Self>) -> Result<Vc<Box<dyn Module>>> {
-        let this = self.await?;
+    async fn userland_module(&self) -> Result<Vc<Box<dyn Module>>> {
+        let this = self;
 
         Ok(this
             .asset_context

crates/next-api/src/pages.rs

@@ -640,8 +640,8 @@ impl PageEndpoint {
     }
 
     #[turbo_tasks::function]
-    async fn source(self: Vc<Self>) -> Result<Vc<Box<dyn Source>>> {
-        let this = self.await?;
+    async fn source(&self) -> Result<Vc<Box<dyn Source>>> {
+        let this = self;
         Ok(Vc::upcast(FileSource::new(this.page.project_path())))
     }
 
@@ -946,10 +946,10 @@ impl PageEndpoint {
 
     #[turbo_tasks::function]
     async fn pages_manifest(
-        self: Vc<Self>,
+        &self,
         entry_chunk: Vc<Box<dyn OutputAsset>>,
     ) -> Result<Vc<Box<dyn OutputAsset>>> {
-        let this = self.await?;
+        let this = self;
         let node_root = this.pages_project.project().node_root();
         let chunk_path = entry_chunk.ident().path().await?;
 
@@ -991,10 +991,10 @@ impl PageEndpoint {
 
     #[turbo_tasks::function]
     async fn build_manifest(
-        self: Vc<Self>,
+        &self,
         client_chunks: Vc<OutputAssets>,
     ) -> Result<Vc<Box<dyn OutputAsset>>> {
-        let this = self.await?;
+        let this = self;
         let node_root = this.pages_project.project().node_root();
         let client_relative_path = this.pages_project.project().client_relative_path();
         let client_relative_path_ref = client_relative_path.await?;

crates/next-api/src/project.rs

@@ -301,8 +301,8 @@ impl ProjectContainer {
 #[turbo_tasks::value_impl]
 impl ProjectContainer {
     #[turbo_tasks::function]
-    pub async fn project(self: Vc<Self>) -> Result<Vc<Project>> {
-        let this = self.await?;
+    pub async fn project(&self) -> Result<Vc<Project>> {
+        let this = self;
 
         let env_map: Vc<EnvMap>;
         let next_config;
@@ -385,11 +385,11 @@ impl ProjectContainer {
     /// disabled, this will always return [`OptionSourceMap::none`].
     #[turbo_tasks::function]
     pub async fn get_source_map(
-        self: Vc<Self>,
+        &self,
         file_path: Vc<FileSystemPath>,
         section: Option<RcStr>,
     ) -> Result<Vc<OptionSourceMap>> {
-        Ok(if let Some(map) = self.await?.versioned_content_map {
+        Ok(if let Some(map) = self.versioned_content_map {
             map.get_source_map(file_path, section)
         } else {
             OptionSourceMap::none()
@@ -517,8 +517,8 @@ impl Project {
     }
 
     #[turbo_tasks::function]
-    async fn project_fs(self: Vc<Self>) -> Result<Vc<DiskFileSystem>> {
-        let this = self.await?;
+    async fn project_fs(&self) -> Result<Vc<DiskFileSystem>> {
+        let this = self;
         let disk_fs = DiskFileSystem::new(
             PROJECT_FILESYSTEM_NAME.into(),
             this.root_path.clone(),
@@ -537,15 +537,15 @@ impl Project {
     }
 
     #[turbo_tasks::function]
-    pub async fn output_fs(self: Vc<Self>) -> Result<Vc<DiskFileSystem>> {
-        let this = self.await?;
+    pub async fn output_fs(&self) -> Result<Vc<DiskFileSystem>> {
+        let this = self;
         let disk_fs = DiskFileSystem::new("output".into(), this.project_path.clone(), vec![]);
         Ok(disk_fs)
     }
 
     #[turbo_tasks::function]
-    pub async fn dist_dir(self: Vc<Self>) -> Result<Vc<RcStr>> {
-        Ok(Vc::cell(self.await?.dist_dir.clone()))
+    pub async fn dist_dir(&self) -> Result<Vc<RcStr>> {
+        Ok(Vc::cell(self.dist_dir.clone()))
     }
 
     #[turbo_tasks::function]
@@ -589,23 +589,23 @@ impl Project {
     }
 
     #[turbo_tasks::function]
-    pub(super) async fn env(self: Vc<Self>) -> Result<Vc<Box<dyn ProcessEnv>>> {
-        Ok(self.await?.env)
+    pub(super) async fn env(&self) -> Result<Vc<Box<dyn ProcessEnv>>> {
+        Ok(self.env)
     }
 
     #[turbo_tasks::function]
-    pub(super) async fn next_config(self: Vc<Self>) -> Result<Vc<NextConfig>> {
-        Ok(self.await?.next_config)
+    pub(super) async fn next_config(&self) -> Result<Vc<NextConfig>> {
+        Ok(self.next_config)
     }
 
     #[turbo_tasks::function]
-    pub(super) async fn next_mode(self: Vc<Self>) -> Result<Vc<NextMode>> {
-        Ok(self.await?.mode)
+    pub(super) async fn next_mode(&self) -> Result<Vc<NextMode>> {
+        Ok(self.mode)
     }
 
     #[turbo_tasks::function]
-    pub(super) async fn js_config(self: Vc<Self>) -> Result<Vc<JsConfig>> {
-        Ok(self.await?.js_config)
+    pub(super) async fn js_config(&self) -> Result<Vc<JsConfig>> {
+        Ok(self.js_config)
     }
 
     #[turbo_tasks::function]

crates/next-api/src/versioned_content_map.rs

@@ -95,7 +95,7 @@ impl VersionedContentMap {
     /// operation. When assets change, map_path_to_op is updated.
     #[turbo_tasks::function]
     async fn compute_entry(
-        self: Vc<Self>,
+        &self,
         assets_operation: Vc<OutputAssetsOperation>,
         node_root: Vc<FileSystemPath>,
         client_relative_path: Vc<FileSystemPath>,
@@ -118,7 +118,7 @@ impl VersionedContentMap {
         }
         let entries = get_entries(assets).await.unwrap_or_default();
 
-        self.await?.map_path_to_op.update_conditionally(|map| {
+        self.map_path_to_op.update_conditionally(|map| {
             let mut changed = false;
 
             // get current map's keys, subtract keys that don't exist in operation

crates/next-core/src/app_structure.rs

@@ -732,8 +732,8 @@ struct DuplicateParallelRouteIssue {
 #[turbo_tasks::value_impl]
 impl Issue for DuplicateParallelRouteIssue {
     #[turbo_tasks::function]
-    async fn file_path(self: Vc<Self>) -> Result<Vc<FileSystemPath>> {
-        let this = self.await?;
+    async fn file_path(&self) -> Result<Vc<FileSystemPath>> {
+        let this = self;
         Ok(this.app_dir.join(this.page.to_string().into()))
     }
 

crates/next-core/src/next_client/runtime_entry.rs

@@ -69,12 +69,12 @@ pub struct RuntimeEntries(Vec<Vc<RuntimeEntry>>);
 impl RuntimeEntries {
     #[turbo_tasks::function]
     pub async fn resolve_entries(
-        self: Vc<Self>,
+        &self,
         asset_context: Vc<Box<dyn AssetContext>>,
     ) -> Result<Vc<EvaluatableAssets>> {
         let mut runtime_entries = Vec::new();
 
-        for reference in &self.await? {
+        for reference in &self.0 {
             let resolved_entries = reference.resolve_entry(asset_context).await?;
             runtime_entries.extend(&resolved_entries);
         }