From be555a74b8b7a32d2c85543ca04040ef3db33f26 Mon Sep 17 00:00:00 2001 From: Sebastian Wilzbach Date: Sat, 10 Dec 2016 18:15:11 +0100 Subject: [PATCH] Add ImportSortedness checker --- src/analysis/config.d | 3 + src/analysis/imports_sortedness.d | 191 ++++++++++++++++++++++++++++++ src/analysis/run.d | 5 + 3 files changed, 199 insertions(+) create mode 100644 src/analysis/imports_sortedness.d diff --git a/src/analysis/config.d b/src/analysis/config.d index 3df4124..8fd1eee 100644 --- a/src/analysis/config.d +++ b/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; } diff --git a/src/analysis/imports_sortedness.d b/src/analysis/imports_sortedness.d new file mode 100644 index 0000000..a663fdc --- /dev/null +++ b/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."); +} diff --git a/src/analysis/run.d b/src/analysis/run.d index 8bf9267..9c7a6d9 100644 --- a/src/analysis/run.d +++ b/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,