rust-lang / rust-clippy

A bunch of lints to catch common mistakes and improve your Rust code. Book: https://doc.rust-lang.org/clippy/
https://rust-lang.github.io/rust-clippy/
Other
11.41k stars 1.54k forks source link

Flag unnecessary visiblity attributes on nested functions #13262

Open d-e-s-o opened 2 months ago

d-e-s-o commented 2 months ago

What it does

When a function is nested inside another one, the inner one can only be accessed within the scope of the outer one, but not outside. Yet, it is possible to have a visibility attribute applied to the inner. That conceptually makes little sense. It could be the sign of a bug, wrong conception of the author, or just an oversight. Regardless, it should be uncontroversal that it can be removed:

fn outer() {
  pub(crate) fn inner() {
  }

  inner()
}

I think this could even be a rustc warning, but filing this here for discussion first.

Advantage

Drawbacks

Example

fn outer() {
  pub(crate) fn inner() {
  }

  inner()
}

Could be written as:

fn outer() {
  fn inner() {
  }

  inner()
}
alex-semenyuk commented 2 months ago

Could be needless_attribute_on_inner_fun or unnecessary_attribute_on_inner_fun

jdahlstrom commented 2 weeks ago

This pertains to all locally declared items, not just functions, mind.

There's a related nursery lint redundant_pub_crate which fires if a pub(crate) item is inside a private module and thus not accessible at crate level. Conceptually function scope is like an unnameable private module, so the existing lint could maybe be expanded to cover this case as well?

(BTW, "redundant" doesn't feel like the correct word, arguably something like "misleading" would be better.)