Skip to content

Disable code Action for clippy::needless_return #16542

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

Closed
aminya opened this issue Feb 12, 2024 · 27 comments · Fixed by #16757
Closed

Disable code Action for clippy::needless_return #16542

aminya opened this issue Feb 12, 2024 · 27 comments · Fixed by #16757
Labels
A-diagnostics diagnostics / error reporting C-bug Category: bug

Comments

@aminya
Copy link

aminya commented Feb 12, 2024

Rust-analyzer has started to give code action suggestions for removing return from my code despite me allowing clippy::needless_return via #![allow(clippy::needless_return)].

In my coding style, I do not like my returns to be implicit, and I don't really align with the push to force not using return statements in the Rust tooling (e.g. Clippy, Rust-analyzer, etc).

Could you add an option to remove this code action, or respect the clippy configuration enabled at the module level?

rust-analyzer version: 0.4.1838-standalone

rustc version: rustc 1.75.0-nightly (42b1224e9 2023-10-15)

relevant settings: (eg. client settings, or environment variables like CARGO, RUSTC, RUSTUP_HOME or CARGO_HOME)

The offending feature:

#16460

@aminya aminya added the C-bug Category: bug label Feb 12, 2024
@lnicola
Copy link
Member

lnicola commented Feb 12, 2024

Unfortunately, I don't think we respect #[allow(clippy::needless_return)] yet, but in VS Code you can disable it using:

{
    "rust-analyzer.diagnostics.disabled": [
        "needless_return"
    ]
}

@j0ni
Copy link

j0ni commented Feb 14, 2024

Weirdly, I see this showing up via eglot in emacs on a function which does not contain an explicit return.

It's annotated with a #[tokio::main] macro, so I guess the problem lies somewhere in the expanded code? I look forward to a working lints.rust (or lints.clippy I guess) entry for this!

@Veykril Veykril added the A-diagnostics diagnostics / error reporting label Feb 14, 2024
@I-Info
Copy link

I-Info commented Feb 14, 2024

Maybe this rule should not be applied on the macro expansions. 🤔

@Veykril
Copy link
Member

Veykril commented Feb 14, 2024

Our filtering for diagnostic level attributes is somewhat broken wrt to macros, that is a known issue.

@lnicola
Copy link
Member

lnicola commented Feb 14, 2024

It should respect #[allow(clippy::needless_return)], but tokio::main doesn't even seem to expand to that. It only has #[allow(clippy::expect_used, clippy::diverging_sub_expression)] for me.

@Veykril
Copy link
Member

Veykril commented Feb 14, 2024

Might be that needless_return doesn't trigger on macro expansions with clippy

@lnicola
Copy link
Member

lnicola commented Feb 14, 2024

No, I think it's something kind of heuristic:

// no warning
fn main() {
    return tokio::runtime::Builder::new_current_thread()
        .enable_all()
        .build()
        .expect("Failed building the Runtime")
        .block_on(async {});
}

// warning
fn main() {
    return tokio::runtime::Runtime::new()
        .expect("Failed building the Runtime")
        .block_on(async {});
}

// warning
fn main() {
    return tokio::runtime::Builder::new_current_thread()
        .build()
        .expect("Failed building the Runtime")
        .block_on(async {});
}

@lnicola
Copy link
Member

lnicola commented Feb 14, 2024

@Veykril
Copy link
Member

Veykril commented Feb 14, 2024

Oh, that's probably due to temporaries? (Then we most likelyneed that as well)

@lnicola
Copy link
Member

lnicola commented Feb 14, 2024

Yeah: https://github.com/rust-lang/rust-clippy/blame/48b4aea/clippy_lints/src/returns.rs#L19-L20

Known problems: If the computation returning the value borrows a local variable, removing the return may run afoul of the borrow checker.

