skip to content
Krishna Sundarram

What I learned from contributing to Rust's linter

/ 6 min read

Last week I attended a workshop run by the maintainer of Rust’s linter Clippy. Rust devs run Clippy on their code to improve its performance, correctness and style.

clippy

The goal of the workshop was to help people to contribute to Clippy. We did this (in my opinion) the simplest, most effective way - everyone was assigned an issue and got started. Everyone already knew Rust so this worked well.

I paired on the first issue with another developer, then tackled the next two issues solo. All 3 were fixes to existing lints.

  • #10353 - Change unusual_byte_groupings to require byte groupings of equal size.
  • #10357 - Stop doc_markdown requiring backticks on links to external websites.
  • #10361 - Stop bytes_nth from suggesting code that does not compile.

What I learned from the experience

My day job doesn’t involve Rust or buildng developer tools but I still learned a lot from the experience. The Clippy project and this workshop got a lot of things right. The main thing is providing a welcoming environment to newbies.

  • Maintain a list of tightly scoped, non-urgent tasks, ideal for newcomers to to work on. In the Clippy issue tracker, these are labelled “good-first-issue”.
  • Great documentation
  • Low latency communication. Andre Bogus (@llogiq), the Clippy maintainer running the workshop assigned an issue, explained the background behind it and answered any questions we had. For example, on this lint causing some compile issues we quickly figured out the desired behaviour. Afterwards he reviewed the PRs as soon as we published them. The quick turnaround encouraged me to pick up another issue and another after that.

As an open source project that depends on volunteers, It makes sense that Clippy invests so much in the new developer experience. There have been 899 contributors to Clippy, 321 (35%) of whom have exactly one commit and 742 (82%) 10 commits or fewer. The early contributors and current maintainers have set up a project where anyone can step in, contribute an improvement or two with ease and step out. As someone who onboards 2-4 new developers on my team each year, there’s a lot to learn from this gold standard.

Personally I think I benefited from that feeling of being new and out of my depth in a new problem domain.

Just one more thing

The three issues I worked on were relatively simple. Feeling confident, I picked up a feature request from the Rust for Linux project for a new lint around name mangling.

What is name mangling? (expand)
A #[no_mangle] annotation will tell the Rust compiler not to mangle the name of this function. Mangling is when a compiler changes the name we’ve given a function to a different name that contains more information for other parts of the compilation process to consume but is less human readable. Every programming language compiler mangles names slightly differently, so for a Rust function to be nameable by other languages, we must disable the Rust compiler’s name mangling.
Source: Unsafe Rust, The Rust Programming Language

Example: a function bar() in a module foo within a crate mycrate would appear in the final artifact with the mangled name NvNtC7mycrate3foo3bar() (source). With #[no_mangle] it would appear in the artifact named just bar().



The #[no_mangle] attribute opts a function out of name mangling by the Rust compiler, allowing it to be called from other languages. This works whether the function uses the Rust ABI (application binary interface) or C ABI . Thing is, the Rust ABI isn’t guaranteed to be stable and could change in future. Therefore a function we’re going to call from another language should have the #[no_mangle] attribute and opt in to the stable C ABI with extern "C" like so

#[no_mangle]
extern "C" fn foo() {
}

That’s what this lint needed to do - check for functions with the #[no_mangle] attribute and warn if they didn’t opt into the C ABI. You can see why this is useful for folks writing Rust code within the Linux kernel. Their Rust code will almost always be called from another language (C). If they accidentally use the Rust ABI, the code could fail silently 5 or 10 years later if the Rust ABI changes.

I was comfortable working with Clippy now, so I found this lint simple to implement. What was harder was estimating the potential impact of this lint on users of Clippy. Lints that are on by default (opt-out) need to have few or no false positives. False positives annoy users and they could end up disabling Clippy.

It was unclear if this lint would meet the bar for low false positives. In theory most people would pair #[no_mangle] with extern "C". But that’s not what a search on Github showed.

About a third of functions with this attribute use the Rust ABI. They could be making a conscious choice here, we don’t know. For example, someone on internals.rust-lang.org mentioned they do this for more readable assembly output. And open source code on Github is only the tip of the iceberg; there likely is closed source code doing the same thing. All of these folks would wake up to failing CI if this lint was opt-out.

Instead I asked the Rust for Linux folks if they were ok with this lint being opt-in. Turns out, they’re fine with that because they opt-in to a few lints already. I changed the lint to pedantic (ie, opt-in) and merged it in (PR #10369).

It’ll release with Rust 1.69.0, expected April 20th, 2023.

Conclusion

I improved 3 existing lints and added a new lint to Rust’s linter.

I had a blast contributing to Clippy. I’d highly recommend checking out a “good-first-issue” if you’re interesting in contributing to open source.

Big thanks to @llogiq for organising the workshop, finding issues and reviewing all my PRs!

Also thank you to @bjorn3 and @ojeda for helping me understand exactly what the no_mangle lint needed to do.


Check out posts similar to this one