Merge pull request #309 from dhasenan/master
Added a test for static if...else if mistakes.
This commit is contained in:
commit
c602036e0f
|
@ -113,4 +113,7 @@ struct StaticAnalysisConfig
|
||||||
|
|
||||||
@INI("Check for uses of the old-style alias syntax")
|
@INI("Check for uses of the old-style alias syntax")
|
||||||
bool alias_syntax_check;
|
bool alias_syntax_check;
|
||||||
|
|
||||||
|
@INI("Checks for else if that should be else static if")
|
||||||
|
bool static_if_else_check;
|
||||||
}
|
}
|
||||||
|
|
|
@ -57,6 +57,7 @@ import analysis.auto_ref_assignment;
|
||||||
import analysis.incorrect_infinite_range;
|
import analysis.incorrect_infinite_range;
|
||||||
import analysis.useless_assert;
|
import analysis.useless_assert;
|
||||||
import analysis.alias_syntax_check;
|
import analysis.alias_syntax_check;
|
||||||
|
import analysis.static_if_else;
|
||||||
|
|
||||||
import dsymbol.string_interning : internString;
|
import dsymbol.string_interning : internString;
|
||||||
import dsymbol.scope_;
|
import dsymbol.scope_;
|
||||||
|
@ -273,6 +274,8 @@ MessageSet analyze(string fileName, const Module m, const StaticAnalysisConfig a
|
||||||
checks ~= new UselessAssertCheck(fileName);
|
checks ~= new UselessAssertCheck(fileName);
|
||||||
if (analysisConfig.alias_syntax_check)
|
if (analysisConfig.alias_syntax_check)
|
||||||
checks ~= new AliasSyntaxCheck(fileName);
|
checks ~= new AliasSyntaxCheck(fileName);
|
||||||
|
if (analysisConfig.static_if_else_check)
|
||||||
|
checks ~= new StaticIfElse(fileName);
|
||||||
version (none)
|
version (none)
|
||||||
if (analysisConfig.redundant_if_check)
|
if (analysisConfig.redundant_if_check)
|
||||||
checks ~= new IfStatementCheck(fileName, moduleScope);
|
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