Skip to content

Commit

Permalink
codemod(turbopack): Remove unused Result<...> return types from `#[…
Browse files Browse the repository at this point in the history
…turbo_task::function]`s (#70492)

The `#[turbo_tasks::function]` macro always exposes a `impl
Future<Output = Result<ReadRef<T>>>`, regardless of the function's
actual return type.

We only need to use a `Result<...>` return type when the function can
fail. If it's infallible, this just adds noise.

I left a bunch of these behind in #70412, plus I think we had a lot of
these independent of that PR. This cleans them up.

```yaml
language: rust
id: remove_unused_result_return_type

utils:
  last-expression-inside-block:
    any:
      - inside: # single-line blocks just have the expression as the only child
          kind: block
        nthChild:
          position: 1
          reverse: true
      - inside: # multi-line blocks wrap their final expression in an expression_statement
          kind: expression_statement
          inside:
            kind: block
          nthChild:
            position: 1
            reverse: true
  ok-expression:
    kind: call_expression
    any:
      - pattern: Ok($_ARG)
      - pattern: $$$::Ok($_ARG)
  ok-expression-capturing:
    kind: call_expression
    any:
      - pattern: Ok($ARG)
      - pattern: $$$::Ok($ARG)
  # ast-grep does not appear to allow utils to be recursive, split out "simple blocks", and limit supported nesting
  simple-block-with-implicit-ok-return:
    kind: block
    has:
      nthChild:
        position: 1
        reverse: true
      matches: ok-expression
  simple-expression-ok-value:
    any:
      - matches: simple-block-with-implicit-ok-return
      - kind: if_expression
        all:
          - has:
              field: consequence
              matches: simple-block-with-implicit-ok-return
          - has:
              field: alternative
              has:
                matches: simple-block-with-implicit-ok-return
      - kind: match_expression
        has:
          field: body
          not:
            has:
              kind: match_arm
              has:
                field: value
                not:
                  any:
                    - matches: ok-expression
                    - matches: simple-block-with-implicit-ok-return
  block-with-implicit-ok-return:
    any:
      - matches: simple-block-with-implicit-ok-return
      - kind: block
        has:
          nthChild:
            position: 1
            reverse: true
          any:
            - kind: expression_statement 
              has:
                matches: simple-expression-ok-value
            - matches: simple-expression-ok-value # single-line blocks don't
  result-return-type:
    pattern:
      context: fn func() -> Result<$INNER_RET_TY> {}
      selector: generic_type
  infallible-fn:  # this function only appears to return `Ok(...)`, it does not use try_expression (`?`) or `anyhow::bail!(...)`
    kind: function_item
    not:
      has:
        field: body
        any:
          - not:
              matches: block-with-implicit-ok-return
          - has:
              stopBy:
                kind: function_item
              any:
                - kind: try_expression
                - pattern: "?"
                  inside:
                    kind: macro_invocation
                    stopBy: end
                - pattern: bail!($$$)
                - pattern: $$$::bail!($$$)
                - kind: return_expression
                  not:
                    has:
                      matches: ok-expression

rule:
  all:
    - pattern: $FUNC
    - kind: function_item
      has:
        field: return_type
        matches: result-return-type
      follows:
        pattern:
          context: |
            #[turbo_tasks::function]
          selector: attribute_item
        stopBy:
          not:
            kind: attribute_item
    - matches: infallible-fn

rewriters:  # this rewriter is far from perfect, and might rewrite too much
  - id: rewrite-return-type
    rule:
      matches: result-return-type
      inside:
        kind: function_item
        field: return_type
    fix: $INNER_RET_TY
  - id: unwrap-ok-values
    rule:
      all:
        - matches: ok-expression-capturing
        - any:
          - matches: last-expression-inside-block
          - inside:
              kind: return_expression
          - inside:
              kind: match_arm
    fix: $ARG

transform:
  NEW_FUNC:
    rewrite:
      rewriters:
        - rewrite-return-type
        - unwrap-ok-values
      source: $FUNC

fix: $NEW_FUNC

ignores:
  - "**/turbo-tasks-testing/**"
  - "**/turbo-tasks-memory/tests/**"
```

```
sg scan -U -r ../codemod_remove_unused_result_return_type.yml && cargo fix --lib --allow-dirty && cargo fmt
```

I used `cargo fix` in this case to auto-remove the now-unused
`anyhow::Result` imports.

I manually fixed clippy lints that now violated the `let_and_return`
lint rule (using a separate commit inside this PR), as `cargo clippy
--fix` seemed to hang when processing our `build.rs` files?
  • Loading branch information
bgw authored Sep 26, 2024
1 parent 3ead2d1 commit e1ea01d
Show file tree
Hide file tree
Showing 103 changed files with 571 additions and 643 deletions.
101 changes: 49 additions & 52 deletions crates/next-api/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,26 +152,26 @@ impl AppProject {
}

#[turbo_tasks::function]
fn client_module_options_context(self: Vc<Self>) -> Result<Vc<ModuleOptionsContext>> {
Ok(get_client_module_options_context(
fn client_module_options_context(self: Vc<Self>) -> Vc<ModuleOptionsContext> {
get_client_module_options_context(
self.project().project_path(),
self.project().execution_context(),
self.project().client_compile_time_info().environment(),
Value::new(self.client_ty()),
self.project().next_mode(),
self.project().next_config(),
))
)
}

#[turbo_tasks::function]
fn client_resolve_options_context(self: Vc<Self>) -> Result<Vc<ResolveOptionsContext>> {
Ok(get_client_resolve_options_context(
fn client_resolve_options_context(self: Vc<Self>) -> Vc<ResolveOptionsContext> {
get_client_resolve_options_context(
self.project().project_path(),
Value::new(self.client_ty()),
self.project().next_mode(),
self.project().next_config(),
self.project().execution_context(),
))
)
}

#[turbo_tasks::function]
Expand All @@ -180,101 +180,101 @@ impl AppProject {
}

#[turbo_tasks::function]
fn client_transition(self: Vc<Self>) -> Result<Vc<FullContextTransition>> {
fn client_transition(self: Vc<Self>) -> Vc<FullContextTransition> {
let module_context = self.client_module_context();
Ok(FullContextTransition::new(module_context))
FullContextTransition::new(module_context)
}

#[turbo_tasks::function]
fn rsc_module_options_context(self: Vc<Self>) -> Result<Vc<ModuleOptionsContext>> {
Ok(get_server_module_options_context(
fn rsc_module_options_context(self: Vc<Self>) -> Vc<ModuleOptionsContext> {
get_server_module_options_context(
self.project().project_path(),
self.project().execution_context(),
Value::new(self.rsc_ty()),
self.project().next_mode(),
self.project().next_config(),
NextRuntime::NodeJs,
))
)
}

#[turbo_tasks::function]
fn edge_rsc_module_options_context(self: Vc<Self>) -> Result<Vc<ModuleOptionsContext>> {
Ok(get_server_module_options_context(
fn edge_rsc_module_options_context(self: Vc<Self>) -> Vc<ModuleOptionsContext> {
get_server_module_options_context(
self.project().project_path(),
self.project().execution_context(),
Value::new(self.rsc_ty()),
self.project().next_mode(),
self.project().next_config(),
NextRuntime::Edge,
))
)
}

#[turbo_tasks::function]
fn route_module_options_context(self: Vc<Self>) -> Result<Vc<ModuleOptionsContext>> {
Ok(get_server_module_options_context(
fn route_module_options_context(self: Vc<Self>) -> Vc<ModuleOptionsContext> {
get_server_module_options_context(
self.project().project_path(),
self.project().execution_context(),
Value::new(self.route_ty()),
self.project().next_mode(),
self.project().next_config(),
NextRuntime::NodeJs,
))
)
}

#[turbo_tasks::function]
fn edge_route_module_options_context(self: Vc<Self>) -> Result<Vc<ModuleOptionsContext>> {
Ok(get_server_module_options_context(
fn edge_route_module_options_context(self: Vc<Self>) -> Vc<ModuleOptionsContext> {
get_server_module_options_context(
self.project().project_path(),
self.project().execution_context(),
Value::new(self.route_ty()),
self.project().next_mode(),
self.project().next_config(),
NextRuntime::Edge,
))
)
}

#[turbo_tasks::function]
fn rsc_resolve_options_context(self: Vc<Self>) -> Result<Vc<ResolveOptionsContext>> {
Ok(get_server_resolve_options_context(
fn rsc_resolve_options_context(self: Vc<Self>) -> Vc<ResolveOptionsContext> {
get_server_resolve_options_context(
self.project().project_path(),
Value::new(self.rsc_ty()),
self.project().next_mode(),
self.project().next_config(),
self.project().execution_context(),
))
)
}

#[turbo_tasks::function]
fn edge_rsc_resolve_options_context(self: Vc<Self>) -> Result<Vc<ResolveOptionsContext>> {
Ok(get_edge_resolve_options_context(
fn edge_rsc_resolve_options_context(self: Vc<Self>) -> Vc<ResolveOptionsContext> {
get_edge_resolve_options_context(
self.project().project_path(),
Value::new(self.rsc_ty()),
self.project().next_mode(),
self.project().next_config(),
self.project().execution_context(),
))
)
}

#[turbo_tasks::function]
fn route_resolve_options_context(self: Vc<Self>) -> Result<Vc<ResolveOptionsContext>> {
Ok(get_server_resolve_options_context(
fn route_resolve_options_context(self: Vc<Self>) -> Vc<ResolveOptionsContext> {
get_server_resolve_options_context(
self.project().project_path(),
Value::new(self.route_ty()),
self.project().next_mode(),
self.project().next_config(),
self.project().execution_context(),
))
)
}

#[turbo_tasks::function]
fn edge_route_resolve_options_context(self: Vc<Self>) -> Result<Vc<ResolveOptionsContext>> {
Ok(get_edge_resolve_options_context(
fn edge_route_resolve_options_context(self: Vc<Self>) -> Vc<ResolveOptionsContext> {
get_edge_resolve_options_context(
self.project().project_path(),
Value::new(self.route_ty()),
self.project().next_mode(),
self.project().next_config(),
self.project().execution_context(),
))
)
}

#[turbo_tasks::function]
Expand Down Expand Up @@ -444,49 +444,49 @@ impl AppProject {
}

#[turbo_tasks::function]
fn ssr_module_options_context(self: Vc<Self>) -> Result<Vc<ModuleOptionsContext>> {
Ok(get_server_module_options_context(
fn ssr_module_options_context(self: Vc<Self>) -> Vc<ModuleOptionsContext> {
get_server_module_options_context(
self.project().project_path(),
self.project().execution_context(),
Value::new(self.ssr_ty()),
self.project().next_mode(),
self.project().next_config(),
NextRuntime::NodeJs,
))
)
}

#[turbo_tasks::function]
fn edge_ssr_module_options_context(self: Vc<Self>) -> Result<Vc<ModuleOptionsContext>> {
Ok(get_server_module_options_context(
fn edge_ssr_module_options_context(self: Vc<Self>) -> Vc<ModuleOptionsContext> {
get_server_module_options_context(
self.project().project_path(),
self.project().execution_context(),
Value::new(self.ssr_ty()),
self.project().next_mode(),
self.project().next_config(),
NextRuntime::Edge,
))
)
}

#[turbo_tasks::function]
fn ssr_resolve_options_context(self: Vc<Self>) -> Result<Vc<ResolveOptionsContext>> {
Ok(get_server_resolve_options_context(
fn ssr_resolve_options_context(self: Vc<Self>) -> Vc<ResolveOptionsContext> {
get_server_resolve_options_context(
self.project().project_path(),
Value::new(self.ssr_ty()),
self.project().next_mode(),
self.project().next_config(),
self.project().execution_context(),
))
)
}

#[turbo_tasks::function]
fn edge_ssr_resolve_options_context(self: Vc<Self>) -> Result<Vc<ResolveOptionsContext>> {
Ok(get_edge_resolve_options_context(
fn edge_ssr_resolve_options_context(self: Vc<Self>) -> Vc<ResolveOptionsContext> {
get_edge_resolve_options_context(
self.project().project_path(),
Value::new(self.ssr_ty()),
self.project().next_mode(),
self.project().next_config(),
self.project().execution_context(),
))
)
}

#[turbo_tasks::function]
Expand Down Expand Up @@ -530,11 +530,8 @@ impl AppProject {
}

#[turbo_tasks::function]
fn runtime_entries(self: Vc<Self>) -> Result<Vc<RuntimeEntries>> {
Ok(get_server_runtime_entries(
Value::new(self.rsc_ty()),
self.project().next_mode(),
))
fn runtime_entries(self: Vc<Self>) -> Vc<RuntimeEntries> {
get_server_runtime_entries(Value::new(self.rsc_ty()), self.project().next_mode())
}

#[turbo_tasks::function]
Expand All @@ -558,15 +555,15 @@ impl AppProject {
}

#[turbo_tasks::function]
fn client_runtime_entries(self: Vc<Self>) -> Result<Vc<EvaluatableAssets>> {
Ok(get_client_runtime_entries(
fn client_runtime_entries(self: Vc<Self>) -> Vc<EvaluatableAssets> {
get_client_runtime_entries(
self.project().project_path(),
Value::new(self.client_ty()),
self.project().next_mode(),
self.project().next_config(),
self.project().execution_context(),
)
.resolve_entries(Vc::upcast(self.client_module_context())))
.resolve_entries(Vc::upcast(self.client_module_context()))
}

#[turbo_tasks::function]
Expand Down
6 changes: 2 additions & 4 deletions crates/next-api/src/global_module_id_strategy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,7 @@ impl GlobalModuleIdStrategyBuilder {
// NOTE(LichuAcu) We can't move this function to `turbopack-core` because we need access to
// `Endpoint`, which is not available there.
#[turbo_tasks::function]
fn preprocess_module_ids(
endpoint: Vc<Box<dyn Endpoint>>,
) -> Result<Vc<PreprocessedChildrenIdents>> {
fn preprocess_module_ids(endpoint: Vc<Box<dyn Endpoint>>) -> Vc<PreprocessedChildrenIdents> {
let root_modules = endpoint.root_modules();
Ok(children_modules_idents(root_modules))
children_modules_idents(root_modules)
}
6 changes: 3 additions & 3 deletions crates/next-api/src/instrumentation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ impl InstrumentationEndpoint {
}

#[turbo_tasks::function]
fn core_modules(&self) -> Result<Vc<InstrumentationCoreModules>> {
fn core_modules(&self) -> Vc<InstrumentationCoreModules> {
let userland_module = self
.asset_context
.process(
Expand All @@ -81,11 +81,11 @@ impl InstrumentationEndpoint {
"instrumentation".into(),
);

Ok(InstrumentationCoreModules {
InstrumentationCoreModules {
userland_module,
edge_entry_module,
}
.cell())
.cell()
}

#[turbo_tasks::function]
Expand Down
11 changes: 5 additions & 6 deletions crates/next-api/src/middleware.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,14 +255,13 @@ impl MiddlewareEndpoint {
}

#[turbo_tasks::function]
fn userland_module(&self) -> Result<Vc<Box<dyn Module>>> {
Ok(self
.asset_context
fn userland_module(&self) -> Vc<Box<dyn Module>> {
self.asset_context
.process(
self.source,
Value::new(ReferenceType::Entry(EntryReferenceSubType::Middleware)),
)
.module())
.module()
}
}

Expand Down Expand Up @@ -311,7 +310,7 @@ impl Endpoint for MiddlewareEndpoint {
}

#[turbo_tasks::function]
fn root_modules(self: Vc<Self>) -> Result<Vc<Modules>> {
Ok(Vc::cell(vec![self.userland_module()]))
fn root_modules(self: Vc<Self>) -> Vc<Modules> {
Vc::cell(vec![self.userland_module()])
}
}
Loading

0 comments on commit e1ea01d

Please sign in to comment.