From 2a4fcbbd9dd83676ce92f309bb9583a65b68178a Mon Sep 17 00:00:00 2001
From: Matthew Brennan Jones <matthew.brennan.jones@gmail.com>
Date: Fri, 30 May 2014 20:47:56 -0700
Subject: [PATCH] Added checker for having opEquals without toHash.

---
 README.md                          |   2 +-
 analysis/opequals_without_tohash.d | 122 +++++++++++++++++++++++++++++
 analysis/run.d                     |   3 +
 3 files changed, 126 insertions(+), 1 deletion(-)
 create mode 100644 analysis/opequals_without_tohash.d

diff --git a/README.md b/README.md
index c8f4828..8873d31 100644
--- a/README.md
+++ b/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
diff --git a/analysis/opequals_without_tohash.d b/analysis/opequals_without_tohash.d
new file mode 100644
index 0000000..5936f68
--- /dev/null
+++ b/analysis/opequals_without_tohash.d
@@ -0,0 +1,122 @@
+// 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 an opEquals method without a toHash 
+ * method.
+ */
+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 = "Should override toHash if it has 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 struct
+		struct Tarantula // [warn]: Should override toHash if it has opEquals
+		{
+			const bool opEquals(Object a, Object b)
+			{
+				return true;
+			}
+		}
+
+		// Fail on class
+		class Kangaroo // [warn]: Should override toHash if it has opEquals
+		{
+			const bool opEquals(Object a, Object b)
+			{
+				return true;
+			}
+		}
+
+		// Fail on class with inheritance
+		class ChaosMonkey : Chimp // [warn]: Should override toHash if it has opEquals
+		{
+			override const bool opEquals(Object a, Object b)
+			{
+				return true;
+			}
+		}
+	}c, analysis.run.AnalyzerCheck.opequals_tohash_check);
+
+	stderr.writeln("Unittest for OpEqualsWithoutToHashCheck passed.");
+}
+
diff --git a/analysis/run.d b/analysis/run.d
index 331e64b..1866d58 100644
--- a/analysis/run.d
+++ b/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)
 	{