Add JSON report output for the Sonar plugin

.gitignore

@@ -16,4 +16,7 @@
 
 # D Scanner binaries
 dscanner
-dscanner.o
+dscanner.o
+
+# Static analysis reports
+dscanner-report.json

analysis/base.d

@@ -7,9 +7,15 @@ import std.array;
 
 struct Message
 {
+	/// Name of the file where the warning was triggered
 	string fileName;
+	/// Line number where the warning was triggered
 	size_t line;
+	/// Column number where the warning was triggered (in bytes)
 	size_t column;
+	/// Name of the warning
+	string key;
+	/// Warning message
 	string message;
 }
 
@@ -47,9 +53,9 @@ protected:
 
 	import core.vararg;
 
-	void addErrorMessage(size_t line, size_t column, string message)
+	void addErrorMessage(size_t line, size_t column, string key, string message)
 	{
-		_messages.insert(Message(fileName, line, column, message));
+		_messages.insert(Message(fileName, line, column, key, message));
 	}
 
 	/**

analysis/builtin_property_names.d

@@ -29,6 +29,8 @@ class BuiltinPropertyNameCheck : BaseAnalyzer
 {
 	alias visit = BaseAnalyzer.visit;
 
+	enum string KEY = "dscanner.confusing.builtin_property_names";
+
 	this(string fileName)
 	{
 		super(fileName);
@@ -38,7 +40,7 @@ class BuiltinPropertyNameCheck : BaseAnalyzer
 	{
 		if (depth > 0 && isBuiltinProperty(fd.name.text))
 		{
-			addErrorMessage(fd.name.line, fd.name.column, generateErrorMessage(fd.name.text));
+			addErrorMessage(fd.name.line, fd.name.column, KEY, generateErrorMessage(fd.name.text));
 		}
 		fd.accept(this);
 	}
@@ -48,14 +50,14 @@ class BuiltinPropertyNameCheck : BaseAnalyzer
 		if (depth > 0) foreach (i; ad.identifiers)
 		{
 			if (isBuiltinProperty(i.text))
-				addErrorMessage(i.line, i.column, generateErrorMessage(i.text));
+				addErrorMessage(i.line, i.column, KEY, generateErrorMessage(i.text));
 		}
 	}
 
 	override void visit(const Declarator d)
 	{
 		if (depth > 0 && isBuiltinProperty(d.name.text))
-			addErrorMessage(d.name.line, d.name.column, generateErrorMessage(d.name.text));
+			addErrorMessage(d.name.line, d.name.column, KEY, generateErrorMessage(d.name.text));
 	}
 
 	override void visit(const StructBody sb)

analysis/constructors.d

@@ -28,9 +28,9 @@ class ConstructorCheck : BaseAnalyzer
 		if (hasNoArgConstructor && hasDefaultArgConstructor)
 		{
 			addErrorMessage(classDeclaration.name.line,
-				classDeclaration.name.column, "This class has a zero-argument"
+				classDeclaration.name.column, "dscanner.confusing.constructor_args",
-				~ " constructor as well as a constructor with one default"
+				"This class has a zero-argument constructor as well as a"
-				~ " argument. This can be confusing.");
+				~ " constructor with one default argument. This can be confusing.");
 		}
 		hasDefaultArgConstructor = oldHasDefault;
 		hasNoArgConstructor = oldHasNoArg;
@@ -54,6 +54,7 @@ class ConstructorCheck : BaseAnalyzer
 				&& constructor.parameters.parameters[0].default_ !is null)
 			{
 				addErrorMessage(constructor.line, constructor.column,
+					"dscanner.confusing.struct_constructor_default_args",
 					"This struct constructor can never be called with its "
 					~ "default argument.");
 			}

analysis/del.d

@@ -25,7 +25,8 @@ class DeleteCheck : BaseAnalyzer
 
 	override void visit(const DeleteExpression d)
 	{
-		addErrorMessage(d.line, d.column, "Avoid using the 'delete' keyword.");
+		addErrorMessage(d.line, d.column, "dscanner.deprecated.delete_keyword",
+			"Avoid using the 'delete' keyword.");
 		d.accept(this);
 	}
 }

analysis/duplicate_attribute.d

@@ -91,7 +91,7 @@ class DuplicateAttributeCheck : BaseAnalyzer
 		if (hasAttribute)
 		{
 			string message = "Attribute '%s' is duplicated.".format(attributeName);
-			addErrorMessage(line, column, message);
+			addErrorMessage(line, column, "dscanner.unnecessary.duplicate_attribute", message);
 		}
 
 		// Mark it as having that attribute

analysis/enumarrayliteral.d

@@ -51,9 +51,10 @@ class EnumArrayLiteralCheck : BaseAnalyzer
 				if (initializer.nonVoidInitializer is null) continue;
 				if (initializer.nonVoidInitializer.arrayInitializer is null) continue;
 				addErrorMessage(autoDec.identifiers[i].line,
-					autoDec.identifiers[i].column, "This enum may lead to "
+					autoDec.identifiers[i].column, "dscanner.performance.enum_array_literal",
-					~ "unnecessary allocation at run-time. Use 'static immutable "
+					"This enum may lead to unnecessary allocation at run-time."
-					~ autoDec.identifiers[i].text ~ " = [ ...' instead.");
+					~ " Use 'static immutable " ~ autoDec.identifiers[i].text
+					~ " = [ ...' instead.");
 			}
 		}
 		autoDec.accept(this);

analysis/fish.d

@@ -18,6 +18,8 @@ class FloatOperatorCheck : BaseAnalyzer
 {
 	alias visit = BaseAnalyzer.visit;
 
+	enum string KEY = "dscanner.deprecated.floating_point_operators";
+
 	this(string fileName)
 	{
 		super(fileName);
@@ -34,7 +36,8 @@ class FloatOperatorCheck : BaseAnalyzer
 			|| r.operator == tok!"!>="
 			|| r.operator == tok!"!<=")
 		{
-			addErrorMessage(r.line, r.column, "Avoid using the deprecated floating-point operators.");
+			addErrorMessage(r.line, r.column, KEY,
+				"Avoid using the deprecated floating-point operators.");
 		}
 		r.accept(this);
 	}

analysis/helpers.d

@@ -12,6 +12,7 @@ import std.stdio;
 import std.d.ast;
 import analysis.config;
 import analysis.run;
+import analysis.base;
 
 
 S between(S)(S value, S before, S after)
@@ -54,23 +55,23 @@ void assertAnalyzerWarnings(string code, const StaticAnalysisConfig config, stri
 	import analysis.run;
 
 	// Run the code and get any warnings
-	string[] rawWarnings = analyze("test", cast(ubyte[]) code, config);
+	MessageSet rawWarnings = analyze("test", cast(ubyte[]) code, config);
 	string[] codeLines = code.split("\n");
 
 	// Get the warnings ordered by line
 	string[size_t] warnings;
-	for (size_t i=0; i<rawWarnings.length; ++i)
+	foreach (rawWarning; rawWarnings[])
 	{
 		// Skip the warning if it is on line zero
-		size_t rawLine = std.conv.to!size_t(rawWarnings[i].between("test(", ":"));
+		size_t rawLine = rawWarning.line;
 		if (rawLine == 0)
 		{
-			stderr.writefln("!!! Skipping warning because it is on line zero:\n%s", rawWarnings[i]);
+			stderr.writefln("!!! Skipping warning because it is on line zero:\n%s", rawWarning.message);
 			continue;
 		}
 
 		size_t warnLine = line - 1 + rawLine;
-		warnings[warnLine] = rawWarnings[i].after(")");
+		warnings[warnLine] = format("[warn]: %s", rawWarning.message);
 	}
 
 	// Get all the messages from the comments in the code

analysis/ifelsesame.d

@@ -34,6 +34,7 @@ class IfElseSameCheck : BaseAnalyzer
 	{
 		if (ifStatement.thenStatement == ifStatement.elseStatement)
 			addErrorMessage(ifStatement.line, ifStatement.column,
+				"dscanner.bugs.if_else_same",
 				"'Else' branch is identical to 'Then' branch.");
 		ifStatement.accept(this);
 	}
@@ -45,6 +46,7 @@ class IfElseSameCheck : BaseAnalyzer
 			&& e.ternaryExpression == assignExpression.ternaryExpression)
 		{
 			addErrorMessage(assignExpression.line, assignExpression.column,
+				"dscanner.bugs.self_assignment",
 				"Left side of assignment operatior is identical to the right side.");
 		}
 		assignExpression.accept(this);
@@ -56,6 +58,7 @@ class IfElseSameCheck : BaseAnalyzer
 			&& andAndExpression.left == andAndExpression.right)
 		{
 			addErrorMessage(andAndExpression.line, andAndExpression.column,
+				"dscanner.bugs.logic_operator_operands",
 				"Left side of logical and is identical to right side.");
 		}
 		andAndExpression.accept(this);
@@ -67,6 +70,7 @@ class IfElseSameCheck : BaseAnalyzer
 			&& orOrExpression.left == orOrExpression.right)
 		{
 			addErrorMessage(orOrExpression.line, orOrExpression.column,
+				"dscanner.bugs.logic_operator_operands",
 				"Left side of logical or is identical to right side.");
 		}
 		orOrExpression.accept(this);

analysis/length_subtraction.d

@@ -49,6 +49,7 @@ class LengthSubtractionCheck : BaseAnalyzer
 			}
 			const(Token) token = l.identifierOrTemplateInstance.identifier;
 			addErrorMessage(token.line, token.column,
+				"dscanner.suspicious.length_subtraction",
 				"Avoid subtracting from '.length' as it may be unsigned.");
 		}
 	end:

analysis/numbers.d

@@ -31,7 +31,7 @@ class NumberStyleCheck : BaseAnalyzer
 			&& ((t.text.startsWith("0b") && !t.text.matchFirst(badBinaryRegex).empty)
 				|| !t.text.matchFirst(badDecimalRegex).empty))
 		{
-			addErrorMessage(t.line, t.column,
+			addErrorMessage(t.line, t.column, "dscanner.style.number_literals",
 				"Use underscores to improve number constant readability.");
 		}
 	}

analysis/objectconst.d

@@ -38,8 +38,8 @@ class ObjectConstCheck : BaseAnalyzer
 			&& !hasConst(d.functionDeclaration.memberFunctionAttributes)))
 		{
 			addErrorMessage(d.functionDeclaration.name.line,
-				d.functionDeclaration.name.column, "Methods 'opCmp', 'toHash',"
+				d.functionDeclaration.name.column, "dscanner.suspicious.object_const",
-				~ " 'opEquals', and 'toString' are non-const.");
+				"Methods 'opCmp', 'toHash', 'opEquals', and 'toString' are non-const.");
 		}
 		d.accept(this);
 	}

analysis/opequals_without_tohash.d

@@ -70,21 +70,23 @@ class OpEqualsWithoutToHashCheck : BaseAnalyzer
 		if (hasOpEquals && !hasToHash)
 		{
 			string message = "'" ~ name.text ~ "' has method 'opEquals', but not 'toHash'.";
-			addErrorMessage(name.line, name.column, message);
+			addErrorMessage(name.line, name.column, KEY, message);
 		}
 		// Warn if has toHash, but not opEquals
 		else if (!hasOpEquals && hasToHash)
 		{
 			string message = "'" ~ name.text ~ "' has method 'toHash', but not 'opEquals'.";
-			addErrorMessage(name.line, name.column, message);
+			addErrorMessage(name.line, name.column, KEY, message);
 		}
 
 		if (hasOpCmp && !hasOpEquals)
 		{
-			addErrorMessage(name.line, name.column,
+			addErrorMessage(name.line, name.column, KEY,
 				"'" ~ name.text ~ "' has method 'opCmp', but not 'opEquals'.");
 		}
 	}
+
+	enum string KEY = "dscanner.suspicious.incomplete_operator_overloading";
 }
 
 unittest

analysis/pokemon.d

@@ -25,7 +25,8 @@ import analysis.helpers;
  */
 class PokemonExceptionCheck : BaseAnalyzer
 {
-	enum message = "Catching Error or Throwable is almost always a bad idea.";
+	enum MESSAGE = "Catching Error or Throwable is almost always a bad idea.";
+	enum string KEY = "dscanner.suspicious.catch_em_all";
 
 	alias visit = BaseAnalyzer.visit;
 
@@ -36,7 +37,7 @@ class PokemonExceptionCheck : BaseAnalyzer
 
 	override void visit(const LastCatch lc)
 	{
-		addErrorMessage(lc.line, lc.column, message);
+		addErrorMessage(lc.line, lc.column, KEY, MESSAGE);
 		lc.accept(this);
 	}
 
@@ -76,7 +77,7 @@ class PokemonExceptionCheck : BaseAnalyzer
 		{
 			immutable column = identOrTemplate.identifier.column;
 			immutable line = identOrTemplate.identifier.line;
-			addErrorMessage(line, column, message);
+			addErrorMessage(line, column, KEY, MESSAGE);
 		}
 	}
 }