Sounds like a limitation of Clippy itself (it's too conservative here), but we probably want to match it anyway.

@arnfaldur
Copy link

Is there any way to disable this behavior?

@lnicola
Copy link
Member

lnicola commented Feb 19, 2024

@arnfaldur see my comment at the top.

@arnfaldur
Copy link

@lnicola ah, sorry, didn't think too much about that as I don't use VSCode. There must be some way to configure LSPs in vim/emacs like that. I'll figure it out, thanks.

@j0ni
Copy link

j0ni commented Feb 24, 2024

@arnfaldur and anyone else who lands here looking for a workaround in emacs using eglot, here is the configuration tweak I have, which appears to work:

(with-eval-after-load 'eglot
  (add-to-list 'eglot-server-programs
               '((rust-ts-mode rust-mode) .
                 ("rust-analyzer"
                  :initializationOptions
                  (:check
                   (:command "clippy")
                   :diagnostics
                   (:disabled ["needless_return"
                               "clippy::needless_return"]))))))

The :check (:command "clippy") part is unrelated, but demonstrates how to compose different config options. I'm not sure whether the clippy:: prefixed item is doing any work here, it was just a piece of spaghetti I threw at the wall which was stuck there when it started working :)

@SergSpice
Copy link

SergSpice commented Mar 5, 2024

Why is this is even in the project?, one of the things that I hate about Go lsp and ESLint is that they force you to have a code style that is horrendous, I think the whole "needless_return" is an overreach, I haven't even been able to remove this in my Neovim config, and is not like I have an old soul and I'm not open to new things, it's because my project now is full of warnings since I updated the version, and this doesn't even makes sense, is a fricking macro!!!!:
image

@leelhn2345
Copy link

leelhn2345 commented Mar 5, 2024

Why is this is even in the project?, one of the things that I hate about Go linter and ESLint is that they force you to have a code style that is horrendous, I think the whole "needless_return" is an overreach, I haven't even been able to remove this in my Neovim config, and is not like I have an old soul and I'm not open to new things, it's because my project now is full of warnings since I updated the version, and this doesn't even makes sense, is a fricking macro!!!!: image

until there's a permanent change to this,
you can configure rust analyzer to disable the warning.

          ["rust-analyzer"] = {
            diagnostics = {
              disabled = {
                "needless_return",
              },
            },
          }

this is from my rustacean.nvim plugin, but i believe the settings should be similiar to other lsp plugins.

@nilehmann

This comment was marked as off-topic.

@conradogarciaberrotaran
Copy link

for lspconfig within mason:

require("mason-lspconfig").setup_handlers({
  ["rust_analyzer"] = function()
    require("lspconfig").rust_analyzer.setup({
      on_attach = on_attach,
      capabilities = capabilities,
      settings = {
        ["rust-analyzer"] = {
          diagnostics = {
            disabled = {
              "needless_return",
            },
          },
        },
      },
    })
  end,
})

@Veykril
Copy link
Member

Veykril commented Mar 6, 2024

The lint shouldn't trigger by default anymore next release until opted-in, 676455f

@conradogarciaberrotaran
Copy link

It has to be released right?
If so, would you update this: https://github.com/mason-org/mason-registry/blob/main/packages/rust-analyzer/package.yaml
or let me know to do it myself.

Thanks!

@lnicola
Copy link
Member

lnicola commented Mar 6, 2024

It will be released on Monday, or you could use the nightly (I've triggered a build now, should finish in about 15 minutes). We don't own mason-registry, but you can probably download a binary and replace it manually.

@conradogarciaberrotaran
Copy link

I'll create a PR when it's released.

Thanks :)

@lnicola
Copy link
Member

lnicola commented Mar 6, 2024

I imagine they might not want to use nightlies, so it's probably fine to wait until Monday.

@conradogarciaberrotaran

mason-registry's versions are auto updated :)
mason-org/mason-registry@696a25a

@gamedev-pnc
Copy link

{
    "rust-analyzer.diagnostics.disabled": [
        "needless_return"
    ]
}

Didn't work for me on VSCode and clippy 0.1.79 (129f3b99 2024-06-10). Are there other solutions?

@gamedev-pnc
Copy link

#![allow(clippy::needless_return)] -- works in clippy 0.1.80.
Thanks!

@Ritzier
Copy link

Ritzier commented Oct 10, 2024

default_settings = {
    checkOnSave = {
        false
    }
}

For my configuraiotn, disable checkOnSave

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics diagnostics / error reporting C-bug Category: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.