Implement import-sorting

makefile

@@ -3,7 +3,7 @@ SRC := $(shell find src -name "*.d") \
 INCLUDE_PATHS := -Ilibdparse/src -Isrc
 DMD_COMMON_FLAGS := -dip25 -w $(INCLUDE_PATHS) -Jviews
 DMD_DEBUG_FLAGS := -debug -g $(DMD_COMMON_FLAGS)
-DMD_FLAGS := -O -inline $(DMD_COMMON_FLAGS)
+DMD_FLAGS :=  $(DMD_COMMON_FLAGS) -g
 DMD_TEST_FLAGS := -unittest -g $(DMD_COMMON_FLAGS)
 LDC_FLAGS := -g -w -oq $(INCLUDE_PATHS)
 GDC_FLAGS := -g -w -oq $(INCLUDE_PATHS)

src/dfmt/ast_info.d

@@ -8,12 +8,32 @@ module dfmt.ast_info;
 import dparse.lexer;
 import dparse.ast;
 
+struct Import
+{
+    string[] importStrings;
+    string renamedAs;
+}
+
+extern (C) static bool importStringLess(const Import a, const Import b)
+{
+    return a.importStrings < b.importStrings;
+}
+
+
 /// AST information that is needed by the formatter.
 struct ASTInformation
 {
+    struct LocationRange
+    {
+        size_t startLocation;
+        size_t endLocation;
+    }
+
     /// Sorts the arrays so that binary search will work on them
     void cleanup()
     {
+        finished = true;
+
         import std.algorithm : sort;
 
         sort(doubleNewlineLocations);
@@ -38,6 +58,66 @@ struct ASTInformation
     /// Locations of tokens where a space is needed (such as the '*' in a type)
     size_t[] spaceAfterLocations;
 
+    /// Location-Ranges where scopes begin and end
+    LocationRange[] scopeLocationRanges;
+
+    /// zero is module scope
+    size_t scopeOrdinalOfLocation(const size_t location) const
+    {
+        size_t bestOrdinal = 0;
+
+        LocationRange bestRange = scopeLocationRanges[bestOrdinal];
+
+        foreach (i; 1 .. scopeLocationRanges.length)
+        {
+            LocationRange nextRange = scopeLocationRanges[i];
+
+            if (nextRange.startLocation > location)
+                break;
+
+            if (nextRange.endLocation > location)
+            {
+                bestRange = nextRange;
+                bestOrdinal = i;
+            }
+
+        }
+
+        return bestOrdinal;
+    }
+
+
+    /// returns an array of indecies into the token array
+    /// which are the indecies of the imports to be written
+    /// in sorted order
+
+    string[] importsFor(const size_t scopeOrdinal) const
+    {
+        import std.algorithm;
+        import std.range;
+
+        uint idx = 0;
+        string[] result;
+
+        auto imports = importScopes[scopeOrdinal];
+
+        if (imports.length)
+        {
+            result.length = imports.length;
+
+            foreach(imp;(cast(Import[])imports).sort!(importStringLess))
+            {
+                result[idx++] = imp.importStrings.join(".");
+            }
+        }
+        else
+        {
+            result = null;
+        }
+
+        return result;
+    }
+
     /// Locations of unary operators
     size_t[] unaryLocations;
 
@@ -73,6 +153,13 @@ struct ASTInformation
 
     /// Locations of template constraint "if" tokens
     size_t[] constraintLocations;
+
+    /// cleanup run;
+    bool finished;
+
+    /// contains all imports inside scope
+    Import[][] importScopes;
+
 }
 
 /// Collects information from the AST that is useful for the formatter
@@ -85,6 +172,18 @@ final class FormatVisitor : ASTVisitor
     this(ASTInformation* astInformation)
     {
         this.astInformation = astInformation;
+        if (this.astInformation.scopeLocationRanges.length != 0)
+            assert(0, "astinformation seems to be dirty");
+
+        this.astInformation.scopeLocationRanges ~= ASTInformation.LocationRange(0, size_t.max);
+        this.astInformation.importScopes.length = 1;
+    }
+
+    void addScope(const size_t startLocation, const size_t endLocation)
+    {
+        astInformation.scopeLocationRanges ~= ASTInformation.LocationRange(startLocation,
+                endLocation);
+        astInformation.importScopes.length += 1;
     }
 
     override void visit(const ArrayInitializer arrayInitializer)
@@ -93,6 +192,42 @@ final class FormatVisitor : ASTVisitor
         arrayInitializer.accept(this);
     }
 
