From cad1f0536751ee71303ad81d5ef0ac7a0974ec94 Mon Sep 17 00:00:00 2001 From: Hackerpilot Date: Thu, 4 Jun 2015 17:35:20 -0700 Subject: [PATCH] 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;