Use static analysis tools to detect obvious errors

[shaomeng]

Background

In my recent PR reviews I discovered repeated occurrence of errors (see examples below) that can be easily detected by a static analyzer.

Impact

For a reviewer, seeing these errors and pointing out them distract the PR reviewer from focusing on more important aspects of PR review, namely the structure and logic of the code. Ultimately, it's not the best use of developer time to pick out errors that automated tools can easily point out.

For a programmer, errors pointed out by automated tools help saving debug time and improving overall code quality.

Proposed enhancement

Deploy static analysis tools, such as clang-tidy, in a similar fashion of clang-format.

Examples

These are just a few examples that I encountered in the past few days reviewing one single PR. I suspect there are more errors of similar natures that a static analysis tool can spot.

1. Resource Leak:
   There are multiple types of resource leaks and here's just one example:

FILE* f = fopen( name, "r" );
if( f != nullptr ) {
    // do something
    if( error_occured )
        return -1;   // ERROR: FILE LEFT OPEN!
}

2. Type mismatch between function signature and its body:
   This type of errors will result in bugs that are very hard to detect. Here's just one example:

size_t func() { // Note: the signature says return type being size_t
    if( condition_1 ) 
        return 4;
    else if( condition_2)    
        return 8;
    else                             
        return -1;  // ERROR: NOBODY KNOWS WHAT'S GONNA BE RETURNED!
}

3. Type cast resulted wrong arguments:
   Example: when the programmer intends to create a vector with 4 values, all of them being false. The programmer types:

std::vector<bool> periodic (false, 4);  // A vector of size 0 is created, which isn't the programmer's intent. 

This statement will pass the compiler because type casts are allowed: false is cast to 0 and 4 is cast to true. A static analyzer with appropriate configurations might have a chance to spot it.

4. Unnecessary tests:
   A function can possibly return one value only, but code written to test other return values which are impossible.

int func1() {
    // do things
    return 0; // Note: this is the only return statement in this function
}

void func2() {
    int ret = func1();   // ret CANNOT POSSIBLY HAVE VALUES OTHER THAN 0
    if( ret < 0 ) {      /* handle error */ };
    else if( ret > 0 ) { /* handle error */ };
    else {
        // proceed as normal
    }
}