Add NullParameterCheck, closes #49

checkstyle/Main.hx

@@ -116,9 +116,8 @@ class Main {
 		var check:Check = cast info.build(checkConf.type);
 		if (check == null) return;
 		verifyAllowedFields(checkConf, ["type", "props"], check.getModuleName());
-		if (checkConf.props == null) checkConf.props = [];
 
-		var props = Reflect.fields(checkConf.props);
+		var props = (checkConf.props == null) ? [] : Reflect.fields(checkConf.props);
 		for (prop in props) {
 			var val = Reflect.field(checkConf.props, prop);
 			if (!Reflect.hasField(check, prop)) {

checkstyle/checks/NullParameterCheck.hx

@@ -0,0 +1,52 @@
+package checkstyle.checks;
+
+import checkstyle.LintMessage.SeverityLevel;
+import haxe.macro.Expr.FunctionArg;
+import haxe.macro.Expr.Position;
+using haxe.macro.ExprTools;
+
+@name("NullParameter")
+@desc("Checks for parameters with a null default value")
+class NullParameterCheck extends Check {
+
+	public static inline var EITHER:String = "either";
+	public static inline var QUESTION_MARK:String = "questionmark";
+	public static inline var NULL:String = "null";
+
+	public var nullDefaultValueStyle:String;
+
+	public function new() {
+		super();
+		nullDefaultValueStyle = QUESTION_MARK;
+	}
+
+	override function actualRun() {
+		forEachField(function(field, _) {
+			switch (field.kind) {
+				case FFun(f):
+					for (arg in f.args) checkArgument(arg, field.pos);
+				case _:
+			}
+		});
+	}
+
+	function checkArgument(arg:FunctionArg, pos:Position) {
+		var hasNullDefault = arg.value.toString() == "null";
+		var prefix = 'Parameter \'${arg.name}\' ';
+		if (arg.opt && hasNullDefault) {
+			logPos(prefix + "is marked as optional with '?' and has a default value of 'null', which is redundant",
+				arg.value.pos, Reflect.field(SeverityLevel, severity));
+		}
+		else {
+			var line = checker.getLinePos(pos.min).line + 1;
+			if (hasNullDefault && nullDefaultValueStyle == QUESTION_MARK) {
+				log(prefix + "should be marked as optional with '?' instead of using a null default value",
+					line, 0, null, Reflect.field(SeverityLevel, severity));
+			}
+			else if (arg.opt && nullDefaultValueStyle == NULL) {
+				log(prefix + "should have a null default value instead of being marked as optional with '?'",
+					line, 0, null, Reflect.field(SeverityLevel, severity));
+			}
+		}
+	}
+}

resources/sample-config.json

@@ -371,6 +371,13 @@
 			"props": {
 				"severity": "ERROR"
 			}
+		},
+		{
+			"type": "NullParameter",
+			"props": {
+				"severity": "ERROR",
+				"nullDefaultValueStyle": "questionmark"
+			}
 		}
 	]
 }

resources/static-analysis.xml

@@ -62,6 +62,8 @@
 	</file>
 	<file name="checkstyle&#x2F;checks&#x2F;NestedTryDepthCheck.hx">
 	</file>
+	<file name="checkstyle&#x2F;checks&#x2F;NullParameterCheck.hx">
+	</file>
 	<file name="checkstyle&#x2F;checks&#x2F;ParameterNameCheck.hx">
 	</file>
 	<file name="checkstyle&#x2F;checks&#x2F;ParameterNumberCheck.hx">

test/AccessOrderCheckTest.hx

@@ -1,4 +1,4 @@
-package ;
+package;
 
 import checkstyle.checks.AccessOrderCheck;
 

test/DynamicCheckTest.hx

@@ -1,4 +1,4 @@
-package ;
+package;
 
 import checkstyle.checks.DynamicCheck;
 

test/NullParameterCheckTest.hx

@@ -0,0 +1,53 @@
+package;
+
+import checkstyle.checks.NullParameterCheck;
+
+class NullParameterCheckTest extends CheckTestCase {
+
+	public function testDetectRedundancy() {
+		assertMsg(new NullParameterCheck(), NullParameterCheckTests.TEST1,
+			"Parameter 'a' is marked as optional with '?' and has a default value of 'null', which is redundant");
+	}
+
+	public function testSuppress() {
+		assertMsg(new NullParameterCheck(), NullParameterCheckTests.TEST2, '');
+	}
+
+	public function testAllowEitherStyle() {
+		var check = new NullParameterCheck();
+		check.nullDefaultValueStyle = NullParameterCheck.EITHER;
+		assertMsg(check, NullParameterCheckTests.TEST3, '');
+	}
+
+	public function testPreferQuestionMark() {
+		var check = new NullParameterCheck();
+		check.nullDefaultValueStyle = NullParameterCheck.QUESTION_MARK;
+		assertMsg(check, NullParameterCheckTests.TEST3,
+			"Parameter 'b' should be marked as optional with '?' instead of using a null default value");
+	}
+
+	public function testPreferNull() {
+		var check = new NullParameterCheck();
+		check.nullDefaultValueStyle = NullParameterCheck.NULL;
+		assertMsg(check, NullParameterCheckTests.TEST3,
+			"Parameter 'a' should have a null default value instead of being marked as optional with '?'");
+	}
+}
+
+class NullParameterCheckTests {
+	public static inline var TEST1:String = "
+	abstractAndClass Test {
+		function test(?a:Int = null) {}
+	}";
+
+	public static inline var TEST2:String = "
+	abstractAndClass Test {
+		@SuppressWarnings('checkstyle:NullParameter')
+		function test(?a:Int = null, b:Null<Int> = null, ?c:Int) {}
+	}";
+
+	public static inline var TEST3:String = "
+	abstractAndClass Test {
+		function test(?a:Int, b:Null<Int> = null) {}
+	}";
+}

test/TestMain.hx

@@ -1,4 +1,4 @@
-package ;
+package;
 
 class TestMain {
 
@@ -33,6 +33,7 @@ class TestMain {
 		runner.add(new NestedForDepthCheckTest());
 		runner.add(new NestedIfDepthCheckTest());
 		runner.add(new NestedTryDepthCheckTest());
+		runner.add(new NullParameterCheckTest());
 		runner.add(new ParameterNameCheckTest());
 		runner.add(new ParameterNumberCheckTest());
 		runner.add(new PublicPrivateCheckTest());