Merge pull request #421 from WebFreak001/aa

Associative array formatting & space_before_aa_colon option

README.md

@@ -49,6 +49,7 @@ found in .editorconfig files.
 * **--selective_import_space**: See **dfmt_selective_import_space** below
 * **--compact_labeled_statements**: See **dfmt_compact_labeled_statements** below
 * **--template_constraint_style**: See **dfmt_template_constraint_style** below
+* **--space_before_aa_colon**: See **dfmt_space_before_aa_colon** below
 
 ### Example
 ```
@@ -107,6 +108,7 @@ dfmt_selective_import_space | `true`, `false` | `true` | Insert space after the
 dfmt_compact_labeled_statements | `true`, `false` | `true` | Place labels on the same line as the labeled `switch`, `for`, `foreach`, or `while` statement.
 dfmt_template_constraint_style | `conditional_newline_indent` `conditional_newline` `always_newline` `always_newline_indent` | `conditional_newline_indent` | Control the formatting of template constraints.
 dfmt_single_template_constraint_indent | `true`, `false` | `false` | Set if the constraints are indented by a single tab instead of two. Has only an effect for if indentation style if set to `always_newline_indent` or `conditional_newline_indent`.
+dfmt_space_before_aa_colon | `true`, `false` | `false` | Adds a space after an associative array key before the `:` like in older dfmt versions.
 
 ## Terminology
 * Braces - `{` and `}`

src/dfmt/ast_info.d

@@ -45,6 +45,7 @@ struct ASTInformation
         sort(conditionalWithElseLocations);
         sort(conditionalStatementLocations);
         sort(arrayStartLocations);
+        sort(assocArrayStartLocations);
         sort(contractLocations);
         sort(constraintLocations);
         sort(constructorDestructorLocations);
@@ -95,6 +96,9 @@ struct ASTInformation
     /// Locations of start locations of array initializers
     size_t[] arrayStartLocations;
 
+    /// Locations of start locations of associative array initializers
+    size_t[] assocArrayStartLocations;
+
     /// Locations of "in" and "out" tokens that begin contracts
     size_t[] contractLocations;
 
@@ -136,6 +140,19 @@ final class FormatVisitor : ASTVisitor
         arrayInitializer.accept(this);
     }
 
+    override void visit(const ArrayLiteral arrayLiteral)
+    {
+        astInformation.arrayStartLocations ~= arrayLiteral.tokens[0].index;
+        arrayLiteral.accept(this);
+    }
+
+    override void visit(const AssocArrayLiteral assocArrayLiteral)
+    {
+        astInformation.arrayStartLocations ~= assocArrayLiteral.tokens[0].index;
+        astInformation.assocArrayStartLocations ~= assocArrayLiteral.tokens[0].index;
+        assocArrayLiteral.accept(this);
+    }
+
     override void visit (const SharedStaticConstructor sharedStaticConstructor)
     {
         astInformation.sharedStaticConstructorDestructorLocations ~= sharedStaticConstructor.location;

src/dfmt/config.d

@@ -55,6 +55,8 @@ struct Config
     TemplateConstraintStyle dfmt_template_constraint_style;
     ///
     OptionalBoolean dfmt_single_template_constraint_indent;
+    ///
+    OptionalBoolean dfmt_space_before_aa_colon;
 
     mixin StandardEditorConfigFields;
 
@@ -82,6 +84,7 @@ struct Config
         dfmt_compact_labeled_statements = OptionalBoolean.t;
         dfmt_template_constraint_style = TemplateConstraintStyle.conditional_newline_indent;
         dfmt_single_template_constraint_indent = OptionalBoolean.f;
+        dfmt_space_before_aa_colon = OptionalBoolean.f;
     }
 
     /**

src/dfmt/formatter.d

@@ -569,19 +569,38 @@ private:
         // No heuristics apply if we can't look before the opening paren/bracket
         if (index < 1)
             return;
-        immutable bool arrayInitializerStart = p == tok!"[" && linebreakHints.length != 0
+        immutable bool arrayInitializerStart = p == tok!"["
             && astInformation.arrayStartLocations.canFindIndex(tokens[index - 1].index);
-        if (arrayInitializerStart)
+        if (arrayInitializerStart && isMultilineAt(index - 1))
         {
             // Use the close bracket as the indent token to distinguish
             // the array initialiazer from an array index in the newline
             // handling code
-            pushWrapIndent(tok!"]");
+            IndentStack.Details detail;
+            detail.wrap = false;
+            detail.temp = true;
+            // wrap and temp are set manually to the values it would actually
+            // receive here because we want to set isAA for the ] token to know if
+            // we should definitely always new-line after every comma for a big AA
+            detail.isAA = astInformation.assocArrayStartLocations.canFindIndex(tokens[index - 1].index);
+            pushWrapIndent(tok!"]", detail);
+
             newline();
             immutable size_t j = expressionEndIndex(index);
             linebreakHints = chooseLineBreakTokens(index, tokens[index .. j],
                     depths[index .. j], config, currentLineLength, indentLevel);
         }
+        else if (arrayInitializerStart)
+        {
+            // This is a short (non-breaking) AA value
+            IndentStack.Details detail;
+            detail.wrap = false;
+            detail.temp = true;
+            detail.isAA = true;
+            detail.mini = true;
+
+            pushWrapIndent(tok!"]", detail);
+        }
         else if (!currentIs(tok!")") && !currentIs(tok!"]")
                 && (linebreakHints.canFindIndex(index - 1) || (linebreakHints.length == 0
                     && currentLineLength > config.max_line_length)))
@@ -625,6 +644,26 @@ private:
             writeToken();
     }
 
+    void formatRightBracket()
+    in
+    {
+        assert(currentIs(tok!"]"));
+    }
+    body
+    {
+        indents.popWrapIndents();
+        if (indents.topIs(tok!"]"))
+        {
+            if (!indents.topDetails.mini)
+                newline();
+            else
+                indents.pop();
+        }
+        writeToken();
+        if (currentIs(tok!"identifier"))
+            write(" ");
+    }
+
     void formatAt()
     {
         immutable size_t atIndex = tokens[index].index;
@@ -700,6 +739,11 @@ private:
             }
             else
             {
+                const inAA = indents.topIs(tok!"]") && indents.topDetails.isAA;
+
+                if (inAA && !config.dfmt_space_before_aa_colon)
+                    write(": ");
+                else
                     write(" : ");
                 index++;
             }
@@ -790,12 +834,7 @@ private:
             sBraceDepth++;
             if (peekBackIsOneOf(true, tok!")", tok!"identifier"))
                 write(" ");
-            auto e = expressionEndIndex(index);
+            immutable bool multiline = isMultilineAt(index);
-            immutable int l = currentLineLength + tokens[index .. e].map!(a => tokenLength(a))
-                .sum();
-            immutable bool multiline = l > config.dfmt_soft_max_line_length
-                || tokens[index .. e].canFind!(a => a.type == tok!"comment"
-                    || isBlockHeaderToken(a.type))();
             writeToken();
             if (multiline)
             {
@@ -1239,12 +1278,7 @@ private:
             formatColon();
             break;
         case tok!"]":
-            indents.popWrapIndents();
+            formatRightBracket();
-            if (indents.topIs(tok!"]"))
-                newline();
-            writeToken();
-            if (currentIs(tok!"identifier"))
-                write(" ");
             break;
         case tok!";":
             formatSemicolon();
@@ -1368,6 +1402,11 @@ private:
             writeToken();
             newline();
         }
+        else if (indents.topIs(tok!"]") && indents.topDetails.isAA && !indents.topDetails.mini)
+        {
+            writeToken();
+            newline();
+        }
         else if (!peekIs(tok!"}") && (linebreakHints.canFind(index)
                 || (linebreakHints.length == 0 && currentLineLength > config.max_line_length)))
         {
@@ -1671,13 +1710,21 @@ private:
     void pushWrapIndent(IdType type = tok!"")
     {
         immutable t = type == tok!"" ? tokens[index].type : type;
+        IndentStack.Details detail;
+        detail.wrap = isWrapIndent(t);
+        detail.temp = isTempIndent(t);
+        pushWrapIndent(t, detail);
+    }
+
+    void pushWrapIndent(IdType type, IndentStack.Details detail)
+    {
         if (parenDepth == 0)
         {
             if (indents.wrapIndents == 0)
-                indents.push(t);
+                indents.push(type, detail);
         }
         else if (indents.wrapIndents < 1)
-            indents.push(t);
+            indents.push(type, detail);
     }
 
 const pure @safe @nogc:
@@ -1685,6 +1732,7 @@ const pure @safe @nogc:
     size_t expressionEndIndex(size_t i) nothrow
     {
         immutable bool braces = i < tokens.length && tokens[i].type == tok!"{";
+        immutable bool brackets = i < tokens.length && tokens[i].type == tok!"[";
         immutable d = depths[i];
         while (true)
         {
@@ -1692,13 +1740,25 @@ const pure @safe @nogc:
                 break;
             if (depths[i] < d)
                 break;
-            if (!braces && (tokens[i].type == tok!";" || tokens[i].type == tok!"{"))
+            if (!braces && !brackets && (tokens[i].type == tok!";" || tokens[i].type == tok!"{"))
                 break;
             i++;
         }
         return i;
     }
 
+    /// Returns: true when the expression starting at index goes over the line length limit.
+    /// Uses matching `{}` or `[]` or otherwise takes everything up until a semicolon or opening brace using expressionEndIndex.
+    bool isMultilineAt(size_t i)
+    {
+        import std.algorithm : map, sum, canFind;
+
+        auto e = expressionEndIndex(i);
+        immutable int l = currentLineLength + tokens[i .. e].map!(a => tokenLength(a)).sum();
+        return l > config.dfmt_soft_max_line_length
+            || tokens[i .. e].canFind!(a => a.type == tok!"comment" || isBlockHeaderToken(a.type))();
+    }
+
     bool peekIsKeyword() nothrow
     {
         return index + 1 < tokens.length && isKeyword(tokens[index + 1].type);

src/dfmt/indentation.d

@@ -7,6 +7,8 @@ module dfmt.indentation;
 
 import dparse.lexer;
 
+import std.bitmanip : bitfields;
+
 /**
  * Returns: true if the given token type is a wrap indent type
  */
@@ -29,6 +31,20 @@ bool isTempIndent(IdType type) pure nothrow @nogc @safe
  */
 struct IndentStack
 {
+    static struct Details
+    {
+        mixin(bitfields!(
+            // generally true for all operators except {, case, @, ], (, )
+            bool, "wrap", 1,
+            // temporary indentation which get's reverted when a block starts
+            // generally true for all tokens except ), {, case, @
+            bool, "temp", 1,
+            // emit minimal newlines
+            bool, "mini", 1,
+            bool, "isAA", 1,
+            uint, "",     28));
+    }
+
     /**
      * Get the indent size at the most recent occurrence of the given indent type
      */
@@ -55,7 +71,7 @@ struct IndentStack
         int tempIndentCount = 0;
         for (size_t i = index; i > 0; i--)
         {
-            if (!isWrapIndent(arr[i - 1]) && arr[i - 1] != tok!"]")
+            if (!details[i - 1].wrap && arr[i - 1] != tok!"]")
                 break;
             tempIndentCount++;
         }
@@ -66,8 +82,20 @@ struct IndentStack
      * Pushes the given indent type on to the stack.
      */
     void push(IdType item) pure nothrow
+    {
+        Details detail;
+        detail.wrap = isWrapIndent(item);
+        detail.temp = isTempIndent(item);
+        push(item, detail);
+    }
+
+    /**
+     * Pushes the given indent type on to the stack.
+     */
+    void push(IdType item, Details detail) pure nothrow
     {
         arr[index] = item;
+        details[index] = detail;
         //FIXME this is actually a bad thing to do,
         //we should not just override when the stack is
         //at it's limit
@@ -91,7 +119,7 @@ struct IndentStack
      */
     void popWrapIndents() pure nothrow @safe @nogc
     {
-        while (index > 0 && isWrapIndent(arr[index - 1]))
+        while (index > 0 && details[index - 1].wrap)
             index--;
     }
 
@@ -100,7 +128,7 @@ struct IndentStack
      */
     void popTempIndents() pure nothrow @safe @nogc
     {
-        while (index > 0 && isTempIndent(arr[index - 1]))
+        while (index > 0 && details[index - 1].temp)
             index--;
     }
 
@@ -125,7 +153,15 @@ struct IndentStack
      */
     bool topIsTemp()
     {
-        return index > 0 && index <= arr.length && isTempIndent(arr[index - 1]);
+        return index > 0 && index <= arr.length && details[index - 1].temp;
+    }
+
+    /**
+     * Returns: `true` if the top of the indent stack is a temporary indent with the specified token
+     */
+    bool topIsTemp(IdType item)
+    {
+        return index > 0 && index <= arr.length && arr[index - 1] == item && details[index - 1].temp;
     }
 
     /**
@@ -133,7 +169,15 @@ struct IndentStack
      */
     bool topIsWrap()
     {
-        return index > 0 && index <= arr.length && isWrapIndent(arr[index - 1]);
+        return index > 0 && index <= arr.length && details[index - 1].wrap;
+    }
+
+    /**
+     * Returns: `true` if the top of the indent stack is a temporary indent with the specified token
+     */
+    bool topIsWrap(IdType item)
+    {
+        return index > 0 && index <= arr.length && arr[index - 1] == item && details[index - 1].wrap;
     }
 
     /**
@@ -156,6 +200,11 @@ struct IndentStack
         return arr[index - 1];
     }
 
+    Details topDetails() const pure nothrow @property @safe @nogc
+    {
+        return details[index - 1];
+    }
+
     int indentLevel() const pure nothrow @property @safe @nogc
     {
         return indentSize();
@@ -183,6 +232,7 @@ private:
     size_t index;
 
     IdType[256] arr;
+    Details[arr.length] details;
 
     int indentSize(const size_t k = size_t.max) const pure nothrow @safe @nogc
     {
@@ -196,17 +246,17 @@ private:
         {
             immutable int pc = (arr[i] == tok!"!" || arr[i] == tok!"(" || arr[i] == tok!")") ? parenCount + 1
                 : parenCount;
-            if ((isWrapIndent(arr[i]) || arr[i] == tok!"(") && parenCount > 1)
+            if ((details[i].wrap || arr[i] == tok!"(") && parenCount > 1)
             {
                 parenCount = pc;
                 continue;
             }
             if (i + 1 < index)
             {
-                if (arr[i] == tok!"]")
+                if (arr[i] == tok!"]" && details[i].temp)
                     continue;
-                immutable currentIsNonWrapTemp = !isWrapIndent(arr[i])
+                immutable currentIsNonWrapTemp = !details[i].wrap
-                    && isTempIndent(arr[i]) && arr[i] != tok!")" && arr[i] != tok!"!";
+                    && details[i].temp && arr[i] != tok!")" && arr[i] != tok!"!";
                 if (arr[i] == tok!"static"
                     && arr[i + 1].among!(tok!"if", tok!"else", tok!"foreach", tok!"foreach_reverse")
                     && (i + 2 >= index || arr[i + 2] != tok!"{"))

src/dfmt/main.d

@@ -89,6 +89,9 @@ else
             case "single_template_constraint_indent":
                 optConfig.dfmt_single_template_constraint_indent = optVal;
                 break;
+            case "space_before_aa_colon":
+                optConfig.dfmt_space_before_aa_colon = optVal;
+                break;
             default:
                 assert(false, "Invalid command-line switch");
             }
@@ -116,6 +119,7 @@ else
                 "split_operator_at_line_end", &handleBooleans,
                 "compact_labeled_statements", &handleBooleans,
                 "single_template_constraint_indent", &handleBooleans,
+                "space_before_aa_colon", &handleBooleans,
                 "tab_width", &optConfig.tab_width,
                 "template_constraint_style", &optConfig.dfmt_template_constraint_style);
             // dfmt on
@@ -314,6 +318,7 @@ Formatting Options:
     --split_operator_at_line_end
     --compact_labeled_statements
     --template_constraint_style
+    --space_before_aa_colon
         `,
             optionsToString!(typeof(Config.dfmt_template_constraint_style)));
 }

