Merge pull request #179 from workhorsy/master

Added checker for having opEquals without toHash.

README.md

@@ -58,13 +58,13 @@ given source files.
 * Unused variables.
 * Unused parameters (check is skipped if function is marked "override")
 * Duplicate attributes
+* Declaring opEquals without toHash
 
 #### Wishlish
 * Assigning to foreach variables that are not "ref".
 * Unused imports.
 * Variables that are never modified and not declared immutable.
 * Public declarations not documented
-* Declaring opEquals without toHash
 * Assignment in conditionals
 
 ### Line of Code Count

analysis/opequals_without_tohash.d

@@ -0,0 +1,137 @@
+// Copyright (c) 2014, Matthew Brennan Jones <matthew.brennan.jones@gmail.com>
+// 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.opequals_without_tohash;
+
+import std.stdio;
+import std.d.ast;
+import std.d.lexer;
+import analysis.base;
+import analysis.helpers;
+
+/**
+ * Checks for when a class/struct has the method opEquals without toHash, or
+ * toHash without opEquals.
+ */
+class OpEqualsWithoutToHashCheck : BaseAnalyzer
+{
+	alias visit = BaseAnalyzer.visit;
+
+	this(string fileName)
+	{
+		super(fileName);
+	}
+
+	override void visit(const ClassDeclaration node)
+	{
+		actualCheck(node.name, node.structBody);
+		node.accept(this);
+	}
+
+	override void visit(const StructDeclaration node)
+	{
+		actualCheck(node.name, node.structBody);
+		node.accept(this);
+	}
+
+	private void actualCheck(const Token name, const StructBody structBody)
+	{
+		bool hasOpEquals = false;
+		bool hasToHash = false;
+
+		// Just return if missing children
+		if (!structBody
+			|| !structBody.declarations
+			|| name is Token.init)
+			return;
+
+		// Check all the function declarations
+		foreach (declaration; structBody.declarations)
+		{
+			// Skip if not a function declaration
+			if (!declaration
+				|| !declaration.functionDeclaration)
+				continue;
+
+			// Check if opEquals or toHash
+			string methodName = declaration.functionDeclaration.name.text;
+			if (methodName == "opEquals")
+				hasOpEquals = true;
+			else if (methodName == "toHash")
+				hasToHash = true;
+		}
+
+		// Warn if has opEquals, but not toHash
+		if (hasOpEquals && !hasToHash)
+		{
+			string message = "Has method opEquals, but not toHash";
+			addErrorMessage(name.line, name.column, message);
+		}
+		// Warn if has toHash, but not opEquals
+		else if (!hasOpEquals && hasToHash)
+		{
+			string message = "Has method toHash, but not opEquals";
+			addErrorMessage(name.line, name.column, message);
+		}
+	}
+}
+
+unittest
+{
+	assertAnalyzerWarnings(q{
+		// Success because it has opEquals and toHash
+		class Chimp
+		{
+			const bool opEquals(Object a, Object b)
+			{
+				return true;
+			}
+
+			const override hash_t toHash()
+			{
+				return 0;
+			}
+		}
+
+		// Fail on class opEquals
+		class Rabbit // [warn]: Has method opEquals, but not toHash
+		{
+			const bool opEquals(Object a, Object b)
+			{
+				return true;
+			}
+		}
+
+		// Fail on class toHash
+		class Kangaroo // [warn]: Has method toHash, but not opEquals
+		{
+			override const hash_t toHash()
+			{
+				return 0;
+			}
+		}
+
+		// Fail on struct opEquals
+		struct Tarantula // [warn]: Has method opEquals, but not toHash
+		{
+			const bool opEquals(Object a, Object b)
+			{
+				return true;
+			}
+		}
+
+		// Fail on struct toHash
+		struct Puma // [warn]: Has method toHash, but not opEquals
+		{
+			const nothrow @safe hash_t toHash()
+			{
+				return 0;
+			}
+		}
+	}c, analysis.run.AnalyzerCheck.opequals_tohash_check);
+
+	stderr.writeln("Unittest for OpEqualsWithoutToHashCheck passed.");
+}
+

analysis/run.d

@@ -23,6 +23,7 @@ import analysis.ifelsesame;
 import analysis.constructors;
 import analysis.unused;
 import analysis.duplicate_attribute;
+import analysis.opequals_without_tohash;
 
 enum AnalyzerCheck : uint
 {
@@ -38,6 +39,7 @@ enum AnalyzerCheck : uint
 	constructor_check =        0b00000010_00000000,
 	unused_variable_check =    0b00000100_00000000,
 	duplicate_attribute =      0b00001000_00000000,
+	opequals_tohash_check   =  0b00010000_00000000,
 	all =                      0b11111111_11111111
 }
 
@@ -107,6 +109,7 @@ string[] analyze(string fileName, ubyte[] code, AnalyzerCheck analyzers, bool st
 	if (analyzers & AnalyzerCheck.constructor_check) checks ~= new ConstructorCheck(fileName);
 	if (analyzers & AnalyzerCheck.unused_variable_check) checks ~= new UnusedVariableCheck(fileName);
 	if (analyzers & AnalyzerCheck.duplicate_attribute) checks ~= new DuplicateAttributeCheck(fileName);
+	if (analyzers & AnalyzerCheck.opequals_tohash_check) checks ~= new OpEqualsWithoutToHashCheck(fileName);
 
 	foreach (check; checks)
 	{