mirror of
https://github.com/dlang-community/D-Scanner.git
synced 2025-04-27 05:40:01 +03:00
replace libdpase in assert without msg visitor (#50)
This commit is contained in:
parent
a3a5982e2c
commit
e6af600921
4 changed files with 52 additions and 126 deletions
1
changelog/dscanner.assert-without-message.dd
Normal file
1
changelog/dscanner.assert-without-message.dd
Normal file
|
@ -0,0 +1 @@
|
||||||
|
Avoid checking `enforce` calls as it is phobos specific.
|
|
@ -5,163 +5,69 @@
|
||||||
module dscanner.analysis.assert_without_msg;
|
module dscanner.analysis.assert_without_msg;
|
||||||
|
|
||||||
import dscanner.analysis.base;
|
import dscanner.analysis.base;
|
||||||
import dscanner.utils : safeAccess;
|
|
||||||
import dsymbol.scope_ : Scope;
|
|
||||||
import dparse.lexer;
|
|
||||||
import dparse.ast;
|
|
||||||
|
|
||||||
import std.stdio;
|
import std.stdio;
|
||||||
import std.algorithm;
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Check that all asserts have an explanatory message.
|
* 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";
|
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"))
|
if (f.fbody)
|
||||||
{
|
{
|
||||||
// libdparse 0.22.0+
|
f.fbody.accept(this);
|
||||||
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);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
override void visit(const FunctionCallExpression expr)
|
override void visit(AST.AssertExp ae)
|
||||||
{
|
{
|
||||||
if (!isStdExceptionImported)
|
if (!ae.msg)
|
||||||
return;
|
addErrorMessage(ae.loc.linnum, ae.loc.charnum, KEY, MESSAGE);
|
||||||
|
|
||||||
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);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
override void visit(const SingleImport sImport)
|
override void visit(AST.StaticAssert ae)
|
||||||
{
|
{
|
||||||
static immutable stdException = ["std", "exception"];
|
if (!ae.msgs)
|
||||||
if (sImport.identifierChain.identifiers.map!(a => a.text).equal(stdException))
|
addErrorMessage(ae.loc.linnum, ae.loc.charnum, KEY, MESSAGE);
|
||||||
isStdExceptionImported = true;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// 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:
|
private:
|
||||||
bool isStdExceptionImported;
|
enum string KEY = "dscanner.style.assert_without_msg";
|
||||||
|
enum string MESSAGE = "An assert should have an explanatory message";
|
||||||
template ScopedVisit(NodeType)
|
|
||||||
{
|
|
||||||
override void visit(const NodeType n)
|
|
||||||
{
|
|
||||||
bool tmp = isStdExceptionImported;
|
|
||||||
scope(exit) isStdExceptionImported = tmp;
|
|
||||||
n.accept(this);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
unittest
|
unittest
|
||||||
{
|
{
|
||||||
import std.stdio : stderr;
|
import std.stdio : stderr;
|
||||||
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.assert_without_msg = Check.enabled;
|
sac.assert_without_msg = Check.enabled;
|
||||||
|
|
||||||
assertAnalyzerWarnings(q{
|
assertAnalyzerWarningsDMD(q{
|
||||||
unittest {
|
unittest {
|
||||||
assert(0, "foo bar");
|
assert(0, "foo bar");
|
||||||
assert(0); /+
|
assert(0); // [warn]: An assert should have an explanatory message
|
||||||
^^^^^^^^^ [warn]: %s +/
|
|
||||||
}
|
}
|
||||||
}c.format(
|
}c, sac);
|
||||||
AssertWithoutMessageCheck.MESSAGE,
|
|
||||||
), sac);
|
|
||||||
|
|
||||||
assertAnalyzerWarnings(q{
|
assertAnalyzerWarningsDMD(q{
|
||||||
unittest {
|
unittest {
|
||||||
static assert(0, "foo bar");
|
static assert(0, "foo bar");
|
||||||
static assert(0); /+
|
static assert(0); // [warn]: An assert should have an explanatory message
|
||||||
^^^^^^^^^ [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
|
|
||||||
}
|
}
|
||||||
}c, sac);
|
}c, sac);
|
||||||
|
|
||||||
|
|
|
@ -913,9 +913,10 @@ extern(C++) class BaseAnalyzerDmd(AST) : ParseTimeTransitiveVisitor!AST
|
||||||
{
|
{
|
||||||
alias visit = ParseTimeTransitiveVisitor!AST.visit;
|
alias visit = ParseTimeTransitiveVisitor!AST.visit;
|
||||||
|
|
||||||
extern(D) this(string fileName)
|
extern(D) this(string fileName, bool skipTests = false)
|
||||||
{
|
{
|
||||||
this.fileName = fileName;
|
this.fileName = fileName;
|
||||||
|
this.skipTests = skipTests;
|
||||||
_messages = new MessageSet;
|
_messages = new MessageSet;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -933,6 +934,12 @@ extern(C++) class BaseAnalyzerDmd(AST) : ParseTimeTransitiveVisitor!AST
|
||||||
return _messages[].array;
|
return _messages[].array;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
override void visit(AST.UnitTestDeclaration ud)
|
||||||
|
{
|
||||||
|
if (!skipTests)
|
||||||
|
super.visit(ud);
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
protected:
|
protected:
|
||||||
|
|
||||||
|
@ -941,6 +948,8 @@ protected:
|
||||||
_messages.insert(Message(fileName, line, column, key, message, getName()));
|
_messages.insert(Message(fileName, line, column, key, message, getName()));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
extern(D) bool skipTests;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* The file name
|
* The file name
|
||||||
*/
|
*/
|
||||||
|
|
|
@ -99,6 +99,11 @@ import dmd.parse : Parser;
|
||||||
|
|
||||||
bool first = true;
|
bool first = true;
|
||||||
|
|
||||||
|
version (unittest)
|
||||||
|
enum ut = true;
|
||||||
|
else
|
||||||
|
enum ut = false;
|
||||||
|
|
||||||
private alias ASTAllocator = CAllocatorImpl!(
|
private alias ASTAllocator = CAllocatorImpl!(
|
||||||
AllocatorList!(n => Region!Mallocator(1024 * 128), Mallocator));
|
AllocatorList!(n => Region!Mallocator(1024 * 128), Mallocator));
|
||||||
|
|
||||||
|
@ -801,15 +806,18 @@ unittest
|
||||||
assert(test("std.bar.foo", "-barr,+bar"));
|
assert(test("std.bar.foo", "-barr,+bar"));
|
||||||
}
|
}
|
||||||
|
|
||||||
private BaseAnalyzer[] getAnalyzersForModuleAndConfig(string fileName,
|
private
|
||||||
const(Token)[] tokens, const Module m,
|
|
||||||
const StaticAnalysisConfig analysisConfig, const Scope* moduleScope)
|
|
||||||
{
|
{
|
||||||
version (unittest)
|
version (unittest)
|
||||||
enum ut = true;
|
enum ut = true;
|
||||||
else
|
else
|
||||||
enum ut = false;
|
enum ut = false;
|
||||||
|
}
|
||||||
|
|
||||||
|
private BaseAnalyzer[] getAnalyzersForModuleAndConfig(string fileName,
|
||||||
|
const(Token)[] tokens, const Module m,
|
||||||
|
const StaticAnalysisConfig analysisConfig, const Scope* moduleScope)
|
||||||
|
{
|
||||||
BaseAnalyzer[] checks;
|
BaseAnalyzer[] checks;
|
||||||
|
|
||||||
string moduleName;
|
string moduleName;
|
||||||
|
@ -957,10 +965,6 @@ private BaseAnalyzer[] getAnalyzersForModuleAndConfig(string fileName,
|
||||||
checks ~= new HasPublicExampleCheck(args.setSkipTests(
|
checks ~= new HasPublicExampleCheck(args.setSkipTests(
|
||||||
analysisConfig.has_public_example == Check.skipTests && !ut));
|
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))
|
if (moduleName.shouldRun!IfConstraintsIndentCheck(analysisConfig))
|
||||||
checks ~= new IfConstraintsIndentCheck(args.setSkipTests(
|
checks ~= new IfConstraintsIndentCheck(args.setSkipTests(
|
||||||
analysisConfig.if_constraints_indent == Check.skipTests && !ut));
|
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))
|
if (moduleName.shouldRunDmd!(ConstructorCheck!ASTBase)(config))
|
||||||
visitors ~= new ConstructorCheck!ASTBase(fileName);
|
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))
|
if (moduleName.shouldRunDmd!(LocalImportCheck!ASTBase)(config))
|
||||||
visitors ~= new LocalImportCheck!ASTBase(fileName);
|
visitors ~= new LocalImportCheck!ASTBase(fileName);
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue