diff --git a/README.md b/README.md index 551f61b..c1d681e 100644 --- a/README.md +++ b/README.md @@ -116,6 +116,7 @@ Note that the "--skipTests" option is the equivalent of changing each * Lines longer than 120 characters. * Incorrect infinite range definitions. * 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 diff --git a/src/analysis/auto_function.d b/src/analysis/auto_function.d new file mode 100644 index 0000000..98489f4 --- /dev/null +++ b/src/analysis/auto_function.d @@ -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."); +} diff --git a/src/analysis/config.d b/src/analysis/config.d index bd178b0..3df4124 100644 --- a/src/analysis/config.d +++ b/src/analysis/config.d @@ -140,8 +140,11 @@ struct StaticAnalysisConfig string alias_syntax_check = Check.disabled; @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") string lambda_return_check = Check.disabled; + + @INI("Check for auto function without return statement") + string auto_function_check = Check.disabled; } diff --git a/src/analysis/run.d b/src/analysis/run.d index 9ad0624..8bf9267 100644 --- a/src/analysis/run.d +++ b/src/analysis/run.d @@ -59,6 +59,7 @@ import analysis.useless_assert; import analysis.alias_syntax_check; import analysis.static_if_else; import analysis.lambda_return_check; +import analysis.auto_function; import dsymbol.string_interning : internString; import dsymbol.scope_; @@ -354,6 +355,10 @@ MessageSet analyze(string fileName, const Module m, const StaticAnalysisConfig a checks ~= new LambdaReturnCheck(fileName, 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) if (analysisConfig.redundant_if_check != Check.disabled) checks ~= new IfStatementCheck(fileName, moduleScope,