add a check for the auto functions without return statement
This commit is contained in:
parent
32f45847fd
commit
734d47e9a2
|
@ -116,6 +116,7 @@ Note that the "--skipTests" option is the equivalent of changing each
|
||||||
* Lines longer than 120 characters.
|
* Lines longer than 120 characters.
|
||||||
* Incorrect infinite range definitions.
|
* Incorrect infinite range definitions.
|
||||||
* Some assertions that check conditions that will always be true.
|
* Some assertions that check conditions that will always be true.
|
||||||
|
* Auto functions without return statement. The compiler doesn't see an omission and it infers 'void' as return type.
|
||||||
|
|
||||||
#### Wishlist
|
#### Wishlist
|
||||||
|
|
||||||
|
|
|
@ -0,0 +1,87 @@
|
||||||
|
// Copyright Basile Burg 2016.
|
||||||
|
// Distributed under the Boost Software License, Version 1.0.
|
||||||
|
// (See accompanying file LICENSE_1_0.txt or copy at
|
||||||
|
// http://www.boost.org/LICENSE_1_0.txt)
|
||||||
|
|
||||||
|
module analysis.auto_function;
|
||||||
|
|
||||||
|
import analysis.base;
|
||||||
|
import analysis.helpers;
|
||||||
|
import dparse.ast;
|
||||||
|
import dparse.lexer;
|
||||||
|
|
||||||
|
import std.stdio;
|
||||||
|
import std.algorithm.searching : any;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Checks for auto functions without return statement.
|
||||||
|
*
|
||||||
|
* Auto function without return statement can be an omission and are not
|
||||||
|
* detected by the compiler. However sometimes they can be used as a trick
|
||||||
|
* to infer attributes.
|
||||||
|
*/
|
||||||
|
final class AutoFunctionChecker : BaseAnalyzer
|
||||||
|
{
|
||||||
|
|
||||||
|
private:
|
||||||
|
|
||||||
|
enum string KEY = "dscanner.suspicious.missing_return";
|
||||||
|
enum string MESSAGE = "Auto function without return statement, prefer an explicit void";
|
||||||
|
|
||||||
|
bool[] _returns;
|
||||||
|
|
||||||
|
public:
|
||||||
|
|
||||||
|
alias visit = BaseAnalyzer.visit;
|
||||||
|
|
||||||
|
///
|
||||||
|
this(string fileName, bool skipTests = false)
|
||||||
|
{
|
||||||
|
super(fileName, null, skipTests);
|
||||||
|
}
|
||||||
|
|
||||||
|
override void visit(const FunctionDeclaration decl)
|
||||||
|
{
|
||||||
|
_returns.length += 1;
|
||||||
|
scope(exit) _returns.length -= 1;
|
||||||
|
_returns[$-1] = false;
|
||||||
|
|
||||||
|
const bool autoFun = decl.storageClasses
|
||||||
|
.any!(a => a.token.type == tok!"auto");
|
||||||
|
|
||||||
|
decl.accept(this);
|
||||||
|
|
||||||
|
if (autoFun && !_returns[$-1])
|
||||||
|
addErrorMessage(decl.name.line, decl.name.column, KEY, MESSAGE);
|
||||||
|
}
|
||||||
|
|
||||||
|
override void visit(const ReturnStatement rst)
|
||||||
|
{
|
||||||
|
if (_returns.length)
|
||||||
|
_returns[$-1] = true;
|
||||||
|
rst.accept(this);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
unittest
|
||||||
|
{
|
||||||
|
import std.stdio : stderr;
|
||||||
|
import std.format : format;
|
||||||
|
import analysis.config : StaticAnalysisConfig, Check;
|
||||||
|
import analysis.helpers : assertAnalyzerWarnings;
|
||||||
|
|
||||||
|
StaticAnalysisConfig sac;
|
||||||
|
sac.auto_function_check = Check.enabled;
|
||||||
|
assertAnalyzerWarnings(q{
|
||||||
|
auto ref doStuff(){} // [warn]: %s
|
||||||
|
auto doStuff(){} // [warn]: %s
|
||||||
|
int doStuff(){auto doStuff(){}} // [warn]: %s
|
||||||
|
auto doStuff(){return 0;}
|
||||||
|
int doStuff(){/*error but not the aim*/}
|
||||||
|
}c.format(
|
||||||
|
AutoFunctionChecker.MESSAGE,
|
||||||
|
AutoFunctionChecker.MESSAGE,
|
||||||
|
AutoFunctionChecker.MESSAGE,
|
||||||
|
), sac);
|
||||||
|
stderr.writeln("Unittest for AutoFunctionChecker passed.");
|
||||||
|
}
|
|
@ -140,8 +140,11 @@ struct StaticAnalysisConfig
|
||||||
string alias_syntax_check = Check.disabled;
|
string alias_syntax_check = Check.disabled;
|
||||||
|
|
||||||
@INI("Checks for else if that should be else static if")
|
@INI("Checks for else if that should be else static if")
|
||||||
string static_if_else_check = Check.disabled;
|
string static_if_else_check = Check.disabled;
|
||||||
|
|
||||||
@INI("Check for unclear lambda syntax")
|
@INI("Check for unclear lambda syntax")
|
||||||
string lambda_return_check = Check.disabled;
|
string lambda_return_check = Check.disabled;
|
||||||
|
|
||||||
|
@INI("Check for auto function without return statement")
|
||||||
|
string auto_function_check = Check.disabled;
|
||||||
}
|
}
|
||||||
|
|
|
@ -59,6 +59,7 @@ import analysis.useless_assert;
|
||||||
import analysis.alias_syntax_check;
|
import analysis.alias_syntax_check;
|
||||||
import analysis.static_if_else;
|
import analysis.static_if_else;
|
||||||
import analysis.lambda_return_check;
|
import analysis.lambda_return_check;
|
||||||
|
import analysis.auto_function;
|
||||||
|
|
||||||
import dsymbol.string_interning : internString;
|
import dsymbol.string_interning : internString;
|
||||||
import dsymbol.scope_;
|
import dsymbol.scope_;
|
||||||
|
@ -354,6 +355,10 @@ MessageSet analyze(string fileName, const Module m, const StaticAnalysisConfig a
|
||||||
checks ~= new LambdaReturnCheck(fileName,
|
checks ~= new LambdaReturnCheck(fileName,
|
||||||
analysisConfig.lambda_return_check == Check.skipTests && !ut);
|
analysisConfig.lambda_return_check == Check.skipTests && !ut);
|
||||||
|
|
||||||
|
if (analysisConfig.auto_function_check != Check.disabled)
|
||||||
|
checks ~= new AutoFunctionChecker(fileName,
|
||||||
|
analysisConfig.auto_function_check == Check.skipTests && !ut);
|
||||||
|
|
||||||
version (none)
|
version (none)
|
||||||
if (analysisConfig.redundant_if_check != Check.disabled)
|
if (analysisConfig.redundant_if_check != Check.disabled)
|
||||||
checks ~= new IfStatementCheck(fileName, moduleScope,
|
checks ~= new IfStatementCheck(fileName, moduleScope,
|
||||||
|
|
Loading…
Reference in New Issue