Merge pull request #267 from Hackerpilot/0.2.0-dev

0.2.0 dev

Author: Hackerpilot

libdparse

@@ -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;
 }

src/analysis/config.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.");
+}

src/analysis/label_var_same_name_check.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;
 
@@ -115,8 +116,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 +133,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)
@@ -192,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)

src/analysis/run.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,13 +39,19 @@ class UndocumentedDeclarationCheck : BaseAnalyzer
 			if (isProtection(attr.attribute.type))
 				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)
@@ -61,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)
@@ -92,14 +111,15 @@ 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);
 	}
 
 	override void visit(const FunctionBody fb) {}
 	override void visit(const Unittest u) {}
+	override void visit(const TraitsExpression t) {}
 
 	mixin V!ClassDeclaration;
 	mixin V!InterfaceDeclaration;
@@ -156,8 +176,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)
@@ -190,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)
@@ -219,6 +259,8 @@ private:
 	{
 		IdType protection;
 		bool isOverride;
+		bool isDeprecated;
+		bool isDisabled;
 	}
 
 	ProtectionInfo[] stack;
@@ -232,3 +274,5 @@ private immutable string[] ignoredFunctionNames = [
 	"toHash",
 	"main"
 ];
+
+private enum getSetRe = ctRegex!`^(?:get|set)(?:\p{Lu}|_).*`;

src/analysis/undocumented.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)

src/analysis/unmodified.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");
@@ -317,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)
@@ -334,13 +338,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)