Merge pull request #379 from BBasile/autofunc-mixin-asserts

auto functions checker, fix several cases of false warnings
dlang-community · Dec 6, 2016 · f283650 · f283650
2 parents bf3b942 + e041f6e
commit f283650
Showing 1 changed file with 88 additions and 8 deletions.
diff --git a/src/analysis/auto_function.d b/src/analysis/auto_function.d
@@ -29,6 +29,7 @@ private:
 	enum string MESSAGE = "Auto function without return statement, prefer an explicit void";
 
 	bool[] _returns;
+	size_t _mixinDepth;
 
 public:
 
@@ -40,7 +41,7 @@ public:
 		super(fileName, null, skipTests);
 	}
 
-	override void visit(const FunctionDeclaration decl)
+	override void visit(const(FunctionDeclaration) decl)
 	{
 		_returns.length += 1;
 		scope(exit) _returns.length -= 1;
@@ -51,16 +52,52 @@ public:
 
 		decl.accept(this);
 
-		if (autoFun && !_returns[$-1])
+		if (decl.functionBody && autoFun && !_returns[$-1])
 			addErrorMessage(decl.name.line, decl.name.column, KEY, MESSAGE);
 	}
 
-	override void visit(const ReturnStatement rst)
+	override void visit(const(ReturnStatement) rst)
 	{
 		if (_returns.length)
 			_returns[$-1] = true;
 		rst.accept(this);
 	}
+
+	override void visit(const(AssertExpression) exp)
+	{
+		exp.accept(this);
+		if (_returns.length)
+		{
+			const UnaryExpression u = cast(UnaryExpression) exp.assertion;
+			if (!u)
+				return;
+			const PrimaryExpression p = u.primaryExpression;
+			if (!p)
+				return;
+
+			immutable token = p.primary;
+			if (token.type == tok!"false")
+				_returns[$-1] = true;
+			else if (token.text == "0")
+				_returns[$-1] = true;
+		}
+	}
+
+	override void visit(const(MixinExpression) mix)
+	{
+		++_mixinDepth;
+		mix.accept(this);
+		--_mixinDepth;
+	}
+
+	override void visit(const(PrimaryExpression) exp)
+	{
+		exp.accept(this);
+		import std.algorithm.searching : find;
+		import std.range : empty;
+		if (_returns.length && _mixinDepth && !exp.primary.text.find("return").empty)
+			_returns[$-1] = true;
+	}
 }
 
 unittest
@@ -74,14 +111,57 @@ unittest
 	sac.auto_function_check = Check.enabled;
 	assertAnalyzerWarnings(q{
 		auto ref doStuff(){} // [warn]: %s
-        auto doStuff(){} // [warn]: %s
-        int doStuff(){auto doStuff(){}} // [warn]: %s
-        auto doStuff(){return 0;}
-        int doStuff(){/*error but not the aim*/}
+		auto doStuff(){} // [warn]: %s
+		int doStuff(){auto doStuff(){}} // [warn]: %s
+		auto doStuff(){return 0;}
+		int doStuff(){/*error but not the aim*/}
 	}c.format(
+		AutoFunctionChecker.MESSAGE,
+		AutoFunctionChecker.MESSAGE,
+		AutoFunctionChecker.MESSAGE,
+	), sac);
+
+	assertAnalyzerWarnings(q{
+		auto doStuff(){assert(true);} // [warn]: %s
+		auto doStuff(){assert(false);}
+	}c.format(
+		AutoFunctionChecker.MESSAGE,
+	), sac);
+
+	assertAnalyzerWarnings(q{
+		auto doStuff(){assert(1);} // [warn]: %s
+		auto doStuff(){assert(0);}
+	}c.format(
+		AutoFunctionChecker.MESSAGE,
+	), sac);
+
+	assertAnalyzerWarnings(q{
+		auto doStuff(){mixin("0+0");} // [warn]: %s
+		auto doStuff(){mixin("return 0;");}
+	}c.format(
+		AutoFunctionChecker.MESSAGE,
+	), sac);
+
+	assertAnalyzerWarnings(q{
+		auto doStuff(){mixin("0+0");} // [warn]: %s
+		auto doStuff(){mixin("static if (true)" ~ "  return " ~ 0.stringof ~ ";");}
+	}c.format(
+		AutoFunctionChecker.MESSAGE,
+	), sac);
+
+    assertAnalyzerWarnings(q{
+        auto doStuff(){} // [warn]: %s
+        extern(C) auto doStuff();
+    }c.format(
         AutoFunctionChecker.MESSAGE,
-        AutoFunctionChecker.MESSAGE,
+    ), sac);
+
+    assertAnalyzerWarnings(q{
+        auto doStuff(){} // [warn]: %s
+        @disable auto doStuff();
+    }c.format(
         AutoFunctionChecker.MESSAGE,
     ), sac);
+
 	stderr.writeln("Unittest for AutoFunctionChecker passed.");
 }