From a1ed66d13784471b987e215bf5903ac4de6e524e Mon Sep 17 00:00:00 2001 From: Basile Burg Date: Thu, 22 Jun 2017 20:12:52 +0200 Subject: [PATCH 01/11] fix #474 fix #473 fix #476 - Cases of false and non positive with the useless init check --- src/analysis/useless_initializer.d | 31 +++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/src/analysis/useless_initializer.d b/src/analysis/useless_initializer.d index 5deb5650..b6e763bc 100644 --- a/src/analysis/useless_initializer.d +++ b/src/analysis/useless_initializer.d @@ -41,29 +41,36 @@ public: super(fileName, null, skipTests); } + // issue 473, prevent to visit delegate that contain duck type checkers. + override void visit(const(TypeofExpression)) {} + override void visit(const(VariableDeclaration) decl) { + import std.algorithm : among, canFind; + import std.algorithm.iteration : filter; + import std.range : empty; + + if (!decl.type || !decl.type.type2 || + // issue 474, manifest constants HAVE to be initialized. + !decl.storageClasses.filter!(a => a.token == tok!"enum").empty) + return; + foreach (declarator; decl.declarators) { - if (!decl.type || !decl.type.type2) - continue; if (!declarator.initializer || !declarator.initializer.nonVoidInitializer) continue; - import std.format : format; - 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));}; + } // --- Info about the declaration type --- // - import std.algorithm : among, canFind; - import std.algorithm.iteration : filter; - import std.range : empty; - const bool isPtr = decl.type.typeSuffixes && !decl.type.typeSuffixes .filter!(a => a.star != tok!"").empty; const bool isArr = decl.type.typeSuffixes && !decl.type.typeSuffixes @@ -100,7 +107,8 @@ public: case tok!"int", tok!"uint": case tok!"long", tok!"ulong": case tok!"cent", tok!"ucent": - if (intDefs.canFind(value.text)) + case tok!"bool": + if (intDefs.canFind(value.text) || value == tok!"false") mixin(warn); goto default; default: @@ -199,6 +207,7 @@ public: 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 @@ -221,6 +230,10 @@ public: 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;}))); + bool a; }, sac); stderr.writeln("Unittest for UselessInitializerChecker passed."); From 191a694cf6ed1dd71fab8addb314f24f3f600443 Mon Sep 17 00:00:00 2001 From: Basile Burg Date: Thu, 22 Jun 2017 21:39:15 +0200 Subject: [PATCH 02/11] do not warn on documented variables --- src/analysis/useless_initializer.d | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/analysis/useless_initializer.d b/src/analysis/useless_initializer.d index b6e763bc..ea123f59 100644 --- a/src/analysis/useless_initializer.d +++ b/src/analysis/useless_initializer.d @@ -50,15 +50,20 @@ public: import std.algorithm.iteration : filter; import std.range : empty; + + 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.filter!(a => a.token == tok!"enum").empty) - return; + return; foreach (declarator; decl.declarators) { - if (!declarator.initializer || !declarator.initializer.nonVoidInitializer) - continue; + if (!declarator.initializer || !declarator.initializer.nonVoidInitializer || + declarator.comment !is null) + continue; version(unittest) enum warn = q{addErrorMessage(declarator.name.line, declarator.name.column, From ea70c619ded8946183f6e8f70e4e6891cec961d5 Mon Sep 17 00:00:00 2001 From: Basile Burg Date: Thu, 22 Jun 2017 23:26:53 +0200 Subject: [PATCH 03/11] fix #477 - Custom type initialized to init should not trigger a warn --- src/analysis/useless_initializer.d | 49 +++++++++++++++++++++++++----- 1 file changed, 42 insertions(+), 7 deletions(-) diff --git a/src/analysis/useless_initializer.d b/src/analysis/useless_initializer.d index ea123f59..f83dc725 100644 --- a/src/analysis/useless_initializer.d +++ b/src/analysis/useless_initializer.d @@ -5,8 +5,13 @@ module analysis.useless_initializer; import analysis.base; +import containers.dynamicarray; +import containers.hashmap; import dparse.ast; import dparse.lexer; +import std.algorithm : among, canFind; +import std.algorithm.iteration : filter; +import std.range : empty; import std.stdio; /* @@ -33,12 +38,40 @@ private: static immutable strDefs = [`""`, `""c`, `""w`, `""d`, "``", "``c", "``w", "``d", "q{}"]; static immutable intDefs = ["0", "0L", "0UL", "0uL", "0U", "0x0", "0b0"]; + HashMap!(string, bool) _knownStructs; + DynamicArray!(const(Token)*) _nestedStructs; + DynamicArray!(bool) _inStruct; + public: /// this(string fileName, bool skipTests = false) { super(fileName, null, skipTests); + _inStruct.insert(false); + } + + override void visit(const(StructDeclaration) decl) + { + _nestedStructs.insert(&decl.name); + _knownStructs[decl.name.text] = false; + decl.accept(this); + _nestedStructs.removeBack(); + } + + override void visit(const(Declaration) decl) + { + _inStruct.insert(decl.structDeclaration !is null); + decl.accept(this); + if (_inStruct[$-2] && decl.constructor && + (decl.constructor.parameters && decl.constructor.parameters.parameters.length == 0 || + !decl.constructor.parameters)) + { + _knownStructs[_nestedStructs.back().text] = !decl.attributes + .filter!(a => a.atAttribute !is null && a.atAttribute.identifier.text == "disable") + .empty; + } + _inStruct.removeBack(); } // issue 473, prevent to visit delegate that contain duck type checkers. @@ -46,12 +79,6 @@ public: override void visit(const(VariableDeclaration) decl) { - import std.algorithm : among, canFind; - import std.algorithm.iteration : filter; - import std.range : empty; - - - if (!decl.type || !decl.type.type2 || // initializer has to appear clearly in generated ddoc decl.comment !is null || @@ -167,7 +194,11 @@ public: ue.unaryExpression.primaryExpression.identifierOrTemplateInstance.identifier == customType && ue.identifierOrTemplateInstance && ue.identifierOrTemplateInstance.identifier.text == "init") { - mixin(warn); + if (customType.text in _knownStructs) + { + if (!_knownStructs[customType.text]) + mixin(warn); + } } // 'Symbol ArrayInitializer' : assumes Symbol is an array b/c of the Init @@ -193,6 +224,7 @@ public: // fails assertAnalyzerWarnings(q{ + struct S {} ubyte a = 0x0; // [warn]: X int a = 0; // [warn]: X ulong a = 0; // [warn]: X @@ -217,6 +249,7 @@ public: // passes assertAnalyzerWarnings(q{ + struct D {@disable this();} ubyte a = 0xFE; int a = 1; ulong a = 1; @@ -239,6 +272,8 @@ public: enum ubyte a = 0; static assert(is(typeof((){T t = T.init;}))); bool a; + D d = D.init; + NotKnown nk = NotKnown.init; }, sac); stderr.writeln("Unittest for UselessInitializerChecker passed."); From 5d41fd931b5bdfea3a8d659f2f7474951fb98c24 Mon Sep 17 00:00:00 2001 From: Basile Burg Date: Fri, 23 Jun 2017 02:14:57 +0200 Subject: [PATCH 04/11] allow struct.init when know struct has `@disable` ctor --- src/analysis/useless_initializer.d | 37 ++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/src/analysis/useless_initializer.d b/src/analysis/useless_initializer.d index f83dc725..b316a40c 100644 --- a/src/analysis/useless_initializer.d +++ b/src/analysis/useless_initializer.d @@ -38,9 +38,10 @@ private: static immutable strDefs = [`""`, `""c`, `""w`, `""d`, "``", "``c", "``w", "``d", "q{}"]; static immutable intDefs = ["0", "0L", "0UL", "0uL", "0U", "0x0", "0b0"]; - HashMap!(string, bool) _knownStructs; - DynamicArray!(const(Token)*) _nestedStructs; + HashMap!(string, bool) _structCanBeInit; + DynamicArray!(string) _structStack; DynamicArray!(bool) _inStruct; + DynamicArray!(bool) _atDisabled; public: @@ -53,27 +54,43 @@ public: override void visit(const(StructDeclaration) decl) { - _nestedStructs.insert(&decl.name); - _knownStructs[decl.name.text] = false; + _structStack.insert(decl.name.text); + _structCanBeInit[decl.name.text] = false; + _atDisabled.insert(false); decl.accept(this); - _nestedStructs.removeBack(); + _structStack.removeBack(); + _atDisabled.removeBack(); } override void visit(const(Declaration) decl) { _inStruct.insert(decl.structDeclaration !is null); decl.accept(this); - if (_inStruct[$-2] && decl.constructor && + if (_inStruct.length > 1 && _inStruct[$-2] && decl.constructor && (decl.constructor.parameters && decl.constructor.parameters.parameters.length == 0 || !decl.constructor.parameters)) { - _knownStructs[_nestedStructs.back().text] = !decl.attributes + _atDisabled[$-1] = !decl.attributes .filter!(a => a.atAttribute !is null && a.atAttribute.identifier.text == "disable") .empty; } _inStruct.removeBack(); } + override void visit(const(Constructor) decl) + { + if (_inStruct.length > 1 && _inStruct[$-2] && + (decl.parameters && decl.parameters.parameters.length == 0 || !decl.parameters)) + { + _structCanBeInit[_structStack.back()] = !_atDisabled[$-1]; + if (!_structCanBeInit[_structStack.back()]) + _structCanBeInit[_structStack.back()] = !decl.memberFunctionAttributes + .filter!(a => a.atAttribute !is null && a.atAttribute.identifier.text == "disable") + .empty; + } + decl.accept(this); + } + // issue 473, prevent to visit delegate that contain duck type checkers. override void visit(const(TypeofExpression)) {} @@ -194,9 +211,9 @@ public: ue.unaryExpression.primaryExpression.identifierOrTemplateInstance.identifier == customType && ue.identifierOrTemplateInstance && ue.identifierOrTemplateInstance.identifier.text == "init") { - if (customType.text in _knownStructs) + if (customType.text in _structCanBeInit) { - if (!_knownStructs[customType.text]) + if (!_structCanBeInit[customType.text]) mixin(warn); } } @@ -250,6 +267,7 @@ public: // passes assertAnalyzerWarnings(q{ struct D {@disable this();} + struct E {this() @disable;} ubyte a = 0xFE; int a = 1; ulong a = 1; @@ -273,6 +291,7 @@ public: static assert(is(typeof((){T t = T.init;}))); bool a; D d = D.init; + E e = E.init; NotKnown nk = NotKnown.init; }, sac); From 133e3b9ea9a0bbda4f0d47502d769861b730edee Mon Sep 17 00:00:00 2001 From: Basile Burg Date: Fri, 23 Jun 2017 04:52:09 +0200 Subject: [PATCH 05/11] fix last false detection in phobos --- src/analysis/useless_initializer.d | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/analysis/useless_initializer.d b/src/analysis/useless_initializer.d index b316a40c..23243a46 100644 --- a/src/analysis/useless_initializer.d +++ b/src/analysis/useless_initializer.d @@ -16,7 +16,8 @@ import std.stdio; /* Limitations: - - Stuff = Stuff.init doesnot work with type with * [] + - Stuff = Stuff.init does not work with type with postfixes`*` `[]`. + - Stuff = Stuff.init is only detected for struct within the module. */ /** @@ -42,6 +43,7 @@ private: DynamicArray!(string) _structStack; DynamicArray!(bool) _inStruct; DynamicArray!(bool) _atDisabled; + DynamicArray!(bool) _inTest; public: @@ -50,10 +52,21 @@ public: { super(fileName, null, skipTests); _inStruct.insert(false); + _inTest.insert(false); + } + + override void visit(const(Unittest) test) + { + _inTest.insert(true); + test.accept(this); + _inTest.removeBack(); } override void visit(const(StructDeclaration) decl) { + if (_inTest.back()) + return; + _structStack.insert(decl.name.text); _structCanBeInit[decl.name.text] = false; _atDisabled.insert(false); @@ -67,7 +80,7 @@ public: _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 && decl.constructor.parameters.parameters.length == 0) || !decl.constructor.parameters)) { _atDisabled[$-1] = !decl.attributes @@ -80,7 +93,7 @@ public: override void visit(const(Constructor) decl) { if (_inStruct.length > 1 && _inStruct[$-2] && - (decl.parameters && decl.parameters.parameters.length == 0 || !decl.parameters)) + ((decl.parameters && decl.parameters.parameters.length == 0) || !decl.parameters)) { _structCanBeInit[_structStack.back()] = !_atDisabled[$-1]; if (!_structCanBeInit[_structStack.back()]) From 12c92b7eafc78d56858552ff518fcaadde578014 Mon Sep 17 00:00:00 2001 From: Basile Burg Date: Fri, 23 Jun 2017 16:33:39 +0200 Subject: [PATCH 06/11] prevent check in the "compiles" trait --- src/analysis/useless_initializer.d | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/src/analysis/useless_initializer.d b/src/analysis/useless_initializer.d index 23243a46..7815fd16 100644 --- a/src/analysis/useless_initializer.d +++ b/src/analysis/useless_initializer.d @@ -16,8 +16,9 @@ import std.stdio; /* Limitations: - - Stuff = Stuff.init does not work with type with postfixes`*` `[]`. - - Stuff = Stuff.init is only detected for struct within the module. + - 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). */ /** @@ -31,10 +32,15 @@ final class UselessInitializerChecker : BaseAnalyzer 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`; + } static immutable strDefs = [`""`, `""c`, `""w`, `""d`, "``", "``c", "``w", "``d", "q{}"]; static immutable intDefs = ["0", "0L", "0UL", "0uL", "0U", "0x0", "0b0"]; @@ -104,9 +110,22 @@ public: decl.accept(this); } - // issue 473, prevent to visit delegate that contain duck type checkers. + // issue 473, prevent to visit delegates that contain duck type checkers. override void visit(const(TypeofExpression)) {} + // 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); + } + } + override void visit(const(VariableDeclaration) decl) { if (!decl.type || !decl.type.type2 || @@ -123,8 +142,10 @@ public: continue; version(unittest) + { enum warn = q{addErrorMessage(declarator.name.line, declarator.name.column, key, msg);}; + } else { import std.format : format; @@ -302,6 +323,7 @@ public: 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; From b37b39854e7b53d28598b23ce6ec753e66bf8dde Mon Sep 17 00:00:00 2001 From: Basile Burg Date: Mon, 26 Jun 2017 00:02:43 +0200 Subject: [PATCH 07/11] - 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 --- src/analysis/useless_initializer.d | 64 +++++++++++++++++++----------- 1 file changed, 41 insertions(+), 23 deletions(-) diff --git a/src/analysis/useless_initializer.d b/src/analysis/useless_initializer.d index 7815fd16..2ff3a030 100644 --- a/src/analysis/useless_initializer.d +++ b/src/analysis/useless_initializer.d @@ -9,8 +9,7 @@ import containers.dynamicarray; import containers.hashmap; import dparse.ast; import dparse.lexer; -import std.algorithm : among, canFind; -import std.algorithm.iteration : filter; +import std.algorithm; import std.range : empty; import std.stdio; @@ -73,8 +72,14 @@ public: if (_inTest.back()) return; - _structStack.insert(decl.name.text); - _structCanBeInit[decl.name.text] = false; + assert(_inStruct.length > 1); + + const string structName = _inStruct[$-2] ? + _structStack.back() ~ "." ~ decl.name.text : + decl.name.text; + + _structStack.insert(structName); + _structCanBeInit[structName] = false; _atDisabled.insert(false); decl.accept(this); _structStack.removeBack(); @@ -89,9 +94,8 @@ public: ((decl.constructor.parameters && decl.constructor.parameters.parameters.length == 0) || !decl.constructor.parameters)) { - _atDisabled[$-1] = !decl.attributes - .filter!(a => a.atAttribute !is null && a.atAttribute.identifier.text == "disable") - .empty; + _atDisabled[$-1] = decl.attributes + .canFind!(a => a.atAttribute !is null && a.atAttribute.identifier.text == "disable"); } _inStruct.removeBack(); } @@ -101,11 +105,11 @@ public: if (_inStruct.length > 1 && _inStruct[$-2] && ((decl.parameters && decl.parameters.parameters.length == 0) || !decl.parameters)) { - _structCanBeInit[_structStack.back()] = !_atDisabled[$-1]; - if (!_structCanBeInit[_structStack.back()]) + const bool canBeInit = !_atDisabled[$-1]; + _structCanBeInit[_structStack.back()] = canBeInit; + if (!canBeInit) _structCanBeInit[_structStack.back()] = !decl.memberFunctionAttributes - .filter!(a => a.atAttribute !is null && a.atAttribute.identifier.text == "disable") - .empty; + .canFind!(a => a.atAttribute !is null && a.atAttribute.identifier.text == "disable"); } decl.accept(this); } @@ -132,32 +136,37 @@ public: // initializer has to appear clearly in generated ddoc decl.comment !is null || // issue 474, manifest constants HAVE to be initialized. - !decl.storageClasses.filter!(a => a.token == tok!"enum").empty) - return; + decl.storageClasses.canFind!(a => a.token == tok!"enum")) + { + return; + } foreach (declarator; decl.declarators) { - if (!declarator.initializer || !declarator.initializer.nonVoidInitializer || + if (!declarator.initializer || + !declarator.initializer.nonVoidInitializer || declarator.comment !is null) + { continue; + } version(unittest) { - enum warn = q{addErrorMessage(declarator.name.line, declarator.name.column, - key, msg);}; + 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));}; + enum warn = q{addErrorMessage(declarator.name.line, + declarator.name.column, key, msg.format(declarator.name.text));}; } // --- Info about the declaration type --- // - 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; + 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; @@ -315,7 +324,16 @@ public: dstring a = "fgh"d; string a = q{int a;}; size_t a = 1; - ptrdiff_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); From 7d01a740c02e5c8e05353defe5d9c4b475b946a8 Mon Sep 17 00:00:00 2001 From: Basile Burg Date: Mon, 26 Jun 2017 00:07:52 +0200 Subject: [PATCH 08/11] fix, self-linting missed a case that was not yet fixed --- src/analysis/unmodified.d | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/analysis/unmodified.d b/src/analysis/unmodified.d index e6617355..37576c97 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) From 7ef6c4ba7086bd137b57fa78daaef25e3cf4cd49 Mon Sep 17 00:00:00 2001 From: Basile Burg Date: Mon, 26 Jun 2017 00:38:26 +0200 Subject: [PATCH 09/11] fix more undetected warns during self linting --- src/analysis/base.d | 2 +- src/analysis/duplicate_attribute.d | 12 ++++++------ src/analysis/enumarrayliteral.d | 2 +- src/analysis/function_attributes.d | 4 ++-- src/analysis/line_length.d | 1 - src/analysis/objectconst.d | 2 +- src/analysis/opequals_without_tohash.d | 6 +++--- src/analysis/run.d | 2 +- src/analysis/undocumented.d | 8 ++++---- 9 files changed, 19 insertions(+), 20 deletions(-) diff --git a/src/analysis/base.d b/src/analysis/base.d index 193b971c..e0c9f80e 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 d10cd6b8..7471adb3 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 e31a660e..99933fec 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 5dde5f22..871ff4b7 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 542f330d..d411c713 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 b7b4d4b4..d2b0a983 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 4c29f08e..f0e2fa2d 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 942ef01c..2e5e79b2 100644 --- a/src/analysis/run.d +++ b/src/analysis/run.d @@ -160,7 +160,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 6bde2564..013e5183 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)) From 49dbb332defcb2adf7000e3526b62e737fc0b01a Mon Sep 17 00:00:00 2001 From: Basile Burg Date: Wed, 28 Jun 2017 07:20:51 +0200 Subject: [PATCH 10/11] use a flag instead of a stack + apply skipTests --- src/analysis/duplicate_attribute.d | 4 ++-- src/analysis/useless_initializer.d | 11 ++++++----- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/analysis/duplicate_attribute.d b/src/analysis/duplicate_attribute.d index 7471adb3..887367b3 100644 --- a/src/analysis/duplicate_attribute.d +++ b/src/analysis/duplicate_attribute.d @@ -35,8 +35,8 @@ class DuplicateAttributeCheck : BaseAnalyzer void checkAttributes(const Declaration node) { bool hasProperty; - bool hasSafe ; - bool hasTrusted ; + bool hasSafe; + bool hasTrusted; bool hasSystem; bool hasPure; bool hasNoThrow; diff --git a/src/analysis/useless_initializer.d b/src/analysis/useless_initializer.d index 2ff3a030..e92e716c 100644 --- a/src/analysis/useless_initializer.d +++ b/src/analysis/useless_initializer.d @@ -48,7 +48,7 @@ private: DynamicArray!(string) _structStack; DynamicArray!(bool) _inStruct; DynamicArray!(bool) _atDisabled; - DynamicArray!(bool) _inTest; + bool _inTest; public: @@ -57,19 +57,20 @@ public: { super(fileName, null, skipTests); _inStruct.insert(false); - _inTest.insert(false); } override void visit(const(Unittest) test) { - _inTest.insert(true); + if (skipTests) + return; + _inTest = true; test.accept(this); - _inTest.removeBack(); + _inTest = false; } override void visit(const(StructDeclaration) decl) { - if (_inTest.back()) + if (_inTest) return; assert(_inStruct.length > 1); From e9b792ac8517588b22fa32014ef79001d527f748 Mon Sep 17 00:00:00 2001 From: Basile Burg Date: Wed, 28 Jun 2017 07:41:45 +0200 Subject: [PATCH 11/11] convert spaces to tabs --- src/analysis/useless_initializer.d | 628 ++++++++++++++--------------- 1 file changed, 314 insertions(+), 314 deletions(-) diff --git a/src/analysis/useless_initializer.d b/src/analysis/useless_initializer.d index e92e716c..6fae7a86 100644 --- a/src/analysis/useless_initializer.d +++ b/src/analysis/useless_initializer.d @@ -15,9 +15,9 @@ import std.stdio; /* Limitations: - - 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). + - 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). */ /** @@ -26,329 +26,329 @@ Limitations: */ final class UselessInitializerChecker : BaseAnalyzer { - alias visit = BaseAnalyzer.visit; + alias visit = BaseAnalyzer.visit; private: - enum key = "dscanner.useless-initializer"; + 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`; - } + 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"]; + 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; + 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); - _inStruct.insert(false); - } - - override void visit(const(Unittest) test) - { - if (skipTests) - return; - _inTest = true; - test.accept(this); - _inTest = false; - } - - override void visit(const(StructDeclaration) decl) - { - if (_inTest) - return; - - assert(_inStruct.length > 1); - - const string structName = _inStruct[$-2] ? - _structStack.back() ~ "." ~ decl.name.text : - decl.name.text; - - _structStack.insert(structName); - _structCanBeInit[structName] = false; - _atDisabled.insert(false); - decl.accept(this); - _structStack.removeBack(); - _atDisabled.removeBack(); - } - - 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(); - } - - 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); - } - - // issue 473, prevent to visit delegates that contain duck type checkers. - override void visit(const(TypeofExpression)) {} - - // 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); - } - } - - 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; - } - - foreach (declarator; decl.declarators) - { - if (!declarator.initializer || - !declarator.initializer.nonVoidInitializer || - declarator.comment !is null) - { - continue; - } - - 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));}; - } - - // --- 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); - } + /// + this(string fileName, bool skipTests = false) + { + super(fileName, null, skipTests); + _inStruct.insert(false); + } + + override void visit(const(Unittest) test) + { + if (skipTests) + return; + _inTest = true; + test.accept(this); + _inTest = false; + } + + override void visit(const(StructDeclaration) decl) + { + if (_inTest) + return; + + assert(_inStruct.length > 1); + + const string structName = _inStruct[$-2] ? + _structStack.back() ~ "." ~ decl.name.text : + decl.name.text; + + _structStack.insert(structName); + _structCanBeInit[structName] = false; + _atDisabled.insert(false); + decl.accept(this); + _structStack.removeBack(); + _atDisabled.removeBack(); + } + + 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(); + } + + 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); + } + + // issue 473, prevent to visit delegates that contain duck type checkers. + override void visit(const(TypeofExpression)) {} + + // 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); + } + } + + 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; + } + + foreach (declarator; decl.declarators) + { + if (!declarator.initializer || + !declarator.initializer.nonVoidInitializer || + declarator.comment !is null) + { + continue; + } + + 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));}; + } + + // --- 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; - - StaticAnalysisConfig sac = disabledConfig; - sac.useless_initializer = Check.enabled; - - // 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{ - 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."); + import analysis.config : Check, disabledConfig, StaticAnalysisConfig; + import analysis.helpers: assertAnalyzerWarnings; + import std.stdio : stderr; + + StaticAnalysisConfig sac = disabledConfig; + sac.useless_initializer = Check.enabled; + + // 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{ + 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."); }