+    void addImport(size_t scopeId, string[] importString, string renamedAs = null)
+    {
+        astInformation.importScopes[scopeId] ~= Import(importString, renamedAs);
+    }
+
+    override void visit(const SingleImport singleImport)
+    {
+        auto scopeOrdinal = size_t.max;
+
+        if (singleImport.identifierChain)
+        {
+            string[] importString;
+            string renamedAs = null;
+
+            auto ic = singleImport.identifierChain;
+            foreach (ident; ic.identifiers)
+            {
+                importString ~= ident.text;
+            }
+
+            scopeOrdinal = astInformation.scopeOrdinalOfLocation(ic.identifiers[0].index);
+
+            if (singleImport.rename.text && singleImport.rename.text.length)
+                renamedAs = singleImport.rename.text;
+
+            addImport(scopeOrdinal, importString, renamedAs);
+
+        }
+        else
+        {
+            assert (0, "singleImport without identifierChain");
+        }
+
+
+    }
+
     override void visit(const ConditionalDeclaration dec)
     {
         if (dec.hasElse)
@@ -137,7 +272,8 @@ final class FormatVisitor : ASTVisitor
     {
         if (funcLit.functionBody !is null)
         {
-            astInformation.funLitStartLocations ~= funcLit.functionBody.blockStatement.startLocation;
+            astInformation.funLitStartLocations ~= funcLit.functionBody
+                .blockStatement.startLocation;
             astInformation.funLitEndLocations ~= funcLit.functionBody.blockStatement.endLocation;
         }
         funcLit.accept(this);
@@ -164,11 +300,22 @@ final class FormatVisitor : ASTVisitor
     override void visit(const FunctionBody functionBody)
     {
         if (functionBody.blockStatement !is null)
-            astInformation.doubleNewlineLocations ~= functionBody.blockStatement.endLocation;
+        {
+            auto bs = functionBody.blockStatement;
+
+            addScope(bs.startLocation, bs.endLocation);
+            astInformation.doubleNewlineLocations ~= bs.endLocation;
+        }
+
         if (functionBody.bodyStatement !is null && functionBody.bodyStatement
                 .blockStatement !is null)
-            astInformation.doubleNewlineLocations
+        {
-                ~= functionBody.bodyStatement.blockStatement.endLocation;
+            auto bs = functionBody.bodyStatement.blockStatement;
+
+            addScope(bs.startLocation, bs.endLocation);
+            astInformation.doubleNewlineLocations ~= bs.endLocation;
+        }
+
         functionBody.accept(this);
     }
 
@@ -199,6 +346,7 @@ final class FormatVisitor : ASTVisitor
 
     override void visit(const StructBody structBody)
     {
+        addScope(structBody.startLocation, structBody.endLocation);
         astInformation.doubleNewlineLocations ~= structBody.endLocation;
         structBody.accept(this);
     }

src/dfmt/config.d

@@ -50,6 +50,8 @@ struct Config
     ///
     OptionalBoolean dfmt_selective_import_space;
     ///
+    OptionalBoolean dfmt_sort_imports;
+    ///
     OptionalBoolean dfmt_compact_labeled_statements;
     ///
     TemplateConstraintStyle dfmt_template_constraint_style;
@@ -77,6 +79,7 @@ struct Config
         dfmt_space_before_function_parameters = OptionalBoolean.f;
         dfmt_split_operator_at_line_end = OptionalBoolean.f;
         dfmt_selective_import_space = OptionalBoolean.t;
+        dfmt_sort_imports = OptionalBoolean.f;
         dfmt_compact_labeled_statements = OptionalBoolean.t;
         dfmt_template_constraint_style = TemplateConstraintStyle.conditional_newline_indent;
     }

src/dfmt/formatter.d

@@ -4,7 +4,6 @@
 //          http://www.boost.org/LICENSE_1_0.txt)
 
 module dfmt.formatter;
-
 import dparse.lexer;
 import dparse.parser;
 import dparse.rollback_allocator;
@@ -170,6 +169,9 @@ private:
     /// True if we're in an ASM block
     bool inAsm;
 
+    /// formarts a variable number of tokes per call
+    /// called until no tokens remain
+    /// maybe called recursively
     void formatStep()
     {
         import std.range : assumeSorted;
@@ -191,7 +193,7 @@ private:
                     write(" ");
             }
         }
-        else if (currentIs(tok!"module") || currentIs(tok!"import"))
+        else if (currentIs(tok!"import") || currentIs(tok!"module"))
         {
             formatModuleOrImport();
         }
@@ -281,11 +283,12 @@ private:
         {
             writeToken();
             if (index < tokens.length && (currentIs(tok!"identifier")
-                    || ( index < 1 && ( isBasicType(peekBack(2).type) || peekBack2Is(tok!"identifier") ) &&
+                    || (index < 1 && (isBasicType(peekBack(2).type)
-                         currentIs(tok!("(")) && config.dfmt_space_before_function_parameters )
+                    || peekBack2Is(tok!"identifier")) && currentIs(tok!("("))
-                    || isBasicType(current.type) || currentIs(tok!"@") || currentIs(tok!"if")
+                    && config.dfmt_space_before_function_parameters)
-                    || isNumberLiteral(tokens[index].type) || (inAsm
+                    || isBasicType(current.type) || currentIs(tok!"@")
-                    && peekBack2Is(tok!";") && currentIs(tok!"["))))
+                    || currentIs(tok!"if") || isNumberLiteral(tokens[index].type)
+                    || (inAsm && peekBack2Is(tok!";") && currentIs(tok!"["))))
             {
                 write(" ");
             }