src/dfmt/tokens.d

@@ -134,7 +134,6 @@ int breakCost(IdType p, IdType c) pure nothrow @safe @nogc
     case tok!"||":
     case tok!"&&":
     case tok!",":
-    case tok!":":
     case tok!"?":
         return 0;
     case tok!"(":
@@ -184,6 +183,10 @@ int breakCost(IdType p, IdType c) pure nothrow @safe @nogc
     case tok!"~":
     case tok!"+=":
         return 200;
+    case tok!":":
+        // colon could be after a label or an import, where it should normally wrap like before
+        // for everything else (associative arrays) try not breaking around colons
+        return p == tok!"identifier" ? 0 : 300;
     case tok!".":
         return p == tok!")" ? 0 : 300;
     default:

tests/allman/associative_array.d.ref

@@ -0,0 +1,26 @@
+unittest
+{
+    Bson base = Bson([
+            "maps": Bson([
+                Bson(["id": Bson(4), "comment": Bson("hello")]),
+                Bson(["id": Bson(49), "comment": Bson(null)])
+            ]),
+            "short": Bson(["a": "b", "c": "d"]),
+            "numbers": Bson([
+                1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0,
+                1, 2, 3, 4, 5, 6, 7, 8, 9, 0
+            ]),
+            "shuffleOnReset": serializeToBson([
+                "all": false,
+                "selected": true,
+                "maybe": false
+            ]),
+            "resetOnEmpty": Bson(false),
+            "applyMods": Bson(true),
+            "sendComments": Bson(true)
+            ]);
+    int[] x = [
+        1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3,
+        4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0
+    ];
+}