analysis/range.d

@@ -26,6 +26,7 @@ class BackwardsRangeCheck : BaseAnalyzer
 	size_t column;
 	size_t line;
 	enum State { ignore, left, right }
+	enum string KEY = "dscanner.bugs.backwards_slices";
 	State state = State.ignore;
 
 	this(string fileName)
@@ -48,7 +49,7 @@ class BackwardsRangeCheck : BaseAnalyzer
 				string message = format(
 					"%d is larger than %d. Did you mean to use 'foreach_reverse( ... ; %d .. %d)'?",
 					left, right, right, left);
-				addErrorMessage(line, this.column, message);
+				addErrorMessage(line, this.column, KEY, message);
 			}
 			hasLeft = false;
 			hasRight = false;
@@ -125,7 +126,7 @@ class BackwardsRangeCheck : BaseAnalyzer
 				string message = format(
 					"%d is larger than %d. This slice is likely incorrect.",
 					left, right);
-				addErrorMessage(line, this.column, message);
+				addErrorMessage(line, this.column, KEY, message);
 			}
 			hasLeft = false;
 			hasRight = false;

analysis/run.d

@@ -29,6 +29,8 @@ import analysis.length_subtraction;
 import analysis.builtin_property_names;
 import analysis.asm_style;
 
+bool first = true;
+
 void messageFunction(string fileName, size_t line, size_t column, string message,
 	bool isError)
 {
@@ -36,15 +38,45 @@ void messageFunction(string fileName, size_t line, size_t column, string message
 		isError ? "error" : "warn", message);
 }
 
