forks
/
D-Scanner
mirror of https://github.com/dlang-community/D-Scanner.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Replace libdparse with DMD in AutoFunctionChecker (#103)

This commit is contained in:
Vladiwostok 2024-04-15 16:52:47 +03:00 committed by Albert24GG
parent a9f1348cd4
commit af525da448
2 changed files with 111 additions and 203 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

304
src/dscanner/analysis/auto_function.d
View File

@ -6,12 +6,8 @@
module dscanner.analysis.auto_function; module dscanner.analysis.auto_function;
import dscanner.analysis.base; import dscanner.analysis.base;
import dscanner.analysis.helpers; import std.conv : to;
import dparse.ast; import std.algorithm.searching : canFind;
import dparse.lexer;
import std.stdio;
import std.algorithm : map, filter;
/** /**
* Checks for auto functions without return statement. * Checks for auto functions without return statement.
@ -20,166 +16,104 @@ import std.algorithm : map, filter;
* detected by the compiler. However sometimes they can be used as a trick * detected by the compiler. However sometimes they can be used as a trick
* to infer attributes. * to infer attributes.
*/ */
final class AutoFunctionChecker : BaseAnalyzer // TODO: Fix Autofix
extern (C++) class AutoFunctionChecker(AST) : BaseAnalyzerDmd
{ {
alias visit = BaseAnalyzerDmd.visit;
private:
enum string KEY = "dscanner.suspicious.missing_return";
enum string MESSAGE = "Auto function without return statement, prefer replacing auto with void";
enum string MESSAGE_INSERT = "Auto function without return statement, prefer inserting void to be explicit";
bool[] _returns;
size_t _mixinDepth;
string[] _literalWithReturn;
public:
alias visit = BaseAnalyzer.visit;
mixin AnalyzerInfo!"auto_function_check"; mixin AnalyzerInfo!"auto_function_check";
private bool foundReturn;
private bool foundFalseAssert;
private bool inMixin;
private bool foundReturnLiteral;
private string[] literalsWithReturn;
private enum string KEY = "dscanner.suspicious.missing_return";
private enum string MESSAGE = "Auto function without return statement, prefer replacing auto with void";
private enum string MESSAGE_INSERT = "Auto function without return statement, prefer inserting void to be explicit";
/// ///
this(BaseAnalyzerArguments args) extern (D) this(string fileName, bool skipTests = false)
{ {
super(args); super(fileName, skipTests);
} }
package static const(Token)[] findAutoReturnType(const(FunctionDeclaration) decl) override void visit(AST.FuncDeclaration d)
{ {
const(Token)[] lastAtAttribute; import dmd.astenums : STC, STMT;
foreach (storageClass; decl.storageClasses)
if (d.storage_class & STC.disable || d.fbody is null || (d.fbody && d.fbody.isReturnStatement()))
return;
ulong lineNum = cast(ulong) d.loc.linnum;
ulong charNum = cast(ulong) d.loc.charnum;
auto oldFoundReturn = foundReturn;
auto oldFoundFalseAssert = foundFalseAssert;
foundReturn = false;
foundFalseAssert = false;
super.visitFuncBody(d);
if (!foundReturn && !foundFalseAssert)
{ {
if (storageClass.token.type == tok!"auto") if (d.storage_class & STC.auto_)
return storageClass.tokens; addErrorMessage(lineNum, charNum, KEY, MESSAGE);
else if (storageClass.atAttribute) else if (auto returnType = cast(AST.TypeFunction) d.type)
lastAtAttribute = storageClass.atAttribute.tokens; if (returnType.next is null)
addErrorMessage(lineNum, charNum, KEY, MESSAGE_INSERT);
} }
return lastAtAttribute;
foundReturn = oldFoundReturn;
foundFalseAssert = oldFoundFalseAssert;
} }
override void visit(const(FunctionDeclaration) decl) override void visit(AST.ReturnStatement s)
{ {
_returns.length += 1; foundReturn = true;
scope(exit) _returns.length -= 1;
_returns[$-1] = false;
auto autoTokens = findAutoReturnType(decl);
bool isAtAttribute = autoTokens.length > 1;
decl.accept(this);
if (decl.functionBody.specifiedFunctionBody && autoTokens.length && !_returns[$-1])
{
if (isAtAttribute)
{
// highlight on the whitespace between attribute and function name
auto tok = autoTokens[$ - 1];
auto whitespace = tok.column + (tok.text.length ? tok.text.length : str(tok.type).length);
auto whitespaceIndex = tok.index + (tok.text.length ? tok.text.length : str(tok.type).length);
addErrorMessage([whitespaceIndex, whitespaceIndex + 1], tok.line, [whitespace, whitespace + 1], KEY, MESSAGE_INSERT,
[AutoFix.insertionAt(whitespaceIndex + 1, "void ")]);
}
else
addErrorMessage(autoTokens, KEY, MESSAGE,
[AutoFix.replacement(autoTokens[0], "", "Replace `auto` with `void`")
.concat(AutoFix.insertionAt(decl.name.index, "void "))]);
}
} }
override void visit(const(ReturnStatement) rst) override void visit(AST.AssertExp assertExpr)
{ {
if (_returns.length) auto ie = assertExpr.e1.isIntegerExp();
_returns[$-1] = true; if (ie && ie.getInteger() == 0)
rst.accept(this); foundFalseAssert = true;
} }
override void visit(const(AssertArguments) exp) override void visit(AST.MixinStatement mixinStatement)
{ {
exp.accept(this); auto oldInMixin = inMixin;
if (_returns.length) inMixin = true;
{ super.visit(mixinStatement);
const UnaryExpression u = cast(UnaryExpression) exp.assertion; inMixin = oldInMixin;
if (!u)
return;
const PrimaryExpression p = u.primaryExpression;
if (!p)
return;
immutable token = p.primary;
if (token.type == tok!"false")
_returns[$-1] = true;
else if (token.text == "0")
_returns[$-1] = true;
}
} }
override void visit(const(MixinExpression) mix) override void visit(AST.StringExp stringExpr)
{ {
++_mixinDepth; foundReturnLiteral = foundReturnLiteral || canFind(stringExpr.toStringz(), "return");
mix.accept(this);
--_mixinDepth; if (inMixin)
foundReturn = foundReturn || foundReturnLiteral;
} }
override void visit(const(PrimaryExpression) exp) override void visit(AST.IdentifierExp ie)
{ {
exp.accept(this); if (inMixin)
foundReturn = foundReturn || canFind(literalsWithReturn, to!string(ie.ident.toString()));
import std.algorithm.searching : canFind; super.visit(ie);
if (_returns.length && _mixinDepth)
{
if (findReturnInLiteral(exp.primary.text))
_returns[$-1] = true;
else if (exp.identifierOrTemplateInstance &&
_literalWithReturn.canFind(exp.identifierOrTemplateInstance.identifier.text))
_returns[$-1] = true;
}
} }
private bool findReturnInLiteral(const(string) value) override void visit(AST.VarDeclaration vd)
{ {
import std.algorithm.searching : find; auto oldFoundReturnLiteral = foundReturnLiteral;
import std.range : empty; foundFalseAssert = false;
super.visit(vd);
return value == "return" || !value.find("return ").empty; if (foundReturnLiteral)
} literalsWithReturn ~= to!string(vd.ident.toString());
private bool stringliteralHasReturn(const(NonVoidInitializer) nvi) foundReturnLiteral = oldFoundReturnLiteral;
{
bool result;
if (!nvi.assignExpression || (cast(UnaryExpression) nvi.assignExpression) is null)
return result;
const(UnaryExpression) u = cast(UnaryExpression) nvi.assignExpression;
if (u.primaryExpression &&
u.primaryExpression.primary.type.isStringLiteral &&
findReturnInLiteral(u.primaryExpression.primary.text))
result = true;
return result;
}
override void visit(const(AutoDeclaration) decl)
{
decl.accept(this);
foreach(const(AutoDeclarationPart) p; decl.parts)
if (p.initializer &&
p.initializer.nonVoidInitializer &&
stringliteralHasReturn(p.initializer.nonVoidInitializer))
_literalWithReturn ~= p.identifier.text.idup;
}
override void visit(const(VariableDeclaration) decl)
{
decl.accept(this);
foreach(const(Declarator) d; decl.declarators)
if (d.initializer &&
d.initializer.nonVoidInitializer &&
stringliteralHasReturn(d.initializer.nonVoidInitializer))
_literalWithReturn ~= d.name.text.idup;
} }
} }
@ -188,95 +122,67 @@ unittest
import std.stdio : stderr; import std.stdio : stderr;
import std.format : format; import std.format : format;
import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig; import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
import dscanner.analysis.helpers : assertAnalyzerWarnings; import dscanner.analysis.helpers : assertAnalyzerWarningsDMD;
StaticAnalysisConfig sac = disabledConfig(); StaticAnalysisConfig sac = disabledConfig();
sac.auto_function_check = Check.enabled; sac.auto_function_check = Check.enabled;
assertAnalyzerWarnings(q{
auto ref doStuff(){} /+ string MESSAGE = "Auto function without return statement, prefer replacing auto with void";
^^^^ [warn]: %s +/ string MESSAGE_INSERT = "Auto function without return statement, prefer inserting void to be explicit";
auto doStuff(){} /+
^^^^ [warn]: %s +/ assertAnalyzerWarningsDMD(q{
auto ref doStuff(){} // [warn]: %s
auto doStuff(){} // [warn]: %s
@Custom @Custom
auto doStuff(){} /+ auto doStuff(){} // [warn]: %s
^^^^ [warn]: %s +/ int doStuff(){auto doStuff(){}} // [warn]: %s
int doStuff(){auto doStuff(){}} /+
^^^^ [warn]: %s +/
auto doStuff(){return 0;} auto doStuff(){return 0;}
int doStuff(){/*error but not the aim*/} int doStuff(){/*error but not the aim*/}
}c.format( }c.format(MESSAGE, MESSAGE, MESSAGE, MESSAGE), sac);
AutoFunctionChecker.MESSAGE,
AutoFunctionChecker.MESSAGE,
AutoFunctionChecker.MESSAGE,
AutoFunctionChecker.MESSAGE,
), sac);
assertAnalyzerWarnings(q{ assertAnalyzerWarningsDMD(q{
auto doStuff(){assert(true);} /+ auto doStuff(){assert(true);} // [warn]: %s
^^^^ [warn]: %s +/
auto doStuff(){assert(false);} auto doStuff(){assert(false);}
}c.format( }c.format(MESSAGE), sac);
AutoFunctionChecker.MESSAGE,
), sac);
assertAnalyzerWarnings(q{ assertAnalyzerWarningsDMD(q{
auto doStuff(){assert(1);} /+ auto doStuff(){assert(1);} // [warn]: %s
^^^^ [warn]: %s +/
auto doStuff(){assert(0);} auto doStuff(){assert(0);}
}c.format( }c.format(MESSAGE), sac);
AutoFunctionChecker.MESSAGE,
), sac);
assertAnalyzerWarnings(q{ assertAnalyzerWarningsDMD(q{
auto doStuff(){mixin("0+0");} /+ auto doStuff(){mixin("0+0");} // [warn]: %s
^^^^ [warn]: %s +/
auto doStuff(){mixin("return 0;");} auto doStuff(){mixin("return 0;");}
}c.format( }c.format(MESSAGE), sac);
AutoFunctionChecker.MESSAGE,
), sac);
assertAnalyzerWarnings(q{ assertAnalyzerWarningsDMD(q{
auto doStuff(){mixin("0+0");} /+ auto doStuff(){mixin("0+0");} // [warn]: %s
^^^^ [warn]: %s +/
auto doStuff(){mixin("static if (true)" ~ " return " ~ 0.stringof ~ ";");} auto doStuff(){mixin("static if (true)" ~ " return " ~ 0.stringof ~ ";");}
}c.format( }c.format(MESSAGE), sac);
AutoFunctionChecker.MESSAGE,
), sac);
assertAnalyzerWarnings(q{ assertAnalyzerWarningsDMD(q{
auto doStuff(){} /+ auto doStuff(){} // [warn]: %s
^^^^ [warn]: %s +/
extern(C) auto doStuff(); extern(C) auto doStuff();
}c.format( }c.format(MESSAGE), sac);
AutoFunctionChecker.MESSAGE,
), sac);
assertAnalyzerWarnings(q{ assertAnalyzerWarningsDMD(q{
auto doStuff(){} /+ auto doStuff(){} // [warn]: %s
^^^^ [warn]: %s +/
@disable auto doStuff(); @disable auto doStuff();
}c.format( }c.format(MESSAGE), sac);
AutoFunctionChecker.MESSAGE,
), sac);
assertAnalyzerWarnings(q{ assertAnalyzerWarningsDMD(q{
@property doStuff(){} /+ @property doStuff(){} // [warn]: %s
^ [warn]: %s +/ @safe doStuff(){} // [warn]: %s
@safe doStuff(){} /+
^ [warn]: %s +/
@disable doStuff(); @disable doStuff();
@safe void doStuff(); @safe void doStuff();
}c.format( }c.format(MESSAGE_INSERT, MESSAGE_INSERT), sac);
AutoFunctionChecker.MESSAGE_INSERT,
AutoFunctionChecker.MESSAGE_INSERT,
), sac);
assertAnalyzerWarnings(q{ assertAnalyzerWarningsDMD(q{
enum _genSave = "return true;"; enum _genSave = "return true;";
auto doStuff(){ mixin(_genSave);} auto doStuff(){ mixin(_genSave);}
}, sac); }, sac);
/+ TODO: Fix Autofix
assertAutoFix(q{ assertAutoFix(q{
auto ref doStuff(){} // fix auto ref doStuff(){} // fix
auto doStuff(){} // fix auto doStuff(){} // fix
@ -291,7 +197,7 @@ unittest
@safe void doStuff(){} // fix @safe void doStuff(){} // fix
@Custom @Custom
void doStuff(){} // fix void doStuff(){} // fix
}c, sac); }c, sac);+/
stderr.writeln("Unittest for AutoFunctionChecker passed."); stderr.writeln("Unittest for AutoFunctionChecker passed.");
} }

10
src/dscanner/analysis/run.d
View File

@ -858,10 +858,6 @@ private BaseAnalyzer[] getAnalyzersForModuleAndConfig(string fileName,
analysisConfig.long_line_check == Check.skipTests && !ut), analysisConfig.long_line_check == Check.skipTests && !ut),
analysisConfig.max_line_length); analysisConfig.max_line_length);
if (moduleName.shouldRun!AutoFunctionChecker(analysisConfig))
checks ~= new AutoFunctionChecker(args.setSkipTests(
analysisConfig.auto_function_check == Check.skipTests && !ut));
if (moduleName.shouldRun!VcallCtorChecker(analysisConfig)) if (moduleName.shouldRun!VcallCtorChecker(analysisConfig))
checks ~= new VcallCtorChecker(args.setSkipTests( checks ~= new VcallCtorChecker(args.setSkipTests(
analysisConfig.vcall_in_ctor == Check.skipTests && !ut)); analysisConfig.vcall_in_ctor == Check.skipTests && !ut));
@ -1348,6 +1344,12 @@ MessageSet analyzeDmd(string fileName, ASTCodegen.Module m, const char[] moduleN
config.style_check == Check.skipTests && !ut config.style_check == Check.skipTests && !ut
); );
if (moduleName.shouldRunDmd!(AutoFunctionChecker!ASTCodegen)(config))
visitors ~= new AutoFunctionChecker!ASTCodegen(
fileName,
config.auto_function_check == Check.skipTests && !ut
);
foreach (visitor; visitors) foreach (visitor; visitors)
{ {
m.accept(visitor); m.accept(visitor);