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

new lint: absolute symbol path usage #10568

Closed
d-e-s-o opened this issue Mar 30, 2023 · 1 comment · Fixed by #11003
Closed

new lint: absolute symbol path usage #10568

d-e-s-o opened this issue Mar 30, 2023 · 1 comment · Fixed by #11003
Labels
A-lint Area: New lints

Comments

@d-e-s-o
Copy link

d-e-s-o commented Mar 30, 2023

What it does

We get a lot of contributions that, for one reason or another, reference symbols with absolute paths instead of having a use declaration at the top. E.g.,

fn main() {
  println!("{}", std::cmp::max(1, 2));
}

instead of

use std::cmp::max;

fn main() {
  println!("{}", max(1, 2));
}

It's not wrong, of course, and variations are possible. E.g., some projects prefer use std::cmp; over use std::cmp::max etc. pp.

But: I am not aware of a single project that uses absolute style. As such, I think there is value in flagging it (without necessarily suggesting an alternative, given that there is more than one). I would certainly find it useful, as it's cumbersome to point out those cases during review, as opposed to having a machine provide assurance that there are none.

Lint Name

absolute-symbol-path or perhaps symbol-reference-without-use

Category

style, pedantic

Advantage

Point out inconsistent use style. For better or worse, most code bases and guides have explicit use statements that import relevant symbols, so that they can be used without having to provide an absolute path/full reference to the symbol at each call/use site. Having some symbols use absolute paths in the middle of code while others have explicit imports seems l;ike an inconsistency that, for no good reason, and could give a false impression of what functionality is used by a module when just checking use statements.

Drawbacks

I don't believe there are drawbacks, but it's a subjective matter, so there are differing opinions on what folks prefer. Should probably be disabled by default.

That being said, macros should probably be excluded from this lint, as it absolutely makes sense for macros to use absolute imports (and it's even the right thing to do in most cases, but that's a different story).

Example

fn main() {
  println!("{}", std::cmp::max(1, 2));
}

Could be written as:

use std::cmp::max;

fn main() {
  println!("{}", max(1, 2));
}
@d-e-s-o d-e-s-o added the A-lint Area: New lints label Mar 30, 2023
@bors bors closed this as completed in 58df1e6 Jul 21, 2023
@d-e-s-o
Copy link
Author

d-e-s-o commented Jul 21, 2023

Wow, thanks @Centri3 for implementing this lint so quickly!

d-e-s-o added a commit to d-e-s-o/libbpf-rs that referenced this issue Dec 4, 2023
In the past we had many submissions that, for one reason or another,
were using absolute paths to functions/constants in a very inconsistent
manner. It is not particularly great use of anybody's time pointing
these out and it's certainly a job that computers can do better. Clippy
folks seem to concur that this can be a useful lint [0] and we got
blessed with clippy::absolute_paths.
Enable it throughout libbpf-rs and libbpf-cargo and fix all violations.

[0] rust-lang/rust-clippy#10568

Signed-off-by: Daniel Müller <[email protected]>
danielocfb pushed a commit to libbpf/libbpf-rs that referenced this issue Dec 4, 2023
In the past we had many submissions that, for one reason or another,
were using absolute paths to functions/constants in a very inconsistent
manner. It is not particularly great use of anybody's time pointing
these out and it's certainly a job that computers can do better. Clippy
folks seem to concur that this can be a useful lint [0] and we got
blessed with clippy::absolute_paths.
Enable it throughout libbpf-rs and libbpf-cargo and fix all violations.

[0] rust-lang/rust-clippy#10568

Signed-off-by: Daniel Müller <[email protected]>
d-e-s-o added a commit to d-e-s-o/blazesym that referenced this issue Dec 26, 2023
In the past we had many submissions that, for one reason or another,
were using absolute paths to functions/constants in a very inconsistent
manner. It is not particularly great use of anybody's time pointing
these out and it's certainly a job that computers can do better. Clippy
folks seem to concur that this can be a useful lint [0] and we got
blessed with clippy::absolute_paths.
Enable it throughout the crate and fix all violations.

[0] rust-lang/rust-clippy#10568

Signed-off-by: Daniel Müller <[email protected]>
d-e-s-o added a commit to libbpf/blazesym that referenced this issue Dec 26, 2023
In the past we had many submissions that, for one reason or another,
were using absolute paths to functions/constants in a very inconsistent
manner. It is not particularly great use of anybody's time pointing
these out and it's certainly a job that computers can do better. Clippy
folks seem to concur that this can be a useful lint [0] and we got
blessed with clippy::absolute_paths.
Enable it throughout the crate and fix all violations.

[0] rust-lang/rust-clippy#10568

Signed-off-by: Daniel Müller <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant