This PR fixes many problems detected by clang-tidy in the codebase, and integrates clang-tidy into the CI. What this means is that future PRs will now cause the CI to fail if clang-tidy finds problems with their code. This will be used to enforce, amongst others, the style of the codebase (i.e. it will not be a CI error to create a variable called isSelected, rather than is_selected) and avoid common performance pitfalls (i.e. it will be a CI error to write a function that takes as input a vector<int> but only reads from it, rather than a const vector<int> &).

Code Changes

Some style changes as a result of this PR are that functions should always be written in CamelCase, even if they are non-member functions, i.e.:

// correct!
static void MyStaticFunction() {
}
// error!
static void my_static_function() {
}

Parameter names (and variable names in general) can no longer have trailing underscores.

// correct!
void MyClass(string parameter_p, ...) : parameter(move(parameter_p)) {
}
// error!
void MyClass(string parameter_, ...) : parameter(move(parameter_)) {
}

When non-trivial objects are used in constructs (e.g. strings, vectors, unordered_maps, LogicalType) they need to be passed as const & if not modified:

// correct!
void MyFunction(const string &parameter) {
   ...
}
// error!
void MyFunction(string parameter) {
   ...
}

When non-trivial objects are used in constructs (e.g. strings, vectors, unordered_maps, LogicalType, shared_ptr) they need to be passed as const & if not modified:

// correct!
void MyFunction(const string &parameter) {
   ...
}
// error!
void MyFunction(string parameter) {
   ...
}

When non-trivial objects are copied only once, they should be moved instead, i.e.:

// correct!
void MyFunction(string parameter) {
   auto object = make_unique<MyObject>(move(parameter));
  ...
}
// error!
void MyFunction(string parameter) {
   auto object = make_unique<MyObject>(parameter);
  ...
}

.empty() needs to be used instead of .size() == 0:

// correct!
if (list.empty()) {
   ...
}
// error!
if (list.size() == 0) {
   ...
}

using namespace X is now forbidden:

// correct!
namespace duckdb {
...
}
// error!
using namespace duckdb;

Running clang-tidy

Clang-tidy has been integrated into the makefile, and can be run using the command make tidy-check, or make tidy-check-parallel (with optional thread count TIDY_THREADS=16 make tidy-check-parallel). This uses the cmake file to get a list of sources to compile (with the flag -DCLANG_TIDY). Currently only the src directory and the parquet extension are checked using clang-tidy.

Fixing errors

In general it is preferable to fix errors manually. There is also a make tidy-fix command that instructs clang-tidy to automatically fix errors, but be careful when using this, as it will break the code in certain cases (specifically when changing names, as it does not check for potential name conflicts/collisions).

Disabling clang-tidy for specific lines

If required, clang-tidy can be disabled for individual lines by appending a // NOLINT comment, i.e.:

void MyType(int x); // NOLINT: allow implicit conversion from int -> MyType

In general you would need a good reason for this, though.