@@ -433,7 +436,9 @@ private:
 
     void formatModuleOrImport()
     {
-        immutable t = current.type;
+        immutable isImport = (current.type == tok!"import");
+        if (!config.dfmt_sort_imports || peekIs(tok!("(")) || !isImport)
+        {
             writeToken();
             if (currentIs(tok!"("))
             {
@@ -441,6 +446,7 @@ private:
                 return;
             }
             write(" ");
+
             while (index < tokens.length)
             {
                 if (currentIs(tok!";"))
@@ -457,7 +463,9 @@ private:
                     }
                     else if (currentIs(tok!"{") && config.dfmt_brace_style == BraceStyle.allman)
                         break;
-                else if (t == tok!"import" && !currentIs(tok!"import")
+                    else if (currentIs(tok!"{") && config.dfmt_brace_style == BraceStyle.allman)
+                        break;
+                    else if (isImport && !currentIs(tok!"import")
                         && !currentIs(tok!"}")
                         && !((currentIs(tok!"public")
                             || currentIs(tok!"private")
@@ -495,7 +503,8 @@ private:
                     }
                     assert(lengthOfNextChunk > 0);
                     writeToken();
-                if (currentLineLength + 1 + lengthOfNextChunk >= config.dfmt_soft_max_line_length)
+                    if (currentLineLength + 1 + lengthOfNextChunk >= config
+                            .dfmt_soft_max_line_length)
                     {
                         pushWrapIndent(tok!",");
                         newline();
@@ -507,6 +516,18 @@ private:
                       formatStep();
               }
         }
+        else
+        {
+            while(currentIs(tok!"import"))
+            {
+                // skip to the ending ; of the import statement
+                while(!currentIs(tok!";")) { index++; }
+                // skip past the ;
+                index++;
+            }
+            newline();
+        }
+    }
 
     void formatLeftParenOrBracket()
     in
@@ -749,6 +770,19 @@ private:
         }
         else
         {
+            bool writeImports = false;
+            size_t scopeOrdinal = void;
+            if (config.dfmt_sort_imports)
+            {
+                scopeOrdinal = astInformation.scopeOrdinalOfLocation(current.index);
+                if (scopeOrdinal)
+                {
+                    auto range = astInformation.scopeLocationRanges[scopeOrdinal];
+                    if (range.startLocation == current.index)
+                        writeImports = true;
+                }
+            }
+
             if (peekBackIsSlashSlash())
             {
                 if (peekBack2Is(tok!";"))
@@ -773,9 +807,23 @@ private:
             }
 
             indents.push(tok!"{");
+
             if (!currentIs(tok!"{"))
                 newline();
             linebreakHints = [];
+
+            if (writeImports && astInformation.importScopes[scopeOrdinal].length)
+            {
+                foreach(importString;astInformation.importsFor(scopeOrdinal))
+                {
+                    newline();
+                    write("import ");
+                    write(importString);
+                    write(";");
+                }
+                newline();
+
+            }
         }
     }
 
@@ -1037,8 +1085,7 @@ private:
 
     bool currentIsIndentedTemplateConstraint()
     {
-        return index < tokens.length
+        return index < tokens.length && astInformation.constraintLocations.canFindIndex(current.index)
-            && astInformation.constraintLocations.canFindIndex(current.index)
             && (config.dfmt_template_constraint_style == TemplateConstraintStyle.always_newline
                     || currentLineLength >= config.dfmt_soft_max_line_length);
     }
@@ -1424,8 +1471,8 @@ private:
 
     void writeToken()
     {
-        import std.range:retro;
+        import std.range : retro;
-        import std.algorithm.searching:countUntil;
+        import std.algorithm.searching : countUntil;
 
         if (current.text is null)
         {

src/dfmt/main.d

@@ -87,7 +87,7 @@ else
             import dfmt.editorconfig : OptionalBoolean;
             import std.exception : enforceEx;
 
-            enforceEx!GetOptException(value == "true" || value == "false", "Invalid argument");
+            enforceEx!GetOptException(value == "true" || value == "false", "Invalid argument '" ~ value ~ "' for " ~ option ~ " should be 'true' or 'false'");
             immutable OptionalBoolean optVal = value == "true" ? OptionalBoolean.t
                 : OptionalBoolean.f;
             switch (option)
@@ -110,6 +110,9 @@ else
             case "selective_import_space":
                 optConfig.dfmt_selective_import_space = optVal;
                 break;
+            case "sort_imports":
+                optConfig.dfmt_sort_imports = optVal;
+                break;
             case "compact_labeled_statements":
                 optConfig.dfmt_compact_labeled_statements = optVal;
                 break;
@@ -136,6 +139,7 @@ else
                 "outdent_attributes", &handleBooleans,
                 "space_after_cast", &handleBooleans,
                 "selective_import_space", &handleBooleans,
+                "sort_imports", &handleBooleans,
                 "space_before_function_parameters", &handleBooleans,
                 "split_operator_at_line_end", &handleBooleans,
                 "compact_labeled_statements", &handleBooleans,