-void syntaxCheck(File output, string[] fileNames)
+void messageFunctionJSON(string fileName, size_t line, size_t column, string message, bool)
+{
+	writeJSON("dscanner.syntax", fileName, line, column, message);
+}
+
+void writeJSON(string key, string fileName, size_t line, size_t column, string message)
+{
+	if (!first)
+		writeln(",");
+	else
+		first = false;
+	writeln("    {");
+	writeln(`      "key": "`, key, `",`);
+	writeln(`      "fileName": "`, fileName, `",`);
+	writeln(`      "line": `, line, `,`);
+	writeln(`      "column": `, column, `,`);
+	writeln(`      "message": "`, message.replace(`"`, `\"`), `"`);
+	write(  "    }");
+}
+
+void syntaxCheck(string[] fileNames)
 {
 	StaticAnalysisConfig config = defaultStaticAnalysisConfig();
-	analyze(output, fileNames, config, false);
+	analyze(fileNames, config, false);
 }
 
 // For multiple files
-void analyze(File output, string[] fileNames, StaticAnalysisConfig config, bool staticAnalyze = true)
+void analyze(string[] fileNames, StaticAnalysisConfig config,
+	bool staticAnalyze = true, bool report = false)
 {
+	if (report)
+	{
+		writeln("{");
+		writeln(`  "issues": [`);
+
+	}
+
+	first = true;
+
 	foreach (fileName; fileNames)
 	{
 		File f = File(fileName);
@@ -52,14 +84,33 @@ void analyze(File output, string[] fileNames, StaticAnalysisConfig config, bool
 		auto code = uninitializedArray!(ubyte[])(to!size_t(f.size));
 		f.rawRead(code);
 
-		string[] results = analyze(fileName, code, config, staticAnalyze);
+		MessageSet results = analyze(fileName, code, config, staticAnalyze, report);
-		if (results.length > 0)
+		if (report)
-			output.writeln(results.join("\n"));
+		{
+			foreach (result; results[])
+			{
+				writeJSON(result.key, result.fileName, result.line, result.column, result.message);
+			}
+		}
+		else
+		{
+			foreach (result; results[])
+				writefln("%s(%d:%d)[warn]: %s", result.fileName, result.line,
+					result.column, result.message);
+		}
+	}
+
+	if (report)
+	{
+		writeln();
+		writeln("  ]");
+		writeln("}");
 	}
 }
 
 // For a string
-string[] analyze(string fileName, ubyte[] code, const StaticAnalysisConfig analysisConfig, bool staticAnalyze = true)
+MessageSet analyze(string fileName, ubyte[] code, const StaticAnalysisConfig analysisConfig,
+	bool staticAnalyze = true, bool report = false)
 {
 	import std.parallelism;
 
@@ -72,12 +123,16 @@ string[] analyze(string fileName, ubyte[] code, const StaticAnalysisConfig analy
 
 	foreach (message; lexer.messages)
 	{
-		messageFunction(fileName, message.line, message.column, message.message,
+		if (report)
-			message.isError);
+			messageFunctionJSON(fileName, message.line, message.column, message.message,
+				message.isError);
+		else
+			messageFunction(fileName, message.line, message.column, message.message,
+				message.isError);
 	}
 
 	ParseAllocator p = new ParseAllocator;
-	Module m = parseModule(tokens, fileName, p, &messageFunction);
+	Module m = parseModule(tokens, fileName, p, report ? &messageFunctionJSON : &messageFunction);
 
 	if (!staticAnalyze)
 		return null;
@@ -110,12 +165,7 @@ string[] analyze(string fileName, ubyte[] code, const StaticAnalysisConfig analy
 	foreach (check; checks)
 		foreach (message; check.messages)
 			set.insert(message);
-
-	string[] results;
-	foreach (message; set[])
-		results ~= "%s(%d:%d)[warn]: %s".format(message.fileName, message.line,
-			message.column, message.message);
 	p.deallocateAll();
-	return results;
+	return set;
 }
 

analysis/style.d

@@ -20,10 +20,10 @@ class StyleChecker : BaseAnalyzer
 {
 	alias visit = ASTVisitor.visit;
 
-	// FIXME: All variable and function names seem to never match this
+	enum string varFunNameRegex = `^([\p{Ll}_][_\w\d]*|[\p{Lu}\d_]+)$`;
-	enum varFunNameRegex = `^([\p{Ll}_][_\w\d]*|[\p{Lu}\d_]+)$`;
+	enum string aggregateNameRegex = `^\p{Lu}[\w\d]*$`;
-	enum aggregateNameRegex = `^\p{Lu}[\w\d]*$`;
+	enum string moduleNameRegex = `^[\p{Ll}_\d]+$`;
-	enum moduleNameRegex = `^[\p{Ll}_\d]+$`;
+	enum string KEY = "dscanner.style.phobos_naming_convention";
 
 	this(string fileName)
 	{
@@ -35,7 +35,7 @@ class StyleChecker : BaseAnalyzer
 		foreach (part; dec.moduleName.identifiers)
 		{
 			if (part.text.matchFirst(moduleNameRegex).length == 0)
-				addErrorMessage(part.line, part.column, "Module/package name '"
+				addErrorMessage(part.line, part.column, KEY, "Module/package name '"
 					~ part.text ~ "' does not match style guidelines.");
 		}
 	}
@@ -53,7 +53,7 @@ class StyleChecker : BaseAnalyzer
 	void checkLowercaseName(string type, ref const Token name)
 	{
 		if (name.text.matchFirst(varFunNameRegex).length == 0)
-			addErrorMessage(name.line, name.column, type ~ " name '"
+			addErrorMessage(name.line, name.column, KEY, type ~ " name '"
 				~ name.text ~ "' does not match style guidelines.");
 	}
 
@@ -86,7 +86,7 @@ class StyleChecker : BaseAnalyzer
 	void checkAggregateName(string aggregateType, ref const Token name)
 	{
 		if (name.text.matchFirst(aggregateNameRegex).length == 0)
-			addErrorMessage(name.line, name.column, aggregateType
+			addErrorMessage(name.line, name.column, KEY, aggregateType
 				~ " name '" ~ name.text ~ "' does not match style guidelines.");
 	}
 }

analysis/unused.d

@@ -323,8 +323,10 @@ class UnusedVariableCheck : BaseAnalyzer
 		{
 			if (!uu.isRef && tree.length > 1)
 				addErrorMessage(uu.line, uu.column,
+					uu.isParameter ? "dscanner.suspicious.unused_parameter"
+						: "dscanner.suspicious.unused_variable",
 					(uu.isParameter ? "Parameter " : "Variable ")
-					~ uu.name ~ " is never used.");
+						~ uu.name ~ " is never used.");
 		}
 		tree = tree[0 .. $ - 1];
 	}

libdparse

@@ -1 +1 @@
-Subproject commit f693be2b2c0fb5b015493c7441e14b888647ef0d
+Subproject commit 4feb92c544a9e09fcc56051d962b669327cee386

main.d

@@ -56,6 +56,7 @@ int run(string[] args)
 	bool tokenDump;
 	bool styleCheck;
 	bool defaultConfig;
+	bool report;
 	string symbolName;
 	string configLocation;
 
@@ -67,7 +68,7 @@ int run(string[] args)
 			"ast|xml", &ast, "imports|i", &imports, "outline|o", &outline,
 			"tokenDump", &tokenDump, "styleCheck|S", &styleCheck,
 			"defaultConfig", &defaultConfig, "declaration|d", &symbolName,
-			"config", &configLocation, "muffinButton", &muffin);
+			"config", &configLocation, "report", &report,"muffinButton", &muffin);
 	}
 	catch (ConvException e)
 	{
@@ -98,7 +99,7 @@ int run(string[] args)
 
 	auto optionCount = count!"a"([sloc, highlight, ctags, tokenCount,
 		syntaxCheck, ast, imports, outline, tokenDump, styleCheck, defaultConfig,
-		symbolName !is null]);
+		report, symbolName !is null]);
 	if (optionCount > 1)
 	{
 		stderr.writeln("Too many options specified");
@@ -110,6 +111,9 @@ int run(string[] args)
 		return 1;
 	}
 