tests/allman/issue0017.d.ref

@@ -1,3 +1,4 @@
-immutable NameId[] namesA = [{"Aacgr", 0x00386}, // GREEK CAPITAL LETTER ALPHA WITH TONOS
+immutable NameId[] namesA = [
+{"Aacgr", 0x00386}, // GREEK CAPITAL LETTER ALPHA WITH TONOS
 {"aacgr", 0x003AC}, // GREEK SMALL LETTER ALPHA WITH TONOS
 ];

tests/associative_array.d

@@ -0,0 +1,26 @@
+unittest
+{
+	Bson base = Bson([
+		"maps": Bson([
+			Bson([
+				"id": Bson(4),
+				"comment": Bson("hello")
+			]),
+			Bson([
+				"id": Bson(49),
+				"comment": Bson(null)
+			])
+		]),
+		"short": Bson(["a": "b", "c": "d"]),
+		"numbers": Bson([1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0]),
+		"shuffleOnReset": serializeToBson([
+			"all": false,
+			"selected": true,
+			"maybe": false
+		]),
+		"resetOnEmpty": Bson(false),
+		"applyMods": Bson(true),
+		"sendComments": Bson(true)
+	]);
+	int[] x = [1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0];
+}

tests/issue0128.args

@@ -0,0 +1 @@
+--space_before_aa_colon=true

tests/otbs/associative_array.d.ref

@@ -0,0 +1,25 @@
+unittest {
+    Bson base = Bson([
+            "maps": Bson([
+                Bson(["id": Bson(4), "comment": Bson("hello")]),
+                Bson(["id": Bson(49), "comment": Bson(null)])
+            ]),
+            "short": Bson(["a": "b", "c": "d"]),
+            "numbers": Bson([
+                1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0,
+                1, 2, 3, 4, 5, 6, 7, 8, 9, 0
+            ]),
+            "shuffleOnReset": serializeToBson([
+                "all": false,
+                "selected": true,
+                "maybe": false
+            ]),
+            "resetOnEmpty": Bson(false),
+            "applyMods": Bson(true),
+            "sendComments": Bson(true)
+            ]);
+    int[] x = [
+        1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3,
+        4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0
+    ];
+}

tests/otbs/issue0017.d.ref

@@ -1,3 +1,4 @@
-immutable NameId[] namesA = [{"Aacgr", 0x00386}, // GREEK CAPITAL LETTER ALPHA WITH TONOS
+immutable NameId[] namesA = [
+{"Aacgr", 0x00386}, // GREEK CAPITAL LETTER ALPHA WITH TONOS
 {"aacgr", 0x003AC}, // GREEK SMALL LETTER ALPHA WITH TONOS
 ];