pingcap / style-guide

Style guide for PingCAP and TiKV code
Apache License 2.0
80 stars 7 forks source link

Provide tooling #16

Open nrc opened 3 years ago

nrc commented 3 years ago

Many of the recommendations could be enforced with tools. This could be as simple as flags for rustfmt and clippy, or perhaps custom lints or other tooling

nrc commented 3 years ago

I did some research on enforcing the style guide mechanically, and I am a bit disappointed with what we can expect Clippy or Rustfmt to do. Some things are already being enforced using the default settings. Beyond that, the following settings may be useful:

Clippy

The following for checking that we don't commit TODO macros of various kinds:

dbg_macro
todo
unimplemented

I believe the following could be useful as warnings, but to deny them is to strict (and since we deny warnings, that is not an easy option:

enum_glob_use
fn_params_excessive_bools
if_not_else
macro_use_imports
unused_self
useless_let_if_seq

Currently we allow the following, but I think we should warn (if we had that option):

should_implement_trait
new_without_default
blacklisted_name
needless_range_loop
unnecessary_wraps

Rustfmt

I would like to use blank_lines_lower_bound, but only for multi-line items, which is not an option but would be fairly easy to implement.

We could use error_on_line_overflow, though a warning might be better than an error.

The following might be useful:

condense_wildcard_suffixes = true
imports_granularity = "Crate"
license_template_path = "..."
newline_style = "Unix"
report_todo = "Always"
use_field_init_shorthand = true
use_try_shorthand = true

imports_granularity would be a big change and would need buy-in and some special care to land the change. report_todo, similarly, we have a lot of TODOs in our code, so would be a big (but IMO, useful change). use_field_init_shorthand and use_try_shorthand could be used without change (I assume a bulk change has been done in the past).

nrc commented 3 years ago

cc https://github.com/tikv/tikv/pull/9767