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

add end line/column to warnings

Browse source
This commit is contained in:
WebFreak001 2023-06-29 12:52:01 +02:00 • committed by Jan Jurzitza
parent 5a53c538d0
commit 5c2035ff76
61 changed files with 1238 additions and 629 deletions
Download patch file Download diff file Expand all files Collapse all files

2
dub.json
View file

@ -11,7 +11,7 @@
"built_with_dub"
],
"dependencies": {
"libdparse": ">=0.23.0 <0.24.0",
"libdparse": ">=0.23.1 <0.24.0",
"dcd:dsymbol": ">=0.16.0-beta.2 <0.17.0",
"inifiled": "~>1.3.1",
"emsi_containers": "~>0.9.0",

2
dub.selections.json
View file

@ -6,7 +6,7 @@
"emsi_containers": "0.9.0",
"inifiled": "1.3.3",
"libddoc": "0.8.0",
"libdparse": "0.23.0",
"libdparse": "0.23.1",
"stdx-allocator": "2.77.5"
}
}

2
libdparse

@ -1 +1 @@
Subproject commit 86c9bf44c96e1666eb175c749cc26f62c2008979
Subproject commit e354f917f20c4a1fae04d1680205486c2a2a8317

6
src/dscanner/analysis/alias_syntax_check.d
View file

@ -29,8 +29,7 @@ final class AliasSyntaxCheck : BaseAnalyzer
return;
assert(ad.declaratorIdentifierList.identifiers.length > 0,
"Identifier list length is zero, libdparse has a bug");
addErrorMessage(ad.declaratorIdentifierList.identifiers[0].line,
ad.declaratorIdentifierList.identifiers[0].column, KEY,
addErrorMessage(ad, KEY,
"Prefer the new \"'alias' identifier '=' type ';'\" syntax"
~ " to the old \"'alias' type identifier ';'\" syntax.");
}
@ -48,7 +47,8 @@ unittest
StaticAnalysisConfig sac = disabledConfig();
sac.alias_syntax_check = Check.enabled;
assertAnalyzerWarnings(q{
alias int abcde; // [warn]: Prefer the new "'alias' identifier '=' type ';'" syntax to the old "'alias' type identifier ';'" syntax.
alias int abcde; /+
^^^^^^^^^^^^^^^^ [warn]: Prefer the new "'alias' identifier '=' type ';'" syntax to the old "'alias' type identifier ';'" syntax.+/
alias abcde = int;
}c, sac);

7
src/dscanner/analysis/allman.d
View file

