mirrors/D-Scanner
1
0
Fork 0
mirror of https://github.com/dlang-community/D-Scanner.git synced 2025-04-26 13:20:07 +03:00
Code Issues Projects Releases Packages Wiki Activity Actions

replace libdpase in assert without msg visitor (#50)

Browse source
This commit is contained in:
lucica28 2022-12-01 15:00:22 +02:00 • committed by Eduard Staniloiu
parent 18a5a09a23
commit 82bc26b7be
4 changed files with 52 additions and 126 deletions
Download patch file Download diff file Expand all files Collapse all files

1
changelog/dscanner.assert-without-message.dd Normal file
View file

@ -0,0 +1 @@
Avoid checking `enforce` calls as it is phobos specific.

142
src/dscanner/analysis/assert_without_msg.d
View file

@ -5,163 +5,69 @@
module dscanner.analysis.assert_without_msg;
import dscanner.analysis.base;
import dscanner.utils : safeAccess;
import dsymbol.scope_ : Scope;
import dparse.lexer;
import dparse.ast;
import std.stdio;
import std.algorithm;
/**
* Check that all asserts have an explanatory message.
*/
final class AssertWithoutMessageCheck : BaseAnalyzer
extern(C++) class AssertWithoutMessageCheck(AST) : BaseAnalyzerDmd
{
enum string KEY = "dscanner.style.assert_without_msg";
enum string MESSAGE = "An assert should have an explanatory message";
mixin AnalyzerInfo!"assert_without_msg";
alias visit = BaseAnalyzerDmd.visit;
///
this(BaseAnalyzerArguments args)
extern(D) this(string fileName, bool skipTests = false)
{
super(args);
super(fileName, skipTests);
}
override void visit(const AssertExpression expr)
// Avoid visiting in/out contracts for this check
override void visitFuncBody(AST.FuncDeclaration f)
{
static if (__traits(hasMember, expr.assertArguments, "messageParts"))
{
// libdparse 0.22.0+
bool hasMessage = expr.assertArguments
&& expr.assertArguments.messageParts.length > 0;
}
else
bool hasMessage = expr.assertArguments
&& expr.assertArguments.message !is null;
if (!hasMessage)
addErrorMessage(expr, KEY, MESSAGE);
if (f.fbody)
{
f.fbody.accept(this);
}
}
override void visit(const FunctionCallExpression expr)
override void visit(AST.AssertExp ae)
{
if (!isStdExceptionImported)
return;
if (const IdentifierOrTemplateInstance iot = safeAccess(expr)
.unaryExpression.primaryExpression.identifierOrTemplateInstance)
{
auto ident = iot.identifier;
if (ident.text == "enforce" && expr.arguments !is null && expr.arguments.namedArgumentList !is null &&
expr.arguments.namedArgumentList.items.length < 2)
addErrorMessage(expr, KEY, MESSAGE);
}
if (!ae.msg)
addErrorMessage(ae.loc.linnum, ae.loc.charnum, KEY, MESSAGE);
}
override void visit(const SingleImport sImport)
override void visit(AST.StaticAssert ae)
{
static immutable stdException = ["std", "exception"];
if (sImport.identifierChain.identifiers.map!(a => a.text).equal(stdException))
isStdExceptionImported = true;
if (!ae.msgs)
addErrorMessage(ae.loc.linnum, ae.loc.charnum, KEY, MESSAGE);
}
// revert the stack after new scopes
override void visit(const Declaration decl)
{
// be careful - ImportDeclarations don't introduce a new scope
if (decl.importDeclaration is null)
{
bool tmp = isStdExceptionImported;
scope(exit) isStdExceptionImported = tmp;
decl.accept(this);
}
else
decl.accept(this);
}
mixin ScopedVisit!IfStatement;
mixin ScopedVisit!BlockStatement;
alias visit = BaseAnalyzer.visit;
private:
bool isStdExceptionImported;
template ScopedVisit(NodeType)
{
override void visit(const NodeType n)
{
bool tmp = isStdExceptionImported;
scope(exit) isStdExceptionImported = tmp;
n.accept(this);
}
}
enum string KEY = "dscanner.style.assert_without_msg";
enum string MESSAGE = "An assert should have an explanatory message";
}
unittest
{
import std.stdio : stderr;
import std.format : format;
import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
import dscanner.analysis.helpers : assertAnalyzerWarnings;
import dscanner.analysis.helpers : assertAnalyzerWarningsDMD;
StaticAnalysisConfig sac = disabledConfig();
sac.assert_without_msg = Check.enabled;
assertAnalyzerWarnings(q{
assertAnalyzerWarningsDMD(q{
unittest {
assert(0, "foo bar");
assert(0); /+
^^^^^^^^^ [warn]: %s +/
assert(0); // [warn]: An assert should have an explanatory message
}
}c.format(
AssertWithoutMessageCheck.MESSAGE,
), sac);
}c, sac);
assertAnalyzerWarnings(q{
assertAnalyzerWarningsDMD(q{
unittest {
static assert(0, "foo bar");
static assert(0); /+
^^^^^^^^^ [warn]: %s +/
}
}c.format(
AssertWithoutMessageCheck.MESSAGE,
), sac);
// check for std.exception.enforce
assertAnalyzerWarnings(q{
unittest {
enforce(0); // std.exception not imported yet - could be a user-defined symbol
import std.exception;
enforce(0, "foo bar");
enforce(0); /+
^^^^^^^^^^ [warn]: %s +/
}
}c.format(
AssertWithoutMessageCheck.MESSAGE,
), sac);
// check for std.exception.enforce
assertAnalyzerWarnings(q{
unittest {
import exception;
class C {
import std.exception;
}
enforce(0); // std.exception not imported yet - could be a user-defined symbol
struct S {
import std.exception;
}
enforce(0); // std.exception not imported yet - could be a user-defined symbol
if (false) {
import std.exception;
}
enforce(0); // std.exception not imported yet - could be a user-defined symbol
{
import std.exception;
}
enforce(0); // std.exception not imported yet - could be a user-defined symbol
static assert(0); // [warn]: An assert should have an explanatory message
}
}c, sac);

11
src/dscanner/analysis/base.d
View file

@ -913,9 +913,10 @@ extern(C++) class BaseAnalyzerDmd(AST) : ParseTimeTransitiveVisitor!AST
{
alias visit = ParseTimeTransitiveVisitor!AST.visit;
extern(D) this(string fileName)
extern(D) this(string fileName, bool skipTests = false)
{
this.fileName = fileName;
this.skipTests = skipTests;
_messages = new MessageSet;
}
@ -933,6 +934,12 @@ extern(C++) class BaseAnalyzerDmd(AST) : ParseTimeTransitiveVisitor!AST
return _messages[].array;
}
override void visit(AST.UnitTestDeclaration ud)
{
if (!skipTests)
super.visit(ud);
}
protected:
@ -941,6 +948,8 @@ protected:
_messages.insert(Message(fileName, line, column, key, message, getName()));
}
extern(D) bool skipTests;
/**
* The file name
*/

24
src/dscanner/analysis/run.d
View file

@ -99,6 +99,11 @@ import dmd.parse : Parser;
bool first = true;
version (unittest)
enum ut = true;
else
enum ut = false;
private alias ASTAllocator = CAllocatorImpl!(
AllocatorList!(n => Region!Mallocator(1024 * 128), Mallocator));
@ -801,15 +806,18 @@ unittest
assert(test("std.bar.foo", "-barr,+bar"));
}
private BaseAnalyzer[] getAnalyzersForModuleAndConfig(string fileName,
const(Token)[] tokens, const Module m,
const StaticAnalysisConfig analysisConfig, const Scope* moduleScope)
private
{
version (unittest)
enum ut = true;
else
enum ut = false;
}
private BaseAnalyzer[] getAnalyzersForModuleAndConfig(string fileName,
const(Token)[] tokens, const Module m,
const StaticAnalysisConfig analysisConfig, const Scope* moduleScope)
{
BaseAnalyzer[] checks;
string moduleName;
@ -957,10 +965,6 @@ private BaseAnalyzer[] getAnalyzersForModuleAndConfig(string fileName,
checks ~= new HasPublicExampleCheck(args.setSkipTests(
analysisConfig.has_public_example == Check.skipTests && !ut));
if (moduleName.shouldRun!AssertWithoutMessageCheck(analysisConfig))
checks ~= new AssertWithoutMessageCheck(args.setSkipTests(
analysisConfig.assert_without_msg == Check.skipTests && !ut));
if (moduleName.shouldRun!IfConstraintsIndentCheck(analysisConfig))
checks ~= new IfConstraintsIndentCheck(args.setSkipTests(
analysisConfig.if_constraints_indent == Check.skipTests && !ut));
@ -1318,6 +1322,12 @@ MessageSet analyzeDmd(string fileName, ASTBase.Module m, const char[] moduleName
if (moduleName.shouldRunDmd!(ConstructorCheck!ASTBase)(config))
visitors ~= new ConstructorCheck!ASTBase(fileName);
if (moduleName.shouldRunDmd!(AssertWithoutMessageCheck!ASTBase)(config))
visitors ~= new AssertWithoutMessageCheck!ASTBase(
fileName,
config.assert_without_msg == Check.skipTests && !ut
);
if (moduleName.shouldRunDmd!(LocalImportCheck!ASTBase)(config))
visitors ~= new LocalImportCheck!ASTBase(fileName);