From ad9491b844644e15a87f113f21f19f2bd3241d6f Mon Sep 17 00:00:00 2001 From: Sebastian Wilzbach Date: Wed, 28 Jun 2017 03:03:29 +0200 Subject: [PATCH 01/24] Temporarily disable the unused check for DScanner --- .dscanner.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.dscanner.ini b/.dscanner.ini index 21ce75a..5f3682c 100644 --- a/.dscanner.ini +++ b/.dscanner.ini @@ -23,7 +23,7 @@ if_else_same_check="enabled" ; Checks for some problems with constructors constructor_check="enabled" ; Checks for unused variables and function parameters -unused_variable_check="enabled" +unused_variable_check="disabled" ; Checks for unused labels unused_label_check="enabled" ; Checks for duplicate attributes From f89d356601a42fc14ef4c4929db97951601eacd4 Mon Sep 17 00:00:00 2001 From: Basile Burg Date: Wed, 28 Jun 2017 08:08:33 +0200 Subject: [PATCH 02/24] 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 --- 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 +- src/analysis/unmodified.d | 2 +- src/analysis/useless_initializer.d | 480 ++++++++++++++++--------- 11 files changed, 323 insertions(+), 198 deletions(-) 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."); } From 6ca85419ebf10f53ae786b71eb533e4f7d6740ef Mon Sep 17 00:00:00 2001 From: Sebastian Wilzbach Date: Thu, 29 Jun 2017 09:24:56 +0200 Subject: [PATCH 03/24] Update libdparse to 0.7.1-beta.5 --- dub.json | 2 +- libdparse | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dub.json b/dub.json index 3d790fb..cbcea30 100644 --- a/dub.json +++ b/dub.json @@ -10,7 +10,7 @@ "StdLoggerDisableWarning" ], "dependencies": { - "libdparse": "~>0.7.1-beta.4", + "libdparse": "~>0.7.1-beta.5", "dsymbol": "~>0.2.0", "inifiled": ">=0.0.6", "emsi_containers": "~>0.5.3", diff --git a/libdparse b/libdparse index 5e81535..69ba83e 160000 --- a/libdparse +++ b/libdparse @@ -1 +1 @@ -Subproject commit 5e81535d0aff4ceec2cbf03f5b02a31ae6d3fec2 +Subproject commit 69ba83ed193a8fa2d2ea6d3286376bbba4a5e676 From 45ef861268d077c1124e9620bfa45e13c55a8d59 Mon Sep 17 00:00:00 2001 From: Sebastian Wilzbach Date: Fri, 30 Jun 2017 04:31:07 +0200 Subject: [PATCH 04/24] Fix #457 - Allow to apply checks only for specific modules (#460) * Fix #457 - Allow to apply checks only for specific modules * update inifiled to 1.0.2 * Compile dependencies separately, s.t. their unittests don't get executed --- README.md | 26 +++++++ dub.json | 2 +- inifiled | 2 +- makefile | 17 +++-- src/analysis/config.d | 21 ++++++ src/analysis/run.d | 165 +++++++++++++++++++++++++++++++----------- 6 files changed, 181 insertions(+), 52 deletions(-) diff --git a/README.md b/README.md index 90b0546..ac69b98 100644 --- a/README.md +++ b/README.md @@ -304,3 +304,29 @@ For more readable output, pipe the command through [xmllint](http://xmlsoft.org/ using its formatting switch. $ dscanner --ast helloworld.d | xmllint --format - + +Selecting modules for a specific check +-------------------------------------- + +It is possible to create a new section `analysis.config.ModuleFilters` in the `.dscanner.ini`. +In this optional section a comma-separated list of inclusion and exclusion selectors can +be specified for every check on which selective filtering should be applied. +These given selectors match on the module name and partial matches (`std.` or `.foo.`) are possible. +Moreover, every selectors must begin with either `+` (inclusion) or `-` (exclusion). +Exclusion selectors take precedence over all inclusion operators. +Of course, for every check a different selector set can given: + +```ini +[analysis.config.ModuleFilters] +final_attribute_check = "+std.foo,+std.bar" +useless_initializer = "-std." +``` + +A few examples: + +- `+std.`: Includes all modules matching `std.` +- `+std.bitmanip,+std.json`: Applies the check only for these two modules +- `-std.bitmanip,-std.json`: Applies the check for all modules, but these two +- `+.bar`: Includes all modules matching `.bar` (e.g. `foo.bar`, `a.b.c.barros`) +- `-etc.`: Excludes all modules from `.etc` +- `+std,-std.internal`: Includes entire `std`, except for the internal modules diff --git a/dub.json b/dub.json index cbcea30..f4db0ff 100644 --- a/dub.json +++ b/dub.json @@ -12,7 +12,7 @@ "dependencies": { "libdparse": "~>0.7.1-beta.5", "dsymbol": "~>0.2.0", - "inifiled": ">=0.0.6", + "inifiled": ">=1.0.2", "emsi_containers": "~>0.5.3", "libddoc": "~>0.2.0" }, diff --git a/inifiled b/inifiled index e4f63f1..35f8d2d 160000 --- a/inifiled +++ b/inifiled @@ -1 +1 @@ -Subproject commit e4f63f126ddddb3e496574fec0f76b24e61b1d51 +Subproject commit 35f8d2d914560f8c73cf5e6b80b8e0f47f498d64 diff --git a/makefile b/makefile index d98e9f6..2780216 100644 --- a/makefile +++ b/makefile @@ -5,14 +5,15 @@ DMD := $(DC) GDC := gdc LDC := ldc2 OBJ_DIR := obj -SRC := \ +LIB_SRC := \ $(shell find containers/src -name "*.d")\ $(shell find dsymbol/src -name "*.d")\ $(shell find inifiled/source/ -name "*.d")\ $(shell find libdparse/src/std/experimental/ -name "*.d")\ $(shell find libdparse/src/dparse/ -name "*.d")\ - $(shell find libddoc/src -name "*.d")\ - $(shell find src/ -name "*.d") + $(shell find libddoc/src -name "*.d") +PROJECT_SRC := $(shell find src/ -name "*.d") +SRC := $(LIB_SRC) $(PROJECT_SRC) INCLUDE_PATHS = \ -Iinifiled/source -Isrc\ -Ilibdparse/src\ @@ -21,7 +22,7 @@ INCLUDE_PATHS = \ VERSIONS = DEBUG_VERSIONS = -version=dparse_verbose DMD_FLAGS = -w -inline -release -O -J. -od${OBJ_DIR} -version=StdLoggerDisableWarning -DMD_TEST_FLAGS = -w -g -unittest -J. +DMD_TEST_FLAGS = -w -g -J. -version=StdLoggerDisableWarning all: dmdbuild ldc: ldcbuild @@ -46,8 +47,12 @@ ldcbuild: githash mkdir -p bin ${LDC} -O5 -release -oq -of=bin/dscanner ${VERSIONS} ${INCLUDE_PATHS} ${SRC} -J. -test: githash - ${DC} -w -g -J. -unittest ${INCLUDE_PATHS} ${SRC} -ofbin/dscanner-unittest -version=StdLoggerDisableWarning +# compile the dependencies separately, s.t. their unittests don't get executed +bin/dscanner-unittest-lib.a: ${LIB_SRC} + ${DC} ${DMD_TEST_FLAGS} -c ${INCLUDE_PATHS} ${LIB_SRC} -of$@ + +test: bin/dscanner-unittest-lib.a githash + ${DC} ${DMD_TEST_FLAGS} -unittest ${INCLUDE_PATHS} bin/dscanner-unittest-lib.a ${PROJECT_SRC} -ofbin/dscanner-unittest ./bin/dscanner-unittest rm -f bin/dscanner-unittest diff --git a/src/analysis/config.d b/src/analysis/config.d index 77c57fe..1966e98 100644 --- a/src/analysis/config.d +++ b/src/analysis/config.d @@ -188,4 +188,25 @@ struct StaticAnalysisConfig @INI("Check public declarations without a documented unittest") string has_public_example = Check.disabled; + + @INI("Module-specific filters") + ModuleFilters filters; +} + +private template ModuleFiltersMixin(A) +{ + const string ModuleFiltersMixin = () { + string s; + foreach (mem; __traits(allMembers, StaticAnalysisConfig)) + static if (is(typeof(__traits(getMember, StaticAnalysisConfig, mem)) == string)) + s ~= `@INI("Exclude/Import modules") string[] ` ~ mem ~ ";\n"; + + return s; + }(); +} + +@INI("ModuleFilters. +std.,-std.internal") +struct ModuleFilters +{ + mixin(ModuleFiltersMixin!int); } diff --git a/src/analysis/run.d b/src/analysis/run.d index af25248..fb909e8 100644 --- a/src/analysis/run.d +++ b/src/analysis/run.d @@ -206,6 +206,77 @@ const(Module) parseModule(string fileName, ubyte[] code, RollbackAllocator* p, ? &messageFunctionJSON : &messageFunction, errorCount, warningCount); } +/** +Checks whether a module is part of a user-specified include/exclude list. +The user can specify a comma-separated list of filters, everyone needs to start with +either a '+' (inclusion) or '-' (exclusion). +If no includes are specified, all modules are included. +*/ +bool shouldRun(string a)(string moduleName, const ref StaticAnalysisConfig config) +{ + if (mixin("config." ~ a) == Check.disabled) + return false; + + // By default, run the check + if (!moduleName.length) + return true; + + auto filters = mixin("config.filters." ~ a); + + // Check if there are filters are defined + // filters starting with a comma are invalid + if (filters.length == 0 || filters[0].length == 0) + return true; + + auto includers = filters.filter!(f => f[0] == '+').map!(f => f[1..$]); + auto excluders = filters.filter!(f => f[0] == '-').map!(f => f[1..$]); + + // exclusion has preference over inclusion + if (!excluders.empty && excluders.any!(s => moduleName.canFind(s))) + return false; + + if (!includers.empty) + return includers.any!(s => moduleName.canFind(s)); + + // by default: include all modules + return true; +} + +/// +unittest +{ + bool test(string moduleName, string filters) + { + StaticAnalysisConfig config; + // it doesn't matter which check we test here + config.asm_style_check = Check.enabled; + // this is done automatically by inifiled + config.filters.asm_style_check = filters.split(","); + return shouldRun!"asm_style_check"(moduleName, config); + } + + // test inclusion + assert(test("std.foo", "+std.")); + // partial matches are ok + assert(test("std.foo", "+bar,+foo")); + // full as well + assert(test("std.foo", "+bar,+std.foo,+foo")); + // mismatch + assert(!test("std.foo", "+bar,+banana")); + + // test exclusion + assert(!test("std.foo", "-std.")); + assert(!test("std.foo", "-bar,-std.foo")); + assert(!test("std.foo", "-bar,-foo")); + // mismatch + assert(test("std.foo", "-bar,-banana")); + + // test combination (exclusion has precedence) + assert(!test("std.foo", "+foo,-foo")); + assert(test("std.foo", "+foo,-bar")); + assert(test("std.bar.foo", "-barr,+bar")); +} + MessageSet analyze(string fileName, const Module m, const StaticAnalysisConfig analysisConfig, ref ModuleCache moduleCache, const(Token)[] tokens, bool staticAnalyze = true) { @@ -220,6 +291,11 @@ MessageSet analyze(string fileName, const Module m, const StaticAnalysisConfig a else enum ut = false; + string moduleName; + if (m !is null && m.moduleDeclaration !is null && + m.moduleDeclaration.moduleName !is null && + m.moduleDeclaration.moduleName.identifiers !is null) + moduleName = m.moduleDeclaration.moduleName.identifiers.map!(e => e.text).join("."); auto first = scoped!FirstPass(m, internString(fileName), symbolAllocator, symbolAllocator, true, &moduleCache, null); @@ -232,171 +308,172 @@ MessageSet analyze(string fileName, const Module m, const StaticAnalysisConfig a scope(exit) typeid(Scope).destroy(first.moduleScope); BaseAnalyzer[] checks; - if (analysisConfig.asm_style_check != Check.disabled) + with(analysisConfig) + if (moduleName.shouldRun!"asm_style_check"(analysisConfig)) checks ~= new AsmStyleCheck(fileName, moduleScope, - analysisConfig.asm_style_check == Check.skipTests && !ut); + asm_style_check == Check.skipTests && !ut); - if (analysisConfig.backwards_range_check != Check.disabled) + if (moduleName.shouldRun!"backwards_range_check"(analysisConfig)) checks ~= new BackwardsRangeCheck(fileName, moduleScope, analysisConfig.backwards_range_check == Check.skipTests && !ut); - if (analysisConfig.builtin_property_names_check != Check.disabled) + if (moduleName.shouldRun!"builtin_property_names_check"(analysisConfig)) checks ~= new BuiltinPropertyNameCheck(fileName, moduleScope, analysisConfig.builtin_property_names_check == Check.skipTests && !ut); - if (analysisConfig.comma_expression_check != Check.disabled) + if (moduleName.shouldRun!"comma_expression_check"(analysisConfig)) checks ~= new CommaExpressionCheck(fileName, moduleScope, analysisConfig.comma_expression_check == Check.skipTests && !ut); - if (analysisConfig.constructor_check != Check.disabled) + if (moduleName.shouldRun!"constructor_check"(analysisConfig)) checks ~= new ConstructorCheck(fileName, moduleScope, analysisConfig.constructor_check == Check.skipTests && !ut); - if (analysisConfig.could_be_immutable_check != Check.disabled) + if (moduleName.shouldRun!"could_be_immutable_check"(analysisConfig)) checks ~= new UnmodifiedFinder(fileName, moduleScope, analysisConfig.could_be_immutable_check == Check.skipTests && !ut); - if (analysisConfig.delete_check != Check.disabled) + if (moduleName.shouldRun!"delete_check"(analysisConfig)) checks ~= new DeleteCheck(fileName, moduleScope, analysisConfig.delete_check == Check.skipTests && !ut); - if (analysisConfig.duplicate_attribute != Check.disabled) + if (moduleName.shouldRun!"duplicate_attribute"(analysisConfig)) checks ~= new DuplicateAttributeCheck(fileName, moduleScope, analysisConfig.duplicate_attribute == Check.skipTests && !ut); - if (analysisConfig.enum_array_literal_check != Check.disabled) + if (moduleName.shouldRun!"enum_array_literal_check"(analysisConfig)) checks ~= new EnumArrayLiteralCheck(fileName, moduleScope, analysisConfig.enum_array_literal_check == Check.skipTests && !ut); - if (analysisConfig.exception_check != Check.disabled) + if (moduleName.shouldRun!"exception_check"(analysisConfig)) checks ~= new PokemonExceptionCheck(fileName, moduleScope, analysisConfig.exception_check == Check.skipTests && !ut); - if (analysisConfig.float_operator_check != Check.disabled) + if (moduleName.shouldRun!"float_operator_check"(analysisConfig)) checks ~= new FloatOperatorCheck(fileName, moduleScope, analysisConfig.float_operator_check == Check.skipTests && !ut); - if (analysisConfig.function_attribute_check != Check.disabled) + if (moduleName.shouldRun!"function_attribute_check"(analysisConfig)) checks ~= new FunctionAttributeCheck(fileName, moduleScope, analysisConfig.function_attribute_check == Check.skipTests && !ut); - if (analysisConfig.if_else_same_check != Check.disabled) + if (moduleName.shouldRun!"if_else_same_check"(analysisConfig)) checks ~= new IfElseSameCheck(fileName, moduleScope, analysisConfig.if_else_same_check == Check.skipTests&& !ut); - if (analysisConfig.label_var_same_name_check != Check.disabled) + if (moduleName.shouldRun!"label_var_same_name_check"(analysisConfig)) checks ~= new LabelVarNameCheck(fileName, moduleScope, analysisConfig.label_var_same_name_check == Check.skipTests && !ut); - if (analysisConfig.length_subtraction_check != Check.disabled) + if (moduleName.shouldRun!"length_subtraction_check"(analysisConfig)) checks ~= new LengthSubtractionCheck(fileName, moduleScope, analysisConfig.length_subtraction_check == Check.skipTests && !ut); - if (analysisConfig.local_import_check != Check.disabled) + if (moduleName.shouldRun!"local_import_check"(analysisConfig)) checks ~= new LocalImportCheck(fileName, moduleScope, analysisConfig.local_import_check == Check.skipTests && !ut); - if (analysisConfig.logical_precedence_check != Check.disabled) + if (moduleName.shouldRun!"logical_precedence_check"(analysisConfig)) checks ~= new LogicPrecedenceCheck(fileName, moduleScope, analysisConfig.logical_precedence_check == Check.skipTests && !ut); - if (analysisConfig.mismatched_args_check != Check.disabled) + if (moduleName.shouldRun!"mismatched_args_check"(analysisConfig)) checks ~= new MismatchedArgumentCheck(fileName, moduleScope, analysisConfig.mismatched_args_check == Check.skipTests && !ut); - if (analysisConfig.number_style_check != Check.disabled) + if (moduleName.shouldRun!"number_style_check"(analysisConfig)) checks ~= new NumberStyleCheck(fileName, moduleScope, analysisConfig.number_style_check == Check.skipTests && !ut); - if (analysisConfig.object_const_check != Check.disabled) + if (moduleName.shouldRun!"object_const_check"(analysisConfig)) checks ~= new ObjectConstCheck(fileName, moduleScope, analysisConfig.object_const_check == Check.skipTests && !ut); - if (analysisConfig.opequals_tohash_check != Check.disabled) + if (moduleName.shouldRun!"opequals_tohash_check"(analysisConfig)) checks ~= new OpEqualsWithoutToHashCheck(fileName, moduleScope, analysisConfig.opequals_tohash_check == Check.skipTests && !ut); - if (analysisConfig.redundant_parens_check != Check.disabled) + if (moduleName.shouldRun!"redundant_parens_check"(analysisConfig)) checks ~= new RedundantParenCheck(fileName, moduleScope, analysisConfig.redundant_parens_check == Check.skipTests && !ut); - if (analysisConfig.style_check != Check.disabled) + if (moduleName.shouldRun!"style_check"(analysisConfig)) checks ~= new StyleChecker(fileName, moduleScope, analysisConfig.style_check == Check.skipTests && !ut); - if (analysisConfig.undocumented_declaration_check != Check.disabled) + if (moduleName.shouldRun!"undocumented_declaration_check"(analysisConfig)) checks ~= new UndocumentedDeclarationCheck(fileName, moduleScope, analysisConfig.undocumented_declaration_check == Check.skipTests && !ut); - if (analysisConfig.unused_label_check != Check.disabled) + if (moduleName.shouldRun!"unused_label_check"(analysisConfig)) checks ~= new UnusedLabelCheck(fileName, moduleScope, analysisConfig.unused_label_check == Check.skipTests && !ut); - if (analysisConfig.unused_variable_check != Check.disabled) + if (moduleName.shouldRun!"unused_variable_check"(analysisConfig)) checks ~= new UnusedVariableCheck(fileName, moduleScope, analysisConfig.unused_variable_check == Check.skipTests && !ut); - if (analysisConfig.long_line_check != Check.disabled) + if (moduleName.shouldRun!"long_line_check"(analysisConfig)) checks ~= new LineLengthCheck(fileName, tokens, analysisConfig.long_line_check == Check.skipTests && !ut); - if (analysisConfig.auto_ref_assignment_check != Check.disabled) + if (moduleName.shouldRun!"auto_ref_assignment_check"(analysisConfig)) checks ~= new AutoRefAssignmentCheck(fileName, analysisConfig.auto_ref_assignment_check == Check.skipTests && !ut); - if (analysisConfig.incorrect_infinite_range_check != Check.disabled) + if (moduleName.shouldRun!"incorrect_infinite_range_check"(analysisConfig)) checks ~= new IncorrectInfiniteRangeCheck(fileName, analysisConfig.incorrect_infinite_range_check == Check.skipTests && !ut); - if (analysisConfig.useless_assert_check != Check.disabled) + if (moduleName.shouldRun!"useless_assert_check"(analysisConfig)) checks ~= new UselessAssertCheck(fileName, analysisConfig.useless_assert_check == Check.skipTests && !ut); - if (analysisConfig.alias_syntax_check != Check.disabled) + if (moduleName.shouldRun!"alias_syntax_check"(analysisConfig)) checks ~= new AliasSyntaxCheck(fileName, analysisConfig.alias_syntax_check == Check.skipTests && !ut); - if (analysisConfig.static_if_else_check != Check.disabled) + if (moduleName.shouldRun!"static_if_else_check"(analysisConfig)) checks ~= new StaticIfElse(fileName, analysisConfig.static_if_else_check == Check.skipTests && !ut); - if (analysisConfig.lambda_return_check != Check.disabled) + if (moduleName.shouldRun!"lambda_return_check"(analysisConfig)) checks ~= new LambdaReturnCheck(fileName, analysisConfig.lambda_return_check == Check.skipTests && !ut); - if (analysisConfig.auto_function_check != Check.disabled) + if (moduleName.shouldRun!"auto_function_check"(analysisConfig)) checks ~= new AutoFunctionChecker(fileName, analysisConfig.auto_function_check == Check.skipTests && !ut); - if (analysisConfig.imports_sortedness != Check.disabled) + if (moduleName.shouldRun!"imports_sortedness"(analysisConfig)) checks ~= new ImportSortednessCheck(fileName, analysisConfig.imports_sortedness == Check.skipTests && !ut); - if (analysisConfig.explicitly_annotated_unittests != Check.disabled) + if (moduleName.shouldRun!"explicitly_annotated_unittests"(analysisConfig)) checks ~= new ExplicitlyAnnotatedUnittestCheck(fileName, analysisConfig.explicitly_annotated_unittests == Check.skipTests && !ut); - if (analysisConfig.properly_documented_public_functions != Check.disabled) + if (moduleName.shouldRun!"properly_documented_public_functions"(analysisConfig)) checks ~= new ProperlyDocumentedPublicFunctions(fileName, analysisConfig.properly_documented_public_functions == Check.skipTests && !ut); - if (analysisConfig.final_attribute_check != Check.disabled) + if (moduleName.shouldRun!"final_attribute_check"(analysisConfig)) checks ~= new FinalAttributeChecker(fileName, analysisConfig.final_attribute_check == Check.skipTests && !ut); - if (analysisConfig.vcall_in_ctor != Check.disabled) + if (moduleName.shouldRun!"vcall_in_ctor"(analysisConfig)) checks ~= new VcallCtorChecker(fileName, analysisConfig.vcall_in_ctor == Check.skipTests && !ut); - if (analysisConfig.useless_initializer != Check.disabled) + if (moduleName.shouldRun!"useless_initializer"(analysisConfig)) checks ~= new UselessInitializerChecker(fileName, analysisConfig.useless_initializer == Check.skipTests && !ut); - if (analysisConfig.allman_braces_check != Check.disabled) + if (moduleName.shouldRun!"allman_braces_check"(analysisConfig)) checks ~= new AllManCheck(fileName, tokens, analysisConfig.allman_braces_check == Check.skipTests && !ut); - if (analysisConfig.redundant_attributes_check != Check.disabled) + if (moduleName.shouldRun!"redundant_attributes_check"(analysisConfig)) checks ~= new RedundantAttributesCheck(fileName, moduleScope, analysisConfig.redundant_attributes_check == Check.skipTests && !ut); @@ -405,7 +482,7 @@ MessageSet analyze(string fileName, const Module m, const StaticAnalysisConfig a analysisConfig.has_public_example == Check.skipTests && !ut); version (none) - if (analysisConfig.redundant_if_check != Check.disabled) + if (moduleName.shouldRun!"redundant_if_check"(analysisConfig)) checks ~= new IfStatementCheck(fileName, moduleScope, analysisConfig.redundant_if_check == Check.skipTests && !ut); From a85393612a4367b180e59666cdf834f7739f8aa9 Mon Sep 17 00:00:00 2001 From: Sebastian Wilzbach Date: Thu, 22 Jun 2017 22:06:04 +0200 Subject: [PATCH 05/24] Apply selective filtering for has_public_example as well --- src/analysis/run.d | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/analysis/run.d b/src/analysis/run.d index fb909e8..16f7e67 100644 --- a/src/analysis/run.d +++ b/src/analysis/run.d @@ -477,7 +477,7 @@ MessageSet analyze(string fileName, const Module m, const StaticAnalysisConfig a checks ~= new RedundantAttributesCheck(fileName, moduleScope, analysisConfig.redundant_attributes_check == Check.skipTests && !ut); - if (analysisConfig.has_public_example!= Check.disabled) + if (moduleName.shouldRun!"has_public_example"(analysisConfig)) checks ~= new HasPublicExampleCheck(fileName, moduleScope, analysisConfig.has_public_example == Check.skipTests && !ut); From 169f784aae68e443204f1db79797e05f31d369f5 Mon Sep 17 00:00:00 2001 From: Basile Burg Date: Tue, 4 Jul 2017 09:18:58 +0200 Subject: [PATCH 06/24] output in bin when using the bat --- build.bat | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.bat b/build.bat index af36b70..28e21c9 100644 --- a/build.bat +++ b/build.bat @@ -26,5 +26,5 @@ for %%x in (containers\src\containers\*.d) do set CONTAINERS=!CONTAINERS! %%x for %%x in (containers\src\containers\internal\*.d) do set CONTAINERS=!CONTAINERS! %%x @echo on -dmd %CORE% %STD% %LIBDPARSE% %LIBDDOC% %ANALYSIS% %INIFILED% %DSYMBOL% %CONTAINERS% %DFLAGS% -I"libdparse\src" -I"dsymbol\src" -I"containers\src" -I"libddoc\src" -ofdscanner.exe +dmd %CORE% %STD% %LIBDPARSE% %LIBDDOC% %ANALYSIS% %INIFILED% %DSYMBOL% %CONTAINERS% %DFLAGS% -I"libdparse\src" -I"dsymbol\src" -I"containers\src" -I"libddoc\src" -ofbin\dscanner.exe From 672c665d51a99d5ee212129582ced7eeac4272ec Mon Sep 17 00:00:00 2001 From: Basile Burg Date: Tue, 4 Jul 2017 09:58:00 +0200 Subject: [PATCH 07/24] DUB project, out put to bin --- dub.json | 39 +++++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/dub.json b/dub.json index f4db0ff..696ebbb 100644 --- a/dub.json +++ b/dub.json @@ -1,19 +1,22 @@ { - "name": "dscanner", - "description": "Swiss-army knife for D source code", - "copyright": "© Brian Schott", - "authors": ["Brian Schott"], - "license" : "Boost Software License - Version 1.0", - "targetType": "autodetect", - "versions": [ - "built_with_dub", - "StdLoggerDisableWarning" - ], - "dependencies": { - "libdparse": "~>0.7.1-beta.5", - "dsymbol": "~>0.2.0", - "inifiled": ">=1.0.2", - "emsi_containers": "~>0.5.3", - "libddoc": "~>0.2.0" - }, -} + "name" : "dscanner", + "description" : "Swiss-army knife for D source code", + "copyright" : "© Brian Schott", + "authors" : [ + "Brian Schott" + ], + "license" : "Boost Software License - Version 1.0", + "targetType" : "autodetect", + "versions" : [ + "built_with_dub", + "StdLoggerDisableWarning" + ], + "dependencies" : { + "libdparse" : "~>0.7.1-beta.5", + "dsymbol" : "~>0.2.0", + "inifiled" : ">=1.0.2", + "emsi_containers" : "~>0.5.3", + "libddoc" : "~>0.2.0" + }, + "targetPath" : "bin" +} \ No newline at end of file From 3cb40148c19deb0bcbdf9ce98bcdeb46aed54ebd Mon Sep 17 00:00:00 2001 From: Basile Burg Date: Wed, 12 Jul 2017 09:05:44 +0200 Subject: [PATCH 08/24] update dparse for the --ast option --- dub.json | 4 ++-- libdparse | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dub.json b/dub.json index 696ebbb..e24801b 100644 --- a/dub.json +++ b/dub.json @@ -12,11 +12,11 @@ "StdLoggerDisableWarning" ], "dependencies" : { - "libdparse" : "~>0.7.1-beta.5", + "libdparse" : "~>0.7.1-beta.6", "dsymbol" : "~>0.2.0", "inifiled" : ">=1.0.2", "emsi_containers" : "~>0.5.3", "libddoc" : "~>0.2.0" }, "targetPath" : "bin" -} \ No newline at end of file +} diff --git a/libdparse b/libdparse index 69ba83e..454a95a 160000 --- a/libdparse +++ b/libdparse @@ -1 +1 @@ -Subproject commit 69ba83ed193a8fa2d2ea6d3286376bbba4a5e676 +Subproject commit 454a95abb2ac7093f1526f262d4a712730dd9ac3 From 7ee23b3d7305530e8172665b5561d382eccad6a4 Mon Sep 17 00:00:00 2001 From: Basile Burg Date: Thu, 27 Jul 2017 12:16:52 +0200 Subject: [PATCH 09/24] fix deprecations messages related to message function (#503) * fix deprecations messages related to message function * update dsymbol as a git submodule too --- dsymbol | 2 +- dub.json | 4 ++-- libdparse | 2 +- src/analysis/run.d | 6 ++++-- src/ctags.d | 3 ++- src/etags.d | 3 ++- src/imports.d | 3 ++- src/main.d | 3 ++- src/symbol_finder.d | 3 ++- 9 files changed, 18 insertions(+), 11 deletions(-) diff --git a/dsymbol b/dsymbol index d22c971..6920a04 160000 --- a/dsymbol +++ b/dsymbol @@ -1 +1 @@ -Subproject commit d22c9714a60ac05cb32db938e81a396cffb5ffa6 +Subproject commit 6920a0489fbef44f105cdfb76d426a03ae14259a diff --git a/dub.json b/dub.json index e24801b..673a4f0 100644 --- a/dub.json +++ b/dub.json @@ -12,8 +12,8 @@ "StdLoggerDisableWarning" ], "dependencies" : { - "libdparse" : "~>0.7.1-beta.6", - "dsymbol" : "~>0.2.0", + "libdparse" : "~>0.7.1-beta.7", + "dsymbol" : "~>0.2.6", "inifiled" : ">=1.0.2", "emsi_containers" : "~>0.5.3", "libddoc" : "~>0.2.0" diff --git a/libdparse b/libdparse index 454a95a..4229f11 160000 --- a/libdparse +++ b/libdparse @@ -1 +1 @@ -Subproject commit 454a95abb2ac7093f1526f262d4a712730dd9ac3 +Subproject commit 4229f11828a901ea5379409015f14a033e742906 diff --git a/src/analysis/run.d b/src/analysis/run.d index 16f7e67..a8244fb 100644 --- a/src/analysis/run.d +++ b/src/analysis/run.d @@ -11,6 +11,7 @@ import std.conv; import std.algorithm; import std.range; import std.array; +import std.functional : toDelegate; import dparse.lexer; import dparse.parser; import dparse.ast; @@ -202,8 +203,9 @@ const(Module) parseModule(string fileName, ubyte[] code, RollbackAllocator* p, tokens = getTokensForParser(code, config, &cache); if (linesOfCode !is null) (*linesOfCode) += count!(a => isLineOfCode(a.type))(tokens); - return dparse.parser.parseModule(tokens, fileName, p, report - ? &messageFunctionJSON : &messageFunction, errorCount, warningCount); + return dparse.parser.parseModule(tokens, fileName, p, + report ? toDelegate(&messageFunctionJSON) : toDelegate(&messageFunction), + errorCount, warningCount); } /** diff --git a/src/ctags.d b/src/ctags.d index c4c76e8..939724a 100644 --- a/src/ctags.d +++ b/src/ctags.d @@ -17,6 +17,7 @@ import std.conv; import std.typecons; import containers.ttree; import std.string; +import std.functional : toDelegate; /** * Prints CTAGS information to the given file. @@ -65,7 +66,7 @@ void printCtags(File output, string[] fileNames) } auto tokens = getTokensForParser(bytes, config, &cache); - Module m = parseModule(tokens.array, fileName, &rba, &doNothing); + Module m = parseModule(tokens.array, fileName, &rba, toDelegate(&doNothing)); auto printer = new CTagsPrinter(&tags); printer.fileName = fileName; diff --git a/src/etags.d b/src/etags.d index a89ce6c..a072632 100644 --- a/src/etags.d +++ b/src/etags.d @@ -17,6 +17,7 @@ import std.path; import std.array; import std.conv; import std.string; +import std.functional : toDelegate; // Prefix tags with their module name. Seems like correct behavior, but just // in case, make it an option. @@ -48,7 +49,7 @@ void printEtags(File output, bool tagAll, string[] fileNames) auto bytes = uninitializedArray!(ubyte[])(to!size_t(f.size)); f.rawRead(bytes); auto tokens = getTokensForParser(bytes, config, &cache); - Module m = parseModule(tokens.array, fileName, &rba, &doNothing); + Module m = parseModule(tokens.array, fileName, &rba, toDelegate(&doNothing)); auto printer = new EtagsPrinter; printer.moduleName = m.moduleFullName(fileName); diff --git a/src/imports.d b/src/imports.d index 392ebe4..6389608 100644 --- a/src/imports.d +++ b/src/imports.d @@ -11,6 +11,7 @@ import dparse.parser; import dparse.rollback_allocator; import std.stdio; import std.container.rbtree; +import std.functional : toDelegate; import readers; /** @@ -63,7 +64,7 @@ private void visitFile(bool usingStdin, string fileName, RedBlackTree!string imp config.stringBehavior = StringBehavior.source; auto visitor = new ImportPrinter; auto tokens = getTokensForParser(usingStdin ? readStdin() : readFile(fileName), config, cache); - auto mod = parseModule(tokens, fileName, &rba, &doNothing); + auto mod = parseModule(tokens, fileName, &rba, toDelegate(&doNothing)); visitor.visit(mod); importedModules.insert(visitor.imports[]); } diff --git a/src/main.d b/src/main.d index 6b0a9b0..8f87c4a 100644 --- a/src/main.d +++ b/src/main.d @@ -15,6 +15,7 @@ import std.stdio; import std.range; import std.experimental.lexer; import std.typecons : scoped; +import std.functional : toDelegate; import dparse.lexer; import dparse.parser; import dparse.rollback_allocator; @@ -283,7 +284,7 @@ else config.stringBehavior = StringBehavior.source; auto tokens = getTokensForParser(usingStdin ? readStdin() : readFile(args[1]), config, &cache); - auto mod = parseModule(tokens, fileName, &rba, &doNothing); + auto mod = parseModule(tokens, fileName, &rba, toDelegate(&doNothing)); if (ast) { diff --git a/src/symbol_finder.d b/src/symbol_finder.d index baf63da..67cc6f3 100644 --- a/src/symbol_finder.d +++ b/src/symbol_finder.d @@ -12,6 +12,7 @@ import dparse.ast; import dparse.rollback_allocator; import std.stdio; import std.file : isFile; +import std.functional : toDelegate; void findDeclarationOf(File output, string symbolName, string[] fileNames) { @@ -31,7 +32,7 @@ void findDeclarationOf(File output, string symbolName, string[] fileNames) f.rawRead(bytes); auto tokens = getTokensForParser(bytes, config, &cache); RollbackAllocator rba; - Module m = parseModule(tokens.array, fileName, &rba, &doNothing); + Module m = parseModule(tokens.array, fileName, &rba, toDelegate(&doNothing)); visitor.fileName = fileName; visitor.visit(m); } From ce6056d4bc369deea0b0a4c7b7dde2462b1bef70 Mon Sep 17 00:00:00 2001 From: Basile Burg Date: Thu, 27 Jul 2017 16:01:02 +0200 Subject: [PATCH 10/24] fix #275 - cant run tests under windows (#504) * fix #275 - cant run tests under windows * fix typo --- README.md | 5 +++++ build.bat | 24 +++++++++++++++++++----- src/analysis/helpers.d | 3 ++- src/analysis/lambda_return_check.d | 1 + test.sh | 17 ----------------- 5 files changed, 27 insertions(+), 23 deletions(-) delete mode 100755 test.sh diff --git a/README.md b/README.md index ac69b98..553c0ab 100644 --- a/README.md +++ b/README.md @@ -12,6 +12,11 @@ makefile has "ldc" and "gdc" targets if you'd prefer to compile with one of thes compilers instead of DMD. To install, simply place the generated binary (in the "bin" folder) somewhere on your $PATH. +### Testing +Testing does not work with DUB. +Under linux or OSX run the tests with `make test`. +Under Windows run the tests with `build.bat test`. + ### Installing with DUB ```sh diff --git a/build.bat b/build.bat index 28e21c9..01bd910 100644 --- a/build.bat +++ b/build.bat @@ -1,7 +1,8 @@ @echo off setlocal enabledelayedexpansion -set DFLAGS=-O -release -inline +set DFLAGS=-O -release -inline -version=StdLoggerDisableWarning +set TESTFLAGS=-g -w -version=StdLoggerDisableWarning set CORE= set LIBDPARSE= set STD= @@ -13,8 +14,6 @@ set LIBDDOC= for %%x in (src\*.d) do set CORE=!CORE! %%x for %%x in (src\analysis\*.d) do set ANALYSIS=!ANALYSIS! %%x -for %%x in (libdparse\experimental_allocator\src\std\experimental\allocator\*.d) do set STD=!STD! %%x -for %%x in (libdparse\experimental_allocator\src\std\experimental\allocator\building_blocks\*.d) do set STD=!STD! %%x for %%x in (libdparse\src\dparse\*.d) do set LIBDPARSE=!LIBDPARSE! %%x for %%x in (libdparse\src\std\experimental\*.d) do set LIBDPARSE=!LIBDPARSE! %%x for %%x in (libddoc\src\ddoc\*.d) do set LIBDDOC=!LIBDDOC! %%x @@ -25,6 +24,21 @@ for %%x in (dsymbol\src\dsymbol\conversion\*.d) do set DSYMBOL=!DSYMBOL! %%x for %%x in (containers\src\containers\*.d) do set CONTAINERS=!CONTAINERS! %%x for %%x in (containers\src\containers\internal\*.d) do set CONTAINERS=!CONTAINERS! %%x -@echo on -dmd %CORE% %STD% %LIBDPARSE% %LIBDDOC% %ANALYSIS% %INIFILED% %DSYMBOL% %CONTAINERS% %DFLAGS% -I"libdparse\src" -I"dsymbol\src" -I"containers\src" -I"libddoc\src" -ofbin\dscanner.exe +if "%1" == "test" goto test_cmd +@echo on +dmd %CORE% %STD% %LIBDPARSE% %LIBDDOC% %ANALYSIS% %INIFILED% %DSYMBOL% %CONTAINERS% %DFLAGS% -I"libdparse\src" -I"dsymbol\src" -I"containers\src" -I"libddoc\src" -ofbin\dscanner.exe +goto eof + +:test_cmd +@echo on +set TESTNAME="bin\dscanner-unittest" +dmd %STD% %LIBDPARSE% %LIBDDOC% %INIFILED% %DSYMBOL% %CONTAINERS% -I"libdparse\src" -I"dsymbol\src" -I"containers\src" -I"libddoc\src" -lib %TESTFLAGS% -of%TESTNAME%.lib +if exist %TESTNAME%.lib dmd %CORE% %ANALYSIS% bin\dscanner-unittest.lib -I"src" -I"inifiled\source" -I"libdparse\src" -I"dsymbol\src" -I"containers\src" -I"libddoc\src" -unittest %TESTFLAGS% -of%TESTNAME%.exe +if exist %TESTNAME%.exe %TESTNAME%.exe + +if exist %TESTNAME%.obj del %TESTNAME%.obj +if exist %TESTNAME%.lib del %TESTNAME%.lib +if exist %TESTNAME%.exe del %TESTNAME%.exe + +:eof \ No newline at end of file diff --git a/src/analysis/helpers.d b/src/analysis/helpers.d index 5312ff9..390c7b2 100644 --- a/src/analysis/helpers.d +++ b/src/analysis/helpers.d @@ -50,6 +50,7 @@ void assertAnalyzerWarnings(string code, const StaticAnalysisConfig config, { import analysis.run : parseModule; import dparse.lexer : StringCache, Token; + import std.ascii : newline; StringCache cache = StringCache(StringCache.defaultBucketCount); RollbackAllocator r; @@ -60,7 +61,7 @@ void assertAnalyzerWarnings(string code, const StaticAnalysisConfig config, // Run the code and get any warnings MessageSet rawWarnings = analyze("test", m, config, moduleCache, tokens); - string[] codeLines = code.split("\n"); + string[] codeLines = code.split(newline); // Get the warnings ordered by line string[size_t] warnings; diff --git a/src/analysis/lambda_return_check.d b/src/analysis/lambda_return_check.d index 2ddf874..e85d05b 100644 --- a/src/analysis/lambda_return_check.d +++ b/src/analysis/lambda_return_check.d @@ -44,6 +44,7 @@ private: enum KEY = "dscanner.confusing.lambda_returns_lambda"; } +version(Windows) {/*because of newline in code*/} else unittest { import analysis.helpers : assertAnalyzerWarnings; diff --git a/test.sh b/test.sh deleted file mode 100755 index 1648b6b..0000000 --- a/test.sh +++ /dev/null @@ -1,17 +0,0 @@ - -rm -f test -rm -f test.o - -dmd\ - src/*.d\ - libdparse/src/std/*.d\ - libdparse/src/std/d/*.d\ - inifiled/source/*.d\ - src/analysis/*.d\ - -oftest\ - -g -unittest\ - -J. - -./test - -rm -f test test.o From 7b91483943e74ab0a31d53e3a699598e8aa6423a Mon Sep 17 00:00:00 2001 From: Basile Burg Date: Fri, 28 Jul 2017 00:30:03 +0200 Subject: [PATCH 11/24] fix #312 - spurious warnings about non-const toString inside const block (#505) --- src/analysis/objectconst.d | 48 ++++++++++++++++++++++++++++++-------- 1 file changed, 38 insertions(+), 10 deletions(-) diff --git a/src/analysis/objectconst.d b/src/analysis/objectconst.d index d2b0a98..64f4184 100644 --- a/src/analysis/objectconst.d +++ b/src/analysis/objectconst.d @@ -21,6 +21,7 @@ class ObjectConstCheck : BaseAnalyzer { alias visit = BaseAnalyzer.visit; + /// this(string fileName, const(Scope)* sc, bool skipTests = false) { super(fileName, sc, skipTests); @@ -31,24 +32,40 @@ class ObjectConstCheck : BaseAnalyzer mixin visitTemplate!UnionDeclaration; mixin visitTemplate!StructDeclaration; + override void visit(const AttributeDeclaration d) + { + if (d.attribute.attribute == tok!"const" && inAggregate) + { + constColon = true; + } + d.accept(this); + } + override void visit(const Declaration d) { - if (inAggregate && d.functionDeclaration !is null - && isInteresting(d.functionDeclaration.name.text) && (!hasConst(d.attributes) - && !hasConst(d.functionDeclaration.memberFunctionAttributes))) + import std.algorithm : any; + bool setConstBlock; + if (inAggregate && d.attributes && d.attributes.any!(a => a.attribute == tok!"const")) + { + setConstBlock = true; + constBlock = true; + } + + if (inAggregate && d.functionDeclaration !is null && !constColon && !constBlock + && isInteresting(d.functionDeclaration.name.text) + && !hasConst(d.functionDeclaration.memberFunctionAttributes)) { addErrorMessage(d.functionDeclaration.name.line, d.functionDeclaration.name.column, "dscanner.suspicious.object_const", "Methods 'opCmp', 'toHash', 'opEquals', 'opCast', and/or 'toString' are non-const."); } + d.accept(this); - } - private static bool hasConst(const Attribute[] attributes) - { - import std.algorithm : any; - - return attributes.any!(a => a.attribute == tok!"const"); + if (!inAggregate) + constColon = false; + if (setConstBlock) + constBlock = false; } private static bool hasConst(const MemberFunctionAttribute[] attributes) @@ -65,7 +82,8 @@ class ObjectConstCheck : BaseAnalyzer || name == "toString" || name == "opCast"; } - private bool looking; + private bool constBlock; + private bool constColon; } @@ -102,6 +120,16 @@ unittest } } + class Bat + { + const: override string toString() { return "foo"; } // ok + } + + class Fox + { + const{ override string toString() { return "foo"; }} // ok + } + // Will warn, because none are const class Dog { From de0b7191062fc23b3e06120730af5012f1cfeafc Mon Sep 17 00:00:00 2001 From: Basile Burg Date: Fri, 28 Jul 2017 13:20:09 +0200 Subject: [PATCH 12/24] fixup for last bat update - use var instead of plain text (#506) --- build.bat | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build.bat b/build.bat index 01bd910..04a988b 100644 --- a/build.bat +++ b/build.bat @@ -34,11 +34,11 @@ goto eof @echo on set TESTNAME="bin\dscanner-unittest" dmd %STD% %LIBDPARSE% %LIBDDOC% %INIFILED% %DSYMBOL% %CONTAINERS% -I"libdparse\src" -I"dsymbol\src" -I"containers\src" -I"libddoc\src" -lib %TESTFLAGS% -of%TESTNAME%.lib -if exist %TESTNAME%.lib dmd %CORE% %ANALYSIS% bin\dscanner-unittest.lib -I"src" -I"inifiled\source" -I"libdparse\src" -I"dsymbol\src" -I"containers\src" -I"libddoc\src" -unittest %TESTFLAGS% -of%TESTNAME%.exe +if exist %TESTNAME%.lib dmd %CORE% %ANALYSIS% %TESTNAME%.lib -I"src" -I"inifiled\source" -I"libdparse\src" -I"dsymbol\src" -I"containers\src" -I"libddoc\src" -unittest %TESTFLAGS% -of%TESTNAME%.exe if exist %TESTNAME%.exe %TESTNAME%.exe if exist %TESTNAME%.obj del %TESTNAME%.obj if exist %TESTNAME%.lib del %TESTNAME%.lib if exist %TESTNAME%.exe del %TESTNAME%.exe -:eof \ No newline at end of file +:eof From a916a64fb7f2b0df6d4f8fccd33730e2f6999338 Mon Sep 17 00:00:00 2001 From: Jan Jurzitza Date: Fri, 28 Jul 2017 21:05:19 +0200 Subject: [PATCH 13/24] Fix #501 (#502) --- src/analysis/useless_initializer.d | 37 +++++++++--------------------- 1 file changed, 11 insertions(+), 26 deletions(-) diff --git a/src/analysis/useless_initializer.d b/src/analysis/useless_initializer.d index 6fae7a8..6240a64 100644 --- a/src/analysis/useless_initializer.d +++ b/src/analysis/useless_initializer.d @@ -41,7 +41,6 @@ private: 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; @@ -217,7 +216,7 @@ public: if (intDefs.canFind(value.text)) mixin(warn); } - else if (isPtr) + else if (isPtr || isStr) { if (str(value.type) == "null") mixin(warn); @@ -228,24 +227,6 @@ public: 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); } } @@ -293,15 +274,14 @@ public: 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 + 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 = ""; // [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 @@ -336,6 +316,11 @@ public: wstring a; dstring a; string a = ['a']; + string a = ""; + string a = ""c; + wstring a = ""w; + dstring a = ""d; + string a = q{}; char[] a = "ze"; S s = S(0,1); S s = s.call(); From e7ea632ea4e1f371d6bd522afebac2d45943678d Mon Sep 17 00:00:00 2001 From: Mark Barbone <> Date: Wed, 2 Aug 2017 15:32:48 -0400 Subject: [PATCH 14/24] No longer uses deprecated string.removechars --- src/analysis/range.d | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/analysis/range.d b/src/analysis/range.d index 32fd0ca..84ac157 100644 --- a/src/analysis/range.d +++ b/src/analysis/range.d @@ -141,9 +141,10 @@ private: long parseNumber(string te) { import std.conv : to; - import std.string : removechars; + import std.regex : ctRegex, replaceAll; - string t = te.removechars("_uUlL"); + enum re = ctRegex!("[_uUlL]", ""); + string t = te.replaceAll(re, ""); if (t.length > 2) { if (t[1] == 'x' || t[1] == 'X') From e4b0ecc7cfe5971d0630078e36cff92e68f786e1 Mon Sep 17 00:00:00 2001 From: Basile Burg Date: Fri, 4 Aug 2017 16:57:16 +0200 Subject: [PATCH 15/24] final attrib checker - handle static (#508) --- src/analysis/final_attribute.d | 79 ++++++++++++++++++++++++++++------ 1 file changed, 66 insertions(+), 13 deletions(-) diff --git a/src/analysis/final_attribute.d b/src/analysis/final_attribute.d index 684f230..15de025 100644 --- a/src/analysis/final_attribute.d +++ b/src/analysis/final_attribute.d @@ -30,6 +30,7 @@ private: static immutable class_t = "templated functions declared within a class are never virtual"; static immutable class_p = "private functions declared within a class are never virtual"; static immutable class_f = "functions declared within a final class are never virtual"; + static immutable class_s = "static functions are never virtual"; static immutable interface_t = "templated functions declared within an interface are never virtual"; static immutable struct_f = "functions declared within a struct are never virtual"; static immutable union_f = "functions declared within an union are never virtual"; @@ -49,6 +50,8 @@ private: bool[] _private; bool _finalAggregate; + bool _alwaysStatic; + bool _blockStatic; Parent _parent = Parent.module_; void addError(T)(T t, string msg) @@ -75,6 +78,7 @@ public: const Parent saved = _parent; _parent = Parent.struct_; _private.length += 1; + _alwaysStatic = false; sd.accept(this); _private.length -= 1; _parent = saved; @@ -85,6 +89,7 @@ public: const Parent saved = _parent; _parent = Parent.interface_; _private.length += 1; + _alwaysStatic = false; id.accept(this); _private.length -= 1; _parent = saved; @@ -95,6 +100,7 @@ public: const Parent saved = _parent; _parent = Parent.union_; _private.length += 1; + _alwaysStatic = false; ud.accept(this); _private.length -= 1; _parent = saved; @@ -105,6 +111,7 @@ public: const Parent saved = _parent; _parent = Parent.class_; _private.length += 1; + _alwaysStatic = false; cd.accept(this); _private.length -= 1; _parent = saved; @@ -120,14 +127,34 @@ public: // regular template are also mixable } + override void visit(const(AttributeDeclaration) decl) + { + if (_parent == Parent.class_ && decl.attribute && + decl.attribute.attribute == tok!"static") + _alwaysStatic = true; + } + override void visit(const(Declaration) d) { + import std.algorithm.iteration : filter; + import std.algorithm.searching : canFind; + const Parent savedParent = _parent; + bool undoBlockStatic; + if (_parent == Parent.class_ && d.attributes && + d.attributes.canFind!(a => a.attribute == tok!"static")) + { + _blockStatic = true; + undoBlockStatic = true; + } + scope(exit) { d.accept(this); _parent = savedParent; + if (undoBlockStatic) + _blockStatic = false; } if (!d.attributeDeclaration && @@ -138,30 +165,27 @@ public: !d.functionDeclaration) return; - import std.algorithm.iteration : filter; - import std.algorithm.searching : find; - import std.range.primitives : empty; - if (d.attributeDeclaration && d.attributeDeclaration.attribute) { const tp = d.attributeDeclaration.attribute.attribute.type; _private[$-1] = isProtection(tp) & (tp == tok!"private"); } - const bool isFinal = !d.attributes - .find!(a => a.attribute.type == tok!"final") - .empty; + const bool isFinal = d.attributes + .canFind!(a => a.attribute.type == tok!"final"); + + const bool isStaticOnce = d.attributes + .canFind!(a => a.attribute.type == tok!"static"); // determine if private - const bool changeProtectionOnce = !d.attributes - .filter!(a => a.attribute.type.isProtection) - .empty; + const bool changeProtectionOnce = d.attributes + .canFind!(a => a.attribute.type.isProtection); - const bool isPrivateOnce = !d.attributes - .find!(a => a.attribute.type == tok!"private") - .empty; + const bool isPrivateOnce = d.attributes + .canFind!(a => a.attribute.type == tok!"private"); bool isPrivate; + if (isPrivateOnce) isPrivate = true; else if (_private[$-1] && !changeProtectionOnce) @@ -194,6 +218,8 @@ public: addError(fd, MESSAGE.class_t); if (isPrivate) addError(fd, MESSAGE.class_p); + else if (isStaticOnce || _alwaysStatic || _blockStatic) + addError(fd, MESSAGE.class_s); else if (_finalAggregate) addError(fd, MESSAGE.class_f); break; @@ -349,5 +375,32 @@ public: FinalAttributeChecker.MSGB.format(FinalAttributeChecker.MESSAGE.class_p) ), sac); + assertAnalyzerWarnings(q{ + class Foo {final static void foo(){}} // [warn]: %s + }c.format( + FinalAttributeChecker.MSGB.format(FinalAttributeChecker.MESSAGE.class_s) + ), sac); + + assertAnalyzerWarnings(q{ + class Foo + { + void foo(){} + static: final void foo(){} // [warn]: %s + } + }c.format( + FinalAttributeChecker.MSGB.format(FinalAttributeChecker.MESSAGE.class_s) + ), sac); + + assertAnalyzerWarnings(q{ + class Foo + { + void foo(){} + static{ final void foo(){}} // [warn]: %s + void foo(){} + } + }c.format( + FinalAttributeChecker.MSGB.format(FinalAttributeChecker.MESSAGE.class_s) + ), sac); + stderr.writeln("Unittest for FinalAttributeChecker passed."); } From 55ecfbe47962e0ac07ae020aed7813f6dad9a266 Mon Sep 17 00:00:00 2001 From: Basile Burg Date: Sat, 5 Aug 2017 19:10:08 +0200 Subject: [PATCH 16/24] fix #370 - False positive for duplicate variable name check with structs (#509) fix #370 - False positive for duplicate variable name check with structs merged-on-behalf-of: Petar Kirov --- src/analysis/label_var_same_name_check.d | 87 +++++++++++++++++++++++- 1 file changed, 84 insertions(+), 3 deletions(-) diff --git a/src/analysis/label_var_same_name_check.d b/src/analysis/label_var_same_name_check.d index eb4172b..13151ff 100644 --- a/src/analysis/label_var_same_name_check.d +++ b/src/analysis/label_var_same_name_check.d @@ -28,6 +28,11 @@ class LabelVarNameCheck : BaseAnalyzer mixin ScopedVisit!IfStatement; mixin ScopedVisit!TemplateDeclaration; + mixin AggregateVisit!ClassDeclaration; + mixin AggregateVisit!StructDeclaration; + mixin AggregateVisit!InterfaceDeclaration; + mixin AggregateVisit!UnionDeclaration; + override void visit(const VariableDeclaration var) { foreach (dec; var.declarators) @@ -63,6 +68,16 @@ private: Thing[string][] stack; + template AggregateVisit(NodeType) + { + override void visit(const NodeType n) + { + pushAggregateName(n.name); + n.accept(this); + popAggregateName(); + } + } + template ScopedVisit(NodeType) { override void visit(const NodeType n) @@ -81,15 +96,16 @@ private: size_t i; foreach (s; retro(stack)) { - const(Thing)* thing = name.text in s; + string fqn = parentAggregateText ~ name.text; + const(Thing)* thing = fqn in s; if (thing is null) - currentScope[name.text] = Thing(name.text, name.line, name.column, !fromLabel /+, isConditional+/ ); + currentScope[fqn] = Thing(fqn, name.line, name.column, !fromLabel /+, isConditional+/ ); else if (i != 0 || !isConditional) { immutable thisKind = fromLabel ? "Label" : "Variable"; immutable otherKind = thing.isVar ? "variable" : "label"; addErrorMessage(name.line, name.column, "dscanner.suspicious.label_var_same_name", - thisKind ~ " \"" ~ name.text ~ "\" has the same name as a " + thisKind ~ " \"" ~ fqn ~ "\" has the same name as a " ~ otherKind ~ " defined on line " ~ to!string(thing.line) ~ "."); } ++i; @@ -121,6 +137,32 @@ private: } int conditionalDepth; + + void pushAggregateName(Token name) + { + parentAggregates ~= name; + updateAggregateText(); + } + + void popAggregateName() + { + parentAggregates.length -= 1; + updateAggregateText(); + } + + void updateAggregateText() + { + import std.algorithm : map; + import std.array : join; + + if (parentAggregates.length) + parentAggregateText = parentAggregates.map!(a => a.text).join(".") ~ "."; + else + parentAggregateText = ""; + } + + Token[] parentAggregates; + string parentAggregateText; } unittest @@ -190,6 +232,45 @@ unittest else version(BigEndian) { enum string NAME = "UTF-16BE"; } } +unittest +{ + int a; + struct A {int a;} +} + +unittest +{ + int a; + struct A { struct A {int a;}} +} + +unittest +{ + int a; + class A { class A {int a;}} +} + +unittest +{ + int a; + interface A { interface A {int a;}} +} + +unittest +{ + interface A + { + int a; + int a; // [warn]: Variable "A.a" has the same name as a variable defined on line 89. + } +} + +unittest +{ + int aa; + struct a { int a; } +} + }c, sac); stderr.writeln("Unittest for LabelVarNameCheck passed."); } From 511cee29dc361cf103f3b369b4034297039a4760 Mon Sep 17 00:00:00 2001 From: Basile Burg Date: Sun, 6 Aug 2017 02:09:06 +0200 Subject: [PATCH 17/24] fix #452 - false pos for namz style for VariableDecl with the "enum" storage class --- src/analysis/style.d | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/analysis/style.d b/src/analysis/style.d index d699c88..3e76180 100644 --- a/src/analysis/style.d +++ b/src/analysis/style.d @@ -70,18 +70,12 @@ final class StyleChecker : BaseAnalyzer override void visit(const VariableDeclaration vd) { - import std.algorithm.iteration : filter; - - varIsEnum = !vd.storageClasses.filter!(a => a.token == tok!"enum").empty; vd.accept(this); } override void visit(const Declarator dec) { - if (varIsEnum) - checkAggregateName("Variable", dec.name); - else - checkLowercaseName("Variable", dec.name); + checkLowercaseName("Variable", dec.name); } override void visit(const FunctionDeclaration dec) @@ -143,8 +137,6 @@ final class StyleChecker : BaseAnalyzer aggregateType ~ " name '" ~ name.text ~ "' does not match style guidelines."); } - bool varIsEnum; - bool[] _winStyles = [false]; bool winStyle() @@ -183,7 +175,10 @@ unittest 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 bool someThing = false; + enum Cat { fritz, } + enum Cat = Cat.fritz; }c, sac); assertAnalyzerWarnings(q{ From 0d48f27873c3449e8ad11600cd3c65095ebfab60 Mon Sep 17 00:00:00 2001 From: Jan Jurzitza Date: Sun, 6 Aug 2017 12:30:32 +0200 Subject: [PATCH 18/24] Don't warn about unused identifers named `_` (#511) Fix #490 --- src/analysis/unused.d | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/analysis/unused.d b/src/analysis/unused.d index 1c080a6..789ed38 100644 --- a/src/analysis/unused.d +++ b/src/analysis/unused.d @@ -11,6 +11,7 @@ import std.container; import std.regex : Regex, regex, matchAll; import dsymbol.scope_ : Scope; import std.algorithm.iteration : map; +import std.algorithm : all; /** * Checks for unused variables. @@ -372,7 +373,7 @@ private: void variableDeclared(string name, size_t line, size_t column, bool isParameter, bool isRef) { - if (inAggregateScope) + if (inAggregateScope || name.all!(a => a == '_')) return; tree[$ - 1].insert(new UnUsed(name, line, column, isParameter, isRef)); } @@ -506,6 +507,15 @@ private: void f(Bat** bat) {*bat = bats.ptr + 8;} } + // Issue 490 + void test490() + { + auto cb1 = delegate(size_t _) {}; + cb1(3); + auto cb2 = delegate(size_t a) {}; // [warn]: Parameter a is never used. + cb2(3); + } + }c, sac); stderr.writeln("Unittest for UnusedVariableCheck passed."); } From 34f893d29f7ededc7291a688855503656ad62ccb Mon Sep 17 00:00:00 2001 From: Jan Jurzitza Date: Sat, 12 Aug 2017 23:14:18 +0200 Subject: [PATCH 19/24] Add a findDeclarationOf overload that takes a callback (#512) * Add a findDeclarationOf overload that takes a callback This makes getting the declarations when working with dscanner as a library much easier. Another possible improvement for the future could be directly passing File objects or an input range of Files * Fix formatting of delegate in findDeclarationOf --- src/symbol_finder.d | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/src/symbol_finder.d b/src/symbol_finder.d index 67cc6f3..7fabd01 100644 --- a/src/symbol_finder.d +++ b/src/symbol_finder.d @@ -15,6 +15,22 @@ import std.file : isFile; import std.functional : toDelegate; void findDeclarationOf(File output, string symbolName, string[] fileNames) +{ + findDeclarationOf((string fileName, size_t line, size_t column) + { + output.writefln("%s(%d:%d)", fileName, line, column); + }, symbolName, fileNames); +} + +/// Delegate that gets called every time a declaration gets found +alias OutputHandler = void delegate(string fileName, size_t line, size_t column); + +/// Finds all declarations of a symbol in the given fileNames and calls a handler on every occurence. +/// Params: +/// output = Callback which gets called when a declaration is found +/// symbolName = Symbol name to search for +/// fileNames = An array of file names which might contain stdin to read from stdin +void findDeclarationOf(scope OutputHandler output, string symbolName, string[] fileNames) { import std.array : uninitializedArray, array; import std.conv : to; @@ -46,7 +62,7 @@ void doNothing(string, size_t, size_t, string, bool) class FinderVisitor : ASTVisitor { - this(File output, string symbolName) + this(OutputHandler output, string symbolName) { this.output = output; this.symbolName = symbolName; @@ -62,19 +78,19 @@ class FinderVisitor : ASTVisitor override void visit(const EnumDeclaration dec) { if (dec.name.text == symbolName) - output.writefln("%s(%d:%d)", fileName, dec.name.line, dec.name.column); + output(fileName, dec.name.line, dec.name.column); } override void visit(const AnonymousEnumMember member) { if (member.name.text == symbolName) - output.writefln("%s(%d:%d)", fileName, member.name.line, member.name.column); + output(fileName, member.name.line, member.name.column); } override void visit(const EnumMember member) { if (member.name.text == symbolName) - output.writefln("%s(%d:%d)", fileName, member.name.line, member.name.column); + output(fileName, member.name.line, member.name.column); } override void visit(const AliasDeclaration dec) @@ -84,13 +100,13 @@ class FinderVisitor : ASTVisitor foreach (ident; dec.identifierList.identifiers) { if (ident.text == symbolName) - output.writefln("%s(%d:%d)", fileName, ident.line, ident.column); + output(fileName, ident.line, ident.column); } } foreach (initializer; dec.initializers) { if (initializer.name.text == symbolName) - output.writefln("%s(%d:%d)", fileName, initializer.name.line, + output(fileName, initializer.name.line, initializer.name.column); } } @@ -98,7 +114,7 @@ class FinderVisitor : ASTVisitor override void visit(const Declarator dec) { if (dec.name.text == symbolName) - output.writefln("%s(%d:%d)", fileName, dec.name.line, dec.name.column); + output(fileName, dec.name.line, dec.name.column); } override void visit(const AutoDeclaration ad) @@ -106,7 +122,7 @@ class FinderVisitor : ASTVisitor foreach (part; ad.parts) { if (part.identifier.text == symbolName) - output.writefln("%s(%d:%d)", fileName, part.identifier.line, part.identifier.column); + output(fileName, part.identifier.line, part.identifier.column); } } @@ -119,14 +135,14 @@ class FinderVisitor : ASTVisitor override void visit(const T t) { if (t.name.text == symbolName) - output.writefln("%s(%d:%d)", fileName, t.name.line, t.name.column); + output(fileName, t.name.line, t.name.column); t.accept(this); } } alias visit = ASTVisitor.visit; - File output; + OutputHandler output; string symbolName; string fileName; } From 142e2845880a8e55602597a400131807319d83ba Mon Sep 17 00:00:00 2001 From: Jan Jurzitza Date: Sun, 13 Aug 2017 14:14:02 +0200 Subject: [PATCH 20/24] foreach missing body crash fix (#515) --- src/analysis/unmodified.d | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/analysis/unmodified.d b/src/analysis/unmodified.d index 37576c9..7acc649 100644 --- a/src/analysis/unmodified.d +++ b/src/analysis/unmodified.d @@ -160,7 +160,8 @@ class UnmodifiedFinder : BaseAnalyzer foreachStatement.low.accept(this); interest--; } - foreachStatement.declarationOrStatement.accept(this); + if (foreachStatement.declarationOrStatement !is null) + foreachStatement.declarationOrStatement.accept(this); } override void visit(const TraitsExpression) From c98fa77f95e9abbc8b96f24cead821493f50fba2 Mon Sep 17 00:00:00 2001 From: Jan Jurzitza Date: Sun, 13 Aug 2017 14:14:37 +0200 Subject: [PATCH 21/24] Fix #513 (#514) --- src/analysis/unused.d | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/analysis/unused.d b/src/analysis/unused.d index 789ed38..b079f01 100644 --- a/src/analysis/unused.d +++ b/src/analysis/unused.d @@ -132,7 +132,8 @@ class UnusedVariableCheck : BaseAnalyzer ifStatement.expression.accept(this); interestDepth--; } - ifStatement.thenStatement.accept(this); + if (ifStatement.thenStatement !is null) + ifStatement.thenStatement.accept(this); if (ifStatement.elseStatement !is null) ifStatement.elseStatement.accept(this); } From 8d53f8887e1ebbda5d92317a2df66092b1602356 Mon Sep 17 00:00:00 2001 From: Jan Jurzitza Date: Sun, 13 Aug 2017 19:06:23 +0200 Subject: [PATCH 22/24] More null checks (#517) same as #513, just for a few more constructs. The switch statement already had the same thing in --- src/analysis/unused.d | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/src/analysis/unused.d b/src/analysis/unused.d index b079f01..38b246e 100644 --- a/src/analysis/unused.d +++ b/src/analysis/unused.d @@ -91,18 +91,26 @@ class UnusedVariableCheck : BaseAnalyzer override void visit(const WhileStatement whileStatement) { - interestDepth++; - whileStatement.expression.accept(this); - interestDepth--; - whileStatement.declarationOrStatement.accept(this); + if (whileStatement.expression !is null) + { + interestDepth++; + whileStatement.expression.accept(this); + interestDepth--; + } + if (whileStatement.declarationOrStatement !is null) + whileStatement.declarationOrStatement.accept(this); } override void visit(const DoStatement doStatement) { - interestDepth++; - doStatement.expression.accept(this); - interestDepth--; - doStatement.statementNoCaseNoDefault.accept(this); + if (doStatement.expression !is null) + { + interestDepth++; + doStatement.expression.accept(this); + interestDepth--; + } + if (doStatement.statementNoCaseNoDefault !is null) + doStatement.statementNoCaseNoDefault.accept(this); } override void visit(const ForStatement forStatement) @@ -121,7 +129,8 @@ class UnusedVariableCheck : BaseAnalyzer forStatement.increment.accept(this); interestDepth--; } - forStatement.declarationOrStatement.accept(this); + if (forStatement.declarationOrStatement !is null) + forStatement.declarationOrStatement.accept(this); } override void visit(const IfStatement ifStatement) @@ -157,7 +166,8 @@ class UnusedVariableCheck : BaseAnalyzer override void visit(const AssignExpression assignExp) { - assignExp.ternaryExpression.accept(this); + if (assignExp.ternaryExpression !is null) + assignExp.ternaryExpression.accept(this); if (assignExp.expression !is null) { interestDepth++; From d27263a0ae5ec7fc72d423b8ffc46f73888ba9a3 Mon Sep 17 00:00:00 2001 From: Basile Burg Date: Thu, 24 Aug 2017 11:03:22 +0200 Subject: [PATCH 23/24] update dparse - could stuck because of AA lit bug --- dub.json | 2 +- libdparse | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dub.json b/dub.json index 673a4f0..92ceadd 100644 --- a/dub.json +++ b/dub.json @@ -12,7 +12,7 @@ "StdLoggerDisableWarning" ], "dependencies" : { - "libdparse" : "~>0.7.1-beta.7", + "libdparse" : "~>0.7.1", "dsymbol" : "~>0.2.6", "inifiled" : ">=1.0.2", "emsi_containers" : "~>0.5.3", diff --git a/libdparse b/libdparse index 4229f11..a4cdc47 160000 --- a/libdparse +++ b/libdparse @@ -1 +1 @@ -Subproject commit 4229f11828a901ea5379409015f14a033e742906 +Subproject commit a4cdc474a130ecf5ba3be9424cfb23815eeefd3a From c3452932b9f2a4947678cfeee7c8bac482b9b9d7 Mon Sep 17 00:00:00 2001 From: Sebastian Wilzbach Date: Mon, 11 Sep 2017 02:33:36 +0200 Subject: [PATCH 24/24] Upgrade libdparse for static foreach --- dub.json | 2 +- libdparse | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dub.json b/dub.json index 92ceadd..82f62e9 100644 --- a/dub.json +++ b/dub.json @@ -12,7 +12,7 @@ "StdLoggerDisableWarning" ], "dependencies" : { - "libdparse" : "~>0.7.1", + "libdparse" : "~>0.7.2-alpha.1", "dsymbol" : "~>0.2.6", "inifiled" : ">=1.0.2", "emsi_containers" : "~>0.5.3", diff --git a/libdparse b/libdparse index a4cdc47..222548f 160000 --- a/libdparse +++ b/libdparse @@ -1 +1 @@ -Subproject commit a4cdc474a130ecf5ba3be9424cfb23815eeefd3a +Subproject commit 222548fe610ee33dc60a87c9c1322aedd487dcdb