@ -47,7 +47,7 @@ final class AllManCheck : BaseAnalyzer
continue;
// ignore inline { } braces
if (curLine != tokens[i + 1].line)
addErrorMessage(tokens[i].line, tokens[i].column, KEY, MESSAGE);
addErrorMessage(tokens[i], KEY, MESSAGE);
}
if (tokens[i].type == tok!"}" && curLine == prevTokenLine)
{
@ -56,7 +56,7 @@ final class AllManCheck : BaseAnalyzer
continue;
// ignore inline { } braces
if (!tokens[0 .. i].retro.until!(t => t.line != curLine).canFind!(t => t.type == tok!"{"))
addErrorMessage(tokens[i].line, tokens[i].column, KEY, MESSAGE);
addErrorMessage(tokens[i], KEY, MESSAGE);
}
}
}
@ -79,7 +79,8 @@ unittest
assertAnalyzerWarnings(q{
void testAllman()
{
while (true) { // [warn]: %s
while (true) { /+
^ [warn]: %s +/
auto f = 1;
}

5
src/dscanner/analysis/asm_style.d
View file

@ -32,7 +32,7 @@ final class AsmStyleCheck : BaseAnalyzer
if (brExp.asmBrExp !is null && brExp.asmBrExp.asmUnaExp !is null
&& brExp.asmBrExp.asmUnaExp.asmPrimaryExp !is null)
{
addErrorMessage(brExp.line, brExp.column, "dscanner.confusing.brexp",
addErrorMessage(brExp, "dscanner.confusing.brexp",
"This is confusing because it looks like an array index. Rewrite a[1] as [a + 1] to clarify.");
}
brExp.accept(this);
@ -50,7 +50,8 @@ unittest
{
asm
{
mov a, someArray[1]; // [warn]: This is confusing because it looks like an array index. Rewrite a[1] as [a + 1] to clarify.
mov a, someArray[1]; /+
^^^^^^^^^^^^ [warn]: This is confusing because it looks like an array index. Rewrite a[1] as [a + 1] to clarify. +/
add near ptr [EAX], 3;
}
}

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

@ -41,7 +41,7 @@ final class AssertWithoutMessageCheck : BaseAnalyzer
&& expr.assertArguments.message !is null;
if (!hasMessage)
addErrorMessage(expr.line, expr.column, KEY, MESSAGE);
addErrorMessage(expr, KEY, MESSAGE);
}
override void visit(const FunctionCallExpression expr)
@ -55,7 +55,7 @@ final class AssertWithoutMessageCheck : BaseAnalyzer
auto ident = iot.identifier;
if (ident.text == "enforce" && expr.arguments !is null && expr.arguments.namedArgumentList !is null &&
expr.arguments.namedArgumentList.items.length < 2)
addErrorMessage(ident.line, ident.column, KEY, MESSAGE);
addErrorMessage(expr, KEY, MESSAGE);
}
}
@ -112,7 +112,8 @@ unittest
assertAnalyzerWarnings(q{
unittest {
assert(0, "foo bar");
assert(0); // [warn]: %s
assert(0); /+
^^^^^^^^^ [warn]: %s +/
}
}c.format(
AssertWithoutMessageCheck.MESSAGE,
@ -121,7 +122,8 @@ unittest
assertAnalyzerWarnings(q{
unittest {
static assert(0, "foo bar");
static assert(0); // [warn]: %s
static assert(0); /+
^^^^^^^^^ [warn]: %s +/
}
}c.format(
AssertWithoutMessageCheck.MESSAGE,
@ -133,7 +135,8 @@ 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
enforce(0); /+
^^^^^^^^^^ [warn]: %s +/
}
}c.format(
AssertWithoutMessageCheck.MESSAGE,

72
src/dscanner/analysis/auto_function.d
View file

@ -11,7 +11,7 @@ import dparse.ast;
import dparse.lexer;
import std.stdio;
import std.algorithm.searching : any;
import std.algorithm : map, filter;
/**
* Checks for auto functions without return statement.
@ -26,7 +26,8 @@ final class AutoFunctionChecker : BaseAnalyzer
private:
enum string KEY = "dscanner.suspicious.missing_return";
enum string MESSAGE = "Auto function without return statement, prefer an explicit void";
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;
@ -44,19 +45,41 @@ public:
super(fileName, null, skipTests);
}
package static const(Token)[] findAutoReturnType(const(FunctionDeclaration) decl)
{
auto autoFunTokens = decl.storageClasses
.map!(a => a.token.type == tok!"auto"
? [a.token]
: a.atAttribute
? a.atAttribute.tokens
: null)
.filter!(a => a.length > 0);
return autoFunTokens.empty ? null : autoFunTokens.front;
}
override void visit(const(FunctionDeclaration) decl)
{
_returns.length += 1;
scope(exit) _returns.length -= 1;
_returns[$-1] = false;
const bool autoFun = decl.storageClasses
.any!(a => a.token.type == tok!"auto" || a.atAttribute !is null);
auto autoTokens = findAutoReturnType(decl);
bool isAtAttribute = autoTokens.length > 1;
decl.accept(this);
if (decl.functionBody.specifiedFunctionBody && autoFun && !_returns[$-1])
addErrorMessage(decl.name.line, decl.name.column, KEY, MESSAGE);
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);
addErrorMessage(tok.line, whitespace, whitespace + 1, KEY, MESSAGE_INSERT);
}
else
addErrorMessage(autoTokens, KEY, MESSAGE);
}
}
override void visit(const(ReturnStatement) rst)
@ -165,9 +188,12 @@ unittest
StaticAnalysisConfig sac = disabledConfig();
sac.auto_function_check = Check.enabled;
assertAnalyzerWarnings(q{
auto ref doStuff(){} // [warn]: %s
auto doStuff(){} // [warn]: %s
int doStuff(){auto doStuff(){}} // [warn]: %s
auto ref doStuff(){} /+
^^^^ [warn]: %s +/
auto doStuff(){} /+
^^^^ [warn]: %s +/
int doStuff(){auto doStuff(){}} /+
^^^^ [warn]: %s +/
auto doStuff(){return 0;}
int doStuff(){/*error but not the aim*/}
}c.format(
@ -177,55 +203,63 @@ unittest
), sac);
assertAnalyzerWarnings(q{
auto doStuff(){assert(true);} // [warn]: %s
auto doStuff(){assert(true);} /+
^^^^ [warn]: %s +/
auto doStuff(){assert(false);}
}c.format(
AutoFunctionChecker.MESSAGE,
), sac);
assertAnalyzerWarnings(q{
auto doStuff(){assert(1);} // [warn]: %s
auto doStuff(){assert(1);} /+
^^^^ [warn]: %s +/
auto doStuff(){assert(0);}
}c.format(
AutoFunctionChecker.MESSAGE,
), sac);
assertAnalyzerWarnings(q{
auto doStuff(){mixin("0+0");} // [warn]: %s
auto doStuff(){mixin("0+0");} /+
^^^^ [warn]: %s +/
auto doStuff(){mixin("return 0;");}
}c.format(
AutoFunctionChecker.MESSAGE,
), sac);
assertAnalyzerWarnings(q{
auto doStuff(){mixin("0+0");} // [warn]: %s
auto doStuff(){mixin("0+0");} /+
^^^^ [warn]: %s +/
auto doStuff(){mixin("static if (true)" ~ " return " ~ 0.stringof ~ ";");}
}c.format(
AutoFunctionChecker.MESSAGE,
), sac);
assertAnalyzerWarnings(q{
auto doStuff(){} // [warn]: %s
auto doStuff(){} /+
^^^^ [warn]: %s +/
extern(C) auto doStuff();
}c.format(
AutoFunctionChecker.MESSAGE,
), sac);
assertAnalyzerWarnings(q{
auto doStuff(){} // [warn]: %s
auto doStuff(){} /+
^^^^ [warn]: %s +/
@disable auto doStuff();
}c.format(
AutoFunctionChecker.MESSAGE,
), sac);
assertAnalyzerWarnings(q{
@property doStuff(){} // [warn]: %s
@safe doStuff(){} // [warn]: %s
@property doStuff(){} /+
^ [warn]: %s +/
@safe doStuff(){} /+
^ [warn]: %s +/
@disable doStuff();
@safe void doStuff();
}c.format(
AutoFunctionChecker.MESSAGE,
AutoFunctionChecker.MESSAGE,
AutoFunctionChecker.MESSAGE_INSERT,
AutoFunctionChecker.MESSAGE_INSERT,
), sac);
assertAnalyzerWarnings(q{

22
src/dscanner/analysis/auto_ref_assignment.d
View file

@ -54,29 +54,29 @@ final class AutoRefAssignmentCheck : BaseAnalyzer
{
if (assign.operator == tok!"" || scopes.length == 0)
return;
interest++;
interest ~= assign;
assign.ternaryExpression.accept(this);
interest--;
interest.length--;
}
override void visit(const IdentifierOrTemplateInstance ioti)
{
import std.algorithm.searching : canFind;
if (ioti.identifier == tok!"" || interest <= 0)
if (ioti.identifier == tok!"" || !interest.length)
return;
if (scopes[$ - 1].canFind(ioti.identifier.text))
addErrorMessage(ioti.identifier.line, ioti.identifier.column, KEY, MESSAGE);
addErrorMessage(interest[$ - 1], KEY, MESSAGE);
}
override void visit(const IdentifierChain ic)
{
import std.algorithm.searching : canFind;
if (ic.identifiers.length == 0 || interest <= 0)
if (ic.identifiers.length == 0 || !interest.length)
return;
if (scopes[$ - 1].canFind(ic.identifiers[0].text))
addErrorMessage(ic.identifiers[0].line, ic.identifiers[0].column, KEY, MESSAGE);
addErrorMessage(interest[$ - 1], KEY, MESSAGE);
}
alias visit = BaseAnalyzer.visit;
@ -86,12 +86,7 @@ private:
enum string MESSAGE = "Assignment to auto-ref function parameter.";
enum string KEY = "dscanner.suspicious.auto_ref_assignment";
invariant
{
assert(interest >= 0);
}
int interest;
const(AssignExpression)[] interest;
void addSymbol(string symbolName)
{
@ -123,7 +118,8 @@ unittest
assertAnalyzerWarnings(q{
int doStuff(T)(auto ref int a)
{
a = 10; // [warn]: %s
a = 10; /+
^^^^^^ [warn]: %s +/
}
int doStuff(T)(ref int a)

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

@ -5,24 +5,94 @@ import std.string;
import dparse.ast;
import std.array;
import dsymbol.scope_ : Scope;
import dparse.lexer : Token, str, IdType;
struct Message
{
/// Name of the file where the warning was triggered
struct Diagnostic
{
/// Name of the file where the warning was triggered.
string fileName;
/// Line number where the warning was triggered
size_t line;
/// Column number where the warning was triggered (in bytes)
size_t column;
/// Line number where the warning was triggered, 1-based.
size_t startLine, endLine;
/// Column number where the warning was triggered. (in bytes)
size_t startColumn, endColumn;
/// Warning message, may be null for supplemental diagnostics.
string message;
// TODO: add auto-fix suggestion API here
deprecated("Use startLine instead") alias line = startLine;
deprecated("Use startColumn instead") alias column = startColumn;
static Diagnostic from(string fileName, const BaseNode node, string message)
{
return from(fileName, node !is null ? node.tokens : [], message);
}
static Diagnostic from(string fileName, const Token token, string message)
{
auto text = token.text.length ? token.text : str(token.type);
return from(fileName, token.line, token.column, token.column + text.length, message);
}
static Diagnostic from(string fileName, const Token[] tokens, string message)
{
auto start = tokens.length ? tokens[0] : Token.init;
auto end = tokens.length ? tokens[$ - 1] : Token.init;
auto endText = end.text.length ? end.text : str(end.type);
return from(fileName, start.line, end.line, start.column, end.column + endText.length, message);
}
static Diagnostic from(string fileName, size_t line, size_t startColumn, size_t endColumn, string message)
{
return Message.Diagnostic(fileName, line, line, startColumn, endColumn, message);
}
static Diagnostic from(string fileName, size_t startLine, size_t endLine, size_t startColumn, size_t endColumn, string message)
{
return Message.Diagnostic(fileName, startLine, endLine, startColumn, endColumn, message);
}
}
/// Primary warning
Diagnostic diagnostic;
/// List of supplemental warnings / hint locations
Diagnostic[] supplemental;
/// Name of the warning
string key;
/// Warning message
string message;
/// Check name
string checkName;
deprecated this(string fileName, size_t line, size_t column, string key = null, string message = null, string checkName = null)
{
diagnostic.fileName = fileName;
diagnostic.startLine = diagnostic.endLine = line;
diagnostic.startColumn = diagnostic.endColumn = column;
diagnostic.message = message;
this.key = key;
this.checkName = checkName;
}
this(Diagnostic diagnostic, string key = null, string checkName = null)
{
this.diagnostic = diagnostic;
this.key = key;
this.checkName = checkName;
}
this(Diagnostic diagnostic, Diagnostic[] supplemental, string key = null, string checkName = null)
{
this.diagnostic = diagnostic;
this.supplemental = supplemental;
this.key = key;
this.checkName = checkName;
}
alias diagnostic this;
}
enum comparitor = q{ a.line < b.line || (a.line == b.line && a.column < b.column) };
enum comparitor = q{ a.startLine < b.startLine || (a.startLine == b.startLine && a.startColumn < b.startColumn) };
alias MessageSet = RedBlackTree!(Message, comparitor, true);
@ -86,11 +156,48 @@ protected:
}
}
deprecated("Use the overload taking start and end locations or a Node instead")
void addErrorMessage(size_t line, size_t column, string key, string message)
{
_messages.insert(Message(fileName, line, column, key, message, getName()));
}
void addErrorMessage(const BaseNode node, string key, string message)
{
addErrorMessage(Message.Diagnostic.from(fileName, node, message), key);
}
void addErrorMessage(const Token token, string key, string message)
{
addErrorMessage(Message.Diagnostic.from(fileName, token, message), key);
}
void addErrorMessage(const Token[] tokens, string key, string message)
{
addErrorMessage(Message.Diagnostic.from(fileName, tokens, message), key);
}
void addErrorMessage(size_t line, size_t startColumn, size_t endColumn, string key, string message)
{
addErrorMessage(line, line, startColumn, endColumn, key, message);
}
void addErrorMessage(size_t startLine, size_t endLine, size_t startColumn, size_t endColumn, string key, string message)
{
auto d = Message.Diagnostic(fileName, startLine, endLine, startColumn, endColumn, message);
_messages.insert(Message(d, key, getName()));
}
void addErrorMessage(Message.Diagnostic diagnostic, string key)
{
_messages.insert(Message(diagnostic, key, getName()));
}
void addErrorMessage(Message.Diagnostic diagnostic, Message.Diagnostic[] supplemental, string key)
{
_messages.insert(Message(diagnostic, supplemental, key, getName()));
}
/**
* The file name
*/
@ -100,3 +207,17 @@ protected:
MessageSet _messages;
}
/// Find the token with the given type, otherwise returns the whole range or a user-specified fallback, if set.
const(Token)[] findTokenForDisplay(const BaseNode node, IdType type, const(Token)[] fallback = null)
{
return node.tokens.findTokenForDisplay(type, fallback);
}
/// ditto
const(Token)[] findTokenForDisplay(const Token[] tokens, IdType type, const(Token)[] fallback = null)
{
foreach (i, token; tokens)
if (token.type == type)
return tokens[i .. i + 1];
return fallback is null ? tokens : fallback;
}

45
src/dscanner/analysis/body_on_disabled_funcs.d
View file

@ -83,7 +83,7 @@ private:
&& dec.functionDeclaration.functionBody !is null
&& dec.functionDeclaration.functionBody.missingFunctionBody is null)
{
addErrorMessage(dec.functionDeclaration.name.line, dec.functionDeclaration.name.column,
addErrorMessage(dec.functionDeclaration.functionBody,
KEY, "Function marked with '@disabled' should not have a body");
}
else if (dec.constructor !is null
@ -91,7 +91,7 @@ private:
&& dec.constructor.functionBody !is null
&& dec.constructor.functionBody.missingFunctionBody is null)
{
addErrorMessage(dec.constructor.line, dec.constructor.column,
addErrorMessage(dec.constructor.functionBody,
KEY, "Constructor marked with '@disabled' should not have a body");
}
else if (dec.destructor !is null
@ -99,7 +99,7 @@ private:
&& dec.destructor.functionBody !is null
&& dec.destructor.functionBody.missingFunctionBody is null)
{
addErrorMessage(dec.destructor.line, dec.destructor.column,
addErrorMessage(dec.destructor.functionBody,
KEY, "Destructor marked with '@disabled' should not have a body");
}
}
@ -159,9 +159,12 @@ unittest
}
}
this() {} // [warn]: Constructor marked with '@disabled' should not have a body
void doThing() {} // [warn]: Function marked with '@disabled' should not have a body
~this() {} // [warn]: Destructor marked with '@disabled' should not have a body
this() {} /+
^^ [warn]: Constructor marked with '@disabled' should not have a body +/
void doThing() {} /+
^^ [warn]: Function marked with '@disabled' should not have a body +/
~this() {} /+
^^ [warn]: Destructor marked with '@disabled' should not have a body +/
this();
void doThing();
@ -170,18 +173,28 @@ unittest
class C2
{
@disable this() {} // [warn]: Constructor marked with '@disabled' should not have a body
@disable { this() {} } // [warn]: Constructor marked with '@disabled' should not have a body
this() @disable {} // [warn]: Constructor marked with '@disabled' should not have a body
@disable this() {} /+
^^ [warn]: Constructor marked with '@disabled' should not have a body +/
@disable { this() {} } /+
^^ [warn]: Constructor marked with '@disabled' should not have a body +/
this() @disable {} /+
^^ [warn]: Constructor marked with '@disabled' should not have a body +/
@disable void doThing() {} // [warn]: Function marked with '@disabled' should not have a body
@disable doThing() {} // [warn]: Function marked with '@disabled' should not have a body
@disable { void doThing() {} } // [warn]: Function marked with '@disabled' should not have a body
void doThing() @disable {} // [warn]: Function marked with '@disabled' should not have a body
@disable void doThing() {} /+
^^ [warn]: Function marked with '@disabled' should not have a body +/
@disable doThing() {} /+
^^ [warn]: Function marked with '@disabled' should not have a body +/
@disable { void doThing() {} } /+
^^ [warn]: Function marked with '@disabled' should not have a body +/
void doThing() @disable {} /+
^^ [warn]: Function marked with '@disabled' should not have a body +/
@disable ~this() {} // [warn]: Destructor marked with '@disabled' should not have a body
@disable { ~this() {} } // [warn]: Destructor marked with '@disabled' should not have a body
~this() @disable {} // [warn]: Destructor marked with '@disabled' should not have a body
@disable ~this() {} /+
^^ [warn]: Destructor marked with '@disabled' should not have a body +/
@disable { ~this() {} } /+
^^ [warn]: Destructor marked with '@disabled' should not have a body +/
~this() @disable {} /+
^^ [warn]: Destructor marked with '@disabled' should not have a body +/
@disable this();
@disable { this(); }

15
src/dscanner/analysis/builtin_property_names.d
View file

@ -42,7 +42,7 @@ final class BuiltinPropertyNameCheck : BaseAnalyzer
{
if (depth > 0 && isBuiltinProperty(fd.name.text))
{
addErrorMessage(fd.name.line, fd.name.column, KEY, generateErrorMessage(fd.name.text));
addErrorMessage(fd.name, KEY, generateErrorMessage(fd.name.text));
}
fd.accept(this);
}
@ -62,14 +62,14 @@ final class BuiltinPropertyNameCheck : BaseAnalyzer
foreach (i; ad.parts.map!(a => a.identifier))
{
if (isBuiltinProperty(i.text))
addErrorMessage(i.line, i.column, KEY, generateErrorMessage(i.text));
addErrorMessage(i, KEY, generateErrorMessage(i.text));
}
}
override void visit(const Declarator d)
{
if (depth > 0 && isBuiltinProperty(d.name.text))
addErrorMessage(d.name.line, d.name.column, KEY, generateErrorMessage(d.name.text));
addErrorMessage(d.name, KEY, generateErrorMessage(d.name.text));
}
override void visit(const StructBody sb)
@ -111,9 +111,12 @@ unittest
assertAnalyzerWarnings(q{
class SomeClass
{
void init(); // [warn]: Avoid naming members 'init'. This can confuse code that depends on the '.init' property of a type.
int init; // [warn]: Avoid naming members 'init'. This can confuse code that depends on the '.init' property of a type.
auto init = 10; // [warn]: Avoid naming members 'init'. This can confuse code that depends on the '.init' property of a type.
void init(); /+
^^^^ [warn]: Avoid naming members 'init'. This can confuse code that depends on the '.init' property of a type. +/
int init; /+
^^^^ [warn]: Avoid naming members 'init'. This can confuse code that depends on the '.init' property of a type. +/
auto init = 10; /+
^^^^ [warn]: Avoid naming members 'init'. This can confuse code that depends on the '.init' property of a type. +/
}
}c, sac);

2
src/dscanner/analysis/comma_expression.d
View file

@ -28,7 +28,7 @@ final class CommaExpressionCheck : BaseAnalyzer
{
if (ex.items.length > 1 && interest > 0)
{
addErrorMessage(ex.line, ex.column, KEY, "Avoid using the comma expression.");
addErrorMessage(ex, KEY, "Avoid using the comma expression.");
}
ex.accept(this);
}

42
src/dscanner/analysis/constructors.d
View file

@ -3,6 +3,7 @@ module dscanner.analysis.constructors;
import dparse.ast;
import dparse.lexer;
import std.stdio;
import std.typecons : Rebindable;
import dscanner.analysis.base;
import dscanner.analysis.helpers;
import dsymbol.scope_ : Scope;
@ -20,19 +21,25 @@ final class ConstructorCheck : BaseAnalyzer
override void visit(const ClassDeclaration classDeclaration)
{
immutable bool oldHasDefault = hasDefaultArgConstructor;
immutable bool oldHasNoArg = hasNoArgConstructor;
hasNoArgConstructor = false;
hasDefaultArgConstructor = false;
const oldHasDefault = hasDefaultArgConstructor;
const oldHasNoArg = hasNoArgConstructor;
hasNoArgConstructor = null;
hasDefaultArgConstructor = null;
immutable State prev = state;
state = State.inClass;
classDeclaration.accept(this);
if (hasNoArgConstructor && hasDefaultArgConstructor)
{
addErrorMessage(classDeclaration.name.line,
classDeclaration.name.column, "dscanner.confusing.constructor_args",
addErrorMessage(
Message.Diagnostic.from(fileName, classDeclaration.name,
"This class has a zero-argument constructor as well as a"
~ " constructor with one default argument. This can be confusing.");
~ " constructor with one default argument. This can be confusing."),
[
Message.Diagnostic.from(fileName, hasNoArgConstructor, "zero-argument constructor defined here"),
Message.Diagnostic.from(fileName, hasDefaultArgConstructor, "default argument constructor defined here")
],
"dscanner.confusing.constructor_args"
);
}
hasDefaultArgConstructor = oldHasDefault;
hasNoArgConstructor = oldHasNoArg;
@ -55,7 +62,11 @@ final class ConstructorCheck : BaseAnalyzer
if (constructor.parameters.parameters.length == 1
&& constructor.parameters.parameters[0].default_ !is null)
{
addErrorMessage(constructor.line, constructor.column,
const(Token)[] tokens = constructor.parameters.parameters[0].default_.tokens;
assert(tokens.length);
// we extend the token range to the `=` sign, since it's continuous
tokens = (tokens.ptr - 1)[0 .. tokens.length + 1];
addErrorMessage(tokens,
"dscanner.confusing.struct_constructor_default_args",
"This struct constructor can never be called with its "
~ "default argument.");
@ -65,10 +76,10 @@ final class ConstructorCheck : BaseAnalyzer
if (constructor.parameters.parameters.length == 1
&& constructor.parameters.parameters[0].default_ !is null)
{
hasDefaultArgConstructor = true;
hasDefaultArgConstructor = constructor;
}
else if (constructor.parameters.parameters.length == 0)
hasNoArgConstructor = true;
hasNoArgConstructor = constructor;
break;
case State.ignoring:
break;
@ -86,8 +97,8 @@ private:
State state;
bool hasNoArgConstructor;
bool hasDefaultArgConstructor;
Rebindable!(const Constructor) hasNoArgConstructor;
Rebindable!(const Constructor) hasDefaultArgConstructor;
}
unittest
@ -96,8 +107,10 @@ unittest
StaticAnalysisConfig sac = disabledConfig();
sac.constructor_check = Check.enabled;
// TODO: test supplemental diagnostics
assertAnalyzerWarnings(q{
class Cat // [warn]: This class has a zero-argument constructor as well as a constructor with one default argument. This can be confusing.
class Cat /+
^^^ [warn]: This class has a zero-argument constructor as well as a constructor with one default argument. This can be confusing. +/
{
this() {}
this(string name = "kittie") {}
@ -106,7 +119,8 @@ unittest
struct Dog
{
this() {}
this(string name = "doggie") {} // [warn]: This struct constructor can never be called with its default argument.
this(string name = "doggie") {} /+
^^^^^^^^^^ [warn]: This struct constructor can never be called with its default argument. +/
}
}c, sac);

32
src/dscanner/analysis/cyclomatic_complexity.d
View file

@ -105,7 +105,7 @@ final class CyclomaticComplexityCheck : BaseAnalyzer
inLoop.length--;
}
fun.functionBody.accept(this);
testComplexity(fun.name.line, fun.name.column);
testComplexity(fun.name);
}
override void visit(const Unittest unittest_)
@ -120,7 +120,7 @@ final class CyclomaticComplexityCheck : BaseAnalyzer
inLoop.length--;
}
unittest_.accept(this);
testComplexity(unittest_.line, unittest_.column);
testComplexity(unittest_.findTokenForDisplay(tok!"unittest"));
}
}
@ -129,12 +129,12 @@ private:
int[] complexityStack = [0];
bool[] inLoop = [false];
void testComplexity(size_t line, size_t column)
void testComplexity(T)(T annotatable)
{
auto complexity = complexityStack[$ - 1];
if (complexity > maxCyclomaticComplexity)
{
addErrorMessage(line, column, KEY, format!MESSAGE(complexity));
addErrorMessage(annotatable, KEY, format!MESSAGE(complexity));
}
}
@ -169,24 +169,28 @@ unittest
sac.cyclomatic_complexity = Check.enabled;
sac.max_cyclomatic_complexity = 0;
assertAnalyzerWarnings(q{
unittest // [warn]: Cyclomatic complexity of this function is 1.
unittest /+
^^^^^^^^ [warn]: Cyclomatic complexity of this function is 1. +/
{
}
unittest // [warn]: Cyclomatic complexity of this function is 1.
unittest /+
^^^^^^^^ [warn]: Cyclomatic complexity of this function is 1. +/
{
writeln("hello");
writeln("world");
}
void main(string[] args) // [warn]: Cyclomatic complexity of this function is 3.
void main(string[] args) /+
^^^^ [warn]: Cyclomatic complexity of this function is 3. +/
{
if (!args.length)
return;
writeln("hello ", args);
}
unittest // [warn]: Cyclomatic complexity of this function is 1.
unittest /+
^^^^^^^^ [warn]: Cyclomatic complexity of this function is 1. +/
{
// static if / static foreach does not increase cyclomatic complexity
static if (stuff)
@ -194,7 +198,8 @@ unittest // [warn]: Cyclomatic complexity of this function is 1.
int a;
}
unittest // [warn]: Cyclomatic complexity of this function is 2.
unittest /+
^^^^^^^^ [warn]: Cyclomatic complexity of this function is 2. +/
{
foreach (i; 0 .. 2)
{
@ -202,7 +207,8 @@ unittest // [warn]: Cyclomatic complexity of this function is 2.
int a;
}
unittest // [warn]: Cyclomatic complexity of this function is 3.
unittest /+
^^^^^^^^ [warn]: Cyclomatic complexity of this function is 3. +/
{
foreach (i; 0 .. 2)
{
@ -211,7 +217,8 @@ unittest // [warn]: Cyclomatic complexity of this function is 3.
int a;
}
unittest // [warn]: Cyclomatic complexity of this function is 2.
unittest /+
^^^^^^^^ [warn]: Cyclomatic complexity of this function is 2. +/
{
switch (x)
{
@ -223,7 +230,8 @@ unittest // [warn]: Cyclomatic complexity of this function is 2.
int a;
}
bool shouldRun(check : BaseAnalyzer)( // [warn]: Cyclomatic complexity of this function is 20.
bool shouldRun(check : BaseAnalyzer)( /+
^^^^^^^^^ [warn]: Cyclomatic complexity of this function is 20. +/
string moduleName, const ref StaticAnalysisConfig config)
{
enum string a = check.name;

8
src/dscanner/analysis/del.d
View file

@ -27,7 +27,7 @@ final class DeleteCheck : BaseAnalyzer
override void visit(const DeleteExpression d)
{
addErrorMessage(d.line, d.column, "dscanner.deprecated.delete_keyword",
addErrorMessage(d.tokens[0], "dscanner.deprecated.delete_keyword",
"Avoid using the 'delete' keyword.");
d.accept(this);
}
@ -44,10 +44,12 @@ unittest
void testDelete()
{
int[int] data = [1 : 2];
delete data[1]; // [warn]: Avoid using the 'delete' keyword.
delete data[1]; /+
^^^^^^ [warn]: Avoid using the 'delete' keyword. +/
auto a = new Class();
delete a; // [warn]: Avoid using the 'delete' keyword.
delete a; /+
^^^^^^ [warn]: Avoid using the 'delete' keyword. +/
}
}c, sac);

81
src/dscanner/analysis/duplicate_attribute.d
View file

@ -46,18 +46,18 @@ final class DuplicateAttributeCheck : BaseAnalyzer
// Check the attributes
foreach (attribute; node.attributes)
{
size_t line, column;
string attributeName = getAttributeName(attribute, line, column);
if (!attributeName || line == 0 || column == 0)
const(Token)[] tokens;
string attributeName = getAttributeName(attribute, tokens);
if (!attributeName || !tokens.length)
return;
// Check for the attributes
checkDuplicateAttribute(attributeName, "property", line, column, hasProperty);
checkDuplicateAttribute(attributeName, "safe", line, column, hasSafe);
checkDuplicateAttribute(attributeName, "trusted", line, column, hasTrusted);
checkDuplicateAttribute(attributeName, "system", line, column, hasSystem);
checkDuplicateAttribute(attributeName, "pure", line, column, hasPure);
checkDuplicateAttribute(attributeName, "nothrow", line, column, hasNoThrow);
checkDuplicateAttribute(attributeName, "property", tokens, hasProperty);
checkDuplicateAttribute(attributeName, "safe", tokens, hasSafe);
checkDuplicateAttribute(attributeName, "trusted", tokens, hasTrusted);
checkDuplicateAttribute(attributeName, "system", tokens, hasSystem);
checkDuplicateAttribute(attributeName, "pure", tokens, hasPure);
checkDuplicateAttribute(attributeName, "nothrow", tokens, hasNoThrow);
}
// Just return if missing function nodes
@ -67,23 +67,23 @@ final class DuplicateAttributeCheck : BaseAnalyzer
// Check the functions
foreach (memberFunctionAttribute; node.functionDeclaration.memberFunctionAttributes)
{
size_t line, column;
string attributeName = getAttributeName(memberFunctionAttribute, line, column);
if (!attributeName || line == 0 || column == 0)
const(Token)[] tokens;
string attributeName = getAttributeName(memberFunctionAttribute, tokens);
if (!attributeName || !tokens.length)
return;
// Check for the attributes
checkDuplicateAttribute(attributeName, "property", line, column, hasProperty);
checkDuplicateAttribute(attributeName, "safe", line, column, hasSafe);
checkDuplicateAttribute(attributeName, "trusted", line, column, hasTrusted);
checkDuplicateAttribute(attributeName, "system", line, column, hasSystem);
checkDuplicateAttribute(attributeName, "pure", line, column, hasPure);
checkDuplicateAttribute(attributeName, "nothrow", line, column, hasNoThrow);
checkDuplicateAttribute(attributeName, "property", tokens, hasProperty);
checkDuplicateAttribute(attributeName, "safe", tokens, hasSafe);
checkDuplicateAttribute(attributeName, "trusted", tokens, hasTrusted);
checkDuplicateAttribute(attributeName, "system", tokens, hasSystem);
checkDuplicateAttribute(attributeName, "pure", tokens, hasPure);
checkDuplicateAttribute(attributeName, "nothrow", tokens, hasNoThrow);
}
}
void checkDuplicateAttribute(const string attributeName,
const string attributeDesired, size_t line, size_t column, ref bool hasAttribute)
const string attributeDesired, const(Token)[] tokens, ref bool hasAttribute)
{
// Just return if not an attribute
if (attributeName != attributeDesired)
@ -93,14 +93,14 @@ final class DuplicateAttributeCheck : BaseAnalyzer
if (hasAttribute)
{
string message = "Attribute '%s' is duplicated.".format(attributeName);
addErrorMessage(line, column, "dscanner.unnecessary.duplicate_attribute", message);
addErrorMessage(tokens, "dscanner.unnecessary.duplicate_attribute", message);
}
// Mark it as having that attribute
hasAttribute = true;
}
string getAttributeName(const Attribute attribute, ref size_t line, ref size_t column)
string getAttributeName(const Attribute attribute, ref const(Token)[] outTokens)
{
// Get the name from the attribute identifier
if (attribute && attribute.atAttribute && attribute.atAttribute.identifier !is Token.init
@ -108,16 +108,14 @@ final class DuplicateAttributeCheck : BaseAnalyzer
&& attribute.atAttribute.identifier.text.length)
{
auto token = attribute.atAttribute.identifier;
line = token.line;
column = token.column;
outTokens = attribute.atAttribute.tokens;
return token.text;
}
// Get the attribute from the storage class token
if (attribute && attribute.attribute.type != tok!"")
{
line = attribute.attribute.line;
column = attribute.attribute.column;
outTokens = attribute.tokens;
return attribute.attribute.type.str;
}
@ -125,14 +123,14 @@ final class DuplicateAttributeCheck : BaseAnalyzer
}
string getAttributeName(const MemberFunctionAttribute memberFunctionAttribute,
ref size_t line, ref size_t column)
ref const(Token)[] outTokens)
{
// Get the name from the tokenType
if (memberFunctionAttribute && memberFunctionAttribute.tokenType !is IdType.init
&& memberFunctionAttribute.tokenType.str
&& memberFunctionAttribute.tokenType.str.length)
{
// FIXME: How do we get the line/column number?
outTokens = memberFunctionAttribute.tokens;
return memberFunctionAttribute.tokenType.str;
}
@ -144,8 +142,7 @@ final class DuplicateAttributeCheck : BaseAnalyzer
&& memberFunctionAttribute.atAttribute.identifier.text.length)
{
auto iden = memberFunctionAttribute.atAttribute.identifier;
line = iden.line;
column = iden.column;
outTokens = memberFunctionAttribute.atAttribute.tokens;
return iden.text;
}
@ -168,25 +165,29 @@ unittest
}
// Duplicate before
@property @property bool aaa() // [warn]: Attribute 'property' is duplicated.
@property @property bool aaa() /+
^^^^^^^^^ [warn]: Attribute 'property' is duplicated. +/
{
return false;
}
// Duplicate after
bool bbb() @safe @safe // [warn]: Attribute 'safe' is duplicated.
bool bbb() @safe @safe /+
^^^^^ [warn]: Attribute 'safe' is duplicated. +/
{
return false;
}
// Duplicate before and after
@system bool ccc() @system // [warn]: Attribute 'system' is duplicated.
@system bool ccc() @system /+
^^^^^^^ [warn]: Attribute 'system' is duplicated. +/
{
return false;
}
// Duplicate before and after
@trusted bool ddd() @trusted // [warn]: Attribute 'trusted' is duplicated.
@trusted bool ddd() @trusted /+
^^^^^^^^ [warn]: Attribute 'trusted' is duplicated. +/
{
return false;
}
@ -199,24 +200,26 @@ unittest
return false;
}
pure pure bool bbb() // [warn]: Attribute 'pure' is duplicated.
pure pure bool bbb() /+
^^^^ [warn]: Attribute 'pure' is duplicated. +/
{
return false;
}
// FIXME: There is no way to get the line/column number of the attribute like this
bool ccc() pure pure // FIXME: [warn]: Attribute 'pure' is duplicated.
bool ccc() pure pure /+
^^^^ [warn]: Attribute 'pure' is duplicated. +/
{
return false;
}
nothrow nothrow bool ddd() // [warn]: Attribute 'nothrow' is duplicated.
nothrow nothrow bool ddd() /+
^^^^^^^ [warn]: Attribute 'nothrow' is duplicated. +/
{
return false;
}
// FIXME: There is no way to get the line/column number of the attribute like this
bool eee() nothrow nothrow // FIXME: [warn]: Attribute 'nothrow' is duplicated.
bool eee() nothrow nothrow /+
^^^^^^^ [warn]: Attribute 'nothrow' is duplicated. +/
{
return false;
}

2
src/dscanner/analysis/enumarrayliteral.d
View file

@ -45,7 +45,7 @@ final class EnumArrayLiteralCheck : BaseAnalyzer
continue;
if (part.initializer.nonVoidInitializer.arrayInitializer is null)
continue;
addErrorMessage(part.identifier.line, part.identifier.column,
addErrorMessage(part.initializer.nonVoidInitializer,
"dscanner.performance.enum_array_literal",
"This enum may lead to unnecessary allocation at run-time."
~ " Use 'static immutable "

14
src/dscanner/analysis/explicitly_annotated_unittests.d
View file

@ -44,7 +44,7 @@ final class ExplicitlyAnnotatedUnittestCheck : BaseAnalyzer
}
}
if (!isSafeOrSystem)
addErrorMessage(decl.unittest_.line, decl.unittest_.column, KEY, MESSAGE);
addErrorMessage(decl.unittest_.findTokenForDisplay(tok!"unittest"), KEY, MESSAGE);
}
decl.accept(this);
}
@ -68,8 +68,10 @@ unittest
@system unittest {}
pure nothrow @system @nogc unittest {}
unittest {} // [warn]: %s
pure nothrow @nogc unittest {} // [warn]: %s
unittest {} /+
^^^^^^^^ [warn]: %s +/
pure nothrow @nogc unittest {} /+
^^^^^^^^ [warn]: %s +/
}c.format(
ExplicitlyAnnotatedUnittestCheck.MESSAGE,
ExplicitlyAnnotatedUnittestCheck.MESSAGE,
@ -82,8 +84,10 @@ unittest
@safe unittest {}
@system unittest {}
unittest {} // [warn]: %s
pure nothrow @nogc unittest {} // [warn]: %s
unittest {} /+
^^^^^^^^ [warn]: %s +/
pure nothrow @nogc unittest {} /+
^^^^^^^^ [warn]: %s +/
}
}c.format(
ExplicitlyAnnotatedUnittestCheck.MESSAGE,

72
src/dscanner/analysis/final_attribute.d
View file

@ -54,12 +54,10 @@ private:
bool _blockStatic;
Parent _parent = Parent.module_;
void addError(T)(T t, string msg)
void addError(T)(const Token finalToken, T t, string msg)
{
import std.format : format;
const size_t lne = t.name.line;
const size_t col = t.name.column;
addErrorMessage(lne, col, KEY, MSGB.format(msg));
addErrorMessage(finalToken.type ? finalToken : t.name, KEY, MSGB.format(msg));
}
public:
@ -178,6 +176,11 @@ public:
const bool isFinal = d.attributes
.canFind!(a => a.attribute.type == tok!"final");
const Token finalToken = isFinal
? d.attributes
.filter!(a => a.attribute.type == tok!"final")
.front.attribute
: Token.init;
const bool isStaticOnce = d.attributes
.canFind!(a => a.attribute.type == tok!"static");
@ -203,9 +206,9 @@ public:
if (_finalAggregate && savedParent == Parent.module_)
{
if (d.structDeclaration)
addError(d.structDeclaration, MESSAGE.struct_i);
addError(finalToken, d.structDeclaration, MESSAGE.struct_i);
else if (d.unionDeclaration)
addError(d.unionDeclaration, MESSAGE.union_i);
addError(finalToken, d.unionDeclaration, MESSAGE.union_i);
}
}
@ -220,29 +223,29 @@ public:
{
case Parent.class_:
if (fd.templateParameters)
addError(fd, MESSAGE.class_t);
addError(finalToken, fd, MESSAGE.class_t);
if (isPrivate)
addError(fd, MESSAGE.class_p);
addError(finalToken, fd, MESSAGE.class_p);
else if (isStaticOnce || _alwaysStatic || _blockStatic)
addError(fd, MESSAGE.class_s);
addError(finalToken, fd, MESSAGE.class_s);
else if (_finalAggregate)
addError(fd, MESSAGE.class_f);
addError(finalToken, fd, MESSAGE.class_f);
break;
case Parent.interface_:
if (fd.templateParameters)
addError(fd, MESSAGE.interface_t);
addError(finalToken, fd, MESSAGE.interface_t);
break;
case Parent.struct_:
addError(fd, MESSAGE.struct_f);
addError(finalToken, fd, MESSAGE.struct_f);
break;
case Parent.union_:
addError(fd, MESSAGE.union_f);
addError(finalToken, fd, MESSAGE.union_f);
break;
case Parent.function_:
addError(fd, MESSAGE.func_n);
addError(finalToken, fd, MESSAGE.func_n);
break;
case Parent.module_:
addError(fd, MESSAGE.func_g);
addError(finalToken, fd, MESSAGE.func_g);
break;
}
}
@ -317,13 +320,15 @@ public:
// fail
assertAnalyzerWarnings(q{
final void foo(){} // [warn]: %s
final void foo(){} /+
^^^^^ [warn]: %s +/
}c.format(
FinalAttributeChecker.MSGB.format(FinalAttributeChecker.MESSAGE.func_g)
), sac);
assertAnalyzerWarnings(q{
void foo(){final void foo(){}} // [warn]: %s
void foo(){final void foo(){}} /+
^^^^^ [warn]: %s +/
}c.format(
FinalAttributeChecker.MSGB.format(FinalAttributeChecker.MESSAGE.func_n)
), sac);
@ -332,56 +337,65 @@ public:
void foo()
{
static if (true)
final class A{ private: final protected void foo(){}} // [warn]: %s
final class A{ private: final protected void foo(){}} /+
^^^^^ [warn]: %s +/
}
}c.format(
FinalAttributeChecker.MSGB.format(FinalAttributeChecker.MESSAGE.class_f)
), sac);
assertAnalyzerWarnings(q{
final struct Foo{} // [warn]: %s
final struct Foo{} /+
^^^^^ [warn]: %s +/
}c.format(
FinalAttributeChecker.MSGB.format(FinalAttributeChecker.MESSAGE.struct_i)
), sac);
assertAnalyzerWarnings(q{
final union Foo{} // [warn]: %s
final union Foo{} /+
^^^^^ [warn]: %s +/
}c.format(
FinalAttributeChecker.MSGB.format(FinalAttributeChecker.MESSAGE.union_i)
), sac);
assertAnalyzerWarnings(q{
class Foo{private final void foo(){}} // [warn]: %s
class Foo{private final void foo(){}} /+
^^^^^ [warn]: %s +/
}c.format(
FinalAttributeChecker.MSGB.format(FinalAttributeChecker.MESSAGE.class_p)
), sac);
assertAnalyzerWarnings(q{
class Foo{private: final void foo(){}} // [warn]: %s
class Foo{private: final void foo(){}} /+
^^^^^ [warn]: %s +/
}c.format(
FinalAttributeChecker.MSGB.format(FinalAttributeChecker.MESSAGE.class_p)
), sac);
assertAnalyzerWarnings(q{
interface Foo{final void foo(T)(){}} // [warn]: %s
interface Foo{final void foo(T)(){}} /+
^^^^^ [warn]: %s +/
}c.format(
FinalAttributeChecker.MSGB.format(FinalAttributeChecker.MESSAGE.interface_t)
), sac);
assertAnalyzerWarnings(q{
final class Foo{final void foo(){}} // [warn]: %s
final class Foo{final void foo(){}} /+
^^^^^ [warn]: %s +/
}c.format(
FinalAttributeChecker.MSGB.format(FinalAttributeChecker.MESSAGE.class_f)
), sac);
assertAnalyzerWarnings(q{
private: final class Foo {public: private final void foo(){}} // [warn]: %s
private: final class Foo {public: private final void foo(){}} /+
^^^^^ [warn]: %s +/
}c.format(
FinalAttributeChecker.MSGB.format(FinalAttributeChecker.MESSAGE.class_p)
), sac);
assertAnalyzerWarnings(q{
class Foo {final static void foo(){}} // [warn]: %s
class Foo {final static void foo(){}} /+
^^^^^ [warn]: %s +/
}c.format(
FinalAttributeChecker.MSGB.format(FinalAttributeChecker.MESSAGE.class_s)
), sac);
@ -390,7 +404,8 @@ public:
class Foo
{
void foo(){}
static: final void foo(){} // [warn]: %s
static: final void foo(){} /+
^^^^^ [warn]: %s +/
}
}c.format(
FinalAttributeChecker.MSGB.format(FinalAttributeChecker.MESSAGE.class_s)
@ -400,7 +415,8 @@ public:
class Foo
{
void foo(){}
static{ final void foo(){}} // [warn]: %s
static{ final void foo(){}} /+
^^^^^ [warn]: %s +/
void foo(){}
}
}c.format(

26
src/dscanner/analysis/fish.d
View file

@ -34,7 +34,7 @@ final class FloatOperatorCheck : BaseAnalyzer
|| r.operator == tok!"!<" || r.operator == tok!"!<>="
|| r.operator == tok!"!>=" || r.operator == tok!"!<=")
{
addErrorMessage(r.line, r.column, KEY,
addErrorMessage(r, KEY,
"Avoid using the deprecated floating-point operators.");
}
r.accept(this);
@ -52,14 +52,22 @@ unittest
{
float z = 1.5f;
bool a;
a = z !<>= z; // [warn]: Avoid using the deprecated floating-point operators.
a = z !<> z; // [warn]: Avoid using the deprecated floating-point operators.
a = z <> z; // [warn]: Avoid using the deprecated floating-point operators.
a = z <>= z; // [warn]: Avoid using the deprecated floating-point operators.
a = z !> z; // [warn]: Avoid using the deprecated floating-point operators.
a = z !>= z; // [warn]: Avoid using the deprecated floating-point operators.
a = z !< z; // [warn]: Avoid using the deprecated floating-point operators.
a = z !<= z; // [warn]: Avoid using the deprecated floating-point operators.
a = z !<>= z; /+
^^^^^^^^ [warn]: Avoid using the deprecated floating-point operators. +/
a = z !<> z; /+
^^^^^^^ [warn]: Avoid using the deprecated floating-point operators. +/
a = z <> z; /+
^^^^^^ [warn]: Avoid using the deprecated floating-point operators. +/
a = z <>= z; /+
^^^^^^^ [warn]: Avoid using the deprecated floating-point operators. +/
a = z !> z; /+
^^^^^^ [warn]: Avoid using the deprecated floating-point operators. +/
a = z !>= z; /+
^^^^^^^ [warn]: Avoid using the deprecated floating-point operators. +/
a = z !< z; /+
^^^^^^ [warn]: Avoid using the deprecated floating-point operators. +/
a = z !<= z; /+
^^^^^^^ [warn]: Avoid using the deprecated floating-point operators. +/
}
}c, sac);

