From f4db04bcb00ae6014806b3353c78311a6d69c75f Mon Sep 17 00:00:00 2001 From: Hackerpilot Date: Thu, 4 Jun 2015 13:48:20 -0700 Subject: [PATCH 01/18] Delete commented code --- src/analysis/unused.d | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/analysis/unused.d b/src/analysis/unused.d index 7b23dbe..12440ea 100644 --- a/src/analysis/unused.d +++ b/src/analysis/unused.d @@ -277,7 +277,6 @@ class UnusedVariableCheck : BaseAnalyzer import std.array : array; if (parameter.name != tok!"") { -// stderr.writeln("Adding parameter ", parameter.name.text); immutable bool isRef = canFind(parameter.parameterAttributes, cast(IdType) tok!"ref") || canFind(parameter.parameterAttributes, cast(IdType) tok!"in") || canFind(parameter.parameterAttributes, cast(IdType) tok!"out"); @@ -334,13 +333,11 @@ private: { if (inAggregateScope) return; -// stderr.writeln("Adding ", name, " ", isParameter, " ", isRef); tree[$ - 1].insert(new UnUsed(name, line, column, isParameter, isRef)); } void variableUsed(string name) { -// writeln("Marking ", name, " used"); size_t treeIndex = tree.length - 1; auto uu = UnUsed(name); while (true) From 0d80bcf5e3cf26f9d7a7e3389f0d5f510382bc90 Mon Sep 17 00:00:00 2001 From: Hackerpilot Date: Thu, 4 Jun 2015 13:51:16 -0700 Subject: [PATCH 02/18] Implement #258 --- src/ctags.d | 64 ++++++++++++++++++++++++++--------------------------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/src/ctags.d b/src/ctags.d index 0968b5f..f40f570 100644 --- a/src/ctags.d +++ b/src/ctags.d @@ -53,8 +53,8 @@ class CTagsPrinter : ASTVisitor { override void visit(const ClassDeclaration dec) { - tagLines ~= "%s\t%s\t%d;\"\tc%s\n".format(dec.name.text, fileName, dec.name.line, context); - auto c = context; + tagLines ~= "%s\t%s\t%d;\"\tc\tline:%d%s\n".format(dec.name.text, fileName, dec.name.line, dec.name.line, context); + const c = context; context = "\tclass:" ~ dec.name.text; dec.accept(this); context = c; @@ -62,8 +62,8 @@ class CTagsPrinter : ASTVisitor override void visit(const StructDeclaration dec) { - tagLines ~= "%s\t%s\t%d;\"\ts%s\n".format(dec.name.text, fileName, dec.name.line, context); - auto c = context; + tagLines ~= "%s\t%s\t%d;\"\ts\tline:%d%s\n".format(dec.name.text, fileName, dec.name.line, dec.name.line, context); + const c = context; context = "\tstruct:" ~ dec.name.text; dec.accept(this); context = c; @@ -71,8 +71,8 @@ class CTagsPrinter : ASTVisitor override void visit(const InterfaceDeclaration dec) { - tagLines ~= "%s\t%s\t%d;\"\ti%s\n".format(dec.name.text, fileName, dec.name.line, context); - auto c = context; + tagLines ~= "%s\t%s\t%d;\"\ti\tline:%d%s\n".format(dec.name.text, fileName, dec.name.line, dec.name.line, context); + const c = context; context = "\tclass:" ~ dec.name.text; dec.accept(this); context = c; @@ -80,8 +80,8 @@ class CTagsPrinter : ASTVisitor override void visit(const TemplateDeclaration dec) { - tagLines ~= "%s\t%s\t%d;\"\tT%s\n".format(dec.name.text, fileName, dec.name.line, context); - auto c = context; + tagLines ~= "%s\t%s\t%d;\"\tT\tline:%d%s\n".format(dec.name.text, fileName, dec.name.line, dec.name.line, context); + const c = context; context = "\ttemplate:" ~ dec.name.text; dec.accept(this); context = c; @@ -89,9 +89,9 @@ class CTagsPrinter : ASTVisitor override void visit(const FunctionDeclaration dec) { - tagLines ~= "%s\t%s\t%d;\"\tf\tarity:%d%s\n".format(dec.name.text, fileName, - dec.name.line, dec.parameters.parameters.length, context); - auto c = context; + tagLines ~= "%s\t%s\t%d;\"\tf\tarity:%d\tline:%d%s\n".format(dec.name.text, fileName, + dec.name.line, dec.parameters.parameters.length, dec.name.line, context); + const c = context; context = "\tfunction:" ~ dec.name.text; dec.accept(this); context = c; @@ -99,9 +99,9 @@ class CTagsPrinter : ASTVisitor override void visit(const Constructor dec) { - tagLines ~= "this\t%s\t%d;\"\tf\tarity:%d%s\n".format(fileName, - dec.line, dec.parameters.parameters.length, context); - auto c = context; + tagLines ~= "this\t%s\t%d;\"\tf\tarity:%d\tline:%d%s\n".format(fileName, + dec.line, dec.parameters.parameters.length, dec.line, context); + const c = context; context = "\tfunction: this"; dec.accept(this); context = c; @@ -109,9 +109,9 @@ class CTagsPrinter : ASTVisitor override void visit(const Destructor dec) { - tagLines ~= "~this\t%s\t%d;\"\tf%s\n".format(fileName, dec.line, - context); - auto c = context; + tagLines ~= "~this\t%s\t%d;\"\tf\tline:%d%s\n".format(fileName, dec.line, + dec.line, context); + const c = context; context = "\tfunction: this"; dec.accept(this); context = c; @@ -119,9 +119,9 @@ class CTagsPrinter : ASTVisitor override void visit(const EnumDeclaration dec) { - tagLines ~= "%s\t%s\t%d;\"\tg%s\n".format(dec.name.text, fileName, - dec.name.line, context); - auto c = context; + tagLines ~= "%s\t%s\t%d;\"\tg\tline:%d%s\n".format(dec.name.text, fileName, + dec.name.line, dec.name.line, context); + const c = context; context = "\tenum:" ~ dec.name.text; dec.accept(this); context = c; @@ -134,9 +134,9 @@ class CTagsPrinter : ASTVisitor dec.accept(this); return; } - tagLines ~= "%s\t%s\t%d;\"\tu%s\n".format(dec.name.text, fileName, - dec.name.line, context); - auto c = context; + tagLines ~= "%s\t%s\t%d;\"\tu\tline:%d%s\n".format(dec.name.text, fileName, + dec.name.line, dec.name.line, context); + const c = context; context = "\tunion:" ~ dec.name.text; dec.accept(this); context = c; @@ -144,22 +144,22 @@ class CTagsPrinter : ASTVisitor override void visit(const AnonymousEnumMember mem) { - tagLines ~= "%s\t%s\t%d;\"\te%s\n".format(mem.name.text, fileName, - mem.name.line, context); + tagLines ~= "%s\t%s\t%d;\"\te\tline:%d%s\n".format(mem.name.text, fileName, + mem.name.line, mem.name.line, context); } override void visit(const EnumMember mem) { - tagLines ~= "%s\t%s\t%d;\"\te%s\n".format(mem.name.text, fileName, - mem.name.line, context); + tagLines ~= "%s\t%s\t%d;\"\te\tline:%d%s\n".format(mem.name.text, fileName, + mem.name.line, mem.name.line, context); } override void visit(const VariableDeclaration dec) { foreach (d; dec.declarators) { - tagLines ~= "%s\t%s\t%d;\"\tv%s\n".format(d.name.text, fileName, - d.name.line, context); + tagLines ~= "%s\t%s\t%d;\"\tv\tline:%d%s\n".format(d.name.text, fileName, + d.name.line, d.name.line, context); } dec.accept(this); } @@ -168,15 +168,15 @@ class CTagsPrinter : ASTVisitor { foreach (i; dec.identifiers) { - tagLines ~= "%s\t%s\t%d;\"\tv%s\n".format(i.text, fileName, - i.line, context); + tagLines ~= "%s\t%s\t%d;\"\tv\tline:%d%s\n".format(i.text, fileName, + i.line, i.line, context); } dec.accept(this); } override void visit(const Invariant dec) { - tagLines ~= "invariant\t%s\t%d;\"\tv%s\n".format(fileName, dec.line, context); + tagLines ~= "invariant\t%s\t%d;\"\tv\tline:%d%s\n".format(fileName, dec.line, dec.line, context); } alias visit = ASTVisitor.visit; From 066c44070e818be1ad0b02dada162d3ea722a7fd Mon Sep 17 00:00:00 2001 From: Hackerpilot Date: Thu, 4 Jun 2015 14:41:57 -0700 Subject: [PATCH 03/18] Fix #266 --- src/analysis/unmodified.d | 5 +++++ src/analysis/unused.d | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/src/analysis/unmodified.d b/src/analysis/unmodified.d index b5621ce..011f598 100644 --- a/src/analysis/unmodified.d +++ b/src/analysis/unmodified.d @@ -153,6 +153,11 @@ class UnmodifiedFinder:BaseAnalyzer foreachStatement.declarationOrStatement.accept(this); } + override void visit(const TraitsExpression) + { + // Issue #266. Ignore everything inside of __traits expressions. + } + private: template PartsMightModify(T) diff --git a/src/analysis/unused.d b/src/analysis/unused.d index 12440ea..30b39ed 100644 --- a/src/analysis/unused.d +++ b/src/analysis/unused.d @@ -316,6 +316,11 @@ class UnusedVariableCheck : BaseAnalyzer variableUsed(primary.identifierChain.identifiers[0].text); } + override void visit(const TraitsExpression) + { + // Issue #266. Ignore everything inside of __traits expressions. + } + private: mixin template PartsUseVariables(NodeType) From 81192e8cc6d25ff5ad0396e18e4331ae5f91766b Mon Sep 17 00:00:00 2001 From: Hackerpilot Date: Thu, 4 Jun 2015 14:46:58 -0700 Subject: [PATCH 04/18] Fix #265 --- libdparse | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libdparse b/libdparse index 32f6d63..018a2c1 160000 --- a/libdparse +++ b/libdparse @@ -1 +1 @@ -Subproject commit 32f6d638e38888e1bb11cf43e93fe2d11132a98f +Subproject commit 018a2c13073b713b2805d912aaa193bfed5732cd From 377c9955f5aec147ca9c71251fc6a3ed01255271 Mon Sep 17 00:00:00 2001 From: Hackerpilot Date: Thu, 4 Jun 2015 15:44:12 -0700 Subject: [PATCH 05/18] Update libdparse to fix #263 --- libdparse | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libdparse b/libdparse index 018a2c1..bd7c1c2 160000 --- a/libdparse +++ b/libdparse @@ -1 +1 @@ -Subproject commit 018a2c13073b713b2805d912aaa193bfed5732cd +Subproject commit bd7c1c2dbb08bf160c4b646e0aede2af1ef6e0e4 From a078e7bbe1d9c13519674292a3212e7db3ad1669 Mon Sep 17 00:00:00 2001 From: Hackerpilot Date: Thu, 4 Jun 2015 16:17:56 -0700 Subject: [PATCH 06/18] Use regex for better getter/setter function detection --- src/analysis/undocumented.d | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/analysis/undocumented.d b/src/analysis/undocumented.d index fef8ab1..85ab00e 100644 --- a/src/analysis/undocumented.d +++ b/src/analysis/undocumented.d @@ -9,6 +9,7 @@ import std.d.ast; import std.d.lexer; import analysis.base; +import std.regex : ctRegex, matchAll; import std.stdio; /** @@ -38,9 +39,7 @@ class UndocumentedDeclarationCheck : BaseAnalyzer if (isProtection(attr.attribute.type)) set(attr.attribute.type); else if (attr.attribute == tok!"override") - { setOverride(true); - } } immutable bool shouldPop = dec.attributeDeclaration is null; @@ -92,7 +91,7 @@ class UndocumentedDeclarationCheck : BaseAnalyzer override void visit(const ConditionalDeclaration cond) { const VersionCondition ver = cond.compileCondition.versionCondition; - if (ver is null || ver.token != tok!"unittest" && ver.token.text != "none") + if ((ver is null || ver.token != tok!"unittest") && ver.token.text != "none") cond.accept(this); else if (cond.falseDeclaration !is null) visit(cond.falseDeclaration); @@ -156,8 +155,7 @@ private: static bool isGetterOrSetter(string name) { - import std.algorithm:startsWith; - return name.startsWith("get") || name.startsWith("set"); + return !matchAll(name, getSetRe).empty; } static bool isProperty(const FunctionDeclaration dec) @@ -232,3 +230,5 @@ private immutable string[] ignoredFunctionNames = [ "toHash", "main" ]; + +private enum getSetRe = ctRegex!`^(?:get|set)(?:\p{Lu}|_).*`; From 81c38d5ee8c403a581ef1fb8195d7cfab399d1c1 Mon Sep 17 00:00:00 2001 From: Hackerpilot Date: Thu, 4 Jun 2015 16:33:16 -0700 Subject: [PATCH 07/18] Fix #255 --- src/analysis/undocumented.d | 46 ++++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/src/analysis/undocumented.d b/src/analysis/undocumented.d index 85ab00e..834886d 100644 --- a/src/analysis/undocumented.d +++ b/src/analysis/undocumented.d @@ -40,10 +40,18 @@ class UndocumentedDeclarationCheck : BaseAnalyzer set(attr.attribute.type); else if (attr.attribute == tok!"override") setOverride(true); + else if (attr.deprecated_ !is null) + setDeprecated(true); + else if (attr.atAttribute !is null && attr.atAttribute.identifier.text == "disable") + setDisabled(true); } immutable bool shouldPop = dec.attributeDeclaration is null; immutable bool prevOverride = getOverride(); + immutable bool prevDisabled = getDisabled(); + immutable bool prevDeprecated = getDeprecated(); + bool dis = false; + bool dep = false; bool ovr = false; bool pushed = false; foreach (attribute; dec.attributes) @@ -60,14 +68,26 @@ class UndocumentedDeclarationCheck : BaseAnalyzer } else if (attribute.attribute == tok!"override") ovr = true; + else if (attribute.deprecated_ !is null) + dep = true; + else if (attribute.atAttribute !is null && attribute.atAttribute.identifier.text == "disable") + dis = true; } if (ovr) setOverride(true); + if (dis) + setDisabled(true); + if (dep) + setDeprecated(true); dec.accept(this); if (shouldPop && pushed) pop(); if (ovr) setOverride(prevOverride); + if (dis) + setDisabled(prevDisabled); + if (dep) + setDeprecated(prevDeprecated); } override void visit(const VariableDeclaration variable) @@ -99,6 +119,7 @@ class UndocumentedDeclarationCheck : BaseAnalyzer override void visit(const FunctionBody fb) {} override void visit(const Unittest u) {} + override void visit(const TraitsExpression t) {} mixin V!ClassDeclaration; mixin V!InterfaceDeclaration; @@ -188,9 +209,30 @@ private: stack[$ - 1].isOverride = o; } + bool getDisabled() + { + return stack[$ - 1].isDisabled; + } + + void setDisabled(bool d = true) + { + stack[$ - 1].isDisabled = d; + } + + bool getDeprecated() + { + return stack[$ - 1].isDeprecated; + } + + void setDeprecated(bool d = true) + { + stack[$ - 1].isDeprecated = d; + } + bool currentIsInteresting() { - return stack[$ - 1].protection == tok!"public" && !(stack[$ - 1].isOverride); + return stack[$ - 1].protection == tok!"public" && !stack[$ - 1].isOverride + && !stack[$ - 1].isDisabled && !stack[$ - 1].isDeprecated; } void set(IdType p) @@ -217,6 +259,8 @@ private: { IdType protection; bool isOverride; + bool isDeprecated; + bool isDisabled; } ProtectionInfo[] stack; From 017d2084055f0c9c0e1c7f7564dc74adb8277d85 Mon Sep 17 00:00:00 2001 From: Hackerpilot Date: Thu, 4 Jun 2015 16:35:02 -0700 Subject: [PATCH 08/18] Get rid of run function --- src/main.d | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/main.d b/src/main.d index 69f85f2..923d0cf 100644 --- a/src/main.d +++ b/src/main.d @@ -34,17 +34,8 @@ import inifiled; int main(string[] args) { version (unittest) - { return 0; - } - else - { - return run(args); - } -} -int run(string[] args) -{ bool sloc; bool highlight; bool ctags; @@ -289,7 +280,7 @@ string[] expandArgs(string[] args) return false; } } - + string[] rVal; if (args.length == 1) args ~= "."; From 77a7b4f5d2b895b27706713944c825e5c0dd9371 Mon Sep 17 00:00:00 2001 From: Hackerpilot Date: Thu, 4 Jun 2015 16:50:29 -0700 Subject: [PATCH 09/18] Implement #239 --- src/analysis/run.d | 12 ++++++++---- src/main.d | 15 ++++++++++----- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/analysis/run.d b/src/analysis/run.d index 2d1a9e6..f137dbe 100644 --- a/src/analysis/run.d +++ b/src/analysis/run.d @@ -115,8 +115,11 @@ void generateReport(string[] fileNames, const StaticAnalysisConfig config) writeln("}"); } -/// For multiple files -/// Returns: true if there were errors +/** + * For multiple files + * + * Returns: true if there were errors or if there were warnings and `staticAnalyze` was true. + */ bool analyze(string[] fileNames, const StaticAnalysisConfig config, bool staticAnalyze = true) { bool hasErrors = false; @@ -129,10 +132,11 @@ bool analyze(string[] fileNames, const StaticAnalysisConfig config, bool staticA ParseAllocator p = new ParseAllocator; StringCache cache = StringCache(StringCache.defaultBucketCount); uint errorCount = 0; + uint warningCount = 0; const Module m = parseModule(fileName, code, p, cache, false, null, - &errorCount, null); + &errorCount, &warningCount); assert (m); - if (errorCount > 0) + if (errorCount > 0 || (staticAnalyze && warningCount > 0)) hasErrors = true; MessageSet results = analyze(fileName, m, config, staticAnalyze); if (results is null) diff --git a/src/main.d b/src/main.d index 923d0cf..366d9a3 100644 --- a/src/main.d +++ b/src/main.d @@ -182,11 +182,11 @@ int main(string[] args) if (report) generateReport(expandArgs(args), config); else - analyze(expandArgs(args), config, true); + return analyze(expandArgs(args), config, true) ? 1 : 0; } else if (syntaxCheck) { - return .syntaxCheck(expandArgs(args)); + return .syntaxCheck(expandArgs(args)) ? 1 : 0; } else { @@ -265,6 +265,7 @@ int main(string[] args) return 0; } +private: string[] expandArgs(string[] args) { @@ -359,11 +360,13 @@ options: --syntaxCheck | -s [sourceFile] Lexes and parses sourceFile, printing the line and column number of any syntax errors to stdout. One error or warning is printed per line. - If no files are specified, input is read from stdin. + If no files are specified, input is read from stdin. %1$s will exit with + a status code of zero if no errors are found, 1 otherwise. --styleCheck | -S [sourceFiles] Lexes and parses sourceFiles, printing the line and column number of any - static analysis check failures stdout. + static analysis check failures stdout. %1$s will exit with a status code + of zero if no warnings or errors are found, 1 otherwise. --ctags | -c sourceFile Generates ctags information from the given source code file. Note that @@ -388,7 +391,9 @@ options: current working directory if none are specified. --report [sourceFiles sourceDirectories] - Generate a static analysis report in JSON format. Implies --styleCheck. + Generate a static analysis report in JSON format. Implies --styleCheck, + however the exit code will still be zero if errors or warnings are + found. --config configFile Use the given configuration file instead of the default located in From cad1f0536751ee71303ad81d5ef0ac7a0974ec94 Mon Sep 17 00:00:00 2001 From: Hackerpilot Date: Thu, 4 Jun 2015 17:35:20 -0700 Subject: [PATCH 10/18] Implement #243 --- src/analysis/config.d | 3 + src/analysis/label_var_same_name_check.d | 119 +++++++++++++++++++++++ src/analysis/run.d | 2 + src/analysis/unused_label.d | 93 ++++++++++-------- 4 files changed, 175 insertions(+), 42 deletions(-) create mode 100644 src/analysis/label_var_same_name_check.d diff --git a/src/analysis/config.d b/src/analysis/config.d index 8136f05..522c11c 100644 --- a/src/analysis/config.d +++ b/src/analysis/config.d @@ -92,4 +92,7 @@ struct StaticAnalysisConfig @INI("Checks for redundant parenthesis") bool redundant_parens_check; + + @INI("Checks for labels with the same name as variables") + bool label_var_same_name_check; } diff --git a/src/analysis/label_var_same_name_check.d b/src/analysis/label_var_same_name_check.d new file mode 100644 index 0000000..e07fe6e --- /dev/null +++ b/src/analysis/label_var_same_name_check.d @@ -0,0 +1,119 @@ +// Distributed under the Boost Software License, Version 1.0. +// (See accompanying file LICENSE_1_0.txt or copy at +// http://www.boost.org/LICENSE_1_0.txt) + +module analysis.label_var_same_name_check; + +import std.d.ast; +import std.d.lexer; + +import analysis.base; +import analysis.helpers; + +/** + * Checks for labels and variables that have the same name. + */ +class LabelVarNameCheck : BaseAnalyzer +{ + this(string fileName) + { + super(fileName); + } + + override void visit(const Module mod) + { + pushScope(); + mod.accept(this); + popScope(); + } + + override void visit(const BlockStatement block) + { + pushScope(); + block.accept(this); + popScope(); + } + + override void visit(const StructBody structBody) + { + pushScope(); + structBody.accept(this); + popScope(); + } + + override void visit(const VariableDeclaration var) + { + foreach (dec; var.declarators) + duplicateCheck(dec.name, false); + } + + override void visit(const LabeledStatement labeledStatement) + { + duplicateCheck(labeledStatement.identifier, true); + if (labeledStatement.declarationOrStatement !is null) + labeledStatement.declarationOrStatement.accept(this); + } + + alias visit = BaseAnalyzer.visit; + +private: + + Thing[string][] stack; + + void duplicateCheck(const Token name, bool fromLabel) + { + import std.conv : to; + const(Thing)* thing = name.text in currentScope; + if (thing is null) + currentScope[name.text] = Thing(name.text, name.line, name.column, false); + else + { + 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 " + ~ otherKind ~ " defined on line " ~ to!string(thing.line) ~ "."); + } + } + + static struct Thing + { + string name; + size_t line; + size_t column; + bool isVar; + } + + ref currentScope() @property + { + return stack[$-1]; + } + + void pushScope() + { + stack.length++; + } + + void popScope() + { + stack.length--; + } +} + +unittest +{ + import analysis.config : StaticAnalysisConfig; + import std.stdio : stderr; + + StaticAnalysisConfig sac; + sac.label_var_same_name_check = true; + assertAnalyzerWarnings(q{ +unittest +{ +blah: + int blah; // [warn]: Variable "blah" has the same name as a label defined on line 4. +} +int blah; + }c, sac); + stderr.writeln("Unittest for LabelVarNameCheck passed."); +} diff --git a/src/analysis/run.d b/src/analysis/run.d index f137dbe..f2b9846 100644 --- a/src/analysis/run.d +++ b/src/analysis/run.d @@ -43,6 +43,7 @@ import analysis.local_imports; import analysis.unmodified; import analysis.if_statements; import analysis.redundant_parens; +import analysis.label_var_same_name_check; bool first = true; @@ -196,6 +197,7 @@ MessageSet analyze(string fileName, const Module m, if (analysisConfig.local_import_check) checks ~= new LocalImportCheck(fileName); if (analysisConfig.could_be_immutable_check) checks ~= new UnmodifiedFinder(fileName); if (analysisConfig.redundant_parens_check) checks ~= new RedundantParenCheck(fileName); + if (analysisConfig.label_var_same_name_check) checks ~= new LabelVarNameCheck(fileName); version(none) if (analysisConfig.redundant_if_check) checks ~= new IfStatementCheck(fileName); foreach (check; checks) diff --git a/src/analysis/unused_label.d b/src/analysis/unused_label.d index 32c0cc7..90ef08c 100644 --- a/src/analysis/unused_label.d +++ b/src/analysis/unused_label.d @@ -4,7 +4,6 @@ module analysis.unused_label; -import std.stdio; import std.d.ast; import std.d.lexer; import analysis.base; @@ -19,39 +18,11 @@ class UnusedLabelCheck : BaseAnalyzer super(fileName); } - static struct Label + override void visit(const Module mod) { - string name; - size_t line; - size_t column; - bool used; - } - - Label[string][] stack; - - auto ref current() @property - { - return stack[$-1]; - } - - void pushScope() - { - stack.length++; - } - - void popScope() - { - foreach (label; current.byValue()) - { - assert(label.line != size_t.max && label.column != size_t.max); - if (!label.used) - { - addErrorMessage(label.line, label.column, - "dscanner.suspicious.unused_label", - "Label \"" ~ label.name ~ "\" is not used."); - } - } - stack.length--; + pushScope(); + mod.accept(this); + popScope(); } override void visit(const FunctionBody functionBody) @@ -99,15 +70,6 @@ class UnusedLabelCheck : BaseAnalyzer labeledStatement.declarationOrStatement.accept(this); } - void labelUsed(string name) - { - Label* entry = name in current; - if (entry is null) - current[name] = Label(name, size_t.max, size_t.max, true); - else - entry.used = true; - } - override void visit(const ContinueStatement contStatement) { if (contStatement.label.text.length) @@ -123,11 +85,58 @@ class UnusedLabelCheck : BaseAnalyzer if (gotoStatement.label.text.length) labelUsed(gotoStatement.label.text); } + +private: + + static struct Label + { + string name; + size_t line; + size_t column; + bool used; + } + + Label[string][] stack; + + auto ref current() @property + { + return stack[$-1]; + } + + void pushScope() + { + stack.length++; + } + + void popScope() + { + foreach (label; current.byValue()) + { + assert(label.line != size_t.max && label.column != size_t.max); + if (!label.used) + { + addErrorMessage(label.line, label.column, + "dscanner.suspicious.unused_label", + "Label \"" ~ label.name ~ "\" is not used."); + } + } + stack.length--; + } + + void labelUsed(string name) + { + Label* entry = name in current; + if (entry is null) + current[name] = Label(name, size_t.max, size_t.max, true); + else + entry.used = true; + } } unittest { import analysis.config : StaticAnalysisConfig; + import std.stdio : stderr; StaticAnalysisConfig sac; sac.unused_label_check = true; From 7a0eb5f13d140a172eb5fc567a91e7d31ee90060 Mon Sep 17 00:00:00 2001 From: Hackerpilot Date: Thu, 4 Jun 2015 17:35:58 -0700 Subject: [PATCH 11/18] Update to beta1 --- src/dscanner_version.d | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dscanner_version.d b/src/dscanner_version.d index f154668..e218dfc 100644 --- a/src/dscanner_version.d +++ b/src/dscanner_version.d @@ -8,7 +8,7 @@ module dscanner_version; /** * Human-readable version number */ -enum DSCANNER_VERSION = "v0.2.0-dev"; +enum DSCANNER_VERSION = "v0.2.0-beta1"; version (Windows) {} else From 6bd1e8e92e0f69a67fb8cc7d332ab5f4cd2c46a5 Mon Sep 17 00:00:00 2001 From: Hackerpilot Date: Thu, 4 Jun 2015 17:45:39 -0700 Subject: [PATCH 12/18] Update README for 0.2.0 --- README.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 3e55f1b..d5be4ea 100644 --- a/README.md +++ b/README.md @@ -79,12 +79,13 @@ you do not want to use the one created by the "--defaultConfig" option. * Confusing asm syntax. * Placement of const, immutable, or inout before a function return type instead of after the parameters. * Functions in interface declarations redundantly marked 'abstract'. +* Declaring a variable with the same name as a label. +* Variables that could have been declared const or immutable (experimental) +* Redundant parenthesis. +* Unused labels. #### Wishlish -* Assigning to foreach variables that are not "ref". -* Unused imports. -* Variables that are never modified and not declared immutable. -* Assignment in conditionals +https://github.com/Hackerpilot/Dscanner/issues?q=is%3Aopen+is%3Aissue+label%3Aenhancement ### Reports The "--report" option writes a JSON report on the static analysis checks From ae7df7615420e4ddaf0f286a41a2eb52342ff8eb Mon Sep 17 00:00:00 2001 From: Hackerpilot Date: Thu, 11 Jun 2015 15:50:15 -0700 Subject: [PATCH 13/18] Fix sefgault in undocumented declaration check^C --- src/analysis/undocumented.d | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/analysis/undocumented.d b/src/analysis/undocumented.d index 834886d..b2b4551 100644 --- a/src/analysis/undocumented.d +++ b/src/analysis/undocumented.d @@ -111,7 +111,7 @@ class UndocumentedDeclarationCheck : BaseAnalyzer override void visit(const ConditionalDeclaration cond) { const VersionCondition ver = cond.compileCondition.versionCondition; - if ((ver is null || ver.token != tok!"unittest") && ver.token.text != "none") + if (ver is null || (ver.token != tok!"unittest" && ver.token.text != "none")) cond.accept(this); else if (cond.falseDeclaration !is null) visit(cond.falseDeclaration); From 81bc94e8730032eb6f2ef3e3f15751c129928700 Mon Sep 17 00:00:00 2001 From: Joakim Brannstrom Date: Fri, 8 May 2015 21:29:01 +0200 Subject: [PATCH 14/18] Extended ctags generation with Exuberant metadata used by Vim. --- src/ctags.d | 256 ++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 197 insertions(+), 59 deletions(-) diff --git a/src/ctags.d b/src/ctags.d index f40f570..c4b2fc1 100644 --- a/src/ctags.d +++ b/src/ctags.d @@ -13,11 +13,13 @@ import std.range; import std.stdio; import std.array; import std.conv; +import std.typecons; /** * Prints CTAGS information to the given file. + * Includes metadata according to exuberant format used by Vim. * Params: - * outpt = the file that CTAGS info is written to + * outpt = the file that Exuberant TAGS info is written to * fileNames = tags will be generated from these files */ void printCtags(File output, string[] fileNames) @@ -28,101 +30,143 @@ void printCtags(File output, string[] fileNames) foreach (fileName; fileNames) { File f = File(fileName); - if (f.size == 0) continue; + if (f.size == 0) + continue; auto bytes = uninitializedArray!(ubyte[])(to!size_t(f.size)); f.rawRead(bytes); auto tokens = getTokensForParser(bytes, config, &cache); Module m = parseModule(tokens.array, fileName, null, &doNothing); + auto printer = new CTagsPrinter; printer.fileName = fileName; printer.visit(m); tags ~= printer.tagLines; } - output.write("!_TAG_FILE_FORMAT\t2\n" - ~ "!_TAG_FILE_SORTED\t1\n" - ~ "!_TAG_FILE_AUTHOR\tBrian Schott\n" - ~ "!_TAG_PROGRAM_URL\thttps://github.com/Hackerpilot/Dscanner/\n"); + output.write( + "!_TAG_FILE_FORMAT\t2\n" ~ "!_TAG_FILE_SORTED\t1\n" ~ "!_TAG_FILE_AUTHOR\tBrian Schott\n" ~ "!_TAG_PROGRAM_URL\thttps://github.com/Hackerpilot/Dscanner/\n"); tags.sort().copy(output.lockingTextWriter); } private: -void doNothing(string, size_t, size_t, string, bool) {} +/// States determining how an access modifier affects the parsning. +/// Reset, when ascending the AST reset back to the previous access. +/// Keep, when ascending the AST keep the new access. +enum AccessState +{ + Reset, + Keep +} -class CTagsPrinter : ASTVisitor +alias ContextType = Tuple!(string, "c", string, "access"); + +void doNothing(string, size_t, size_t, string, bool) +{ +} + +string paramsToString(Dec)(const Dec dec) +{ + import std.d.formatter : Formatter; + + auto app = appender!string(); + auto formatter = new Formatter!(typeof(app))(app); + + static if (is(Dec == FunctionDeclaration)) + { + formatter.format(dec.parameters); + } + else static if (is(Dec == TemplateDeclaration)) + { + formatter.format(dec.templateParameters); + } + else static if (is(Dec == Constructor)) + { + formatter.format(dec.parameters); + } + + return app.data; +} + +final class CTagsPrinter + : ASTVisitor { override void visit(const ClassDeclaration dec) { - tagLines ~= "%s\t%s\t%d;\"\tc\tline:%d%s\n".format(dec.name.text, fileName, dec.name.line, dec.name.line, context); - const c = context; - context = "\tclass:" ~ dec.name.text; + tagLines ~= "%s\t%s\t%d;\"\tc\tline:%d%s%s\n".format(dec.name.text, + fileName, dec.name.line, dec.name.line, context.c, context.access); + auto c = context; + context = ContextType("\tclass:" ~ dec.name.text, "\taccess:public"); dec.accept(this); context = c; } override void visit(const StructDeclaration dec) { - tagLines ~= "%s\t%s\t%d;\"\ts\tline:%d%s\n".format(dec.name.text, fileName, dec.name.line, dec.name.line, context); - const c = context; - context = "\tstruct:" ~ dec.name.text; + if (dec.name == tok!"") + { + dec.accept(this); + return; + } + tagLines ~= "%s\t%s\t%d;\"\ts\tline:%d%s%s\n".format(dec.name.text, + fileName, dec.name.line, dec.name.line, context.c, context.access); + auto c = context; + context = ContextType("\tstruct:" ~ dec.name.text, "\taccess:public"); dec.accept(this); context = c; } override void visit(const InterfaceDeclaration dec) { - tagLines ~= "%s\t%s\t%d;\"\ti\tline:%d%s\n".format(dec.name.text, fileName, dec.name.line, dec.name.line, context); - const c = context; - context = "\tclass:" ~ dec.name.text; + tagLines ~= "%s\t%s\t%d;\"\ti\tline:%d%s%s\n".format(dec.name.text, + fileName, dec.name.line, dec.name.line, context.c, context.access); + auto c = context; + context = ContextType("\tclass:" ~ dec.name.text, context.access); dec.accept(this); context = c; } override void visit(const TemplateDeclaration dec) { - tagLines ~= "%s\t%s\t%d;\"\tT\tline:%d%s\n".format(dec.name.text, fileName, dec.name.line, dec.name.line, context); - const c = context; - context = "\ttemplate:" ~ dec.name.text; + auto params = paramsToString(dec); + + tagLines ~= "%s\t%s\t%d;\"\tT\tline:%d%s%s\tsignature:%s\n".format( + dec.name.text, fileName, dec.name.line, dec.name.line, context.c, + context.access, params); + auto c = context; + context = ContextType("\ttemplate:" ~ dec.name.text, context.access); dec.accept(this); context = c; } override void visit(const FunctionDeclaration dec) { - tagLines ~= "%s\t%s\t%d;\"\tf\tarity:%d\tline:%d%s\n".format(dec.name.text, fileName, - dec.name.line, dec.parameters.parameters.length, dec.name.line, context); - const c = context; - context = "\tfunction:" ~ dec.name.text; - dec.accept(this); - context = c; + auto params = paramsToString(dec); + + tagLines ~= "%s\t%s\t%d;\"\tf\tline:%d%s%s\tsignature:%s\n".format( + dec.name.text, fileName, dec.name.line, dec.name.line, context.c, + context.access, params); } override void visit(const Constructor dec) { - tagLines ~= "this\t%s\t%d;\"\tf\tarity:%d\tline:%d%s\n".format(fileName, - dec.line, dec.parameters.parameters.length, dec.line, context); - const c = context; - context = "\tfunction: this"; - dec.accept(this); - context = c; + auto params = paramsToString(dec); + + tagLines ~= "this\t%s\t%d;\"\tf\tline:%d%s\tsignature:%s%s\n".format(fileName, + dec.line, dec.line, context.c, params, context.access); } override void visit(const Destructor dec) { - tagLines ~= "~this\t%s\t%d;\"\tf\tline:%d%s\n".format(fileName, dec.line, - dec.line, context); - const c = context; - context = "\tfunction: this"; - dec.accept(this); - context = c; + tagLines ~= "~this\t%s\t%d;\"\tf\tline:%d%s\n".format(fileName, + dec.line, dec.line, context.c); } override void visit(const EnumDeclaration dec) { - tagLines ~= "%s\t%s\t%d;\"\tg\tline:%d%s\n".format(dec.name.text, fileName, - dec.name.line, dec.name.line, context); - const c = context; - context = "\tenum:" ~ dec.name.text; + tagLines ~= "%s\t%s\t%d;\"\tg\tline:%d%s%s\n".format(dec.name.text, + fileName, dec.name.line, dec.name.line, context.c, context.access); + auto c = context; + context = ContextType("\tenum:" ~ dec.name.text, context.access); dec.accept(this); context = c; } @@ -134,32 +178,32 @@ class CTagsPrinter : ASTVisitor dec.accept(this); return; } - tagLines ~= "%s\t%s\t%d;\"\tu\tline:%d%s\n".format(dec.name.text, fileName, - dec.name.line, dec.name.line, context); - const c = context; - context = "\tunion:" ~ dec.name.text; + tagLines ~= "%s\t%s\t%d;\"\tu\tline:%d%s\n".format(dec.name.text, + fileName, dec.name.line, dec.name.line, context.c); + auto c = context; + context = ContextType("\tunion:" ~ dec.name.text, context.access); dec.accept(this); context = c; } override void visit(const AnonymousEnumMember mem) { - tagLines ~= "%s\t%s\t%d;\"\te\tline:%d%s\n".format(mem.name.text, fileName, - mem.name.line, mem.name.line, context); + tagLines ~= "%s\t%s\t%d;\"\te\tline:%d%s\n".format(mem.name.text, + fileName, mem.name.line, mem.name.line, context.c); } override void visit(const EnumMember mem) { - tagLines ~= "%s\t%s\t%d;\"\te\tline:%d%s\n".format(mem.name.text, fileName, - mem.name.line, mem.name.line, context); + tagLines ~= "%s\t%s\t%d;\"\te\tline:%d%s\n".format(mem.name.text, + fileName, mem.name.line, mem.name.line, context.c); } override void visit(const VariableDeclaration dec) { foreach (d; dec.declarators) { - tagLines ~= "%s\t%s\t%d;\"\tv\tline:%d%s\n".format(d.name.text, fileName, - d.name.line, d.name.line, context); + tagLines ~= "%s\t%s\t%d;\"\tv\tline:%d%s%s\n".format(d.name.text, + fileName, d.name.line, d.name.line, context.c, context.access); } dec.accept(this); } @@ -168,22 +212,116 @@ class CTagsPrinter : ASTVisitor { foreach (i; dec.identifiers) { - tagLines ~= "%s\t%s\t%d;\"\tv\tline:%d%s\n".format(i.text, fileName, - i.line, i.line, context); + tagLines ~= "%s\t%s\t%d;\"\tv\tline:%d%s%s\n".format(i.text, + fileName, i.line, i.line, context.c, context.access); } dec.accept(this); } override void visit(const Invariant dec) { - tagLines ~= "invariant\t%s\t%d;\"\tv\tline:%d%s\n".format(fileName, dec.line, dec.line, context); + tagLines ~= "invariant\t%s\t%d;\"\tv\tline:%d%s%s\n".format(fileName, + dec.line, dec.line, context.c, context.access); + } + + override void visit(const ModuleDeclaration dec) + { + context = ContextType("", "\taccess:public"); + dec.accept(this); + } + + override void visit(const Attribute attribute) + { + if (attribute.attribute != tok!"") + { + switch (attribute.attribute.type) + { + case tok!"export": + context.access = "\taccess:public"; + break; + case tok!"public": + context.access = "\taccess:public"; + break; + case tok!"package": + context.access = "\taccess:protected"; + break; + case tok!"protected": + context.access = "\taccess:protected"; + break; + case tok!"private": + context.access = "\taccess:private"; + break; + default: + } + } + + attribute.accept(this); + } + + override void visit(const AttributeDeclaration dec) + { + accessSt = AccessState.Keep; + dec.accept(this); + } + + override void visit(const Declaration dec) + { + auto c = context; + dec.accept(this); + + final switch (accessSt) with (AccessState) + { + case Reset: + context = c; + break; + case Keep: + break; + } + accessSt = AccessState.Reset; + } + + override void visit(const Unittest dec) + { + // skipping symbols inside a unit test. + //TODO investigate if it would be useful to show the unittests that was + //found when std.experimental.unittest is released. Could be possible + //to then use the UDA to show the unittest with a "name". + } + + override void visit(const AliasDeclaration dec) + { + // Old style alias + if (dec.identifierList) + { + foreach (i; dec.identifierList.identifiers) + { + tagLines ~= "%s\t%s\t%d;\"\ta\tline:%d%s%s\n".format(i.text, + fileName, i.line, i.line, context.c, context.access); + } + } + dec.accept(this); + } + + override void visit(const AliasInitializer dec) + { + tagLines ~= "%s\t%s\t%d;\"\ta\tline:%d%s%s\n".format(dec.name.text, + fileName, dec.name.line, dec.name.line, context.c, context.access); + + dec.accept(this); + } + + override void visit(const AliasThisDeclaration dec) + { + auto name = dec.identifier; + tagLines ~= "%s\t%s\t%d;\"\ta\tline:%d%s%s\n".format(name.text, + fileName, name.line, name.line, context.c, context.access); + + dec.accept(this); } alias visit = ASTVisitor.visit; - string fileName; string[] tagLines; - int suppressDepth; - string context; + ContextType context; + AccessState accessSt; } - From 3025185a55b95dfeb37a8292221b455ff8e21571 Mon Sep 17 00:00:00 2001 From: Joakim Brannstrom Date: Fri, 17 Jul 2015 13:01:16 +0200 Subject: [PATCH 15/18] Update after code review. --- src/ctags.d | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/src/ctags.d b/src/ctags.d index c4b2fc1..616226b 100644 --- a/src/ctags.d +++ b/src/ctags.d @@ -19,7 +19,7 @@ import std.typecons; * Prints CTAGS information to the given file. * Includes metadata according to exuberant format used by Vim. * Params: - * outpt = the file that Exuberant TAGS info is written to + * output = the file that Exuberant TAGS info is written to * fileNames = tags will be generated from these files */ void printCtags(File output, string[] fileNames) @@ -49,13 +49,15 @@ void printCtags(File output, string[] fileNames) private: -/// States determining how an access modifier affects the parsning. -/// Reset, when ascending the AST reset back to the previous access. -/// Keep, when ascending the AST keep the new access. +/// States determining how an access modifier affects tags when traversing the +/// AST. +/// The assumption is that there are fewer AST nodes and patterns that affects +/// the whole scope. +/// Therefor the default was chosen to be Reset. enum AccessState { - Reset, - Keep + Reset, /// when ascending the AST reset back to the previous access. + Keep /// when ascending the AST keep the new access. } alias ContextType = Tuple!(string, "c", string, "access"); @@ -71,7 +73,7 @@ string paramsToString(Dec)(const Dec dec) auto app = appender!string(); auto formatter = new Formatter!(typeof(app))(app); - static if (is(Dec == FunctionDeclaration)) + static if (is(Dec == FunctionDeclaration) || is(Dec == Constructor)) { formatter.format(dec.parameters); } @@ -79,10 +81,6 @@ string paramsToString(Dec)(const Dec dec) { formatter.format(dec.templateParameters); } - else static if (is(Dec == Constructor)) - { - formatter.format(dec.parameters); - } return app.data; } @@ -282,10 +280,11 @@ final class CTagsPrinter override void visit(const Unittest dec) { - // skipping symbols inside a unit test. - //TODO investigate if it would be useful to show the unittests that was - //found when std.experimental.unittest is released. Could be possible - //to then use the UDA to show the unittest with a "name". + // skipping symbols inside a unit test to not clutter the ctags file + // with "temporary" symbols. + // TODO when phobos have a unittest library investigate how that could + // be used to describe the tests. + // Maybe with UDA's to give the unittest a "name". } override void visit(const AliasDeclaration dec) From af53b74106c659328efa5be5077c021d3d07caa4 Mon Sep 17 00:00:00 2001 From: Whitebyte Date: Thu, 27 Aug 2015 20:43:10 +0600 Subject: [PATCH 16/18] Check args similar to tokenCount --- src/main.d | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main.d b/src/main.d index 366d9a3..dec230b 100644 --- a/src/main.d +++ b/src/main.d @@ -222,11 +222,11 @@ int main(string[] args) } else if (imports) { - const string[] fileNames = usingStdin ? ["stdin"] : args[1 .. $]; + string[] fileNames = usingStdin ? ["stdin"] : args[1 .. $]; LexerConfig config; config.stringBehavior = StringBehavior.source; auto visitor = new ImportPrinter; - foreach (name; fileNames) + foreach (name; expandArgs(fileNames)) { config.fileName = name; auto tokens = getTokensForParser( From 71575a8e18ef908700a9560ecb452ec591d43570 Mon Sep 17 00:00:00 2001 From: Martin Nowak Date: Mon, 14 Sep 2015 19:32:16 +0200 Subject: [PATCH 17/18] fix statement is not reachable error in unittest --- src/main.d | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main.d b/src/main.d index dec230b..b6503ab 100644 --- a/src/main.d +++ b/src/main.d @@ -31,11 +31,11 @@ import dscanner_version; import inifiled; +version (unittest) +void main() {} +else int main(string[] args) { - version (unittest) - return 0; - bool sloc; bool highlight; bool ctags; From eb50bc9e18d04a5643f99940177bd9722effa740 Mon Sep 17 00:00:00 2001 From: Hackerpilot Date: Thu, 17 Sep 2015 23:39:38 -0700 Subject: [PATCH 18/18] Fix #270 --- src/analysis/unmodified.d | 7 ++++++- src/analysis/unused.d | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/analysis/unmodified.d b/src/analysis/unmodified.d index 011f598..eb32ae0 100644 --- a/src/analysis/unmodified.d +++ b/src/analysis/unmodified.d @@ -155,7 +155,12 @@ class UnmodifiedFinder:BaseAnalyzer override void visit(const TraitsExpression) { - // Issue #266. Ignore everything inside of __traits expressions. + // issue #266: Ignore unmodified variables inside of `__traits` expressions + } + + override void visit(const TypeofExpression) + { + // issue #270: Ignore unmodified variables inside of `typeof` expressions } private: diff --git a/src/analysis/unused.d b/src/analysis/unused.d index 30b39ed..06445a8 100644 --- a/src/analysis/unused.d +++ b/src/analysis/unused.d @@ -318,7 +318,12 @@ class UnusedVariableCheck : BaseAnalyzer override void visit(const TraitsExpression) { - // Issue #266. Ignore everything inside of __traits expressions. + // issue #266: Ignore unused variables inside of `__traits` expressions + } + + override void visit(const TypeofExpression) + { + // issue #270: Ignore unused variables inside of `typeof` expressions } private: