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

Fixes cases of false or non positive with the useless init check (#475) 

* fix #474 fix #473 fix #476 - Cases of false and non positive with the useless init check

* do not warn on documented variables

* fix #477 - Custom type initialized to init should not trigger a warn

* allow struct.init when know struct has `@disable` ctor

* fix last false detection in phobos

* prevent check in the "compiles" trait

* - use canFind when filter.empty was negated
- FQN the struct names
- prevent a double query in the canBeInit AA
- import the whole also package
- there was not test on non-initilized variables

* fix, self-linting missed a case that was not yet fixed

* fix more undetected warns during self linting

* use a flag instead of a stack + apply skipTests

* convert spaces to tabs
This commit is contained in:
Basile Burg 2017-06-28 08:08:33 +02:00 committed by GitHub
parent 3aeee9a108
commit f89d356601
11 changed files with 323 additions and 198 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/analysis/base.d
View File

@ -56,7 +56,7 @@ public:
protected:
bool inAggregate = false;
bool inAggregate;
bool skipTests;
template visitTemplate(T)

12
src/analysis/duplicate_attribute.d
View File

@ -34,12 +34,12 @@ class DuplicateAttributeCheck : BaseAnalyzer
void checkAttributes(const Declaration node)
{
bool hasProperty = false;
bool hasSafe = false;
bool hasTrusted = false;
bool hasSystem = false;
bool hasPure = false;
bool hasNoThrow = false;
bool hasProperty;
bool hasSafe;
bool hasTrusted;
bool hasSystem;
bool hasPure;
bool hasNoThrow;
// Check the attributes
foreach (attribute; node.attributes)

2
src/analysis/enumarrayliteral.d
View File

@ -24,7 +24,7 @@ class EnumArrayLiteralCheck : BaseAnalyzer
super(fileName, sc, skipTests);
}
bool looking = false;
bool looking;
mixin visitTemplate!ClassDeclaration;
mixin visitTemplate!InterfaceDeclaration;

4
src/analysis/function_attributes.d
View File

@ -59,8 +59,8 @@ class FunctionAttributeCheck : BaseAnalyzer
{
if (dec.parameters.parameters.length == 0)
{
bool foundConst = false;
bool foundProperty = false;
bool foundConst;
bool foundProperty;
foreach (attribute; dec.attributes)
foundConst = foundConst || attribute.attribute.type == tok!"const"
|| attribute.attribute.type == tok!"immutable"

1
src/analysis/line_length.d
View File

@ -125,7 +125,6 @@ private:
size_t length;
const newLine = tok.line > prevLine;
bool multiLine;
if (tok.text is null)
length += str(tok.type).length;
else

2
src/analysis/objectconst.d
View File

@ -65,7 +65,7 @@ class ObjectConstCheck : BaseAnalyzer
|| name == "toString" || name == "opCast";
}
private bool looking = false;
private bool looking;
}

6
src/analysis/opequals_without_tohash.d
View File

@ -39,9 +39,9 @@ class OpEqualsWithoutToHashCheck : BaseAnalyzer
private void actualCheck(const Token name, const StructBody structBody)
{
bool hasOpEquals = false;
bool hasToHash = false;
bool hasOpCmp = false;
bool hasOpEquals;
bool hasToHash;
bool hasOpCmp;
// Just return if missing children
if (!structBody || !structBody.declarations || name is Token.init)

2
src/analysis/run.d
View File

@ -161,7 +161,7 @@ void generateReport(string[] fileNames, const StaticAnalysisConfig config,
bool analyze(string[] fileNames, const StaticAnalysisConfig config,
ref StringCache cache, ref ModuleCache moduleCache, bool staticAnalyze = true)
{
bool hasErrors = false;
bool hasErrors;
foreach (fileName; fileNames)
{
auto code = readFile(fileName);

8
src/analysis/undocumented.d
View File

@ -51,10 +51,10 @@ class UndocumentedDeclarationCheck : BaseAnalyzer
immutable bool prevOverride = getOverride();
immutable bool prevDisabled = getDisabled();
immutable bool prevDeprecated = getDeprecated();
bool dis = false;
bool dep = false;
bool ovr = false;
bool pushed = false;
bool dis;
bool dep;
bool ovr;
bool pushed;
foreach (attribute; dec.attributes)
{
if (isProtection(attribute.attribute.type))

2
src/analysis/unmodified.d
View File

@ -223,7 +223,7 @@ private:
castExpression.accept(this);
}
bool foundCast = false;
bool foundCast;
}
if (initializer is null)

480
src/analysis/useless_initializer.d
View File

@ -5,13 +5,19 @@
module analysis.useless_initializer;
import analysis.base;
import containers.dynamicarray;
import containers.hashmap;
import dparse.ast;
import dparse.lexer;
import std.algorithm;
import std.range : empty;
import std.stdio;
/*
Limitations:
- Stuff = Stuff.init doesnot work with type with * []
- Stuff s = Stuff.init does not work with type with postfixes`*` `[]`.
- Stuff s = Stuff.init is only detected for struct within the module.
- BasicType b = BasicType(v), default init used in BasicType ctor, e.g int(8).
*/
/**
@ -20,209 +26,329 @@ Limitations:
*/
final class UselessInitializerChecker : BaseAnalyzer
{
alias visit = BaseAnalyzer.visit;
alias visit = BaseAnalyzer.visit;
private:
enum key = "dscanner.useless-initializer";
version(unittest)
enum msg = "X";
else
enum msg = `Variable %s initializer is useless because it does not differ from the default value`;
enum key = "dscanner.useless-initializer";
static immutable strDefs = [`""`, `""c`, `""w`, `""d`, "``", "``c", "``w", "``d", "q{}"];
static immutable intDefs = ["0", "0L", "0UL", "0uL", "0U", "0x0", "0b0"];
version(unittest)
{
enum msg = "X";
}
else
{
enum msg = `Variable %s initializer is useless because it does not differ from the default value`;
}
static immutable strDefs = [`""`, `""c`, `""w`, `""d`, "``", "``c", "``w", "``d", "q{}"];
static immutable intDefs = ["0", "0L", "0UL", "0uL", "0U", "0x0", "0b0"];
HashMap!(string, bool) _structCanBeInit;
DynamicArray!(string) _structStack;
DynamicArray!(bool) _inStruct;
DynamicArray!(bool) _atDisabled;
bool _inTest;
public:
///
this(string fileName, bool skipTests = false)
{
super(fileName, null, skipTests);
}
///
this(string fileName, bool skipTests = false)
{
super(fileName, null, skipTests);
_inStruct.insert(false);
}
override void visit(const(VariableDeclaration) decl)
{
foreach (declarator; decl.declarators)
{
if (!decl.type || !decl.type.type2)
continue;
if (!declarator.initializer || !declarator.initializer.nonVoidInitializer)
continue;
override void visit(const(Unittest) test)
{
if (skipTests)
return;
_inTest = true;
test.accept(this);
_inTest = false;
}
import std.format : format;
override void visit(const(StructDeclaration) decl)
{
if (_inTest)
return;
version(unittest)
enum warn = q{addErrorMessage(declarator.name.line, declarator.name.column,
key, msg);};
else
enum warn = q{addErrorMessage(declarator.name.line, declarator.name.column,
key, msg.format(declarator.name.text));};
assert(_inStruct.length > 1);
// --- Info about the declaration type --- //
import std.algorithm : among, canFind;
import std.algorithm.iteration : filter;
import std.range : empty;
const string structName = _inStruct[$-2] ?
_structStack.back() ~ "." ~ decl.name.text :
decl.name.text;
const bool isPtr = decl.type.typeSuffixes && !decl.type.typeSuffixes
.filter!(a => a.star != tok!"").empty;
const bool isArr = decl.type.typeSuffixes && !decl.type.typeSuffixes
.filter!(a => a.array).empty;
_structStack.insert(structName);
_structCanBeInit[structName] = false;
_atDisabled.insert(false);
decl.accept(this);
_structStack.removeBack();
_atDisabled.removeBack();
}
bool isStr, isSzInt;
Token customType;
override void visit(const(Declaration) decl)
{
_inStruct.insert(decl.structDeclaration !is null);
decl.accept(this);
if (_inStruct.length > 1 && _inStruct[$-2] && decl.constructor &&
((decl.constructor.parameters && decl.constructor.parameters.parameters.length == 0) ||
!decl.constructor.parameters))
{
_atDisabled[$-1] = decl.attributes
.canFind!(a => a.atAttribute !is null && a.atAttribute.identifier.text == "disable");
}
_inStruct.removeBack();
}
if (decl.type.type2.symbol && decl.type.type2.symbol.identifierOrTemplateChain &&
decl.type.type2.symbol.identifierOrTemplateChain.identifiersOrTemplateInstances.length == 1)
{
const IdentifierOrTemplateInstance idt =
decl.type.type2.symbol.identifierOrTemplateChain.identifiersOrTemplateInstances[0];
override void visit(const(Constructor) decl)
{
if (_inStruct.length > 1 && _inStruct[$-2] &&
((decl.parameters && decl.parameters.parameters.length == 0) || !decl.parameters))
{
const bool canBeInit = !_atDisabled[$-1];
_structCanBeInit[_structStack.back()] = canBeInit;
if (!canBeInit)
_structCanBeInit[_structStack.back()] = !decl.memberFunctionAttributes
.canFind!(a => a.atAttribute !is null && a.atAttribute.identifier.text == "disable");
}
decl.accept(this);
}
customType = idt.identifier;
isStr = customType.text.among("string", "wstring", "dstring") != 0;
isSzInt = customType.text.among("size_t", "ptrdiff_t") != 0;
}
// issue 473, prevent to visit delegates that contain duck type checkers.
override void visit(const(TypeofExpression)) {}
// --- 'BasicType/Symbol AssignExpression' ---//
const NonVoidInitializer nvi = declarator.initializer.nonVoidInitializer;
const UnaryExpression ue = cast(UnaryExpression) nvi.assignExpression;
if (ue && ue.primaryExpression)
{
const Token value = ue.primaryExpression.primary;
// issue 473, prevent to check expressions in __traits(compiles, ...)
override void visit(const(TraitsExpression) e)
{
if (e.identifier.text == "compiles")
{
return;
}
else
{
e.accept(this);
}
}
if (!isPtr && !isArr && !isStr && decl.type.type2.builtinType != tok!"")
{
switch(decl.type.type2.builtinType)
{
// check for common cases of default values
case tok!"byte", tok!"ubyte":
case tok!"short", tok!"ushort":
case tok!"int", tok!"uint":
case tok!"long", tok!"ulong":
case tok!"cent", tok!"ucent":
if (intDefs.canFind(value.text))
mixin(warn);
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);
}
}
else if (isSzInt)
{
if (intDefs.canFind(value.text))
mixin(warn);
}
else if (isPtr)
{
if (str(value.type) == "null")
mixin(warn);
}
else if (isArr)
{
if (str(value.type) == "null")
mixin(warn);
else if (nvi.arrayInitializer && nvi.arrayInitializer.arrayMemberInitializations.length == 0)
mixin(warn);
else if (decl.type.type2.builtinType != tok!"")
{
switch(decl.type.type2.builtinType)
{
case tok!"char", tok!"wchar", tok!"dchar":
if (strDefs.canFind(value.text))
mixin(warn);
break;
default:
}
}
}
else if (isStr)
{
if (strDefs.canFind(value.text))
mixin(warn);
else if (nvi.arrayInitializer && nvi.arrayInitializer.arrayMemberInitializations.length == 0)
mixin(warn);
}
}
override void visit(const(VariableDeclaration) decl)
{
if (!decl.type || !decl.type.type2 ||
// initializer has to appear clearly in generated ddoc
decl.comment !is null ||
// issue 474, manifest constants HAVE to be initialized.
decl.storageClasses.canFind!(a => a.token == tok!"enum"))
{
return;
}
// Symbol s = Symbol.init
else if (ue && customType != tok!"" && ue.unaryExpression && ue.unaryExpression.primaryExpression &&
ue.unaryExpression.primaryExpression.identifierOrTemplateInstance &&
ue.unaryExpression.primaryExpression.identifierOrTemplateInstance.identifier == customType &&
ue.identifierOrTemplateInstance && ue.identifierOrTemplateInstance.identifier.text == "init")
{
mixin(warn);
}
foreach (declarator; decl.declarators)
{
if (!declarator.initializer ||
!declarator.initializer.nonVoidInitializer ||
declarator.comment !is null)
{
continue;
}
// 'Symbol ArrayInitializer' : assumes Symbol is an array b/c of the Init
else if (nvi.arrayInitializer && (isArr || isStr))
{
if (nvi.arrayInitializer.arrayMemberInitializations.length == 0)
mixin(warn);
}
}
version(unittest)
{
enum warn = q{addErrorMessage(declarator.name.line,
declarator.name.column, key, msg);};
}
else
{
import std.format : format;
enum warn = q{addErrorMessage(declarator.name.line,
declarator.name.column, key, msg.format(declarator.name.text));};
}
decl.accept(this);
}
// --- Info about the declaration type --- //
const bool isPtr = decl.type.typeSuffixes && decl.type.typeSuffixes
.canFind!(a => a.star != tok!"");
const bool isArr = decl.type.typeSuffixes && decl.type.typeSuffixes
.canFind!(a => a.array);
bool isStr, isSzInt;
Token customType;
if (decl.type.type2.symbol && decl.type.type2.symbol.identifierOrTemplateChain &&
decl.type.type2.symbol.identifierOrTemplateChain.identifiersOrTemplateInstances.length == 1)
{
const IdentifierOrTemplateInstance idt =
decl.type.type2.symbol.identifierOrTemplateChain.identifiersOrTemplateInstances[0];
customType = idt.identifier;
isStr = customType.text.among("string", "wstring", "dstring") != 0;
isSzInt = customType.text.among("size_t", "ptrdiff_t") != 0;
}
// --- 'BasicType/Symbol AssignExpression' ---//
const NonVoidInitializer nvi = declarator.initializer.nonVoidInitializer;
const UnaryExpression ue = cast(UnaryExpression) nvi.assignExpression;
if (ue && ue.primaryExpression)
{
const Token value = ue.primaryExpression.primary;
if (!isPtr && !isArr && !isStr && decl.type.type2.builtinType != tok!"")
{
switch(decl.type.type2.builtinType)
{
// check for common cases of default values
case tok!"byte", tok!"ubyte":
case tok!"short", tok!"ushort":
case tok!"int", tok!"uint":
case tok!"long", tok!"ulong":
case tok!"cent", tok!"ucent":
case tok!"bool":
if (intDefs.canFind(value.text) || value == tok!"false")
mixin(warn);
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);
}
}
else if (isSzInt)
{
if (intDefs.canFind(value.text))
mixin(warn);
}
else if (isPtr)
{
if (str(value.type) == "null")
mixin(warn);
}
else if (isArr)
{
if (str(value.type) == "null")
mixin(warn);
else if (nvi.arrayInitializer && nvi.arrayInitializer.arrayMemberInitializations.length == 0)
mixin(warn);
else if (decl.type.type2.builtinType != tok!"")
{
switch(decl.type.type2.builtinType)
{
case tok!"char", tok!"wchar", tok!"dchar":
if (strDefs.canFind(value.text))
mixin(warn);
break;
default:
}
}
}
else if (isStr)
{
if (strDefs.canFind(value.text))
mixin(warn);
else if (nvi.arrayInitializer && nvi.arrayInitializer.arrayMemberInitializations.length == 0)
mixin(warn);
}
}
// Symbol s = Symbol.init
else if (ue && customType != tok!"" && ue.unaryExpression && ue.unaryExpression.primaryExpression &&
ue.unaryExpression.primaryExpression.identifierOrTemplateInstance &&
ue.unaryExpression.primaryExpression.identifierOrTemplateInstance.identifier == customType &&
ue.identifierOrTemplateInstance && ue.identifierOrTemplateInstance.identifier.text == "init")
{
if (customType.text in _structCanBeInit)
{
if (!_structCanBeInit[customType.text])
mixin(warn);
}
}
// 'Symbol ArrayInitializer' : assumes Symbol is an array b/c of the Init
else if (nvi.arrayInitializer && (isArr || isStr))
{
if (nvi.arrayInitializer.arrayMemberInitializations.length == 0)
mixin(warn);
}
}
decl.accept(this);
}
}
@system unittest
{
import analysis.config : Check, disabledConfig, StaticAnalysisConfig;
import analysis.helpers: assertAnalyzerWarnings;
import std.stdio : stderr;
import analysis.config : Check, disabledConfig, StaticAnalysisConfig;
import analysis.helpers: assertAnalyzerWarnings;
import std.stdio : stderr;
StaticAnalysisConfig sac = disabledConfig;
sac.useless_initializer = Check.enabled;
StaticAnalysisConfig sac = disabledConfig;
sac.useless_initializer = Check.enabled;
// fails
assertAnalyzerWarnings(q{
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 = ""; // [warn]: X
string a = ""c; // [warn]: X
wstring a = ""w; // [warn]: X
dstring a = ""d; // [warn]: X
string a = q{}; // [warn]: X
size_t a = 0; // [warn]: X
ptrdiff_t a = 0; // [warn]: X
string a = []; // [warn]: X
char[] a = ""; // [warn]: X
int a = int.init; // [warn]: X
char a = char.init; // [warn]: X
S s = S.init; // [warn]: X
}, sac);
// 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 = ""; // [warn]: X
string a = ""c; // [warn]: X
wstring a = ""w; // [warn]: X
dstring a = ""d; // [warn]: X
string a = q{}; // [warn]: X
size_t a = 0; // [warn]: X
ptrdiff_t a = 0; // [warn]: X
string a = []; // [warn]: X
char[] a = ""; // [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
assertAnalyzerWarnings(q{
ubyte a = 0xFE;
int a = 1;
ulong a = 1;
int* a = &a;
Foo* a = &a;
int[] a = &a;
int[] a = [0];
string a = "sdf";
string a = "sdg"c;
wstring a = "sdg"w;
dstring a = "fgh"d;
string a = q{int a;};
size_t a = 1;
ptrdiff_t a = 1;
string a = ['a'];
char[] a = "ze";
S s = S(0,1);
S s = s.call();
}, sac);
// passes
assertAnalyzerWarnings(q{
struct D {@disable this();}
struct E {this() @disable;}
ubyte a = 0xFE;
int a = 1;
ulong a = 1;
int* a = &a;
Foo* a = &a;
int[] a = &a;
int[] a = [0];
string a = "sdf";
string a = "sdg"c;
wstring a = "sdg"w;
dstring a = "fgh"d;
string a = q{int a;};
size_t a = 1;
ptrdiff_t a;
ubyte a;
int a;
ulong a;
int* a;
Foo* a;
int[] a;
string a;
wstring a;
dstring a;
string a = ['a'];
char[] a = "ze";
S s = S(0,1);
S s = s.call();
enum {a}
enum ubyte a = 0;
static assert(is(typeof((){T t = T.init;})));
void foo(){__traits(compiles, (){int a = 0;}).writeln;}
bool a;
D d = D.init;
E e = E.init;
NotKnown nk = NotKnown.init;
}, sac);
stderr.writeln("Unittest for UselessInitializerChecker passed.");
stderr.writeln("Unittest for UselessInitializerChecker passed.");
}