28
src/dscanner/analysis/function_attributes.d
View file

@ -81,8 +81,7 @@ final class FunctionAttributeCheck : BaseAnalyzer
{
if (inInterface && dec.attribute.attribute == tok!"abstract")
{
addErrorMessage(dec.attribute.attribute.line,
dec.attribute.attribute.column, KEY, ABSTRACT_MESSAGE);
addErrorMessage(dec.attribute, KEY, ABSTRACT_MESSAGE);
}
}
@ -105,7 +104,7 @@ final class FunctionAttributeCheck : BaseAnalyzer
}
if (foundProperty && !foundConst)
{
addErrorMessage(dec.name.line, dec.name.column, KEY,
addErrorMessage(dec.name, KEY,
"Zero-parameter '@property' function should be"
~ " marked 'const', 'inout', or 'immutable'.");
}
@ -124,7 +123,7 @@ final class FunctionAttributeCheck : BaseAnalyzer
continue;
if (attr.attribute == tok!"abstract" && inInterface)
{
addErrorMessage(attr.attribute.line, attr.attribute.column, KEY, ABSTRACT_MESSAGE);
addErrorMessage(attr.attribute, KEY, ABSTRACT_MESSAGE);
continue;
}
if (attr.attribute == tok!"static")
@ -137,8 +136,7 @@ final class FunctionAttributeCheck : BaseAnalyzer
import std.string : format;
immutable string attrString = str(attr.attribute.type);
addErrorMessage(dec.functionDeclaration.name.line,
dec.functionDeclaration.name.column, KEY, format(
addErrorMessage(attr.attribute, KEY, format(
"'%s' is not an attribute of the return type." ~ " Place it after the parameter list to clarify.",
attrString));
}
@ -172,31 +170,37 @@ unittest
int foo() @property { return 0; }
class ClassName {
const int confusingConst() { return 0; } // [warn]: 'const' is not an attribute of the return type. Place it after the parameter list to clarify.
const int confusingConst() { return 0; } /+
^^^^^ [warn]: 'const' is not an attribute of the return type. Place it after the parameter list to clarify. +/
int bar() @property { return 0; } // [warn]: Zero-parameter '@property' function should be marked 'const', 'inout', or 'immutable'.
int bar() @property { return 0; } /+
^^^ [warn]: Zero-parameter '@property' function should be marked 'const', 'inout', or 'immutable'. +/
static int barStatic() @property { return 0; }
int barConst() const @property { return 0; }
}
struct StructName {
int bar() @property { return 0; } // [warn]: Zero-parameter '@property' function should be marked 'const', 'inout', or 'immutable'.
int bar() @property { return 0; } /+
^^^ [warn]: Zero-parameter '@property' function should be marked 'const', 'inout', or 'immutable'. +/
static int barStatic() @property { return 0; }
int barConst() const @property { return 0; }
}
union UnionName {
int bar() @property { return 0; } // [warn]: Zero-parameter '@property' function should be marked 'const', 'inout', or 'immutable'.
int bar() @property { return 0; } /+
^^^ [warn]: Zero-parameter '@property' function should be marked 'const', 'inout', or 'immutable'. +/
static int barStatic() @property { return 0; }
int barConst() const @property { return 0; }
}
interface InterfaceName {
int bar() @property; // [warn]: Zero-parameter '@property' function should be marked 'const', 'inout', or 'immutable'.
int bar() @property; /+
^^^ [warn]: Zero-parameter '@property' function should be marked 'const', 'inout', or 'immutable'. +/
static int barStatic() @property { return 0; }
int barConst() const @property;
abstract int method(); // [warn]: 'abstract' attribute is redundant in interface declarations
abstract int method(); /+
^^^^^^^^ [warn]: 'abstract' attribute is redundant in interface declarations +/
}
}c, sac);

36
src/dscanner/analysis/has_public_example.d
View file