+	// --report implies --styleCheck
+	if (report) styleCheck = true;
+
 	StringCache cache = StringCache(StringCache.defaultBucketCount);
 	if (defaultConfig)
 	{
@@ -156,11 +160,11 @@ int run(string[] args)
 		string s = configLocation is null ? getConfigurationLocation() : configLocation;
 		if (s.exists())
 			readINIFile(config, s);
-		stdout.analyze(expandArgs(args), config);
+		analyze(expandArgs(args), config, true, report);
 	}
 	else if (syntaxCheck)
 	{
-		stdout.syntaxCheck(expandArgs(args));
+		.syntaxCheck(expandArgs(args));
 	}
 	else
 	{
@@ -342,6 +346,9 @@ options:
         accurate than "grep". Searches the given files and directories, or the
         current working directory if none are specified.
 
+    --report [sourceFiles sourceDirectories]
+        Generate a static analysis report in JSON format. Implies --styleCheck.
+
     --config configFile
         Use the given configuration file instead of the default located in
         $HOME/.config/dscanner/dscanner.ini

sonar-project.properties

@@ -0,0 +1,5 @@
+sonar.projectKey=dscanner
+sonar.projectName=D Scanner
+sonar.projectVersion=1.0
+sonar.sourceEncoding=UTF-8
+sonar.sources=.