Adds an alternate way to enable our standard lints (#59) for all this crates in a repository through .cargo/config.toml section. Inspired by https://github.com/rust-lang/cargo/issues/5034#issuecomment-927105016.

We are testing this as a workaround for #22 until Cargo/Clippy supports a proper mechanism to configure lints for an entire repository/workspace, and seems to work really well so far. One of our big repositories has 80+ crates and all of them had the previous lint code definitions in every lib.rs/main.rs copy'n'pasted and this will replace all of that and make it dramatically easier to manage and less error prone.

Current plan for v0.3

• Add: map_err_ignore - prevents loosing error context and works well in Ark, easy to ignore when needed
• Add: match_same_arms - seems to work well and simplifies matching code to reduce complexity, with some exceptions.
• Add: large_types_passed_by_value - seems like a good idea, could catch some things should be boxed. no warns in Ark.
• Add: string_add - cleaner & more explicit, only 1 easy fixed warn in Ark.
• Add: string_add_assign - cleaner & more explicit, only 1 easy fixed warn in Ark.
• Add: explicit_iter_loop - we already use explicit_into_iter_loop in v0.2, should enable or disable both.
• Add: unimplemented - good for stubbing out WIP code but want to avoid on main our crates & systems are live at head

The match_same_arms lint is definitely the most intrusive change of these, and may need some opt outs in a few cases when one doesn't want to re-order code, but have been quite favored still.

Under consideration, but currently not in v0.3

Some discussion in the PR but think these require more testing or follow up.

• Add: needless_pass_by_value - helped us catch a lot of unnecessary : Vec<T> arguments and replace them with : &[T], saving a lot of calls to .clone() (very common trap for rust beginners to fall into). quite intrusive change for lots of crates though so safer to do separtely
• Remove: doc_markdown - has a 4y old outstanding issue on acronyms that it wants to force codeformatting backticks on which is quite messy and requires a lot of manual disabling (or a clippy.toml config): https://github.com/rust-lang/rust-clippy/issues/1136
• Remove: pub_enum_variant_names - pretty common for this one to trigger in valid/good cases, so is a bit hit & miss.
• Remove: suboptimal_flops - rearranges code quite significantly with mul_add

Will leave this as a draft for a while and feel free to discuss & suggest additional additions or removals

Thanks to the new Cargo nightly -Z timings I saw that our 3.5 min Windows release build was spending 65 seconds building just the old winapi 0.2 crate.

The wall time itself, while very high, is not a major problem as this was a larger from scratch build and lots of cores available, but what the timings clearly showed was that it pushed out a lot of build steps because there were tons of small and big crates that had dependencies on winapi 0.2 directly or indirectly and as such couldn't even start until that had been built.

So want to fully get rid of winapi 0.2 crate in our builds and migrate the crates that we are using to the faster and better winapi 0.3 or, in some cases, remove them.

Current crates that use (directly or indirectly) that uses winapi 0.2:

• [x] iovec:: This is the most important as huge ecosystem depend on this small crate. PR fix filed
• [x] app_dirs: Abandoned crate, switched to app_dirs2 instead and PR fix was merged & published in 7 minutes (!)
• [x] mio: In-progress v0.7 is using latest winapi, v0.6 will stay on winapi 0.2. May just have to wait for v0.7?
• [x] notify: Unmaintained crate, leaf dependency so less important but should find alternative or get it updated. Related PR: https://github.com/notify-rs/notify/pull/217
• [x] wasmer-runtime-core: From page_size. Is there a more modern/maintained equivalent that they can switch to?
• [x] tokio Update to mio 0.7 https://github.com/tokio-rs/tokio/issues/1190
• [x] Ecosystem upgrade to new tokio 0.3
• [x] winit uses mio 0.6

Once we have those resolved:, we want to allow be able to disallow winapi 0.2 in our projects:

• [x] cargo-deny: https://github.com/EmbarkStudios/cargo-deny/issues/28

This used to be important for us as we have a lot of private crates and dependencies between them.

But we have since moved to a monorepo for all of our Rust crates where it is easy to set up dependencies within the repository and workspace.

But believe for other companies this will be critical to adopt Rust.

One workaround that we've used in the past when having multiple private repositories of crates with dependencies between them is to set up the dependencies as git dependencies. The draw back of that is that it is both slower to download and more to set up on local machines and esp. CI.

• [x] https://github.com/rust-lang/cargo/issues/6589
• [x] https://github.com/rust-lang/rust/issues/44931
• [x] https://github.com/rust-lang/cargo/issues/6691

We use a single large workspace and monorepo and RLS very often hangs / stops working.

Also performance with having RLS build our repo for its own target can be quite bad, esp. working on laptop. Looking forward to future more incremental background service.

Related issues:

• [ ] clippy: force full re-clippy #2604
• [ ] rls: Performance issue of rls on large code base
• [ ] rust gamedev wg: RLS for huge projects

We currently can't debug our WASM modules (that we run not in a browser), this will become a bigger and bigger issue going forward so we should investigate what are the paths for getting some type of debugging support either with wasmer or wasmtime.

We would want it to be super easy to debug the code for a small (or big) WASM module directly in VS Code as well as in other editors.

Related:

• [ ] wasmer: Add support for step debugging and Virtual Machine introspection

Thanks for providing a good clippy base in #59!

These are the extra lints that we have enabled over at Traverse. They were mostly picked at random a while back by looking at default-allowed lints and is likely incomplete. In addition we have these pending, hoping they become useful one day after fixing all violations:

clippy::clone_on_ref_ptr
clippy::cognitive_complexity
clippy::pattern_type_mismatch
clippy::unimplemented
clippy::use_self

Hope these are helpful to others as they are to others. Some might already be considered, or are disabled due to false-positives elsehwere and should be filtered out, let me know.

With these changes our standard lint set now requires Rust 1.52, and builds on Rust 1.53 / beta / nightly without warnings about renamed/changed lints.

Removals

• clippy::pub_enum_variant_names - This was removed in Rust 1.53 so can't be in our standard lint set anymore.
• clippy::suboptimal_flops - Harder to read and perf degradations, see discussions in #63 and https://github.com/rust-lang/rust-clippy/issues/6867

Additions

Enables the following lints that we've been testing with in Ark for a few months. Many of them are somewhat low impact but low cost so just nice to have enabled.

• clippy::char_lit_as_u8
• clippy::checked_conversions
• clippy::expl_impl_clone_on_copy
• clippy::explicit_deref_methods
• clippy::fallible_impl_from
• clippy::float_cmp_const
• clippy::implicit_clone - More explicit have have also found redundant clones with it which is nice
• clippy::invalid_upcast_comparisons
• clippy::manual_ok_or
• clippy::mut_mut
• clippy::mutex_integer
• clippy::path_buf_push_overwrite
• clippy::ptr_as_ptr
• clippy::same_functions_in_if_condition
• clippy::semicolon_if_nothing_returned - This one is really quite pedantic and typically a bunch of small fixups. But is easy to do and keeps it explicit
• clippy::string_lit_as_bytes
• clippy::trait_duplication_in_bounds
• clippy::useless_transmute
• clippy::zero_sized_map_values

Part of #59

clippy::use_self was previously part of our default set of clippy lints but we've had to remove it since it lead to false positives where applying its suggestions would fail to compile.

This comment links to some of the issues.

One specific case we've run into is this

#![deny(clippy::use_self)]

pub struct Foo<T> {
    value: T,
}

impl<T> Foo<T> {
    pub fn new<K>(value: K) -> Foo<K> {
                            // ^^^^^^ changing this to `Self` wont work
        Foo { value }
    }
}

For our internal and open source repositories.

We don't want where we develop to have a legacy name with these associations:

mast•er măs′tər

n. One that has control over another person, a group of persons, or a thing, especially. n. The owner or keeper of an animal. n. The owner of a slave.

and main is shorter!

Good blog post and guide: https://www.hanselman.com/blog/EasilyRenameYourGitDefaultBranchFromMasterToMain.aspx

TODO:

• [x] 🌋 ash-molten
• [x] 👷 buildkite-jobify - @Jake-Shadle
• [x] 📜 cargo-about - @Jake-Shadle
• [x] ❌ cargo-deny - @Jake-Shadle
• [x] 🎁 cargo-fetcher - @Jake-Shadle
• [x] 📦 krates - @Jake-Shadle
• [x] 🎳 physx
• [x] 🐏 rpmalloc-rs
• [x] 🆔 spdx - @Jake-Shadle
• [x] 🔆 superluminal-perf
• [x] 📂 tame-gcs - @Jake-Shadle
• [x] 🔐 tame-oauth - @Jake-Shadle
• [x] 🎨 texture-synthesis
• [x] rust-ecosystem (this repo)
• [x] opensource-template cc @arirawr

For our project we find using LLD instead 2x faster for full builds, and 5x faster for incremental builds, compared to the default MSVC linker. This is huge and essential for us, so we enable it manually in .cargo/config:

[target.x86_64-pc-windows-msvc]
linker = "lld-link.exe"

It would be nice if this was the default instead on Windows, esp. if it also made it possible to not have to install the Visual Studio Build Tools?

When using LLD Windows we do get this warning, but haven't been an issue for us:

warning: not embedding natvis: lld-link may not support the flag

Related umbrella issue in rust-gamedev WG: https://github.com/rust-gamedev/wg/issues/50

Tracking issue:

• [ ] rust: Linking with LLD #39915
• [ ] rust: Use lld by default on x64 msvc windows #71520

Wish list of rust analyzer features & bug fixes we would like to see added/improved/fixed based on the Rust projects we've been developing at Embark.

This is not a complete list, but an attempt to keep a bit of structure for our own sake, and may be of interest for rustfmt developers as well.

Should have

Features or fixes that would directly improve or help our workflows

• [x] https://github.com/rust-lang/rust-analyzer/issues/6777
• [x] https://github.com/rust-lang/rust-analyzer/issues/6556
• [x] https://github.com/rust-lang/rust-analyzer/issues/12936
• [x] https://github.com/rust-lang/rust-analyzer/issues/13125
• [x] https://github.com/rust-lang/rust-analyzer/issues/11654
• [x] https://github.com/rust-lang/rust-analyzer/issues/6895
• [ ] https://github.com/rust-lang/rust-analyzer/issues/11260
• [ ] https://github.com/rust-lang/rust-analyzer/issues/11296

Nice to have

Features or fixes that would be nice to have, but are lower priority to us than the above list.

• [x] https://github.com/rust-lang/rust-analyzer/issues/13254
• [ ] https://github.com/rust-lang/rust-analyzer/issues/12499
• [ ] https://github.com/rust-lang/rust-analyzer/issues/11352
• [ ] https://github.com/rust-lang/rust-analyzer/issues/11181
• [ ] https://github.com/rust-lang/rust-analyzer/issues/11137
• [x] https://github.com/rust-lang/rust-analyzer/issues/9673
• [x] https://github.com/rust-lang/rust-analyzer/issues/9544
• [ ] https://github.com/rust-lang/rust-analyzer/issues/9131
• [ ] https://github.com/rust-lang/rust-analyzer/issues/8314
• [ ] https://github.com/rust-lang/rust-analyzer/issues/3767

Not filed / found

Fixes or enhancements that believe there is no issue for yet, if you find one please do comment and we'll update it here.

• Generally slow performance on our large repositories, unclear how to investigate to share for focused improvements

Related tracking issues for other Rust components:

• #84
• #83
• #79

Wish list of rust compiler & std features & bug fixes we would like to see added/improved/fixed based on the Rust projects we've been developing at Embark.

This is not a complete list, but an attempt to keep a bit of structure for our own sake, and may be of interest for rust compiler developers as well.

Should have

rust compiler features or fixes that would directly improve or help our workflows

• [ ] #41
• [ ] https://github.com/rust-lang/rust/issues/74015
• [ ] https://github.com/rust-lang/rust/issues/68007
• [ ] https://github.com/rust-lang/rust/issues/76578
• [ ] https://github.com/rust-lang/rust/issues/101337
• [ ] https://github.com/rust-lang/rust/issues/101210
• [ ] https://github.com/rust-lang/cargo/issues/10016
• [ ] https://github.com/rust-lang/cargo/issues/7850
• [ ] https://github.com/rust-lang/cargo/issues/7169
• [ ] https://github.com/rust-lang/cargo/issues/9096
• [ ] https://github.com/rust-lang/cargo/issues/10030
• [ ] https://github.com/rust-lang/rustup/issues/988
• [ ] https://github.com/rust-lang/rust/issues/71520

Nice to have

rust compiler features or fixes that would be nice to have, but are lower priority to us than the above list.

• [ ] https://github.com/rust-lang/rust/issues/85444
• [ ] https://github.com/rust-lang/rust/issues/54196
• [ ] https://github.com/rust-lang/cargo/issues/5042
• [ ] https://github.com/rust-lang/cargo/issues/10857
• [ ] https://github.com/rust-lang/rfcs/issues/2790
• [ ] https://github.com/rust-lang/cargo/issues/9455
• [ ] https://github.com/rust-lang/cargo/issues/10711
• [ ] https://github.com/rust-lang/cargo/issues/9506
• [ ] https://github.com/rust-lang/cargo/issues/9058
• [ ] https://github.com/rust-lang/cargo/issues/8850
• [ ] https://github.com/rust-lang/rust/issues/83310

Not filed / found

Fixes or enhancements that believe there is no issue for yet, if you find one please do comment and we'll update it here.

Related tracking issues for other Rust components:

• #79
• #83
• #86

Wish list of rustfmt features & bug fixes we would like to see added/improved/fixed based on the Rust projects we've been developing at Embark.

This is not a complete list, but an attempt to keep a bit of structure for our own sake, and may be of interest for rustfmt developers as well.

Should have

rustfmt features or fixes that would directly improve or help our workflows

• [ ] https://github.com/rust-lang/rustfmt/issues/4991
• [ ] https://github.com/rust-lang/rustfmt/issues/3347
• [ ] https://github.com/rust-lang/rustfmt/issues/3863
• [ ] https://github.com/rust-lang/rustfmt/issues/4914

Nice to have

rustfmt features or fixes that would be nice to have, but are lower priority to us than the above list.

• [ ] https://github.com/rust-lang/rustfmt/issues/3353
• [ ] https://github.com/rust-lang/rustfmt/issues/3348
• [ ] https://github.com/rust-lang/rustfmt/issues/3350
• [ ] https://github.com/rust-lang/rustfmt/issues/5257
• [ ] https://github.com/rust-lang/rustfmt/issues/3363
• [ ] https://github.com/rust-lang/rustfmt/issues/5323

Not filed / found

Fixes or enhancements that believe there is no issue for yet, if you find one please do comment and we'll update it here.

• Could rustfmt automatically reformat literals in compliance with clippy::unreadable_literal? Reopen this or file new? Format long numeric literals · Issue #3594 · rust-lang/rustfmt · GitHub

Related tracking issues for other Rust components:

• #84
• #79
• #86

Wish list of Clippy lints and features & bug fixes we would like to see added/improved/fixed based on the Rust projects we've been developing at Embark.

This is not a complete list, but an attempt to keep a bit of structure for our own sake, and may be of interest for Clippy lint developers as well.

Should have

Lints & Clippy features or fixes that would directly improve or help our workflows

• [ ] https://github.com/rust-lang/rust-clippy/issues/6625 (also see #22)
• [ ] https://github.com/rust-lang/rust-clippy/issues/8679 (Embark filed)
• [ ] https://github.com/rust-lang/rust-clippy/issues/8567 (Embark filed)
• [x] https://github.com/rust-lang/rust-clippy/issues/7005 (Embark filed)
• [x] https://github.com/rust-lang/rust-clippy/issues/3689 (cast_lossless)
• [x] https://github.com/rust-lang/rust-clippy/issues/8505
• [ ] https://github.com/rust-lang/rust-clippy/issues/8349
• [ ] https://github.com/rust-lang/rust-clippy/issues/8246
• [x] https://github.com/rust-lang/rust-clippy/issues/7954
• [ ] https://github.com/rust-lang/rust-clippy/issues/7828
• [ ] https://github.com/rust-lang/rust-clippy/issues/7763
• [ ] https://github.com/rust-lang/rust-clippy/issues/3045
• [ ] https://github.com/rust-lang/rust-clippy/issues/8092
• [x] https://github.com/rust-lang/rust-clippy/issues/6902
• [x] https://github.com/rust-lang/rust-clippy/issues/6880
• [ ] https://github.com/rust-lang/rust-clippy/issues/5736 (by @jhpratt)
• [ ] https://github.com/rust-lang/rust-clippy/issues/3122
• [ ] https://github.com/rust-lang/rust-clippy/issues/9320
• [ ] https://github.com/rust-lang/rust-clippy/issues/9063

Nice to have

Lints & Clippy features or fixes that would be nice to have, but are lower priority to us than the above list.

• [x] https://github.com/rust-lang/rust-clippy/issues/8399
• [x] https://github.com/rust-lang/rust-clippy/issues/8603
• [x] https://github.com/rust-lang/rust-clippy/issues/8368
• [ ] https://github.com/rust-lang/rust-clippy/issues/8088
• [ ] https://github.com/rust-lang/rust-clippy/issues/8022
• [x] https://github.com/rust-lang/rust-clippy/issues/7959
• [ ] https://github.com/rust-lang/rust-clippy/issues/7958
• [ ] https://github.com/rust-lang/rust-clippy/issues/8439
• [x] https://github.com/rust-lang/rust-clippy/issues/7714
• [x] https://github.com/rust-lang/rust-clippy/issues/7381
• [ ] https://github.com/rust-lang/rust-clippy/issues/7500
• [ ] https://github.com/rust-lang/rust-clippy/issues/7459
• [ ] https://github.com/rust-lang/rust-clippy/issues/7328
• [ ] https://github.com/rust-lang/rust-clippy/issues/5974
• [ ] https://github.com/rust-lang/rust-clippy/issues/5794
• [ ] https://github.com/rust-lang/rust-clippy/issues/8522 (by @jhpratt)
• [ ] https://github.com/rust-lang/rust-clippy/issues/8863

Not filed / found

Fixes or enhancements that believe there is no issue for yet, if you find one please do comment and we'll update it here.

• [ ] cargo clippy --fix often doesn't work and it is unclear to the user which lints it works for and which it skips
• [ ] A lint to require types in crate to be used fully qualified, like log::info!("hej") instead of use log::info; info!("hej") by specifying log crate in clippy.toml section for the lint.

Related tracking issues for other Rust components:

• #84
• #83
• #86

Planning to do a larger v7 update to our standard lints (#59) with a bunch of additions and requirement of Rust 1.60

Lint candidates

These are the candidates currently thinking of and are using in our internal main repo but will experiment with all of these in our other private and open repos and see if there are any issues with them there.

Planning to add

Quite trivial but nice ones:

• clippy::disallowed_script_idents
• clippy::equatable_if_let
• clippy::fn_to_numeric_cast_any
• clippy::index_refutable_slice
• clippy::iter_not_returning_iterator
• clippy::negative_feature_names
• clippy::nonstandard_macro_braces
• clippy::trailing_empty_array
• clippy::unnecessary_wraps
• trivial_numeric_casts

Planning to remove

These are now warn-by-default:

• clippy::disallowed_methods
• clippy::disallowed_types

Considering

More opinionated for our engine repository workflow:

• clippy::mod_module_files - the style we use and prefer
• clippy::undocumented_unsafe_blocks - this one we do have a lot of allows with for FFI crates and such, but it is a quite nice default still that we just recently activated
• unsafe_op_in_unsafe_fn - see this tracking issue and deeper discussion in RFC about the direction of getting this to be default in the language as unsafe fn and unsafe blocks are quite different things. https://github.com/rust-lang/rust/issues/71668
• rustdoc::missing_crate_level_docs - this may not be that important for many repos, but we've found it a nice quality to have for both internal and open source crates - and can just be a single-line when you add the crate.

Background

This issue tracks which Rust and Clippy lints we currently use as default for all of our internal and external crates. We have a common base of lints that we configure the same everywhere that is more opinionated than the Rust and Clippy defaults and geared towards how we prefer to (currently) write & maintain Rust code.

Individual crates are able to, and often do override some of these defaults, either on a crate level, or for specific modules or functions. But we still find it helpful to have a standard across everything to keep the code high quality and generally working the same way.

Lint configuration

We currently have two ways to enable our configured lints:

1. Through copy'n'pasting https://github.com/EmbarkStudios/rust-ecosystem/blob/main/lints.rs into every crate
2. Through a .cargo/config.toml section: https://github.com/EmbarkStudios/rust-ecosystem/blob/main/lints.toml

Traditionally we've used option 1, but are testing and converting more to option 2 as it is much easier to just have a single configuration file with everything.

We hope to later in 2021 have proper first-class support for a lints.toml or [lints] section in Cargo and switch to that, but progress for it has been slow. Tracked in:

• #22
• https://github.com/rust-lang/cargo/issues/5034
• https://github.com/rust-lang/rust-clippy/issues/6625.

Lints of interests

Testing / being discussed

• Add: clippy::needless_pass_by_value - helped us catch a lot of unnecessary : Vec<T> arguments and replace them with : &[T], saving a lot of calls to .clone() (very common trap for rust beginners to fall into). quite intrusive change for lots of crates though so safer to do separtely
• Add: rustdoc::missing_crate_level_docs

Blocked

List of Clippy lints we are interested in using, but that have issues or are not mature / specific enough yet for inclusion.

• clippy::use_self - blocked on #53
• clippy::unnecessary_wraps
• clippy::option_if_let_else - blocked on:
  • https://github.com/rust-lang/rust-clippy/issues/5821
  • https://github.com/rust-lang/rust-clippy/issues/5822
  • https://github.com/rust-lang/rust-clippy/issues/6737
  • https://github.com/rust-lang/rust-clippy/issues/6137
• clippy::wildcard_enum_match_arm - blocked on https://github.com/rust-lang/rust-clippy/issues/6862
• clippy::print_stdout / clippy::print_stderr - ineffective due to https://github.com/rust-lang/rust-clippy/issues/5721