@ -157,14 +157,14 @@ private:
{
foreach (property; possibleDeclarations)
if (auto fn = mixin("decl." ~ property))
addMessage(fn.name.line, fn.name.column, fn.name.text);
addMessage(fn.name.type ? [fn.name] : fn.tokens, fn.name.text);
}
void addMessage(size_t line, size_t column, string name)
void addMessage(const Token[] tokens, string name)
{
import std.string : format;
addErrorMessage(line, column, "dscanner.style.has_public_example", name is null
addErrorMessage(tokens, "dscanner.style.has_public_example", name is null
? "Public declaration has no documented example."
: format("Public declaration '%s' has no documented example.", name));
}
@ -220,27 +220,33 @@ unittest
// enums or variables don't need to have public unittest
assertAnalyzerWarnings(q{
/// C
class C{} // [warn]: Public declaration 'C' has no documented example.
class C{} /+
^ [warn]: Public declaration 'C' has no documented example. +/
unittest {}
/// I
interface I{} // [warn]: Public declaration 'I' has no documented example.
interface I{} /+
^ [warn]: Public declaration 'I' has no documented example. +/
unittest {}
/// f
void f(){} // [warn]: Public declaration 'f' has no documented example.
void f(){} /+
^ [warn]: Public declaration 'f' has no documented example. +/
unittest {}
/// S
struct S{} // [warn]: Public declaration 'S' has no documented example.
struct S{} /+
^ [warn]: Public declaration 'S' has no documented example. +/
unittest {}
/// T
template T(){} // [warn]: Public declaration 'T' has no documented example.
template T(){} /+
^ [warn]: Public declaration 'T' has no documented example. +/
unittest {}
/// U
union U{} // [warn]: Public declaration 'U' has no documented example.
union U{} /+
^ [warn]: Public declaration 'U' has no documented example. +/
unittest {}
}, sac);
@ -248,7 +254,8 @@ unittest
assertAnalyzerWarnings(q{
unittest {}
/// C
class C{} // [warn]: Public declaration 'C' has no documented example.
class C{} /+
^ [warn]: Public declaration 'C' has no documented example. +/
}, sac);
// test documented module header unittest
@ -256,13 +263,15 @@ unittest
///
unittest {}
/// C
class C{} // [warn]: Public declaration 'C' has no documented example.
class C{} /+
^ [warn]: Public declaration 'C' has no documented example. +/
}, sac);
// test multiple unittest blocks
assertAnalyzerWarnings(q{
/// C
class C{} // [warn]: Public declaration 'C' has no documented example.
class C{} /+
^ [warn]: Public declaration 'C' has no documented example. +/
unittest {}
unittest {}
unittest {}
@ -318,7 +327,8 @@ unittest
// test reset on private symbols (#500)
assertAnalyzerWarnings(q{
///
void dirName(C)(C[] path) {} // [warn]: Public declaration 'dirName' has no documented example.
void dirName(C)(C[] path) {} /+
^^^^^^^ [warn]: Public declaration 'dirName' has no documented example. +/
private void _dirName(R)(R path) {}
///
unittest {}

84
src/dscanner/analysis/helpers.d
View file

@ -43,7 +43,11 @@ S after(S)(S value, S separator) if (isSomeString!S)
/**
* This assert function will analyze the passed in code, get the warnings,
* and make sure they match the warnings in the comments. Warnings are
* marked like so: // [warn]: Failed to do somethings.
* marked like so if range doesn't matter: // [warn]: Failed to do somethings.
*
* To test for start and end column, mark warnings as multi-line comments like
* this: /+
* ^^^^^ [warn]: Failed to do somethings. +/
*/
void assertAnalyzerWarnings(string code, const StaticAnalysisConfig config,
string file = __FILE__, size_t line = __LINE__)
@ -62,12 +66,18 @@ void assertAnalyzerWarnings(string code, const StaticAnalysisConfig config,
MessageSet rawWarnings = analyze("test", m, config, moduleCache, tokens);
string[] codeLines = code.splitLines();
struct FoundWarning
{
string msg;
size_t startColumn, endColumn;
}
// Get the warnings ordered by line
string[size_t] warnings;
FoundWarning[size_t] warnings;
foreach (rawWarning; rawWarnings[])
{
// Skip the warning if it is on line zero
immutable size_t rawLine = rawWarning.line;
immutable size_t rawLine = rawWarning.endLine;
if (rawLine == 0)
{
stderr.writefln("!!! Skipping warning because it is on line zero:\n%s",
@ -76,28 +86,49 @@ void assertAnalyzerWarnings(string code, const StaticAnalysisConfig config,
}
size_t warnLine = line - 1 + rawLine;
warnings[warnLine] = format("[warn]: %s", rawWarning.message);
warnings[warnLine] = FoundWarning(
format("[warn]: %s", rawWarning.message),
rawWarning.startLine != rawWarning.endLine ? 1 : rawWarning.startColumn,
rawWarning.endColumn,
);
}
// Get all the messages from the comments in the code
string[size_t] messages;
FoundWarning[size_t] messages;
bool lastLineStartedComment = false;
foreach (i, codeLine; codeLines)
{
// Skip if no [warn] comment
if (codeLine.indexOf("// [warn]:") == -1)
continue;
scope (exit)
lastLineStartedComment = codeLine.stripRight.endsWith("/+", "/*") > 0;
// Get the line of this code line
size_t lineNo = i + line;
if (codeLine.stripLeft.startsWith("^") && lastLineStartedComment)
{
auto start = codeLine.indexOf("^") + 1;
assert(start != 0);
auto end = codeLine.indexOfNeither("^", start) + 1;
assert(end != 0);
auto warn = codeLine.indexOf("[warn]:");
assert(warn != -1, "malformed line, expected `[warn]: text` after `^^^^^` part");
auto message = codeLine[warn .. $].stripRight;
if (message.endsWith("+/", "*/"))
message = message[0 .. $ - 2].stripRight;
messages[lineNo - 1] = FoundWarning(message, start, end);
}
// Skip if no [warn] comment
else if (codeLine.indexOf("// [warn]:") != -1)
{
// Skip if there is no comment or code
immutable string codePart = codeLine.before("// ");
immutable string commentPart = codeLine.after("// ");
if (!codePart.length || !commentPart.length)
continue;
// Get the line of this code line
size_t lineNo = i + line;
// Get the message
messages[lineNo] = commentPart;
messages[lineNo] = FoundWarning(commentPart);
}
}
// Throw an assert error if any messages are not listed in the warnings
@ -111,12 +142,39 @@ void assertAnalyzerWarnings(string code, const StaticAnalysisConfig config,
throw new AssertError(errors, file, lineNo);
}
// Different warning
else if (warnings[lineNo] != messages[lineNo])
else if (warnings[lineNo].msg != messages[lineNo].msg)
{
immutable string errors = "Expected warning:\n%s\nBut was:\n%s\nFrom source code at (%s:?):\n%s".format(
messages[lineNo], warnings[lineNo], lineNo, codeLines[lineNo - line]);
throw new AssertError(errors, file, lineNo);
}
// specified column range
if ((message.startColumn || message.endColumn)
&& warnings[lineNo] != message)
{
import std.algorithm : max;
import std.array : array;
import std.range : repeat;
import std.string : replace;
const(char)[] expectedRange = ' '.repeat(max(0, cast(int)message.startColumn - 1)).array
~ '^'.repeat(max(0, cast(int)(message.endColumn - message.startColumn))).array;
const(char)[] actualRange;
if (!warnings[lineNo].startColumn || warnings[lineNo].startColumn == warnings[lineNo].endColumn)
actualRange = "no column range defined!";
else
actualRange = ' '.repeat(max(0, cast(int)warnings[lineNo].startColumn - 1)).array
~ '^'.repeat(max(0, cast(int)(warnings[lineNo].endColumn - warnings[lineNo].startColumn))).array;
size_t paddingWidth = max(expectedRange.length, actualRange.length);
immutable string errors = "Wrong warning range: expected %s, but was %s\nFrom source code at (%s:?):\n%s\n%-*s <-- expected\n%-*s <-- actual".format(
[message.startColumn, message.endColumn],
[warnings[lineNo].startColumn, warnings[lineNo].endColumn],
lineNo, codeLines[lineNo - line].replace("\t", " "),
paddingWidth, expectedRange,
paddingWidth, actualRange);
throw new AssertError(errors, file, lineNo);
}
}
// Throw an assert error if there were any warnings that were not expected

37
src/dscanner/analysis/if_constraints_indent.d
View file

@ -109,17 +109,21 @@ private:
void checkConstraintSpace(const Constraint constraint, size_t line)
{
import std.algorithm : min;
// dscanner lines start at 1
auto pDecl = firstSymbolAtLine[line - 1];
// search for constraint if (might not be on the same line as the expression)
auto r = firstSymbolAtLine[line .. constraint.expression.line].retro.filter!(s => s.isIf);
auto if_ = constraint.tokens.findTokenForDisplay(tok!"if")[0];
// no hit = constraint is on the same line
if (r.empty)
addErrorMessage(constraint.expression.line, constraint.expression.column, KEY, MESSAGE);
addErrorMessage(if_, KEY, MESSAGE);
else if (pDecl.column != r.front.column)
addErrorMessage(constraint.expression.line, constraint.expression.column, KEY, MESSAGE);
addErrorMessage(if_.line, min(if_.column, pDecl.column), if_.column + 2, KEY, MESSAGE);
}
}
@ -138,8 +142,10 @@ void foo(R)(R r)
if (R == null)
{}
// note: since we are using tabs, the ^ look a bit off here. Use 1-wide tab stops to view tests.
void foo(R)(R r)
if (R == null) // [warn]: %s
if (R == null) /+
^^^ [warn]: %s +/
{}
}c.format(
IfConstraintsIndentCheck.MESSAGE,
@ -151,11 +157,13 @@ void foo(R)(R r)
{}
void foo(R)(R r)
if (R == null) // [warn]: %s
if (R == null) /+
^^ [warn]: %s +/
{}
void foo(R)(R r)
if (R == null) // [warn]: %s
if (R == null) /+
^^^ [warn]: %s +/
{}
}c.format(
IfConstraintsIndentCheck.MESSAGE,
@ -168,11 +176,13 @@ if (R == null) // [warn]: %s
{}
struct Foo(R)
if (R == null) // [warn]: %s
if (R == null) /+
^^ [warn]: %s +/
{}
struct Foo(R)
if (R == null) // [warn]: %s
if (R == null) /+
^^^ [warn]: %s +/
{}
}c.format(
IfConstraintsIndentCheck.MESSAGE,
@ -206,8 +216,9 @@ if (is(typeof(Num.init >= 0)) && is(typeof(-Num.init)) &&
{}
struct Foo(R)
if
(R == null) // [warn]: %s
if /+
^^ [warn]: %s +/
(R == null)
{}
struct Foo(R)
@ -222,8 +233,9 @@ if
{}
struct Foo(R)
if (
R == null // [warn]: %s
if ( /+
^^^ [warn]: %s +/
R == null
) {}
}c.format(
IfConstraintsIndentCheck.MESSAGE,
@ -232,7 +244,8 @@ if
// constraint on the same line
assertAnalyzerWarnings(q{
struct CRC(uint N, ulong P) if (N == 32 || N == 64) // [warn]: %s
struct CRC(uint N, ulong P) if (N == 32 || N == 64) /+
^^ [warn]: %s +/
{}
}c.format(
IfConstraintsIndentCheck.MESSAGE,

6
src/dscanner/analysis/if_statements.d
View file

@ -9,6 +9,7 @@ import dparse.lexer;
import dparse.formatter;
import dscanner.analysis.base;
import dsymbol.scope_ : Scope;
import std.typecons : Rebindable, rebindable;
final class IfStatementCheck : BaseAnalyzer
{
@ -81,12 +82,12 @@ private:
immutable size_t prevLocation = alreadyChecked(app.data, line, column);
if (prevLocation != size_t.max)
{
addErrorMessage(line, column, KEY, "Expression %s is true: already checked on line %d.".format(
addErrorMessage(expressions[prevLocation].astNode, KEY, "Expression %s is true: already checked on line %d.".format(
expressions[prevLocation].formatted, expressions[prevLocation].line));
}
else
{
expressions ~= ExpressionInfo(app.data, line, column, depth);
expressions ~= ExpressionInfo(app.data, line, column, depth, (cast(const BaseNode) expression).rebindable);
sort(expressions);
}
}
@ -124,4 +125,5 @@ private struct ExpressionInfo
size_t line;
size_t column;
int depth;
Rebindable!(const BaseNode) astNode;
}

22
src/dscanner/analysis/ifelsesame.d
View file

@ -34,8 +34,13 @@ final class IfElseSameCheck : BaseAnalyzer
override void visit(const IfStatement ifStatement)
{
if (ifStatement.thenStatement && (ifStatement.thenStatement == ifStatement.elseStatement))
addErrorMessage(ifStatement.line, ifStatement.column,
{
const(Token)[] tokens = ifStatement.elseStatement.tokens;
// extend 1 past, so we include the `else` token
tokens = (tokens.ptr - 1)[0 .. tokens.length + 1];
addErrorMessage(tokens,
"dscanner.bugs.if_else_same", "'Else' branch is identical to 'Then' branch.");
}
ifStatement.accept(this);
}
@ -45,7 +50,7 @@ final class IfElseSameCheck : BaseAnalyzer
if (e !is null && assignExpression.operator == tok!"="
&& e.ternaryExpression == assignExpression.ternaryExpression)
{
addErrorMessage(assignExpression.line, assignExpression.column, "dscanner.bugs.self_assignment",
addErrorMessage(assignExpression, "dscanner.bugs.self_assignment",
"Left side of assignment operatior is identical to the right side.");
}
assignExpression.accept(this);
@ -56,7 +61,7 @@ final class IfElseSameCheck : BaseAnalyzer
if (andAndExpression.left !is null && andAndExpression.right !is null
&& andAndExpression.left == andAndExpression.right)
{
addErrorMessage(andAndExpression.line, andAndExpression.column,
addErrorMessage(andAndExpression.right,
"dscanner.bugs.logic_operator_operands",
"Left side of logical and is identical to right side.");
}
@ -68,7 +73,7 @@ final class IfElseSameCheck : BaseAnalyzer
if (orOrExpression.left !is null && orOrExpression.right !is null
&& orOrExpression.left == orOrExpression.right)
{
addErrorMessage(orOrExpression.line, orOrExpression.column,
addErrorMessage(orOrExpression.right,
"dscanner.bugs.logic_operator_operands",
"Left side of logical or is identical to right side.");
}
@ -86,10 +91,13 @@ unittest
void testSizeT()
{
string person = "unknown";
if (person == "unknown") // [warn]: 'Else' branch is identical to 'Then' branch.
person = "bobrick"; // same
if (person == "unknown")
person = "bobrick"; /* same */
else
person = "bobrick"; // same
person = "bobrick"; /* same */ /+
^^^^^^^^^^^^^^^^^^^^^^^ [warn]: 'Else' branch is identical to 'Then' branch. +/
// note: above ^^^ line spans over multiple lines, so it starts at start of line, since we don't have any way to test this here
// look at the tests using 1-wide tab width for accurate visuals.
if (person == "unknown") // ok
person = "ricky"; // not same

138
src/dscanner/analysis/imports_sortedness.d
View file

@ -46,19 +46,21 @@ final class ImportSortednessCheck : BaseAnalyzer
if (id.importBindings is null || id.importBindings.importBinds.length == 0)
{
bool suppress;
foreach (singleImport; id.singleImports)
{
string importModuleName = singleImport.identifierChain.identifiers.map!`a.text`.join(".");
addImport(importModuleName, singleImport);
addImport(importModuleName, singleImport, null, suppress);
}
}
else
{
string importModuleName = id.importBindings.singleImport.identifierChain.identifiers.map!`a.text`.join(".");
bool suppress;
foreach (importBind; id.importBindings.importBinds)
{
addImport(importModuleName ~ "-" ~ importBind.left.text, id.importBindings.singleImport);
addImport(importModuleName ~ "-" ~ importBind.left.text, importBind, id.importBindings.singleImport, suppress);
}
}
}
@ -80,14 +82,29 @@ private:
}
}
void addImport(string importModuleName, const SingleImport singleImport)
void addImport(string importModuleName, const BaseNode range, const BaseNode parent, ref bool suppress)
{
import std.algorithm : findSplit;
import std.string : indexOf;
import std.uni : sicmp;
if (imports[level].length > 0 && imports[level][$ -1].sicmp(importModuleName) > 0)
{
addErrorMessage(singleImport.identifierChain.identifiers[0].line,
singleImport.identifierChain.identifiers[0].column, KEY, MESSAGE);
if (parent !is null)
{
auto parentEnd = importModuleName.indexOf("-");
if (parentEnd != -1 && imports[level][$ -1].findSplit("-")[0].sicmp(importModuleName) > 0)
{
// mark module name as broken, not selected symbols, since it's the module name is not belonging here
if (!suppress)
addErrorMessage(parent, KEY, MESSAGE);
suppress = true;
return;
}
}
if (!suppress)
addErrorMessage(range, KEY, MESSAGE);
suppress = true;
}
else
{
@ -113,7 +130,8 @@ unittest
assertAnalyzerWarnings(q{
import foo.bar;
import bar.foo; // [warn]: %s
import bar.foo; /+
^^^^^^^ [warn]: %s +/
}c.format(
ImportSortednessCheck.MESSAGE,
), sac);
@ -121,19 +139,36 @@ unittest
assertAnalyzerWarnings(q{
import c;
import c.b;
import c.a; // [warn]: %s
import c.a; /+
^^^ [warn]: %s +/
import d.a;
import d; // [warn]: %s
import d; /+
^ [warn]: %s +/
}c.format(
ImportSortednessCheck.MESSAGE,
ImportSortednessCheck.MESSAGE,
), sac);
assertAnalyzerWarnings(q{
unittest
{
import a.b, a.c, a.d;
import a.b, a.d, a.c; // [warn]: %s
import a.c, a.b, a.c; // [warn]: %s
import foo.bar, bar.foo; // [warn]: %s
}
unittest
{
import a.b, a.d, a.c; /+
^^^ [warn]: %s +/
}
unittest
{
import a.c, a.b, a.c; /+
^^^ [warn]: %s +/
}
unittest
{
import foo.bar, bar.foo; /+
^^^^^^^ [warn]: %s +/
}
}c.format(
ImportSortednessCheck.MESSAGE,
ImportSortednessCheck.MESSAGE,
@ -143,8 +178,10 @@ unittest
// multiple items out of order
assertAnalyzerWarnings(q{
import foo.bar;
import bar.foo; // [warn]: %s
import bar.bar.foo; // [warn]: %s
import bar.foo; /+
^^^^^^^ [warn]: %s +/
import bar.bar.foo; /+
^^^^^^^^^^^ [warn]: %s +/
}c.format(
ImportSortednessCheck.MESSAGE,
ImportSortednessCheck.MESSAGE,
@ -158,14 +195,19 @@ unittest
// selective imports
assertAnalyzerWarnings(q{
import test : foo;
import test : bar; // [warn]: %s
import test : bar; /+
^^^ [warn]: %s +/
import before : zzz; /+
^^^^^^ [warn]: %s +/
}c.format(
ImportSortednessCheck.MESSAGE,
ImportSortednessCheck.MESSAGE,
), sac);
// selective imports
assertAnalyzerWarnings(q{
import test : foo, bar; // [warn]: %s
import test : foo, bar; /+
^^^ [warn]: %s +/
}c.format(
ImportSortednessCheck.MESSAGE,
), sac);
@ -173,8 +215,10 @@ unittest
assertAnalyzerWarnings(q{
import b;
import c : foo;
import c : bar; // [warn]: %s
import a; // [warn]: %s
import c : bar; /+
^^^ [warn]: %s +/
import a; /+
^ [warn]: %s +/
}c.format(
ImportSortednessCheck.MESSAGE,
ImportSortednessCheck.MESSAGE,
@ -184,8 +228,10 @@ unittest
import c;
import c : bar;
import d : bar;
import d; // [warn]: %s
import a : bar; // [warn]: %s
import d; /+
^ [warn]: %s +/
import a : bar; /+
^ [warn]: %s +/
}c.format(
ImportSortednessCheck.MESSAGE,
ImportSortednessCheck.MESSAGE,
@ -199,8 +245,10 @@ unittest
assertAnalyzerWarnings(q{
import t1 : a, b = foo;
import t1 : b, a = foo; // [warn]: %s
import t0 : a, b = foo; // [warn]: %s
import t1 : b, a = foo; /+
^^^^^^^ [warn]: %s +/
import t0 : a, b = foo; /+
^^ [warn]: %s +/
}c.format(
ImportSortednessCheck.MESSAGE,
ImportSortednessCheck.MESSAGE,
@ -209,11 +257,13 @@ unittest
// local imports in functions
assertAnalyzerWarnings(q{
import t2;
import t1; // [warn]: %s
import t1; /+
^^ [warn]: %s +/
void foo()
{
import f2;
import f1; // [warn]: %s
import f1; /+
^^ [warn]: %s +/
import f3;
}
void bar()
@ -229,15 +279,18 @@ unittest
// local imports in scopes
assertAnalyzerWarnings(q{
import t2;
import t1; // [warn]: %s
import t1; /+
^^ [warn]: %s +/
void foo()
{
import f2;
import f1; // [warn]: %s
import f1; /+
^^ [warn]: %s +/
import f3;
{
import f2;
import f1; // [warn]: %s
import f1; /+
^^ [warn]: %s +/
import f3;
}
{
@ -255,15 +308,18 @@ unittest
// local imports in functions
assertAnalyzerWarnings(q{
import t2;
import t1; // [warn]: %s
import t1; /+
^^ [warn]: %s +/
void foo()
{
import f2;
import f1; // [warn]: %s
import f1; /+
^^ [warn]: %s +/
import f3;
while (true) {
import f2;
import f1; // [warn]: %s
import f1; /+
^^ [warn]: %s +/
import f3;
}
for (;;) {
@ -273,7 +329,8 @@ unittest
}
foreach (el; arr) {
import f2;
import f1; // [warn]: %s
import f1; /+
^^ [warn]: %s +/
import f3;
}
}
@ -287,23 +344,28 @@ unittest
// nested scopes
assertAnalyzerWarnings(q{
import t2;
import t1; // [warn]: %s
import t1; /+
^^ [warn]: %s +/
void foo()
{
import f2;
import f1; // [warn]: %s
import f1; /+
^^ [warn]: %s +/
import f3;
{
import f2;
import f1; // [warn]: %s
import f1; /+
^^ [warn]: %s +/
import f3;
{
import f2;
import f1; // [warn]: %s
import f1; /+
^^ [warn]: %s +/
import f3;
{
import f2;
import f1; // [warn]: %s
import f1; /+
^^ [warn]: %s +/
import f3;
}
}
@ -320,11 +382,13 @@ unittest
// local imports in functions
assertAnalyzerWarnings(q{
import t2;
import t1; // [warn]: %s
import t1; /+
^^ [warn]: %s +/
struct foo()
{
import f2;
import f1; // [warn]: %s
import f1; /+
^^ [warn]: %s +/
import f3;
}
class bar()

26
src/dscanner/analysis/incorrect_infinite_range.d
View file

@ -10,6 +10,8 @@ import dscanner.analysis.helpers;
import dparse.ast;
import dparse.lexer;
import std.typecons : Rebindable;
/**
* Checks for incorrect infinite range definitions
*/
@ -36,9 +38,10 @@ final class IncorrectInfiniteRangeCheck : BaseAnalyzer
{
if (inStruct > 0 && fd.name.text == "empty")
{
line = fd.name.line;
column = fd.name.column;
auto old = parentFunc;
parentFunc = fd;
fd.accept(this);
parentFunc = old;
}
}
@ -63,7 +66,7 @@ final class IncorrectInfiniteRangeCheck : BaseAnalyzer
override void visit(const ReturnStatement rs)
{
if (inStruct == 0 || line == size_t.max) // not within a struct yet
if (inStruct == 0 || parentFunc == null) // not within a struct yet
return;
visitReturnExpression(rs.expression);
}
@ -79,7 +82,7 @@ final class IncorrectInfiniteRangeCheck : BaseAnalyzer
return;
if (unary.primaryExpression.primary != tok!"false")
return;
addErrorMessage(line, column, KEY, MESSAGE);
addErrorMessage(parentFunc.get, KEY, MESSAGE);
}
override void visit(const Unittest u)
@ -90,8 +93,7 @@ private:
uint inStruct;
enum string KEY = "dscanner.suspicious.incorrect_infinite_range";
enum string MESSAGE = "Use `enum bool empty = false;` to define an infinite range.";
size_t line = size_t.max;
size_t column;
Rebindable!(const FunctionDeclaration) parentFunc;
}
unittest
@ -104,10 +106,12 @@ unittest
sac.incorrect_infinite_range_check = Check.enabled;
assertAnalyzerWarnings(q{struct InfiniteRange
{
bool empty() // [warn]: %1$s
bool empty()
{
return false;
}
} /+
^^ [warn]: %1$s+/
// TODO: test for multiline issues like this
bool stuff()
{
@ -128,7 +132,8 @@ unittest
struct InfiniteRange
{
bool empty() => false; // [warn]: %1$s
bool empty() => false; /+
^^^^^^^^^^^^^^^^^^^^^^ [warn]: %1$s +/
bool stuff() => false;
unittest
{
@ -143,7 +148,8 @@ struct InfiniteRange
}
bool empty() { return false; }
class C { bool empty() { return false; } } // [warn]: %1$s
class C { bool empty() { return false; } } /+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ [warn]: %1$s +/
}c
.format(IncorrectInfiniteRangeCheck.MESSAGE), sac);

17
src/dscanner/analysis/label_var_same_name_check.d
View file

@ -106,7 +106,7 @@ private:
{
immutable thisKind = fromLabel ? "Label" : "Variable";
immutable otherKind = thing.isVar ? "variable" : "label";
addErrorMessage(name.line, name.column, "dscanner.suspicious.label_var_same_name",
addErrorMessage(name, "dscanner.suspicious.label_var_same_name",
thisKind ~ " \"" ~ fqn ~ "\" has the same name as a "
~ otherKind ~ " defined on line " ~ to!string(thing.line) ~ ".");
}
@ -178,14 +178,16 @@ unittest
unittest
{
blah:
int blah; // [warn]: Variable "blah" has the same name as a label defined on line 4.
int blah; /+
^^^^ [warn]: Variable "blah" has the same name as a label defined on line 4. +/
}
int blah;
unittest
{
static if (stuff)
int a;
int a; // [warn]: Variable "a" has the same name as a variable defined on line 11.
int a; /+
^ [warn]: Variable "a" has the same name as a variable defined on line 12. +/
}
unittest
@ -202,7 +204,8 @@ unittest
int a = 10;
else
int a = 20;
int a; // [warn]: Variable "a" has the same name as a variable defined on line 28.
int a; /+
^ [warn]: Variable "a" has the same name as a variable defined on line 30. +/
}
template T(stuff)
{
@ -225,7 +228,8 @@ unittest
int c = 10;
else
int c = 20;
int c; // [warn]: Variable "c" has the same name as a variable defined on line 51.
int c; /+
^ [warn]: Variable "c" has the same name as a variable defined on line 54. +/
}
unittest
@ -263,7 +267,8 @@ unittest
interface A
{
int a;
int a; // [warn]: Variable "A.a" has the same name as a variable defined on line 89.
int a; /+
^ [warn]: Variable "A.a" has the same name as a variable defined on line 93. +/
}
}

15
src/dscanner/analysis/lambda_return_check.d
View file

@ -31,7 +31,11 @@ final class LambdaReturnCheck : BaseAnalyzer
{
return;
}
addErrorMessage(fLit.line, fLit.column, KEY, "This lambda returns a lambda. Add parenthesis to clarify.");
auto start = &fLit.tokens[0];
auto endIncl = &fe.specifiedFunctionBody.tokens[0];
assert(endIncl >= start);
auto tokens = start[0 .. endIncl - start + 1];
addErrorMessage(tokens, KEY, "This lambda returns a lambda. Add parenthesis to clarify.");
}
private:
@ -52,9 +56,12 @@ unittest
void main()
{
int[] b;
auto a = b.map!(a => { return a * a + 2; }).array(); // [warn]: This lambda returns a lambda. Add parenthesis to clarify.
pragma(msg, typeof(a => { return a; })); // [warn]: This lambda returns a lambda. Add parenthesis to clarify.
pragma(msg, typeof((a) => { return a; })); // [warn]: This lambda returns a lambda. Add parenthesis to clarify.
auto a = b.map!(a => { return a * a + 2; }).array(); /+
^^^^^^ [warn]: This lambda returns a lambda. Add parenthesis to clarify. +/
pragma(msg, typeof(a => { return a; })); /+
^^^^^^ [warn]: This lambda returns a lambda. Add parenthesis to clarify. +/
pragma(msg, typeof((a) => { return a; })); /+
^^^^^^^^ [warn]: This lambda returns a lambda. Add parenthesis to clarify. +/
pragma(msg, typeof({ return a; }));
pragma(msg, typeof(a => () { return a; }));
}`c;

14
src/dscanner/analysis/length_subtraction.d
View file

@ -34,23 +34,14 @@ final class LengthSubtractionCheck : BaseAnalyzer
const UnaryExpression l = cast(const UnaryExpression) addExpression.left;
const UnaryExpression r = cast(const UnaryExpression) addExpression.right;
if (l is null || r is null)
{
// stderr.writeln(__FILE__, " ", __LINE__);
goto end;
}
if (r.primaryExpression is null || r.primaryExpression.primary.type != tok!"intLiteral")
{
// stderr.writeln(__FILE__, " ", __LINE__);
goto end;
}
if (l.identifierOrTemplateInstance is null
|| l.identifierOrTemplateInstance.identifier.text != "length")
{
// stderr.writeln(__FILE__, " ", __LINE__);
goto end;
}
const(Token) token = l.identifierOrTemplateInstance.identifier;
addErrorMessage(token.line, token.column, "dscanner.suspicious.length_subtraction",
addErrorMessage(addExpression, "dscanner.suspicious.length_subtraction",
"Avoid subtracting from '.length' as it may be unsigned.");
}
end:
@ -67,7 +58,8 @@ unittest
assertAnalyzerWarnings(q{
void testSizeT()
{
if (i < a.length - 1) // [warn]: Avoid subtracting from '.length' as it may be unsigned.
if (i < a.length - 1) /+
^^^^^^^^^^^^ [warn]: Avoid subtracting from '.length' as it may be unsigned. +/
writeln("something");
}
}c, sac);

4
src/dscanner/analysis/line_length.d
View file

@ -58,9 +58,11 @@ private:
void triggerError(ref const Token tok)
{
import std.algorithm : max;
if (tok.line != lastErrorLine)
{
addErrorMessage(tok.line, tok.column, KEY, message);
addErrorMessage(tok.line, maxLineLength, max(maxLineLength + 1, tok.column + 1), KEY, message);
lastErrorLine = tok.line;
}
}

6
src/dscanner/analysis/local_imports.d
View file

@ -58,8 +58,7 @@ final class LocalImportCheck : BaseAnalyzer
{
if (singleImport.rename.text.length == 0)
{
addErrorMessage(singleImport.identifierChain.identifiers[0].line,
singleImport.identifierChain.identifiers[0].column,
addErrorMessage(singleImport,
"dscanner.suspicious.local_imports", "Local imports should specify"
~ " the symbols being imported to avoid hiding local symbols.");
}
@ -93,7 +92,8 @@ unittest
assertAnalyzerWarnings(q{
void testLocalImport()
{
import std.stdio; // [warn]: Local imports should specify the symbols being imported to avoid hiding local symbols.
import std.stdio; /+
^^^^^^^^^ [warn]: Local imports should specify the symbols being imported to avoid hiding local symbols. +/
import std.fish : scales, head;
import DAGRON = std.experimental.dragon;
}

8
src/dscanner/analysis/logic_precedence.d
View file

@ -41,7 +41,7 @@ final class LogicPrecedenceCheck : BaseAnalyzer
return;
if ((left !is null && left.right is null) && (right !is null && right.right is null))
return;
addErrorMessage(orOr.line, orOr.column, KEY,
addErrorMessage(orOr, KEY,
"Use parenthesis to clarify this expression.");
orOr.accept(this);
}
@ -56,9 +56,11 @@ unittest
assertAnalyzerWarnings(q{
void testFish()
{
if (a && b || c) {} // [warn]: Use parenthesis to clarify this expression.
if (a && b || c) {} /+
^^^^^^^^^^^ [warn]: Use parenthesis to clarify this expression. +/
if ((a && b) || c) {} // Good
if (b || c && d) {} // [warn]: Use parenthesis to clarify this expression.
if (b || c && d) {} /+
^^^^^^^^^^^ [warn]: Use parenthesis to clarify this expression. +/
if (b || (c && d)) {} // Good
}
}c, sac);

32
src/dscanner/analysis/mismatched_args.d
View file

@ -5,7 +5,7 @@ import dscanner.utils : safeAccess;
import dsymbol.scope_;
import dsymbol.symbol;
import dparse.ast;
import dparse.lexer : tok;
import dparse.lexer : tok, Token;
import dsymbol.builtin.names;
/// Checks for mismatched argument and parameter names
@ -42,8 +42,7 @@ final class MismatchedArgumentCheck : BaseAnalyzer
static struct ErrorMessage
{
size_t line;
size_t column;
const(Token)[] range;
string message;
}
@ -60,17 +59,16 @@ final class MismatchedArgumentCheck : BaseAnalyzer
matched = true;
else
{
foreach (size_t i, ref const mm; mismatches)
foreach (ref const mm; mismatches)
{
messages ~= ErrorMessage(argVisitor.lines[i],
argVisitor.columns[i], createWarningFromMismatch(mm));
messages ~= ErrorMessage(argVisitor.tokens[mm.argIndex], createWarningFromMismatch(mm));
}
}
}
if (!matched)
foreach (m; messages)
addErrorMessage(m.line, m.column, KEY, m.message);
addErrorMessage(m.range, KEY, m.message);
}
alias visit = ASTVisitor.visit;
@ -123,8 +121,7 @@ final class ArgVisitor : ASTVisitor
// if we didn't get an identifier in the unary expression,
// assume it's a good argument
args ~= istring.init;
lines ~= a.tokens.length ? a.tokens[0].line : size_t.max;
columns ~= a.tokens.length ? a.tokens[0].column : size_t.max;
tokens ~= a.tokens;
}
}
}
@ -138,16 +135,14 @@ final class ArgVisitor : ASTVisitor
if (iot.identifier == tok!"")
return;
immutable t = iot.identifier;
lines ~= t.line;
columns ~= t.column;
tokens ~= [t];
args ~= internString(t.text);
}
}
alias visit = ASTVisitor.visit;
size_t[] lines;
size_t[] columns;
const(Token[])[] tokens;
istring[] args;
}
@ -271,14 +266,19 @@ unittest
{
int x = 1;
int y = 2;
foo(y, x); // [warn]: Argument 2 is named 'x', but this is the name of parameter 1
foo(y + 1, x); // [warn]: Argument 2 is named 'x', but this is the name of parameter 1
foo(y, x); /+
^ [warn]: Argument 2 is named 'x', but this is the name of parameter 1 +/
foo(y + 1, x); /+
^ [warn]: Argument 2 is named 'x', but this is the name of parameter 1 +/
foo(y + 1, f(x));
foo(x: y, y: x);
foo(y, 0); /+
^ [warn]: Argument 1 is named 'y', but this is the name of parameter 2 +/
// foo(y: y, x); // TODO: this shouldn't error
foo(x, y: x); // TODO: this should error
foo(y, y: x); // [warn]: Argument 1 is named 'y', but this is the name of parameter 2
foo(y, y: x); /+
^ [warn]: Argument 1 is named 'y', but this is the name of parameter 2 +/
}
}c, sac);
stderr.writeln("Unittest for MismatchedArgumentCheck passed.");

13
src/dscanner/analysis/numbers.d
View file

@ -39,7 +39,7 @@ public:
&& ((t.text.startsWith("0b") && !t.text.matchFirst(badBinaryRegex)
.empty) || !t.text.matchFirst(badDecimalRegex).empty))
{
addErrorMessage(t.line, t.column, "dscanner.style.number_literals",
addErrorMessage(t, "dscanner.style.number_literals",
"Use underscores to improve number constant readability.");
}
}
@ -62,10 +62,13 @@ unittest
a = 1; // ok
a = 10; // ok
a = 100; // ok
a = 1000; // FIXME: boom
a = 10000; // [warn]: Use underscores to improve number constant readability.
a = 100000; // [warn]: Use underscores to improve number constant readability.
a = 1000000; // [warn]: Use underscores to improve number constant readability.
a = 1000; // ok
a = 10000; /+
^^^^^ [warn]: Use underscores to improve number constant readability. +/
a = 100000; /+
^^^^^^ [warn]: Use underscores to improve number constant readability. +/
a = 1000000; /+
^^^^^^^ [warn]: Use underscores to improve number constant readability. +/
}
}c, sac);

15
src/dscanner/analysis/objectconst.d
View file

@ -68,8 +68,7 @@ final class ObjectConstCheck : BaseAnalyzer
if (inAggregate && !constColon && !constBlock && !isDeclationDisabled
&& isInteresting(fd.name.text) && !hasConst(fd.memberFunctionAttributes))
{
addErrorMessage(d.functionDeclaration.name.line,
d.functionDeclaration.name.column, "dscanner.suspicious.object_const",
addErrorMessage(d.functionDeclaration.name, "dscanner.suspicious.object_const",
"Methods 'opCmp', 'toHash', 'opEquals', 'opCast', and/or 'toString' are non-const.");
}
}
@ -157,22 +156,26 @@ unittest
// Will warn, because none are const
class Dog
{
bool opEquals(Object a, Object b) // [warn]: Methods 'opCmp', 'toHash', 'opEquals', 'opCast', and/or 'toString' are non-const.
bool opEquals(Object a, Object b) /+
^^^^^^^^ [warn]: Methods 'opCmp', 'toHash', 'opEquals', 'opCast', and/or 'toString' are non-const. +/
{
return true;
}
int opCmp(Object o) // [warn]: Methods 'opCmp', 'toHash', 'opEquals', 'opCast', and/or 'toString' are non-const.
int opCmp(Object o) /+
^^^^^ [warn]: Methods 'opCmp', 'toHash', 'opEquals', 'opCast', and/or 'toString' are non-const. +/
{
return 1;
}
hash_t toHash() // [warn]: Methods 'opCmp', 'toHash', 'opEquals', 'opCast', and/or 'toString' are non-const.
hash_t toHash() /+
^^^^^^ [warn]: Methods 'opCmp', 'toHash', 'opEquals', 'opCast', and/or 'toString' are non-const. +/
{
return 0;
}
string toString() // [warn]: Methods 'opCmp', 'toHash', 'opEquals', 'opCast', and/or 'toString' are non-const.
string toString() /+
^^^^^^^^ [warn]: Methods 'opCmp', 'toHash', 'opEquals', 'opCast', and/or 'toString' are non-const. +/
{
return "Dog";
}

44
src/dscanner/analysis/opequals_without_tohash.d
View file

@ -5,12 +5,13 @@
module dscanner.analysis.opequals_without_tohash;
import std.stdio;
import dparse.ast;
import dparse.lexer;
import dscanner.analysis.base;
import dscanner.analysis.helpers;
import dsymbol.scope_ : Scope;
import std.stdio;
import std.typecons : Rebindable;
/**
* Checks for when a class/struct has the method opEquals without toHash, or
@ -41,9 +42,9 @@ final class OpEqualsWithoutToHashCheck : BaseAnalyzer
private void actualCheck(const Token name, const StructBody structBody)
{
bool hasOpEquals;
bool hasToHash;
bool hasOpCmp;
Rebindable!(const Declaration) hasOpEquals;
Rebindable!(const Declaration) hasToHash;
Rebindable!(const Declaration) hasOpCmp;
// Just return if missing children
if (!structBody || !structBody.declarations || name is Token.init)
@ -72,24 +73,36 @@ final class OpEqualsWithoutToHashCheck : BaseAnalyzer
// Check if opEquals or toHash
immutable string methodName = declaration.functionDeclaration.name.text;
if (methodName == "opEquals")
hasOpEquals = true;
hasOpEquals = declaration;
else if (methodName == "toHash")
hasToHash = true;
hasToHash = declaration;
else if (methodName == "opCmp")
hasOpCmp = true;
hasOpCmp = declaration;
}
// Warn if has opEquals, but not toHash
if (hasOpEquals && !hasToHash)
{
string message = "'" ~ name.text ~ "' has method 'opEquals', but not 'toHash'.";
addErrorMessage(name.line, name.column, KEY, message);
addErrorMessage(
Message.Diagnostic.from(fileName, name, message),
[
Message.Diagnostic.from(fileName, hasOpEquals.get, "'opEquals' defined here")
],
KEY
);
}
// Warn if has toHash, but not opEquals
else if (!hasOpEquals && hasToHash)
{
string message = "'" ~ name.text ~ "' has method 'toHash', but not 'opEquals'.";
addErrorMessage(name.line, name.column, KEY, message);
addErrorMessage(
Message.Diagnostic.from(fileName, name, message),
[
Message.Diagnostic.from(fileName, hasToHash.get, "'toHash' defined here")
],
KEY
);
}
}
@ -102,6 +115,7 @@ unittest
StaticAnalysisConfig sac = disabledConfig();
sac.opequals_tohash_check = Check.enabled;
// TODO: test supplemental diagnostics
assertAnalyzerWarnings(q{
// Success because it has opEquals and toHash
class Chimp
@ -127,7 +141,8 @@ unittest
}
// Fail on class opEquals
class Rabbit // [warn]: 'Rabbit' has method 'opEquals', but not 'toHash'.
class Rabbit /+
^^^^^^ [warn]: 'Rabbit' has method 'opEquals', but not 'toHash'. +/
{
const bool opEquals(Object a, Object b)
{
@ -136,7 +151,8 @@ unittest
}
// Fail on class toHash
class Kangaroo // [warn]: 'Kangaroo' has method 'toHash', but not 'opEquals'.
class Kangaroo /+
^^^^^^^^ [warn]: 'Kangaroo' has method 'toHash', but not 'opEquals'. +/
{
override const hash_t toHash()
{
@ -145,7 +161,8 @@ unittest
}
// Fail on struct opEquals
struct Tarantula // [warn]: 'Tarantula' has method 'opEquals', but not 'toHash'.
struct Tarantula /+
^^^^^^^^^ [warn]: 'Tarantula' has method 'opEquals', but not 'toHash'. +/
{
const bool opEquals(Object a, Object b)
{
@ -154,7 +171,8 @@ unittest
}
// Fail on struct toHash
struct Puma // [warn]: 'Puma' has method 'toHash', but not 'opEquals'.
struct Puma /+
^^^^ [warn]: 'Puma' has method 'toHash', but not 'opEquals'. +/
{
const nothrow @safe hash_t toHash()
{

18
src/dscanner/analysis/pokemon.d
View file

@ -38,7 +38,7 @@ final class PokemonExceptionCheck : BaseAnalyzer
override void visit(const LastCatch lc)
{
addErrorMessage(lc.line, lc.column, KEY, MESSAGE);
addErrorMessage(lc.tokens[0], KEY, MESSAGE);
lc.accept(this);
}
@ -76,9 +76,7 @@ final class PokemonExceptionCheck : BaseAnalyzer
if (identOrTemplate.identifier.text == "Throwable"
|| identOrTemplate.identifier.text == "Error")
{
immutable column = identOrTemplate.identifier.column;
immutable line = identOrTemplate.identifier.line;
addErrorMessage(line, column, KEY, MESSAGE);
addErrorMessage(identOrTemplate, KEY, MESSAGE);
}
}
}
@ -108,19 +106,23 @@ unittest
{
}
catch (Error err) // [warn]: Catching Error or Throwable is almost always a bad idea.
catch (Error err) /+
^^^^^ [warn]: Catching Error or Throwable is almost always a bad idea. +/
{
}
catch (Throwable err) // [warn]: Catching Error or Throwable is almost always a bad idea.
catch (Throwable err) /+
^^^^^^^^^ [warn]: Catching Error or Throwable is almost always a bad idea. +/
{
}
catch (shared(Error) err) // [warn]: Catching Error or Throwable is almost always a bad idea.
catch (shared(Error) err) /+
^^^^^ [warn]: Catching Error or Throwable is almost always a bad idea. +/
{
}
catch // [warn]: Catching Error or Throwable is almost always a bad idea.
catch /+
^^^^^ [warn]: Catching Error or Throwable is almost always a bad idea. +/
{
}

171
src/dscanner/analysis/properly_documented_public_functions.d
View file

@ -157,8 +157,8 @@ final class ProperlyDocumentedPublicFunctions : BaseAnalyzer
override void visit(const TemplateDeclaration decl)
{
setLastDdocParams(decl.name.line, decl.name.column, decl.comment);
checkDdocParams(decl.name.line, decl.name.column, decl.templateParameters);
setLastDdocParams(decl.name, decl.comment);
checkDdocParams(decl.templateParameters);
withinTemplate = true;
scope(exit) withinTemplate = false;
@ -172,15 +172,15 @@ final class ProperlyDocumentedPublicFunctions : BaseAnalyzer
override void visit(const StructDeclaration decl)
{
setLastDdocParams(decl.name.line, decl.name.column, decl.comment);
checkDdocParams(decl.name.line, decl.name.column, decl.templateParameters);
setLastDdocParams(decl.name, decl.comment);
checkDdocParams(decl.templateParameters);
decl.accept(this);
}
override void visit(const ClassDeclaration decl)
{
setLastDdocParams(decl.name.line, decl.name.column, decl.comment);
checkDdocParams(decl.name.line, decl.name.column, decl.templateParameters);
setLastDdocParams(decl.name, decl.comment);
checkDdocParams(decl.templateParameters);
decl.accept(this);
}
@ -205,20 +205,31 @@ final class ProperlyDocumentedPublicFunctions : BaseAnalyzer
{
Appender!(char[]) app;
astFmt(&app, t);
addErrorMessage(decl.name.line, decl.name.column, MISSING_THROW_KEY,
addErrorMessage(decl.name, MISSING_THROW_KEY,
MISSING_THROW_MESSAGE.format(app.data));
}
}
if (nestedFuncs == 1)
{
auto comment = setLastDdocParams(decl.name.line, decl.name.column, decl.comment);
checkDdocParams(decl.name.line, decl.name.column, decl.parameters, decl.templateParameters);
auto comment = setLastDdocParams(decl.name, decl.comment);
checkDdocParams(decl.parameters, decl.templateParameters);
enum voidType = tok!"void";
if (decl.returnType is null || decl.returnType.type2.builtinType != voidType)
if (!(comment.isDitto || withinTemplate || comment.sections.any!(s => s.name == "Returns")))
addErrorMessage(decl.name.line, decl.name.column,
MISSING_RETURNS_KEY, MISSING_RETURNS_MESSAGE);
{
import dscanner.analysis.auto_function : AutoFunctionChecker;
const(Token)[] typeRange;
if (decl.returnType !is null)
typeRange = decl.returnType.tokens;
else
typeRange = AutoFunctionChecker.findAutoReturnType(decl);
if (!typeRange.length)
typeRange = [decl.name];
addErrorMessage(typeRange, MISSING_RETURNS_KEY, MISSING_RETURNS_MESSAGE);
}
}
}
@ -257,7 +268,7 @@ private:
static struct Function
{
bool active;
size_t line, column;
Token name;
const(string)[] ddocParams;
bool[string] params;
}
@ -274,7 +285,7 @@ private:
if (lastSeenFun.active)
foreach (p; lastSeenFun.ddocParams)
if (p !in lastSeenFun.params)
addErrorMessage(lastSeenFun.line, lastSeenFun.column, NON_EXISTENT_PARAMS_KEY,
addErrorMessage(lastSeenFun.name, NON_EXISTENT_PARAMS_KEY,
NON_EXISTENT_PARAMS_MESSAGE.format(p));
lastSeenFun.active = false;
@ -289,7 +300,7 @@ private:
return comment.isDitto || comment.sections.canFind!(s => s.name == "Throws");
}
auto setLastDdocParams(size_t line, size_t column, string commentText)
auto setLastDdocParams(Token name, string commentText)
{
import ddoc.comments : parseComment;
import std.algorithm.searching : find;
@ -312,19 +323,19 @@ private:
const paramSection = comment.sections.find!(s => s.name == "Params");
if (paramSection.empty)
{
lastSeenFun = Function(true, line, column, null);
lastSeenFun = Function(true, name, null);
}
else
{
auto ddocParams = paramSection[0].mapping.map!(a => a[0]).array;
lastSeenFun = Function(true, line, column, ddocParams);
lastSeenFun = Function(true, name, ddocParams);
}
}
return comment;
}
void checkDdocParams(size_t line, size_t column, const Parameters params,
void checkDdocParams(const Parameters params,
const TemplateParameters templateParameters = null)
{
import std.array : array;
@ -338,7 +349,7 @@ private:
if (const tp = templateParameters)
if (const tpl = tp.templateParameterList)
templateList = tpl.items;
string[] tlList = templateList.map!(a => templateParamName(a)).array;
string[] tlList = templateList.map!(a => templateParamName(a).text).array;
// make a copy of all parameters and remove the seen ones later during the loop
size_t[] unseenTemplates = templateList.length.iota.array;
@ -369,7 +380,7 @@ private:
}
if (!lastSeenFun.ddocParams.canFind(p.name.text))
addErrorMessage(line, column, MISSING_PARAMS_KEY,
addErrorMessage(p.name, MISSING_PARAMS_KEY,
format(MISSING_PARAMS_MESSAGE, p.name.text));
else
lastSeenFun.params[p.name.text] = true;
@ -377,45 +388,45 @@ private:
// now check the remaining, not used template parameters
auto unseenTemplatesArr = templateList.indexed(unseenTemplates).array;
checkDdocParams(line, column, unseenTemplatesArr);
checkDdocParams(unseenTemplatesArr);
}
void checkDdocParams(size_t line, size_t column, const TemplateParameters templateParams)
void checkDdocParams(const TemplateParameters templateParams)
{
if (lastSeenFun.active && templateParams !is null &&
templateParams.templateParameterList !is null)
checkDdocParams(line, column, templateParams.templateParameterList.items);
checkDdocParams(templateParams.templateParameterList.items);
}
void checkDdocParams(size_t line, size_t column, const TemplateParameter[] templateParams)
void checkDdocParams(const TemplateParameter[] templateParams)
{
import std.algorithm.searching : canFind;
foreach (p; templateParams)
{
const name = templateParamName(p);
assert(name, "Invalid template parameter name."); // this shouldn't happen
if (!lastSeenFun.ddocParams.canFind(name))
addErrorMessage(line, column, MISSING_PARAMS_KEY,
format(MISSING_TEMPLATE_PARAMS_MESSAGE, name));
assert(name !is Token.init, "Invalid template parameter name."); // this shouldn't happen
if (!lastSeenFun.ddocParams.canFind(name.text))
addErrorMessage(name, MISSING_PARAMS_KEY,
format(MISSING_TEMPLATE_PARAMS_MESSAGE, name.text));
else
lastSeenFun.params[name] = true;
lastSeenFun.params[name.text] = true;
}
}
static string templateParamName(const TemplateParameter p)
static Token templateParamName(const TemplateParameter p)
{
if (p.templateTypeParameter)
return p.templateTypeParameter.identifier.text;
return p.templateTypeParameter.identifier;
if (p.templateValueParameter)
return p.templateValueParameter.identifier.text;
return p.templateValueParameter.identifier;
if (p.templateAliasParameter)
return p.templateAliasParameter.identifier.text;
return p.templateAliasParameter.identifier;
if (p.templateTupleParameter)
return p.templateTupleParameter.identifier.text;
return p.templateTupleParameter.identifier;
if (p.templateThisParameter)
return p.templateThisParameter.templateTypeParameter.identifier.text;
return p.templateThisParameter.templateTypeParameter.identifier;
return null;
return Token.init;
}
bool hasDeclaration(const Declaration decl)
@ -483,7 +494,8 @@ unittest
/**
Some text
*/
void foo(int k){} // [warn]: %s
void foo(int k){} /+
^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_PARAMS_MESSAGE.format("k")
), sac);
@ -492,7 +504,8 @@ unittest
/**
Some text
*/
void foo(int K)(){} // [warn]: %s
void foo(int K)(){} /+
^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_TEMPLATE_PARAMS_MESSAGE.format("K")
), sac);
@ -501,7 +514,8 @@ unittest
/**
Some text
*/
struct Foo(Bar){} // [warn]: %s
struct Foo(Bar){} /+
^^^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_TEMPLATE_PARAMS_MESSAGE.format("Bar")
), sac);
@ -510,7 +524,8 @@ unittest
/**
Some text
*/
class Foo(Bar){} // [warn]: %s
class Foo(Bar){} /+
^^^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_TEMPLATE_PARAMS_MESSAGE.format("Bar")
), sac);
@ -519,7 +534,8 @@ unittest
/**
Some text
*/
template Foo(Bar){} // [warn]: %s
template Foo(Bar){} /+
^^^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_TEMPLATE_PARAMS_MESSAGE.format("Bar")
), sac);
@ -558,7 +574,8 @@ unittest
/**
Some text
*/
int foo(){} // [warn]: %s
int foo(){} /+
^^^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_RETURNS_MESSAGE,
), sac);
@ -567,7 +584,8 @@ unittest
/**
Some text
*/
auto foo(){} // [warn]: %s
auto foo(){} /+
^^^^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_RETURNS_MESSAGE,
), sac);
@ -594,10 +612,12 @@ unittest
*/
private void foo(int k){}
///
public int bar(){} // [warn]: %s
public int bar(){} /+
^^^ [warn]: %s +/
public:
///
int foobar(){} // [warn]: %s
int foobar(){} /+
^^^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_RETURNS_MESSAGE,
ProperlyDocumentedPublicFunctions.MISSING_RETURNS_MESSAGE,
@ -611,10 +631,12 @@ unittest
*/
private template foo(int k){}
///
public template bar(T){} // [warn]: %s
public template bar(T){} /+
^ [warn]: %s +/
public:
///
template foobar(T){} // [warn]: %s
template foobar(T){} /+
^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_TEMPLATE_PARAMS_MESSAGE.format("T"),
ProperlyDocumentedPublicFunctions.MISSING_TEMPLATE_PARAMS_MESSAGE.format("T"),
@ -628,10 +650,12 @@ unittest
*/
private struct foo(int k){}
///
public struct bar(T){} // [warn]: %s
public struct bar(T){} /+
^ [warn]: %s +/
public:
///
struct foobar(T){} // [warn]: %s
struct foobar(T){} /+
^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_TEMPLATE_PARAMS_MESSAGE.format("T"),
ProperlyDocumentedPublicFunctions.MISSING_TEMPLATE_PARAMS_MESSAGE.format("T"),
@ -653,7 +677,8 @@ unittest
* Returns:
* A long description.
*/
int foo(int k){} // [warn]: %s
int foo(int k){} /+
^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_PARAMS_MESSAGE.format("k")
), sac);
@ -667,7 +692,8 @@ int foo(int k){} // [warn]: %s
* Returns:
* A long description.
*/
int foo(int k) => k; // [warn]: %s
int foo(int k) => k; /+
^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_PARAMS_MESSAGE.format("k")
), sac);
@ -683,7 +709,8 @@ k = A stupid parameter
Returns:
A long description.
*/
int foo(int k){} // [warn]: %s
int foo(int k){} /+
^^^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.NON_EXISTENT_PARAMS_MESSAGE.format("val")
), sac);
@ -697,7 +724,8 @@ Params:
Returns:
A long description.
*/
int foo(int k){} // [warn]: %s
int foo(int k){} /+
^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_PARAMS_MESSAGE.format("k")
), sac);
@ -714,7 +742,8 @@ foobar = A stupid parameter
Returns:
A long description.
*/
int foo(int foo, int foobar){} // [warn]: %s
int foo(int foo, int foobar){} /+
^^^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.NON_EXISTENT_PARAMS_MESSAGE.format("bad")
), sac);
@ -731,7 +760,8 @@ foobar = A stupid parameter
Returns:
A long description.
*/
struct foo(int foo, int foobar){} // [warn]: %s
struct foo(int foo, int foobar){} /+
^^^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.NON_EXISTENT_PARAMS_MESSAGE.format("bad")
), sac);
@ -835,7 +865,8 @@ int bar(int f){}
int foo(int k){}
/// ditto
int bar(int bar){} // [warn]: %s
int bar(int bar){} /+
^^^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_PARAMS_MESSAGE.format("bar")
), sac);
@ -854,7 +885,8 @@ int bar(int bar){} // [warn]: %s
* See_Also:
* $(REF takeExactly, std,range)
*/
int foo(int k){} // [warn]: %s
int foo(int k){} /+
^^^ [warn]: %s +/
/// ditto
int bar(int bar){}
@ -956,7 +988,8 @@ Params:
Returns: Awesome values.
+/
string bar(P, R)(R r){}// [warn]: %s
string bar(P, R)(R r){}/+
^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_TEMPLATE_PARAMS_MESSAGE.format("P")
), sac);
@ -1020,7 +1053,8 @@ unittest
/++
Simple case
+/
void bar(){throw new Exception("bla");} // [warn]: %s
void bar(){throw new Exception("bla");} /+
^^^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_THROW_MESSAGE.format("Exception")
), sac);
@ -1051,7 +1085,8 @@ unittest
/++
rethrow
+/
void bar(){try throw new Exception("bla"); catch(Exception) throw new Error();} // [warn]: %s
void bar(){try throw new Exception("bla"); catch(Exception) throw new Error();} /+
^^^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_THROW_MESSAGE.format("Error")
), sac);
@ -1079,7 +1114,8 @@ unittest
/++
case of throw in nested func
+/
void bar() // [warn]: %s
void bar() /+
^^^ [warn]: %s +/
{
void foo(){throw new AssertError("bla");}
foo();
@ -1116,7 +1152,8 @@ unittest
/++
case of double throw in nested func but only 1 caught
+/
void bar() // [warn]: %s
void bar() /+
^^^ [warn]: %s +/
{
void foo(){throw new AssertError("bla");throw new Error("bla");}
try foo();
@ -1136,7 +1173,8 @@ unittest
/++
enforce
+/
void bar() // [warn]: %s
void bar() /+
^^^ [warn]: %s +/
{
enforce(condition);
}
@ -1154,7 +1192,8 @@ unittest
/++
enforce
+/
void bar() // [warn]: %s
void bar() /+
^^^ [warn]: %s +/
{
enforce!AssertError(condition);
}
@ -1172,7 +1211,8 @@ unittest
/++
enforce
+/
void bar() // [warn]: %s
void bar() /+
^^^ [warn]: %s +/
{
enforce!(AssertError)(condition);
}
@ -1190,7 +1230,8 @@ unittest
/++
enforce
+/
void foo() // [warn]: %s
void foo() /+
^^^ [warn]: %s +/
{
void bar()
{

19
src/dscanner/analysis/range.d
View file

@ -50,7 +50,11 @@ final class BackwardsRangeCheck : BaseAnalyzer
string message = format(
"%d is larger than %d. Did you mean to use 'foreach_reverse( ... ; %d .. %d)'?",
left, right, right, left);
addErrorMessage(line, this.column, KEY, message);
auto start = &foreachStatement.low.tokens[0];
auto endIncl = &foreachStatement.high.tokens[$ - 1];
assert(endIncl >= start);
auto tokens = start[0 .. endIncl - start + 1];
addErrorMessage(tokens, KEY, message);
}
hasLeft = false;
hasRight = false;
@ -82,9 +86,6 @@ final class BackwardsRangeCheck : BaseAnalyzer
return;
if (state == State.left)
{
line = primary.primary.line;
this.column = primary.primary.column;
try
left = parseNumber(primary.primary.text);
catch (ConvException e)
@ -116,7 +117,7 @@ final class BackwardsRangeCheck : BaseAnalyzer
string message = format("%d is larger than %d. This slice is likely incorrect.",
left, right);
addErrorMessage(line, this.column, KEY, message);
addErrorMessage(index, KEY, message);
}
hasLeft = false;
hasRight = false;
@ -129,8 +130,6 @@ private:
bool hasRight;
long left;
long right;
size_t column;
size_t line;
enum State
{
ignore,
@ -173,10 +172,12 @@ unittest
int[] data = [1, 2, 3, 4, 5];
data = data[1 .. 3]; // ok
data = data[3 .. 1]; // [warn]: 3 is larger than 1. This slice is likely incorrect.
data = data[3 .. 1]; /+
^^^^^^ [warn]: 3 is larger than 1. This slice is likely incorrect. +/
foreach (n; 1 .. 3) { } // ok
foreach (n; 3 .. 1) { } // [warn]: 3 is larger than 1. Did you mean to use 'foreach_reverse( ... ; 1 .. 3)'?
foreach (n; 3 .. 1) { } /+
^^^^^^ [warn]: 3 is larger than 1. Did you mean to use 'foreach_reverse( ... ; 1 .. 3)'? +/
}
}c, sac);

36
src/dscanner/analysis/redundant_attributes.d
View file

@ -103,8 +103,7 @@ private:
auto match = currentAttributes.find!(a => a.attribute.type == attr.attribute.type);
if (!match.empty)
{
auto token = attr.attribute;
addErrorMessage(token.line, token.column, KEY,
addErrorMessage(attr, KEY,
text("same visibility attribute used as defined on line ",
match.front.attribute.line.to!string, "."));
return false;
@ -194,12 +193,15 @@ unittest
unittest
{
private:
private int blah; // [warn]: same visibility attribute used as defined on line 4.
private int blah; /+
^^^^^^^ [warn]: same visibility attribute used as defined on line 4. +/
protected
{
protected int blah; // [warn]: same visibility attribute used as defined on line 6.
protected int blah; /+
^^^^^^^^^ [warn]: same visibility attribute used as defined on line 7. +/
}
private int blah; // [warn]: same visibility attribute used as defined on line 4.
private int blah; /+
^^^^^^^ [warn]: same visibility attribute used as defined on line 4. +/
}}c, sac);
// test labels vs. block attributes
@ -207,11 +209,14 @@ protected
unittest
{
private:
private: // [warn]: same visibility attribute used as defined on line 4.
private: /+
^^^^^^^ [warn]: same visibility attribute used as defined on line 4. +/
public:
private int a;
public int b; // [warn]: same visibility attribute used as defined on line 6.
public // [warn]: same visibility attribute used as defined on line 6.
public int b; /+
^^^^^^ [warn]: same visibility attribute used as defined on line 7. +/
public /+
^^^^^^ [warn]: same visibility attribute used as defined on line 7. +/
{
int c;
}
@ -222,8 +227,10 @@ unittest
unittest
{
private:
private int foo2; // [warn]: same visibility attribute used as defined on line 4.
private void foo() // [warn]: same visibility attribute used as defined on line 4.
private int foo2; /+
^^^^^^^ [warn]: same visibility attribute used as defined on line 4. +/
private void foo() /+
^^^^^^^ [warn]: same visibility attribute used as defined on line 4. +/
{
private int blah;
}
@ -235,7 +242,8 @@ unittest
{
private:
public int a;
private: // [warn]: same visibility attribute used as defined on line 4.
private: /+
^^^^^^^ [warn]: same visibility attribute used as defined on line 4. +/
}}c, sac);
// test conditional compilation
@ -245,7 +253,8 @@ unittest
version(unittest)
{
private:
private int foo; // [warn]: same visibility attribute used as defined on line 6.
private int foo; /+
^^^^^^^ [warn]: same visibility attribute used as defined on line 6. +/
}
private int foo2;
}}c, sac);
@ -263,7 +272,8 @@ public:
{
public int b;
}
public int b; // [warn]: same visibility attribute used as defined on line 4.
public int b; /+
^^^^^^ [warn]: same visibility attribute used as defined on line 4. +/
}}c, sac);
}

6
src/dscanner/analysis/redundant_parens.d
View file

@ -37,8 +37,7 @@ final class RedundantParenCheck : BaseAnalyzer
goto end;
if (unary.primaryExpression.expression is null)
goto end;
addErrorMessage(unary.primaryExpression.expression.line,
unary.primaryExpression.expression.column, KEY, "Redundant parenthesis.");
addErrorMessage(unary.primaryExpression, KEY, "Redundant parenthesis.");
end:
statement.accept(this);
}
@ -55,8 +54,7 @@ final class RedundantParenCheck : BaseAnalyzer
goto end;
if (unary.primaryExpression.expression is null)
goto end;
addErrorMessage(primaryExpression.expression.line,
primaryExpression.expression.column, KEY, "Redundant parenthesis.");
addErrorMessage(primaryExpression, KEY, "Redundant parenthesis.");
end:
primaryExpression.accept(this);
}

2
src/dscanner/analysis/redundant_storage_class.d
View file

@ -59,7 +59,7 @@ final class RedundantStorageClassCheck : BaseAnalyzer
return;
auto t = vd.declarators[0].name;
string message = REDUNDANT_VARIABLE_ATTRIBUTES.format(t.text, globalAttributes);
addErrorMessage(t.line, t.column, "dscanner.unnecessary.duplicate_attribute", message);
addErrorMessage(t, "dscanner.unnecessary.duplicate_attribute", message);
}
}
}

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

@ -106,8 +106,10 @@ void messageFunctionFormat(string format, Message message, bool isError)
auto s = format;
s = s.replace("{filepath}", message.fileName);
s = s.replace("{line}", to!string(message.line));
s = s.replace("{column}", to!string(message.column));
s = s.replace("{line}", to!string(message.startLine));
s = s.replace("{column}", to!string(message.startColumn));
s = s.replace("{endLine}", to!string(message.endLine));
s = s.replace("{endColumn}", to!string(message.endColumn));
s = s.replace("{type}", isError ? "error" : "warn");
s = s.replace("{message}", message.message);
s = s.replace("{name}", message.checkName);
@ -122,7 +124,7 @@ void messageFunction(Message message, bool isError)
void messageFunctionJSON(string fileName, size_t line, size_t column, string message, bool)
{
writeJSON(Message(fileName, line, column, "dscanner.syntax", message));
writeJSON(Message(Message.Diagnostic.from(fileName, line, column, column, message), "dscanner.syntax"));
}
void writeJSON(Message message)
@ -138,9 +140,35 @@ void writeJSON(Message message)
writeln(` "name": "`, message.checkName, `",`);
}
writeln(` "fileName": "`, message.fileName.replace("\\", "\\\\").replace(`"`, `\"`), `",`);
writeln(` "line": `, message.line, `,`);
writeln(` "column": `, message.column, `,`);
writeln(` "message": "`, message.message.replace("\\", "\\\\").replace(`"`, `\"`), `"`);
writeln(` "line": `, message.startLine, `,`);
writeln(` "column": `, message.startColumn, `,`);
writeln(` "endLine": `, message.endLine, `,`);
writeln(` "endColumn": `, message.endColumn, `,`);
writeln(` "message": "`, message.message.replace("\\", "\\\\").replace(`"`, `\"`), `",`);
if (message.supplemental.length)
{
writeln(` "supplemental": [`);
foreach (i, suppl; message.supplemental)
{
if (i != 0)
writeln(",");
writeln(` {`);
if (message.fileName != suppl.fileName)
writeln(` "fileName": `, suppl.fileName, `,`);
if (suppl.message.length)
writeln(` "message": `, suppl.message, `,`);
writeln(` "line": `, suppl.startLine, `,`);
writeln(` "column": `, suppl.startColumn, `,`);
writeln(` "endLine": `, suppl.endLine, `,`);
writeln(` "endColumn": `, suppl.endColumn);
write(` }`);
}
if (message.supplemental.length)
writeln();
writeln(` ]`);
}
else
writeln(` "supplemental": []`);
write(" }");
}
@ -156,7 +184,9 @@ void generateReport(string[] fileNames, const StaticAnalysisConfig config,
auto reporter = new DScannerJsonReporter();
auto writeMessages = delegate void(string fileName, size_t line, size_t column, string message, bool isError){
reporter.addMessage(Message(fileName, line, column, "dscanner.syntax", message), isError);
reporter.addMessage(
Message(Message.Diagnostic.from(fileName, line, column, column, message), "dscanner.syntax"),
isError);
};
first = true;
@ -194,7 +224,9 @@ void generateSonarQubeGenericIssueDataReport(string[] fileNames, const StaticAna
auto reporter = new SonarQubeGenericIssueDataReporter();
auto writeMessages = delegate void(string fileName, size_t line, size_t column, string message, bool isError){
reporter.addMessage(Message(fileName, line, column, "dscanner.syntax", message), isError);
reporter.addMessage(
Message(Message.Diagnostic.from(fileName, line, column, column, message), "dscanner.syntax"),
isError);
};
foreach (fileName; fileNames)
@ -280,7 +312,9 @@ const(Module) parseModule(string fileName, ubyte[] code, RollbackAllocator* p,
ulong* linesOfCode = null, uint* errorCount = null, uint* warningCount = null)
{
auto writeMessages = delegate(string fileName, size_t line, size_t column, string message, bool isError){
return messageFunctionFormat(errorFormat, Message(fileName, line, column, "dscanner.syntax", message), isError);
return messageFunctionFormat(errorFormat,
Message(Message.Diagnostic.from(fileName, line, column, column, message), "dscanner.syntax"),
isError);
};
return parseModule(fileName, code, p, cache, tokens,

8
src/dscanner/analysis/static_if_else.d
View file

@ -45,7 +45,10 @@ final class StaticIfElse : BaseAnalyzer
{
return;
}
addErrorMessage(ifStmt.line, ifStmt.column, KEY, "Mismatched static if. Use 'else static if' here.");
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.");
}
const(IfStatement) getIfStatement(const ConditionalStatement cc)
@ -68,7 +71,8 @@ unittest
void foo() {
static if (false)
auto a = 0;
else if (true) // [warn]: Mismatched static if. Use 'else static if' here.
else if (true) /+
^^^^^^^ [warn]: Mismatched static if. Use 'else static if' here. +/
auto b = 1;
}
}c, sac);

36
src/dscanner/analysis/style.d
View file

@ -36,7 +36,7 @@ final class StyleChecker : BaseAnalyzer
foreach (part; dec.moduleName.identifiers)
{
if (part.text.matchFirst(moduleNameRegex).length == 0)
addErrorMessage(part.line, part.column, KEY,
addErrorMessage(part, KEY,
"Module/package name '" ~ part.text ~ "' does not match style guidelines.");
}
}
@ -102,7 +102,7 @@ final class StyleChecker : BaseAnalyzer
void checkLowercaseName(string type, ref const Token name)
{
if (name.text.length > 0 && name.text.matchFirst(varFunNameRegex).length == 0)
addErrorMessage(name.line, name.column, KEY,
addErrorMessage(name, KEY,
type ~ " name '" ~ name.text ~ "' does not match style guidelines.");
}
@ -135,7 +135,7 @@ final class StyleChecker : BaseAnalyzer
void checkAggregateName(string aggregateType, ref const Token name)
{
if (name.text.length > 0 && name.text.matchFirst(aggregateNameRegex).length == 0)
addErrorMessage(name.line, name.column, KEY,
addErrorMessage(name, KEY,
aggregateType ~ " name '" ~ name.text ~ "' does not match style guidelines.");
}
@ -166,17 +166,22 @@ unittest
sac.style_check = Check.enabled;
assertAnalyzerWarnings(q{
module AMODULE; // [warn]: Module/package name 'AMODULE' does not match style guidelines.
module AMODULE; /+
^^^^^^^ [warn]: Module/package name 'AMODULE' does not match style guidelines. +/
bool A_VARIABLE; // FIXME:
bool a_variable; // ok
bool aVariable; // ok
void A_FUNCTION() {} // FIXME:
class cat {} // [warn]: Class name 'cat' does not match style guidelines.
interface puma {} // [warn]: Interface name 'puma' does not match style guidelines.
struct dog {} // [warn]: Struct name 'dog' does not match style guidelines.
enum racoon { a } // [warn]: Enum name 'racoon' does not match style guidelines.
class cat {} /+
^^^ [warn]: Class name 'cat' does not match style guidelines. +/
interface puma {} /+
^^^^ [warn]: Interface name 'puma' does not match style guidelines. +/
struct dog {} /+
^^^ [warn]: Struct name 'dog' does not match style guidelines. +/
enum racoon { a } /+
^^^^^^ [warn]: Enum name 'racoon' does not match style guidelines. +/
enum bool something = false;
enum bool someThing = false;
enum Cat { fritz, }
@ -194,7 +199,8 @@ unittest
assertAnalyzerWarnings(q{
extern(Windows)
{
extern(D) bool Fun2(); // [warn]: Function name 'Fun2' does not match style guidelines.
extern(D) bool Fun2(); /+
^^^^ [warn]: Function name 'Fun2' does not match style guidelines. +/
bool Fun3();
}
}c, sac);
@ -203,8 +209,10 @@ unittest
extern(Windows)
{
extern(C):
extern(D) bool Fun4(); // [warn]: Function name 'Fun4' does not match style guidelines.
bool Fun5(); // [warn]: Function name 'Fun5' does not match style guidelines.
extern(D) bool Fun4(); /+
^^^^ [warn]: Function name 'Fun4' does not match style guidelines. +/
bool Fun5(); /+
^^^^ [warn]: Function name 'Fun5' does not match style guidelines. +/
}
}c, sac);
@ -214,12 +222,14 @@ unittest
bool Fun7();
extern(D):
void okOkay();
void NotReallyOkay(); // [warn]: Function name 'NotReallyOkay' does not match style guidelines.
void NotReallyOkay(); /+
^^^^^^^^^^^^^ [warn]: Function name 'NotReallyOkay' does not match style guidelines. +/
}c, sac);
assertAnalyzerWarnings(q{
extern(Windows):
bool WinButWithBody(){} // [warn]: Function name 'WinButWithBody' does not match style guidelines.
bool WinButWithBody(){} /+
^^^^^^^^^^^^^^ [warn]: Function name 'WinButWithBody' does not match style guidelines. +/
}c, sac);
stderr.writeln("Unittest for StyleChecker passed.");

26
src/dscanner/analysis/trust_too_much.d
View file

@ -39,10 +39,7 @@ public:
override void visit(const AtAttribute d)
{
if (checkAtAttribute && d.identifier.text == "trusted")
{
const Token t = d.identifier;
addErrorMessage(t.line, t.column, KEY, MESSAGE);
}
addErrorMessage(d, KEY, MESSAGE);
d.accept(this);
}
@ -91,17 +88,20 @@ unittest
//--- fail cases ---//
assertAnalyzerWarnings(q{
@trusted: // [warn]: %s
@trusted: /+
^^^^^^^^ [warn]: %s +/
void test();
}c.format(msg), sac);
assertAnalyzerWarnings(q{
@trusted @nogc: // [warn]: %s
@trusted @nogc: /+
^^^^^^^^ [warn]: %s +/
void test();
}c.format(msg), sac);
assertAnalyzerWarnings(q{
@trusted { // [warn]: %s
@trusted { /+
^^^^^^^^ [warn]: %s +/
void test();
void test();
}
@ -109,27 +109,31 @@ unittest
assertAnalyzerWarnings(q{
@safe {
@trusted @nogc { // [warn]: %s
@trusted @nogc { /+
^^^^^^^^ [warn]: %s +/
void test();
void test();
}}
}c.format(msg), sac);
assertAnalyzerWarnings(q{
@nogc @trusted { // [warn]: %s
@nogc @trusted { /+
^^^^^^^^ [warn]: %s +/
void test();
void test();
}
}c.format(msg), sac);
assertAnalyzerWarnings(q{
@trusted template foo(){ // [warn]: %s
@trusted template foo(){ /+
^^^^^^^^ [warn]: %s +/
}
}c.format(msg), sac);
assertAnalyzerWarnings(q{
struct foo{
@trusted: // [warn]: %s
@trusted: /+
^^^^^^^^ [warn]: %s +/
}
}c.format(msg), sac);
//--- pass cases ---//

45
src/dscanner/analysis/undocumented.d
View file

@ -100,15 +100,14 @@ final class UndocumentedDeclarationCheck : BaseAnalyzer
return;
if (variable.autoDeclaration !is null)
{
addMessage(variable.autoDeclaration.parts[0].identifier.line,
variable.autoDeclaration.parts[0].identifier.column,
addMessage(variable.autoDeclaration.parts[0].identifier,
variable.autoDeclaration.parts[0].identifier.text);
return;
}
foreach (dec; variable.declarators)
{
if (dec.comment.ptr is null)
addMessage(dec.name.line, dec.name.column, dec.name.text);
addMessage(dec.name, dec.name.text);
return;
}
}
@ -174,20 +173,25 @@ private:
|| isGetterOrSetter(declaration.name.text)
|| isProperty(declaration)))
{
addMessage(declaration.name.line,
declaration.name.column, declaration.name.text);
addMessage(declaration.name, declaration.name.text);
}
}
else
{
if (declaration.name.type != tok!"")
addMessage(declaration.name.line,
declaration.name.column, declaration.name.text);
addMessage(declaration.name, declaration.name.text);
}
}
else
{
addMessage(declaration.line, declaration.column, null);
import std.algorithm : countUntil;
// like constructors
auto tokens = declaration.tokens.findTokenForDisplay(tok!"this");
auto earlyEnd = tokens.countUntil!(a => a == tok!"{" || a == tok!"(" || a == tok!";");
if (earlyEnd != -1)
tokens = tokens[0 .. earlyEnd];
addMessage(tokens, null);
}
}
static if (!(is(T == TemplateDeclaration) || is(T == FunctionDeclaration)))
@ -215,11 +219,11 @@ private:
return false;
}
void addMessage(size_t line, size_t column, string name)
void addMessage(T)(T range, string name)
{
import std.string : format;
addErrorMessage(line, column, "dscanner.style.undocumented_declaration", name is null
addErrorMessage(range, "dscanner.style.undocumented_declaration", name is null
? "Public declaration is undocumented."
: format("Public declaration '%s' is undocumented.", name));
}
@ -315,13 +319,20 @@ unittest
sac.undocumented_declaration_check = Check.enabled;
assertAnalyzerWarnings(q{
class C{} // [warn]: Public declaration 'C' is undocumented.
interface I{} // [warn]: Public declaration 'I' is undocumented.
enum e = 0; // [warn]: Public declaration 'e' is undocumented.
void f(){} // [warn]: Public declaration 'f' is undocumented.
struct S{} // [warn]: Public declaration 'S' is undocumented.
template T(){} // [warn]: Public declaration 'T' is undocumented.
union U{} // [warn]: Public declaration 'U' is undocumented.
class C{} /+
^ [warn]: Public declaration 'C' is undocumented. +/
interface I{} /+
^ [warn]: Public declaration 'I' is undocumented. +/
enum e = 0; /+
^ [warn]: Public declaration 'e' is undocumented. +/
void f(){} /+
^ [warn]: Public declaration 'f' is undocumented. +/
struct S{} /+
^ [warn]: Public declaration 'S' is undocumented. +/
template T(){} /+
^ [warn]: Public declaration 'T' is undocumented. +/
union U{} /+
^ [warn]: Public declaration 'U' is undocumented. +/
}, sac);
assertAnalyzerWarnings(q{

14
src/dscanner/analysis/unmodified.d
View file

@ -63,8 +63,7 @@ final class UnmodifiedFinder : BaseAnalyzer
continue;
if (initializedFromNew(d.initializer))
continue;
tree[$ - 1].insert(new VariableInfo(d.name.text, d.name.line,
d.name.column, isValueTypeSimple(dec.type)));
tree[$ - 1].insert(new VariableInfo(d.name.text, d.name, isValueTypeSimple(dec.type)));
}
}
dec.accept(this);
@ -84,8 +83,7 @@ final class UnmodifiedFinder : BaseAnalyzer
continue;
if (initializedFromNew(part.initializer))
continue;
tree[$ - 1].insert(new VariableInfo(part.identifier.text,
part.identifier.line, part.identifier.column));
tree[$ - 1].insert(new VariableInfo(part.identifier.text, part.identifier));
}
}
autoDeclaration.accept(this);
@ -292,8 +290,7 @@ private:
static struct VariableInfo
{
string name;
size_t line;
size_t column;
Token token;
bool isValueType;
}
@ -303,7 +300,7 @@ private:
{
immutable string errorMessage = "Variable " ~ vi.name
~ " is never modified and could have been declared const or immutable.";
addErrorMessage(vi.line, vi.column, "dscanner.suspicious.unmodified", errorMessage);
addErrorMessage(vi.token, "dscanner.suspicious.unmodified", errorMessage);
}
tree = tree[0 .. $ - 1];
}
@ -346,7 +343,8 @@ bool isValueTypeSimple(const Type type) pure nothrow @nogc
// fails
assertAnalyzerWarnings(q{
void foo(){int i = 1;} // [warn]: Variable i is never modified and could have been declared const or immutable.
void foo(){int i = 1;} /+
^ [warn]: Variable i is never modified and could have been declared const or immutable. +/
}, sac);
// pass

10
src/dscanner/analysis/unused.d
View file

@ -338,11 +338,11 @@ abstract class UnusedIdentifierCheck : BaseAnalyzer
protected Tree[] tree;
protected void variableDeclared(string name, size_t line, size_t column, bool isRef)
protected void variableDeclared(string name, Token token, bool isRef)
{
if (inAggregateScope || name.all!(a => a == '_'))
return;
tree[$ - 1].insert(new UnUsed(name, line, column, isRef));
tree[$ - 1].insert(new UnUsed(name, token, isRef));
}
protected void pushScope()
@ -355,8 +355,7 @@ private:
struct UnUsed
{
string name;
size_t line;
size_t column;
Token token;
bool isRef;
bool uncertain;
}
@ -450,8 +449,7 @@ abstract class UnusedStorageCheck : UnusedIdentifierCheck
if (uu.uncertain)
continue;
immutable string errorMessage = publicType ~ ' ' ~ uu.name ~ " is never used.";
addErrorMessage(uu.line, uu.column,
"dscanner.suspicious." ~ reportType, errorMessage);
addErrorMessage(uu.token, "dscanner.suspicious." ~ reportType, errorMessage);
}
}
}

37
src/dscanner/analysis/unused_label.d
View file

@ -57,16 +57,15 @@ final class UnusedLabelCheck : BaseAnalyzer
override void visit(const LabeledStatement labeledStatement)
{
auto token = &labeledStatement.identifier;
auto token = labeledStatement.identifier;
Label* label = token.text in current;
if (label is null)
{
current[token.text] = Label(token.text, token.line, token.column, false);
current[token.text] = Label(token.text, token, false);
}
else
{
label.line = token.line;
label.column = token.column;
label.token = token;
}
if (labeledStatement.declarationOrStatement !is null)
labeledStatement.declarationOrStatement.accept(this);
@ -119,8 +118,7 @@ private:
static struct Label
{
string name;
size_t line;
size_t column;
Token token;
bool used;
}
@ -140,13 +138,13 @@ private:
{
foreach (label; current.byValue())
{
if (label.line == size_t.max || label.column == size_t.max)
if (label.token is Token.init)
{
// TODO: handle unknown labels
}
else if (!label.used)
{
addErrorMessage(label.line, label.column, "dscanner.suspicious.unused_label",
addErrorMessage(label.token, "dscanner.suspicious.unused_label",
"Label \"" ~ label.name ~ "\" is not used.");
}
}
@ -157,7 +155,7 @@ private:
{
Label* entry = name in current;
if (entry is null)
current[name] = Label(name, size_t.max, size_t.max, true);
current[name] = Label(name, Token.init, true);
else
entry.used = true;
}
@ -174,14 +172,16 @@ unittest
int testUnusedLabel()
{
int x = 0;
A: // [warn]: Label "A" is not used.
A: /+
^ [warn]: Label "A" is not used. +/
if (x) goto B;
x++;
B:
goto C;
void foo()
{
C: // [warn]: Label "C" is not used.
C: /+
^ [warn]: Label "C" is not used. +/
return;
}
C:
@ -191,10 +191,12 @@ unittest
D:
return;
}
D: // [warn]: Label "D" is not used.
D: /+
^ [warn]: Label "D" is not used. +/
goto E;
() {
E: // [warn]: Label "E" is not used.
E: /+
^ [warn]: Label "E" is not used. +/
return;
}();
E:
@ -203,9 +205,11 @@ unittest
F:
return;
}();
F: // [warn]: Label "F" is not used.
F: /+
^ [warn]: Label "F" is not used. +/
return x;
G: // [warn]: Label "G" is not used.
G: /+
^ [warn]: Label "G" is not used. +/
}
}c, sac);
@ -221,7 +225,8 @@ unittest
void testAsm()
{
asm { mov RAX,1;}
lbl: // [warn]: Label "lbl" is not used.
lbl: /+
^^^ [warn]: Label "lbl" is not used. +/
}
}c, sac);

12
src/dscanner/analysis/unused_parameter.d
View file

@ -41,8 +41,7 @@ final class UnusedParameterCheck : UnusedStorageCheck
immutable bool isPtr = parameter.type && !parameter.type
.typeSuffixes.filter!(a => a.star != tok!"").empty;
variableDeclared(parameter.name.text, parameter.name.line,
parameter.name.column, isRef | isPtr);
variableDeclared(parameter.name.text, parameter.name, isRef | isPtr);
if (parameter.default_ !is null)
{
@ -72,9 +71,11 @@ final class UnusedParameterCheck : UnusedStorageCheck
is(StringTypeOf!R));
}
void inPSC(in int a){} // [warn]: Parameter a is never used.
void inPSC(in int a){} /+
^ [warn]: Parameter a is never used. +/
void doStuff(int a, int b) // [warn]: Parameter b is never used.
void doStuff(int a, int b) /+
^ [warn]: Parameter b is never used. +/
{
return a;
}
@ -95,7 +96,8 @@ final class UnusedParameterCheck : UnusedStorageCheck
{
auto cb1 = delegate(size_t _) {};
cb1(3);
auto cb2 = delegate(size_t a) {}; // [warn]: Parameter a is never used.
auto cb2 = delegate(size_t a) {}; /+
^ [warn]: Parameter a is never used. +/
cb2(3);
}

26
src/dscanner/analysis/unused_result.d
View file

@ -4,14 +4,15 @@
// http://www.boost.org/LICENSE_1_0.txt)
module dscanner.analysis.unused_result;
import dparse.ast;
import dparse.lexer;
import dscanner.analysis.base;
import dscanner.analysis.mismatched_args : resolveSymbol, IdentVisitor;
import dscanner.analysis.mismatched_args : IdentVisitor, resolveSymbol;
import dscanner.utils;
import dsymbol.scope_;
import dsymbol.symbol;
import dparse.ast, dparse.lexer;
import std.algorithm.searching : canFind;
import std.range: retro;
import std.algorithm : canFind, countUntil;
import std.range : retro;
/**
* Checks for function call statements which call non-void functions.
@ -93,7 +94,13 @@ public:
return;
}
addErrorMessage(decl.expression.line, decl.expression.column, KEY, MSG);
const(Token)[] tokens = fce.unaryExpression
? fce.unaryExpression.tokens
: fce.type
? fce.type.tokens
: fce.tokens;
addErrorMessage(tokens, KEY, MSG);
}
}
@ -128,7 +135,8 @@ unittest
int fun() { return 1; }
void main()
{
fun(); // [warn]: %s
fun(); /+
^^^ [warn]: %s +/
}
}c.format(UnusedResultChecker.MSG), sac);
@ -143,7 +151,8 @@ unittest
alias Bar = Foo;
void main()
{
Bar.get(); // [warn]: %s
Bar.get(); /+
^^^^^^^ [warn]: %s +/
}
}c.format(UnusedResultChecker.MSG), sac);
@ -160,7 +169,8 @@ unittest
void main()
{
int fun() { return 1; }
fun(); // [warn]: %s
fun(); /+
^^^ [warn]: %s +/
}
}c.format(UnusedResultChecker.MSG), sac);

10
src/dscanner/analysis/unused_variable.d
View file

@ -31,14 +31,14 @@ final class UnusedVariableCheck : UnusedStorageCheck
override void visit(const VariableDeclaration variableDeclaration)
{
foreach (d; variableDeclaration.declarators)
this.variableDeclared(d.name.text, d.name.line, d.name.column, false);
this.variableDeclared(d.name.text, d.name, false);
variableDeclaration.accept(this);
}
override void visit(const AutoDeclaration autoDeclaration)
{
foreach (t; autoDeclaration.parts.map!(a => a.identifier))
this.variableDeclared(t.text, t.line, t.column, false);
this.variableDeclared(t.text, t, false);
autoDeclaration.accept(this);
}
}
@ -62,7 +62,8 @@ final class UnusedVariableCheck : UnusedStorageCheck
unittest
{
int a; // [warn]: Variable a is never used.
int a; /+
^ [warn]: Variable a is never used. +/
}
// Issue 380
@ -75,7 +76,8 @@ final class UnusedVariableCheck : UnusedStorageCheck
// Issue 380
int otherTemplatedEnum()
{
auto a(T) = T.init; // [warn]: Variable a is never used.
auto a(T) = T.init; /+
^ [warn]: Variable a is never used. +/
return 0;
}

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

@ -94,7 +94,7 @@ final class UselessAssertCheck : BaseAnalyzer
default:
return;
}
addErrorMessage(ae.line, ae.column, KEY, MESSAGE);
addErrorMessage(unary, KEY, MESSAGE);
}
private:
@ -113,9 +113,12 @@ unittest
assertAnalyzerWarnings(q{
unittest
{
assert(true); // [warn]: %1$s
assert(1); // [warn]: %1$s
assert([10]); // [warn]: %1$s
assert(true); /+
^^^^ [warn]: %1$s +/
assert(1); /+
^ [warn]: %1$s +/
assert([10]); /+
^^^^ [warn]: %1$s +/
assert(false);
assert(0);
assert(0.0L);

85
src/dscanner/analysis/useless_initializer.d
View file

@ -155,14 +155,18 @@ public:
version(unittest)
{
enum warn = q{addErrorMessage(declarator.name.line,
declarator.name.column, key, msg);};
void warn(const BaseNode range)
{
addErrorMessage(range, key, msg);
}
}
else
{
import std.format : format;
enum warn = q{addErrorMessage(declarator.name.line,
declarator.name.column, key, msg.format(declarator.name.text));};
void warn(const BaseNode range)
{
addErrorMessage(range, key, msg.format(declarator.name.text));
}
}
// --- Info about the declaration type --- //
@ -203,32 +207,32 @@ public:
case tok!"cent", tok!"ucent":
case tok!"bool":
if (intDefs.canFind(value.text) || value == tok!"false")
mixin(warn);
warn(nvi);
goto default;
default:
// check for BasicType.init
if (ue.primaryExpression.basicType.type == decl.type.type2.builtinType &&
ue.primaryExpression.primary.text == "init" &&
!ue.primaryExpression.expression)
mixin(warn);
warn(nvi);
}
}
else if (isSzInt)
{
if (intDefs.canFind(value.text))
mixin(warn);
warn(nvi);
}
else if (isPtr || isStr)
{
if (str(value.type) == "null")
mixin(warn);
warn(nvi);
}
else if (isArr)
{
if (str(value.type) == "null")
mixin(warn);
warn(nvi);
else if (nvi.arrayInitializer && nvi.arrayInitializer.arrayMemberInitializations.length == 0)
mixin(warn);
warn(nvi);
}
}
@ -242,7 +246,7 @@ public:
if (customType.text in _structCanBeInit)
{
if (!_structCanBeInit[customType.text])
mixin(warn);
warn(nvi);
}
}
}
@ -251,7 +255,7 @@ public:
else if (nvi.arrayInitializer && (isArr || isStr))
{
if (nvi.arrayInitializer.arrayMemberInitializations.length == 0)
mixin(warn);
warn(nvi);
}
}
@ -271,25 +275,44 @@ public:
// fails
assertAnalyzerWarnings(q{
struct S {}
ubyte a = 0x0; // [warn]: X
int a = 0; // [warn]: X
ulong a = 0; // [warn]: X
int* a = null; // [warn]: X
Foo* a = null; // [warn]: X
int[] a = null; // [warn]: X
int[] a = []; // [warn]: X
string a = null; // [warn]: X
string a = null; // [warn]: X
wstring a = null; // [warn]: X
dstring a = null; // [warn]: X
size_t a = 0; // [warn]: X
ptrdiff_t a = 0; // [warn]: X
string a = []; // [warn]: X
char[] a = null; // [warn]: X
int a = int.init; // [warn]: X
char a = char.init; // [warn]: X
S s = S.init; // [warn]: X
bool a = false; // [warn]: X
ubyte a = 0x0; /+
^^^ [warn]: X +/
int a = 0; /+
^ [warn]: X +/
ulong a = 0; /+
^ [warn]: X +/
int* a = null; /+
^^^^ [warn]: X +/
Foo* a = null; /+
^^^^ [warn]: X +/
int[] a = null; /+
^^^^ [warn]: X +/
int[] a = []; /+
^^ [warn]: X +/
string a = null; /+
^^^^ [warn]: X +/
string a = null; /+
^^^^ [warn]: X +/
wstring a = null; /+
^^^^ [warn]: X +/
dstring a = null; /+
^^^^ [warn]: X +/
size_t a = 0; /+
^ [warn]: X +/
ptrdiff_t a = 0; /+
^ [warn]: X +/
string a = []; /+
^^ [warn]: X +/
char[] a = null; /+
^^^^ [warn]: X +/
int a = int.init; /+
^^^^^^^^ [warn]: X +/
char a = char.init; /+
^^^^^^^^^ [warn]: X +/
S s = S.init; /+
^^^^^^ [warn]: X +/
bool a = false; /+
^^^^^ [warn]: X +/
}, sac);
// passes

14
src/dscanner/analysis/vcall_in_ctor.d
View file

@ -136,7 +136,7 @@ private:
{
if (call == vm)
{
addErrorMessage(call.line, call.column, KEY, MSG);
addErrorMessage(call, KEY, MSG);
break;
}
}
@ -306,7 +306,8 @@ unittest
assertAnalyzerWarnings(q{
class Bar
{
this(){foo();} // [warn]: %s
this(){foo();} /+
^^^ [warn]: %s +/
private:
public
void foo(){}
@ -319,8 +320,10 @@ unittest
{
this()
{
foo(); // [warn]: %s
foo(); // [warn]: %s
foo(); /+
^^^ [warn]: %s +/
foo(); /+
^^^ [warn]: %s +/
bar();
}
private: void bar();
@ -334,7 +337,8 @@ unittest
this()
{
foo();
bar(); // [warn]: %s
bar(); /+
^^^ [warn]: %s +/
}
private: public void bar();
public private {void foo(){}}

2
src/dscanner/main.d
View file

@ -406,7 +406,7 @@ Options:
Format errors produced by the style/syntax checkers. The default
value for the pattern is: "%2$s".
Supported placeholders are: {filepath}, {line}, {column}, {type},
{message}, and {name}.
{endLine}, {endColumn}, {message}, and {name}.
--ctags <file | directory>..., -c <file | directory>...
Generates ctags information from the given source code file. Note that

63
src/dscanner/reports.d
View file

@ -59,10 +59,24 @@ class DScannerJsonReporter
JSONValue js = JSONValue([
"key": JSONValue(issue.message.key),
"fileName": JSONValue(issue.message.fileName),
"line": JSONValue(issue.message.line),
"column": JSONValue(issue.message.column),
"line": JSONValue(issue.message.startLine),
"column": JSONValue(issue.message.startColumn),
"endLine": JSONValue(issue.message.endLine),
"endColumn": JSONValue(issue.message.endColumn),
"message": JSONValue(issue.message.message),
"type": JSONValue(issue.type)
"type": JSONValue(issue.type),
"supplemental": JSONValue(
issue.message.supplemental.map!(a =>
JSONValue([
"fileName": JSONValue(a.fileName),
"line": JSONValue(a.startLine),
"column": JSONValue(a.startColumn),
"endLine": JSONValue(a.endLine),
"endColumn": JSONValue(a.endColumn),
"message": JSONValue(a.message),
])
).array
)
]);
// dfmt on
@ -129,10 +143,10 @@ class SonarQubeGenericIssueDataReporter
struct TextRange
{
// SonarQube Generic Issue only allows specifying start line only
// or the complete range, for which start and end has to differ
// D-Scanner does not provide the complete range info
long startLine;
long endLine;
long startColumn;
long endColumn;
}
private Appender!(Issue[]) _issues;
@ -160,6 +174,20 @@ class SonarQubeGenericIssueDataReporter
return result.toPrettyString();
}
private static JSONValue toJson(Location location)
{
return JSONValue([
"message": JSONValue(location.message),
"filePath": JSONValue(location.filePath),
"textRange": JSONValue([
"startLine": JSONValue(location.textRange.startLine),
"endLine": JSONValue(location.textRange.endLine),
"startColumn": JSONValue(location.textRange.startColumn),
"endColumn": JSONValue(location.textRange.endColumn)
]),
]);
}
private static JSONValue toJson(Issue issue)
{
// dfmt off
@ -168,13 +196,8 @@ class SonarQubeGenericIssueDataReporter
"ruleId": JSONValue(issue.ruleId),
"severity": JSONValue(issue.severity),
"type": JSONValue(issue.type),
"primaryLocation": JSONValue([
"message": JSONValue(issue.primaryLocation.message),
"filePath": JSONValue(issue.primaryLocation.filePath),
"textRange": JSONValue([
"startLine": JSONValue(issue.primaryLocation.textRange.startLine)
]),
]),
"primaryLocation": toJson(issue.primaryLocation),
"secondaryLocations": JSONValue(issue.secondaryLocations.map!toJson.array),
]);
// dfmt on
}
@ -189,18 +212,20 @@ class SonarQubeGenericIssueDataReporter
// dfmt off
Issue issue = {
engineId: "dscanner",
ruleId : message.key,
severity : (isError) ? Severity.blocker : getSeverity(message.key),
type : getType(message.key),
primaryLocation : getPrimaryLocation(message)
ruleId: message.key,
severity: (isError) ? Severity.blocker : getSeverity(message.key),
type: getType(message.key),
primaryLocation: getLocation(message.diagnostic),
secondaryLocations: message.supplemental.map!getLocation.array
};
// dfmt on
return issue;
}
private static Location getPrimaryLocation(Message message)
private static Location getLocation(Message.Diagnostic diag)
{
return Location(message.message, message.fileName, TextRange(message.line));
return Location(diag.message, diag.fileName,
TextRange(diag.startLine, diag.endLine, diag.startColumn, diag.endColumn));
}
private static string getSeverity(string key)