diff --git a/src/analysis/base.d b/src/analysis/base.d index 193b971..e0c9f80 100644 --- a/src/analysis/base.d +++ b/src/analysis/base.d @@ -56,7 +56,7 @@ public: protected: - bool inAggregate = false; + bool inAggregate; bool skipTests; template visitTemplate(T) diff --git a/src/analysis/duplicate_attribute.d b/src/analysis/duplicate_attribute.d index d10cd6b..887367b 100644 --- a/src/analysis/duplicate_attribute.d +++ b/src/analysis/duplicate_attribute.d @@ -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) diff --git a/src/analysis/enumarrayliteral.d b/src/analysis/enumarrayliteral.d index e31a660..99933fe 100644 --- a/src/analysis/enumarrayliteral.d +++ b/src/analysis/enumarrayliteral.d @@ -24,7 +24,7 @@ class EnumArrayLiteralCheck : BaseAnalyzer super(fileName, sc, skipTests); } - bool looking = false; + bool looking; mixin visitTemplate!ClassDeclaration; mixin visitTemplate!InterfaceDeclaration; diff --git a/src/analysis/function_attributes.d b/src/analysis/function_attributes.d index 5dde5f2..871ff4b 100644 --- a/src/analysis/function_attributes.d +++ b/src/analysis/function_attributes.d @@ -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" diff --git a/src/analysis/line_length.d b/src/analysis/line_length.d index 542f330..d411c71 100644 --- a/src/analysis/line_length.d +++ b/src/analysis/line_length.d @@ -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 diff --git a/src/analysis/objectconst.d b/src/analysis/objectconst.d index b7b4d4b..d2b0a98 100644 --- a/src/analysis/objectconst.d +++ b/src/analysis/objectconst.d @@ -65,7 +65,7 @@ class ObjectConstCheck : BaseAnalyzer || name == "toString" || name == "opCast"; } - private bool looking = false; + private bool looking; } diff --git a/src/analysis/opequals_without_tohash.d b/src/analysis/opequals_without_tohash.d index 4c29f08..f0e2fa2 100644 --- a/src/analysis/opequals_without_tohash.d +++ b/src/analysis/opequals_without_tohash.d @@ -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) diff --git a/src/analysis/run.d b/src/analysis/run.d index a5e63aa..af25248 100644 --- a/src/analysis/run.d +++ b/src/analysis/run.d @@ -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); diff --git a/src/analysis/undocumented.d b/src/analysis/undocumented.d index 6bde256..013e518 100644 --- a/src/analysis/undocumented.d +++ b/src/analysis/undocumented.d @@ -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)) diff --git a/src/analysis/unmodified.d b/src/analysis/unmodified.d index e661735..37576c9 100644 --- a/src/analysis/unmodified.d +++ b/src/analysis/unmodified.d @@ -223,7 +223,7 @@ private: castExpression.accept(this); } - bool foundCast = false; + bool foundCast; } if (initializer is null) diff --git a/src/analysis/useless_initializer.d b/src/analysis/useless_initializer.d index 5deb565..6fae7a8 100644 --- a/src/analysis/useless_initializer.d +++ b/src/analysis/useless_initializer.d @@ -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."); }