mirror of
https://github.com/dlang-community/D-Scanner.git
synced 2025-04-27 13:50:02 +03:00
replace libdparse in statif if else visitor (#56)
This commit is contained in:
parent
235e9d0884
commit
60fd082eb1
2 changed files with 35 additions and 109 deletions
|
@ -910,10 +910,6 @@ private BaseAnalyzer[] getAnalyzersForModuleAndConfig(string fileName,
|
||||||
checks ~= new UselessAssertCheck(args.setSkipTests(
|
checks ~= new UselessAssertCheck(args.setSkipTests(
|
||||||
analysisConfig.useless_assert_check == Check.skipTests && !ut));
|
analysisConfig.useless_assert_check == Check.skipTests && !ut));
|
||||||
|
|
||||||
if (moduleName.shouldRun!StaticIfElse(analysisConfig))
|
|
||||||
checks ~= new StaticIfElse(args.setSkipTests(
|
|
||||||
analysisConfig.static_if_else_check == Check.skipTests && !ut));
|
|
||||||
|
|
||||||
if (moduleName.shouldRun!LambdaReturnCheck(analysisConfig))
|
if (moduleName.shouldRun!LambdaReturnCheck(analysisConfig))
|
||||||
checks ~= new LambdaReturnCheck(args.setSkipTests(
|
checks ~= new LambdaReturnCheck(args.setSkipTests(
|
||||||
analysisConfig.lambda_return_check == Check.skipTests && !ut));
|
analysisConfig.lambda_return_check == Check.skipTests && !ut));
|
||||||
|
@ -1345,6 +1341,12 @@ MessageSet analyzeDmd(string fileName, ASTCodegen.Module m, const char[] moduleN
|
||||||
config.redundant_parens_check == Check.skipTests && !ut
|
config.redundant_parens_check == Check.skipTests && !ut
|
||||||
);
|
);
|
||||||
|
|
||||||
|
if (moduleName.shouldRunDmd!(StaticIfElse!ASTCodegen)(config))
|
||||||
|
visitors ~= new StaticIfElse!ASTCodegen(
|
||||||
|
fileName,
|
||||||
|
config.static_if_else_check == Check.skipTests && !ut
|
||||||
|
);
|
||||||
|
|
||||||
foreach (visitor; visitors)
|
foreach (visitor; visitors)
|
||||||
{
|
{
|
||||||
m.accept(visitor);
|
m.accept(visitor);
|
||||||
|
|
|
@ -5,11 +5,10 @@
|
||||||
|
|
||||||
module dscanner.analysis.static_if_else;
|
module dscanner.analysis.static_if_else;
|
||||||
|
|
||||||
import dparse.ast;
|
|
||||||
import dparse.lexer;
|
|
||||||
import dscanner.analysis.base;
|
import dscanner.analysis.base;
|
||||||
import dscanner.utils : safeAccess;
|
import std.stdio;
|
||||||
|
|
||||||
|
// TODO: check and fix AutoFix
|
||||||
/**
|
/**
|
||||||
* Checks for potentially mistaken static if / else if.
|
* Checks for potentially mistaken static if / else if.
|
||||||
*
|
*
|
||||||
|
@ -22,78 +21,50 @@ import dscanner.utils : safeAccess;
|
||||||
*
|
*
|
||||||
* However, it's more likely that this is a mistake.
|
* However, it's more likely that this is a mistake.
|
||||||
*/
|
*/
|
||||||
final class StaticIfElse : BaseAnalyzer
|
extern(C++) class StaticIfElse(AST) : BaseAnalyzerDmd
|
||||||
{
|
{
|
||||||
alias visit = BaseAnalyzer.visit;
|
alias visit = BaseAnalyzerDmd.visit;
|
||||||
|
|
||||||
mixin AnalyzerInfo!"static_if_else_check";
|
mixin AnalyzerInfo!"static_if_else_check";
|
||||||
|
|
||||||
this(BaseAnalyzerArguments args)
|
extern(D) this(string fileName, bool skipTests = false)
|
||||||
{
|
{
|
||||||
super(args);
|
super(fileName, skipTests);
|
||||||
}
|
}
|
||||||
|
|
||||||
override void visit(const ConditionalStatement cc)
|
override void visit(AST.ConditionalStatement s)
|
||||||
{
|
{
|
||||||
cc.accept(this);
|
import dmd.astenums : STMT;
|
||||||
if (cc.falseStatement is null)
|
|
||||||
|
if (!s.condition.isStaticIfCondition())
|
||||||
{
|
{
|
||||||
|
super.visit(s);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
const(IfStatement) ifStmt = getIfStatement(cc);
|
|
||||||
if (!ifStmt)
|
s.condition.accept(this);
|
||||||
|
|
||||||
|
if (s.ifbody)
|
||||||
|
s.ifbody.accept(this);
|
||||||
|
|
||||||
|
if (s.elsebody)
|
||||||
{
|
{
|
||||||
return;
|
if (s.elsebody.stmt == STMT.If)
|
||||||
|
addErrorMessage(cast(ulong) s.elsebody.loc.linnum, cast(ulong) s.elsebody.loc.charnum,
|
||||||
|
KEY, MESSAGE);
|
||||||
|
|
||||||
|
s.elsebody.accept(this);
|
||||||
}
|
}
|
||||||
auto tokens = ifStmt.tokens[0 .. 1];
|
|
||||||
// extend one token to include `else` before this if
|
|
||||||
tokens = (tokens.ptr - 1)[0 .. 2];
|
|
||||||
addErrorMessage(tokens, KEY, "Mismatched static if. Use 'else static if' here.",
|
|
||||||
[
|
|
||||||
AutoFix.insertionBefore(tokens[$ - 1], "static "),
|
|
||||||
AutoFix.resolveLater("Wrap '{}' block around 'if'", [tokens[0].index, ifStmt.tokens[$ - 1].index, 0])
|
|
||||||
]);
|
|
||||||
}
|
|
||||||
|
|
||||||
const(IfStatement) getIfStatement(const ConditionalStatement cc)
|
|
||||||
{
|
|
||||||
return safeAccess(cc).falseStatement.statement.statementNoCaseNoDefault.ifStatement;
|
|
||||||
}
|
|
||||||
|
|
||||||
override AutoFix.CodeReplacement[] resolveAutoFix(
|
|
||||||
const Module mod,
|
|
||||||
scope const(Token)[] tokens,
|
|
||||||
const AutoFix.ResolveContext context,
|
|
||||||
const AutoFixFormatting formatting,
|
|
||||||
)
|
|
||||||
{
|
|
||||||
import dscanner.analysis.helpers : getLineIndentation;
|
|
||||||
import std.algorithm : countUntil;
|
|
||||||
|
|
||||||
auto beforeElse = tokens.countUntil!(a => a.index == context.params[0]);
|
|
||||||
auto lastToken = tokens.countUntil!(a => a.index == context.params[1]);
|
|
||||||
if (beforeElse == -1 || lastToken == -1)
|
|
||||||
throw new Exception("got different tokens than what was used to generate this autofix");
|
|
||||||
|
|
||||||
auto indentation = getLineIndentation(tokens, tokens[beforeElse].line, formatting);
|
|
||||||
|
|
||||||
string beforeIf = formatting.getWhitespaceBeforeOpeningBrace(indentation, false)
|
|
||||||
~ "{" ~ formatting.eol ~ indentation;
|
|
||||||
string afterIf = formatting.eol ~ indentation ~ "}";
|
|
||||||
|
|
||||||
return AutoFix.replacement([tokens[beforeElse].index + 4, tokens[beforeElse + 1].index], beforeIf, "")
|
|
||||||
.concat(AutoFix.indentLines(tokens[beforeElse + 1 .. lastToken + 1], formatting))
|
|
||||||
.concat(AutoFix.insertionAfter(tokens[lastToken], afterIf))
|
|
||||||
.expectReplacements;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private:
|
||||||
enum KEY = "dscanner.suspicious.static_if_else";
|
enum KEY = "dscanner.suspicious.static_if_else";
|
||||||
|
enum MESSAGE = "Mismatched static if. Use 'else static if' here.";
|
||||||
}
|
}
|
||||||
|
|
||||||
unittest
|
unittest
|
||||||
{
|
{
|
||||||
import dscanner.analysis.config : Check, disabledConfig, StaticAnalysisConfig;
|
import dscanner.analysis.helpers : assertAnalyzerWarnings = assertAnalyzerWarningsDMD;
|
||||||
import dscanner.analysis.helpers : assertAnalyzerWarnings, assertAutoFix;
|
import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
|
||||||
import std.stdio : stderr;
|
import std.stdio : stderr;
|
||||||
|
|
||||||
StaticAnalysisConfig sac = disabledConfig();
|
StaticAnalysisConfig sac = disabledConfig();
|
||||||
|
@ -102,8 +73,7 @@ unittest
|
||||||
void foo() {
|
void foo() {
|
||||||
static if (false)
|
static if (false)
|
||||||
auto a = 0;
|
auto a = 0;
|
||||||
else if (true) /+
|
else if (true) // [warn]: Mismatched static if. Use 'else static if' here.
|
||||||
^^^^^^^ [warn]: Mismatched static if. Use 'else static if' here. +/
|
|
||||||
auto b = 1;
|
auto b = 1;
|
||||||
}
|
}
|
||||||
}c, sac);
|
}c, sac);
|
||||||
|
@ -119,51 +89,5 @@ unittest
|
||||||
}
|
}
|
||||||
}c, sac);
|
}c, sac);
|
||||||
|
|
||||||
assertAutoFix(q{
|
|
||||||
void foo() {
|
|
||||||
static if (false)
|
|
||||||
auto a = 0;
|
|
||||||
else if (true) // fix:0
|
|
||||||
auto b = 1;
|
|
||||||
}
|
|
||||||
void bar() {
|
|
||||||
static if (false)
|
|
||||||
auto a = 0;
|
|
||||||
else if (true) // fix:1
|
|
||||||
auto b = 1;
|
|
||||||
}
|
|
||||||
void baz() {
|
|
||||||
static if (false)
|
|
||||||
auto a = 0;
|
|
||||||
else if (true) { // fix:1
|
|
||||||
auto b = 1;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}c, q{
|
|
||||||
void foo() {
|
|
||||||
static if (false)
|
|
||||||
auto a = 0;
|
|
||||||
else static if (true) // fix:0
|
|
||||||
auto b = 1;
|
|
||||||
}
|
|
||||||
void bar() {
|
|
||||||
static if (false)
|
|
||||||
auto a = 0;
|
|
||||||
else {
|
|
||||||
if (true) // fix:1
|
|
||||||
auto b = 1;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
void baz() {
|
|
||||||
static if (false)
|
|
||||||
auto a = 0;
|
|
||||||
else {
|
|
||||||
if (true) { // fix:1
|
|
||||||
auto b = 1;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}c, sac);
|
|
||||||
|
|
||||||
stderr.writeln("Unittest for StaticIfElse passed.");
|
stderr.writeln("Unittest for StaticIfElse passed.");
|
||||||
}
|
}
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue