Added a test for static if...else if mistakes.
If you write your code as: static if (foo) doFoo(); else if (bar) doBar(); This check will catch it. If you write it as: static if (foo) doFoo(); else { if (bar) doBar(); } This isn't a bug and we won't warn about it. If you write your code in an intermediate style, we'll still warn about it: static if (foo) doFoo(); else if (bar) doBar(); Which probably isn't optimal, but what the hey.
This commit is contained in:
parent
f7868cae73
commit
0943820ebe
|
@ -113,4 +113,7 @@ struct StaticAnalysisConfig
|
|||
|
||||
@INI("Check for uses of the old-style alias syntax")
|
||||
bool alias_syntax_check;
|
||||
|
||||
@INI("Checks for else if that should be else static if")
|
||||
bool static_if_else_check;
|
||||
}
|
||||
|
|
|
@ -56,6 +56,7 @@ import analysis.auto_ref_assignment;
|
|||
import analysis.incorrect_infinite_range;
|
||||
import analysis.useless_assert;
|
||||
import analysis.alias_syntax_check;
|
||||
import analysis.static_if_else;
|
||||
|
||||
import dsymbol.string_interning : internString;
|
||||
import dsymbol.scope_;
|
||||
|
@ -269,6 +270,8 @@ MessageSet analyze(string fileName, const Module m, const StaticAnalysisConfig a
|
|||
checks ~= new UselessAssertCheck(fileName);
|
||||
if (analysisConfig.alias_syntax_check)
|
||||
checks ~= new AliasSyntaxCheck(fileName);
|
||||
if (analysisConfig.static_if_else_check)
|
||||
checks ~= new StaticIfElse(fileName);
|
||||
version (none)
|
||||
if (analysisConfig.redundant_if_check)
|
||||
checks ~= new IfStatementCheck(fileName, moduleScope);
|
||||
|
|
|
@ -0,0 +1,95 @@
|
|||
// Copyright Chris Wright (dhasenan) 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.static_if_else;
|
||||
|
||||
import dparse.ast;
|
||||
import dparse.lexer;
|
||||
import analysis.base;
|
||||
|
||||
/**
|
||||
* Checks for potentially mistaken static if / else if.
|
||||
*
|
||||
* It's potentially valid to write:
|
||||
* ---
|
||||
* static if (foo) {
|
||||
* } else if (bar) {
|
||||
* }
|
||||
* ---
|
||||
*
|
||||
* However, it's more likely that this is a mistake.
|
||||
*/
|
||||
class StaticIfElse : BaseAnalyzer
|
||||
{
|
||||
alias visit = BaseAnalyzer.visit;
|
||||
|
||||
this(string filename)
|
||||
{
|
||||
super(filename, null);
|
||||
}
|
||||
|
||||
override void visit(const ConditionalStatement cc)
|
||||
{
|
||||
cc.accept(this);
|
||||
if (cc.falseStatement is null)
|
||||
{
|
||||
return;
|
||||
}
|
||||
const(IfStatement) ifStmt = getIfStatement(cc);
|
||||
if (!ifStmt)
|
||||
{
|
||||
return;
|
||||
}
|
||||
addErrorMessage(ifStmt.line, ifStmt.column, KEY, "Mismatched static if. Use 'else static if' here.");
|
||||
}
|
||||
|
||||
const(IfStatement) getIfStatement(const ConditionalStatement cc)
|
||||
{
|
||||
if (cc.falseStatement.statement)
|
||||
{
|
||||
if (cc.falseStatement.statement.statementNoCaseNoDefault)
|
||||
{
|
||||
if (cc.falseStatement.statement.statementNoCaseNoDefault.ifStatement)
|
||||
{
|
||||
return cc.falseStatement.statement.statementNoCaseNoDefault.ifStatement;
|
||||
}
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
enum KEY = "dscanner.suspicious.static_if_else";
|
||||
}
|
||||
|
||||
unittest
|
||||
{
|
||||
import analysis.helpers : assertAnalyzerWarnings;
|
||||
import analysis.config : StaticAnalysisConfig;
|
||||
import std.stdio : stderr;
|
||||
|
||||
StaticAnalysisConfig sac;
|
||||
sac.static_if_else_check = true;
|
||||
assertAnalyzerWarnings(q{
|
||||
void foo() {
|
||||
static if (false)
|
||||
auto a = 0;
|
||||
else if (true) // [warn]: Mismatched static if. Use 'else static if' here.
|
||||
auto b = 1;
|
||||
}
|
||||
}c, sac);
|
||||
// Explicit braces, so no warning.
|
||||
assertAnalyzerWarnings(q{
|
||||
void foo() {
|
||||
static if (false)
|
||||
auto a = 0;
|
||||
else {
|
||||
if (true)
|
||||
auto b = 1;
|
||||
}
|
||||
}
|
||||
}c, sac);
|
||||
|
||||
stderr.writeln("Unittest for StaticIfElse passed.");
|
||||
}
|
Loading…
Reference in New Issue