Merge pull request #382 from wilzbach/import-sortedness

Add ImportSortedness checker

src/analysis/config.d

@@ -147,4 +147,7 @@ struct StaticAnalysisConfig
 
 	@INI("Check for auto function without return statement")
 	string auto_function_check = Check.disabled;
+
+	@INI("Check for sortedness of imports")
+	string imports_sortedness = Check.disabled;
 }

src/analysis/imports_sortedness.d

@@ -0,0 +1,191 @@
+// 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.imports_sortedness;
+
+import dparse.lexer;
+import dparse.ast;
+import analysis.base : BaseAnalyzer;
+
+import std.stdio;
+
+/**
+ * Checks the sortedness of module imports
+ */
+class ImportSortednessCheck : BaseAnalyzer
+{
+	enum string KEY = "dscanner.style.imports_sortedness";
+	enum string MESSAGE = "The imports are not sorted in alphabetical order";
+
+	///
+	this(string fileName, bool skipTests = false)
+	{
+		super(fileName, null, skipTests);
+	}
+
+	override void visit(const Module mod)
+	{
+		globalImports = [];
+		mod.accept(this);
+	}
+
+	override void visit(const ImportDeclaration id)
+	{
+		import std.algorithm.iteration : map;
+		import std.array : join;
+		import std.string : strip;
+
+		if (id.importBindings is null || id.importBindings.importBinds.length == 0)
+		{
+			foreach (singleImport; id.singleImports)
+			{
+				string importModuleName = singleImport.identifierChain.identifiers.map!`a.text`.join(".");
+				addImport(importModuleName, singleImport);
+			}
+		}
+		else
+		{
+			string importModuleName = id.importBindings.singleImport.identifierChain.identifiers.map!`a.text`.join(".");
+
+			foreach (importBind; id.importBindings.importBinds)
+			{
+				addImport(importModuleName ~ "_" ~ importBind.left.text, id.importBindings.singleImport);
+			}
+		}
+	}
+
+	alias visit = BaseAnalyzer.visit;
+
+private:
+
+	string[] globalImports;
+
+	void addImport(string importModuleName, const SingleImport singleImport)
+	{
+		import std.uni : sicmp;
+
+		if (globalImports.length > 0 && globalImports[$ -1].sicmp(importModuleName) > 0)
+		{
+			addErrorMessage(singleImport.identifierChain.identifiers[0].line,
+					singleImport.identifierChain.identifiers[0].column, KEY, MESSAGE);
+		}
+		else
+		{
+			globalImports ~= importModuleName;
+		}
+	}
+}
+
+unittest
+{
+	import std.stdio : stderr;
+	import std.format : format;
+	import analysis.config : StaticAnalysisConfig, Check;
+	import analysis.helpers : assertAnalyzerWarnings;
+
+	StaticAnalysisConfig sac;
+	sac.imports_sortedness = Check.enabled;
+
+	assertAnalyzerWarnings(q{
+		import bar.foo;
+		import foo.bar;
+	}c, sac);
+
+	assertAnalyzerWarnings(q{
+		import foo.bar;
+		import bar.foo; // [warn]: %s
+	}c.format(
+		ImportSortednessCheck.MESSAGE,
+	), sac);
+
+	assertAnalyzerWarnings(q{
+		import c;
+		import c.b;
+		import c.a; // [warn]: %s
+		import d.a;
+		import d; // [warn]: %s
+	}c.format(
+		ImportSortednessCheck.MESSAGE,
+		ImportSortednessCheck.MESSAGE,
+	), sac);
+
+	assertAnalyzerWarnings(q{
+		import a.b, a.c, a.d;
+		import a.b, a.d, a.c; // [warn]: %s
+		import a.c, a.b, a.c; // [warn]: %s
+		import foo.bar, bar.foo; // [warn]: %s
+	}c.format(
+		ImportSortednessCheck.MESSAGE,
+		ImportSortednessCheck.MESSAGE,
+		ImportSortednessCheck.MESSAGE,
+	), sac);
+
+	// multiple items out of order
+	assertAnalyzerWarnings(q{
+		import foo.bar;
+		import bar.foo; // [warn]: %s
+		import bar.bar.foo; // [warn]: %s
+	}c.format(
+		ImportSortednessCheck.MESSAGE,
+		ImportSortednessCheck.MESSAGE,
+	), sac);
+
+	assertAnalyzerWarnings(q{
+		import test : bar;
+		import test : foo;
+	}c, sac);
+
+	// selective imports
+	assertAnalyzerWarnings(q{
+		import test : foo;
+		import test : bar; // [warn]: %s
+	}c.format(
+		ImportSortednessCheck.MESSAGE,
+	), sac);
+
+	// selective imports
+	assertAnalyzerWarnings(q{
+		import test : foo, bar; // [warn]: %s
+	}c.format(
+		ImportSortednessCheck.MESSAGE,
+	), sac);
+
+	assertAnalyzerWarnings(q{
+		import b;
+		import c : foo;
+		import c : bar; // [warn]: %s
+		import a; // [warn]: %s
+	}c.format(
+		ImportSortednessCheck.MESSAGE,
+		ImportSortednessCheck.MESSAGE,
+	), sac);
+
+	assertAnalyzerWarnings(q{
+		import c;
+		import c : bar;
+		import d : bar;
+		import d; // [warn]: %s
+		import a : bar; // [warn]: %s
+	}c.format(
+		ImportSortednessCheck.MESSAGE,
+		ImportSortednessCheck.MESSAGE,
+	), sac);
+
+	assertAnalyzerWarnings(q{
+		import t0;
+		import t1 : a, b = foo;
+		import t2;
+	}c, sac);
+
+	assertAnalyzerWarnings(q{
+		import t1 : a, b = foo;
+		import t1 : b, a = foo; // [warn]: %s
+		import t0 : a, b = foo; // [warn]: %s
+	}c.format(
+		ImportSortednessCheck.MESSAGE,
+		ImportSortednessCheck.MESSAGE,
+	), sac);
+
+	stderr.writeln("Unittest for ImportSortednessCheck passed.");
+}

src/analysis/run.d

@@ -60,6 +60,7 @@ import analysis.alias_syntax_check;
 import analysis.static_if_else;
 import analysis.lambda_return_check;
 import analysis.auto_function;
+import analysis.imports_sortedness;
 
 import dsymbol.string_interning : internString;
 import dsymbol.scope_;
@@ -359,6 +360,10 @@ MessageSet analyze(string fileName, const Module m, const StaticAnalysisConfig a
 		checks ~= new AutoFunctionChecker(fileName,
 		analysisConfig.auto_function_check == Check.skipTests && !ut);
 
+	if (analysisConfig.imports_sortedness != Check.disabled)
+		checks ~= new ImportSortednessCheck(fileName,
+		analysisConfig.imports_sortedness == Check.skipTests && !ut);
+
 	version (none)
 		if (analysisConfig.redundant_if_check != Check.disabled)
 			checks ~= new IfStatementCheck(fileName, moduleScope,