From 53d0001e88a7ec9024806f49a4f7134bf65dfc3d Mon Sep 17 00:00:00 2001 From: Stefan Koch Date: Fri, 3 Nov 2017 10:27:17 +0100 Subject: [PATCH 1/9] Implement import-sorting --- makefile | 2 +- src/dfmt/ast_info.d | 156 ++++++++++++++++++++++++++++++++- src/dfmt/config.d | 3 + src/dfmt/formatter.d | 199 ++++++++++++++++++++++++++----------------- src/dfmt/main.d | 20 +++-- 5 files changed, 291 insertions(+), 89 deletions(-) diff --git a/makefile b/makefile index e2a8335..b4710dd 100644 --- a/makefile +++ b/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) diff --git a/src/dfmt/ast_info.d b/src/dfmt/ast_info.d index 8716893..18d1f97 100644 --- a/src/dfmt/ast_info.d +++ b/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); } diff --git a/src/dfmt/config.d b/src/dfmt/config.d index 9dcc8fe..8870a0d 100644 --- a/src/dfmt/config.d +++ b/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; } diff --git a/src/dfmt/formatter.d b/src/dfmt/formatter.d index 932955a..fbdd119 100644 --- a/src/dfmt/formatter.d +++ b/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") ) && - currentIs(tok!("(")) && config.dfmt_space_before_function_parameters ) - || isBasicType(current.type) || currentIs(tok!"@") || currentIs(tok!"if") - || isNumberLiteral(tokens[index].type) || (inAsm - && peekBack2Is(tok!";") && currentIs(tok!"[")))) + || (index < 1 && (isBasicType(peekBack(2).type) + || peekBack2Is(tok!"identifier")) && currentIs(tok!("(")) + && config.dfmt_space_before_function_parameters) + || isBasicType(current.type) || currentIs(tok!"@") + || currentIs(tok!"if") || isNumberLiteral(tokens[index].type) + || (inAsm && peekBack2Is(tok!";") && currentIs(tok!"[")))) { write(" "); } @@ -433,78 +436,96 @@ private: void formatModuleOrImport() { - immutable t = current.type; - writeToken(); - if (currentIs(tok!"(")) + immutable isImport = (current.type == tok!"import"); + if (!config.dfmt_sort_imports || peekIs(tok!("(")) || !isImport) { - writeParens(false); - return; - } - write(" "); - while (index < tokens.length) - { - if (currentIs(tok!";")) + writeToken(); + if (currentIs(tok!"(")) { - writeToken(); - if (index >= tokens.length) - { - newline(); - break; - } - if (currentIs(tok!"comment") && current.line == peekBack().line) - { - break; - } - else if (currentIs(tok!"{") && config.dfmt_brace_style == BraceStyle.allman) - break; - else if (t == tok!"import" && !currentIs(tok!"import") - && !currentIs(tok!"}") - && !((currentIs(tok!"public") - || currentIs(tok!"private") - || currentIs(tok!"static")) - && peekIs(tok!"import")) && !indents.topIsOneOf(tok!"if", - tok!"debug", tok!"version")) - { - simpleNewline(); - currentLineLength = 0; - justAddedExtraNewline = true; - newline(); - } - else - newline(); - break; + writeParens(false); + return; } - else if (currentIs(tok!":")) + write(" "); + + while (index < tokens.length) { - if (config.dfmt_selective_import_space) - write(" "); - writeToken(); - write(" "); - } - else if (currentIs(tok!",")) - { - // compute length until next ',' or ';' - int lengthOfNextChunk; - for (size_t i = index + 1; i < tokens.length; i++) + if (currentIs(tok!";")) { - if (tokens[i].type == tok!"," || tokens[i].type == tok!";") + writeToken(); + if (index >= tokens.length) + { + newline(); break; - const len = tokenLength(tokens[i]); - assert(len >= 0); - lengthOfNextChunk += len; + } + if (currentIs(tok!"comment") && current.line == peekBack().line) + { + break; + } + else if (currentIs(tok!"{") && config.dfmt_brace_style == BraceStyle.allman) + break; + 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") + || currentIs(tok!"static")) + && peekIs(tok!"import")) && !indents.topIsOneOf(tok!"if", + tok!"debug", tok!"version")) + { + simpleNewline(); + currentLineLength = 0; + justAddedExtraNewline = true; + newline(); + } + else + newline(); + break; } - assert(lengthOfNextChunk > 0); - writeToken(); - if (currentLineLength + 1 + lengthOfNextChunk >= config.dfmt_soft_max_line_length) + else if (currentIs(tok!":")) { - pushWrapIndent(tok!","); - newline(); + if (config.dfmt_selective_import_space) + write(" "); + writeToken(); + write(" "); + } + else if (currentIs(tok!",")) + { + // compute length until next ',' or ';' + int lengthOfNextChunk; + for (size_t i = index + 1; i < tokens.length; i++) + { + if (tokens[i].type == tok!"," || tokens[i].type == tok!";") + break; + const len = tokenLength(tokens[i]); + assert(len >= 0); + lengthOfNextChunk += len; + } + assert(lengthOfNextChunk > 0); + writeToken(); + if (currentLineLength + 1 + lengthOfNextChunk >= config + .dfmt_soft_max_line_length) + { + pushWrapIndent(tok!","); + newline(); + } + else + write(" "); } else - write(" "); + formatStep(); + } + } + else + { + while(currentIs(tok!"import")) + { + // skip to the ending ; of the import statement + while(!currentIs(tok!";")) { index++; } + // skip past the ; + index++; } - else - formatStep(); + newline(); } } @@ -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,10 +1085,9 @@ private: bool currentIsIndentedTemplateConstraint() { - return index < tokens.length - && astInformation.constraintLocations.canFindIndex(current.index) + return index < tokens.length && astInformation.constraintLocations.canFindIndex(current.index) && (config.dfmt_template_constraint_style == TemplateConstraintStyle.always_newline - || currentLineLength >= config.dfmt_soft_max_line_length); + || currentLineLength >= config.dfmt_soft_max_line_length); } void formatOperator() @@ -1424,8 +1471,8 @@ private: void writeToken() { - import std.range:retro; - import std.algorithm.searching:countUntil; + import std.range : retro; + import std.algorithm.searching : countUntil; if (current.text is null) { @@ -1592,9 +1639,9 @@ const pure @safe @nogc: const(Token) peekBack(uint distance = 1) nothrow { if (index < distance) - { - assert(0, "Trying to peek before the first token"); - } + { + assert(0, "Trying to peek before the first token"); + } return tokens[index - distance]; } diff --git a/src/dfmt/main.d b/src/dfmt/main.d index 7ed73b6..c70ebd5 100644 --- a/src/dfmt/main.d +++ b/src/dfmt/main.d @@ -18,18 +18,18 @@ static immutable VERSION = () { { // takes the `git describe --tags` output and removes the leading // 'v' as well as any kind of newline - // if the tag is considered malformed it gets used verbatim + // if the tag is considered malformed it gets used verbatim enum gitDescribeOutput = import("VERSION"); - string result; + string result; if (gitDescribeOutput[0] == 'v') result = gitDescribeOutput[1 .. $]; else result = null; - uint minusCount; + uint minusCount; foreach (i, c; result) { @@ -39,14 +39,14 @@ static immutable VERSION = () { break; } - if (c == '-') - { + if (c == '-') + { ++minusCount; } } if (minusCount > 1) - result = null; + result = null; return result ? result ~ DEBUG_SUFFIX : gitDescribeOutput ~ DEBUG_SUFFIX; @@ -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) @@ -103,13 +103,16 @@ else break; case "space_before_function_parameters": optConfig.dfmt_space_before_function_parameters = optVal; - break; + break; case "split_operator_at_line_end": optConfig.dfmt_split_operator_at_line_end = optVal; break; 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, From 7158cff8c7aef8952f8abb1ca5d2a0410053dca8 Mon Sep 17 00:00:00 2001 From: Stefan Koch Date: Mon, 27 Nov 2017 09:39:09 +0100 Subject: [PATCH 2/9] retain attribs on import --- src/dfmt/ast_info.d | 60 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 56 insertions(+), 4 deletions(-) diff --git a/src/dfmt/ast_info.d b/src/dfmt/ast_info.d index 18d1f97..30c1be6 100644 --- a/src/dfmt/ast_info.d +++ b/src/dfmt/ast_info.d @@ -12,6 +12,7 @@ struct Import { string[] importStrings; string renamedAs; + string attribString; } extern (C) static bool importStringLess(const Import a, const Import b) @@ -50,6 +51,8 @@ struct ASTInformation sort(arrayStartLocations); sort(contractLocations); sort(constraintLocations); + sort(skipTokenLocations); + } /// Locations of end braces for struct bodies @@ -107,7 +110,7 @@ struct ASTInformation foreach(imp;(cast(Import[])imports).sort!(importStringLess)) { - result[idx++] = imp.importStrings.join("."); + result[idx++] = imp.attribString ~ imp.importStrings.join("."); } } else @@ -121,6 +124,9 @@ struct ASTInformation /// Locations of unary operators size_t[] unaryLocations; + /// Locations of tokens to be skipped + size_t[] skipTokenLocations; + /// Lines containing attribute declarations size_t[] attributeDeclarationLines; @@ -169,6 +175,7 @@ final class FormatVisitor : ASTVisitor * Params: * astInformation = the AST information that will be filled in */ + this(ASTInformation* astInformation) { this.astInformation = astInformation; @@ -192,9 +199,9 @@ final class FormatVisitor : ASTVisitor arrayInitializer.accept(this); } - void addImport(size_t scopeId, string[] importString, string renamedAs = null) + void addImport(size_t scopeId, string[] importString, string renamedAs, string importAttribString) { - astInformation.importScopes[scopeId] ~= Import(importString, renamedAs); + astInformation.importScopes[scopeId] ~= Import(importString, renamedAs, importAttribString); } override void visit(const SingleImport singleImport) @@ -217,7 +224,7 @@ final class FormatVisitor : ASTVisitor if (singleImport.rename.text && singleImport.rename.text.length) renamedAs = singleImport.rename.text; - addImport(scopeOrdinal, importString, renamedAs); + addImport(scopeOrdinal, importString, renamedAs, importAttribString); } else @@ -285,6 +292,49 @@ final class FormatVisitor : ASTVisitor defaultStatement.accept(this); } + /// this is the very limited usecase of printing attribs which may be + /// attached to imports (therefore it's not compleate at all) + /// HACK this method also adds the original token to the ignore_tokens + + private string toImportAttribString (const (Attribute)[] attributes) + { + string result; + + foreach(attrib;attributes) + { + + if (attrib.attribute.type == tok!"public") + { + result ~= "public "; + astInformation.skipTokenLocations ~= attrib.attribute.index; + } + else if (attrib.attribute.type == tok!"private") + { + result ~= "private "; + astInformation.skipTokenLocations ~= attrib.attribute.index; + } + else if (attrib.attribute.type == tok!"static") + { + result ~= "static "; + astInformation.skipTokenLocations ~= attrib.attribute.index; + } + } + + return result; + } + + override void visit(const Declaration declaration) + { + if (declaration.importDeclaration) + { + importAttribString = toImportAttribString(declaration.attributes); + } + + declaration.accept(this); + + importAttribString = null; + } + override void visit(const CaseStatement caseStatement) { astInformation.caseEndLocations ~= caseStatement.colonLocation; @@ -395,5 +445,7 @@ final class FormatVisitor : ASTVisitor private: ASTInformation* astInformation; + string importAttribString; + alias visit = ASTVisitor.visit; } From 44f550bcb52e3b2358b13e577a334cca0652e818 Mon Sep 17 00:00:00 2001 From: Stefan Koch Date: Mon, 27 Nov 2017 12:36:49 +0100 Subject: [PATCH 3/9] add import grouping into the sorting --- src/dfmt/ast_info.d | 46 +++++++++++++++++++++++++++++++++++--------- src/dfmt/formatter.d | 11 ++++++----- 2 files changed, 43 insertions(+), 14 deletions(-) diff --git a/src/dfmt/ast_info.d b/src/dfmt/ast_info.d index 30c1be6..bebda22 100644 --- a/src/dfmt/ast_info.d +++ b/src/dfmt/ast_info.d @@ -15,12 +15,6 @@ struct Import string attribString; } -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 { @@ -89,11 +83,24 @@ struct ASTInformation return bestOrdinal; } + bool importStringLess(const Import a, const Import b) const + { + bool result; + result = a.importStrings < b.importStrings; + /* + if (moduleNameStrings.length && isCloserTo(a.importStrings, b.importStrings, moduleNameStrings) + { + + } + */ + + return result; + } /// returns an array of indecies into the token array /// which are the indecies of the imports to be written /// in sorted order - + /// newlines for grouping are enoded as a null entry string[] importsFor(const size_t scopeOrdinal) const { import std.algorithm; @@ -106,12 +113,31 @@ struct ASTInformation if (imports.length) { - result.length = imports.length; + const max_sorted_imports_length = imports.length * 2; + // account for newlines + result.length = max_sorted_imports_length; + auto sortedImports = + (cast(Import[])imports).sort!((a, b) => importStringLess(a, b)) + .release; - foreach(imp;(cast(Import[])imports).sort!(importStringLess)) + foreach(i, imp;sortedImports) { + if (i > 0 && imp.importStrings.length > 1) + { + const prev = sortedImports[i-1]; + if (prev.importStrings.length < 2 + || imp.importStrings[0 .. $-1] != prev.importStrings[0 .. $-1] + ) + { + result[idx++] = null; + } + } + result[idx++] = imp.attribString ~ imp.importStrings.join("."); + } + + result = result[0 .. idx]; } else { @@ -166,6 +192,8 @@ struct ASTInformation /// contains all imports inside scope Import[][] importScopes; + ///contain the current fqn of the module + string[] moduleNameStrings; } /// Collects information from the AST that is useful for the formatter diff --git a/src/dfmt/formatter.d b/src/dfmt/formatter.d index fbdd119..59d7ae1 100644 --- a/src/dfmt/formatter.d +++ b/src/dfmt/formatter.d @@ -817,12 +817,13 @@ private: foreach(importString;astInformation.importsFor(scopeOrdinal)) { newline(); - write("import "); - write(importString); - write(";"); + if (importString !is null) + { + write("import "); + write(importString); + write(";"); + } } - newline(); - } } } From 345e394b0181591c503ad81e4ac36b6b95a49d6f Mon Sep 17 00:00:00 2001 From: Stefan Koch Date: Mon, 27 Nov 2017 15:57:17 +0100 Subject: [PATCH 4/9] [WIP] commit - moar import sorting --- src/dfmt/ast_info.d | 27 ++++++++++++++++----- src/dfmt/formatter.d | 56 ++++++++++++++++++++++++++++++++++---------- 2 files changed, 65 insertions(+), 18 deletions(-) diff --git a/src/dfmt/ast_info.d b/src/dfmt/ast_info.d index bebda22..ceb2197 100644 --- a/src/dfmt/ast_info.d +++ b/src/dfmt/ast_info.d @@ -97,17 +97,24 @@ struct ASTInformation return result; } + static struct ImportLine + { + string importString; + string attribString; + string renamedAs; + } + /// returns an array of indecies into the token array /// which are the indecies of the imports to be written /// in sorted order /// newlines for grouping are enoded as a null entry - string[] importsFor(const size_t scopeOrdinal) const + ImportLine[] importLinesFor(const size_t scopeOrdinal) const { import std.algorithm; import std.range; uint idx = 0; - string[] result; + ImportLine[] result; auto imports = importScopes[scopeOrdinal]; @@ -129,12 +136,13 @@ struct ASTInformation || imp.importStrings[0 .. $-1] != prev.importStrings[0 .. $-1] ) { - result[idx++] = null; + result[idx++].importString = null; } } - result[idx++] = imp.attribString ~ imp.importStrings.join("."); - + result[idx].importString = imp.importStrings.join("."); + result[idx].renamedAs = imp.renamedAs; + result[idx++].attribString = imp.attribString; } result = result[0 .. idx]; @@ -156,6 +164,9 @@ struct ASTInformation /// Lines containing attribute declarations size_t[] attributeDeclarationLines; + /// lines in which imports end + size_t[] importEndLines; + /// Case statement colon locations size_t[] caseEndLocations; @@ -226,9 +237,13 @@ final class FormatVisitor : ASTVisitor astInformation.arrayStartLocations ~= arrayInitializer.startLocation; arrayInitializer.accept(this); } - + void addImport(size_t scopeId, string[] importString, string renamedAs, string importAttribString) { + import std.stdio; + + writeln("addImport(", scopeId, ", ", importString, ", ", renamedAs, ", ", importAttribString, ")"); + astInformation.importScopes[scopeId] ~= Import(importString, renamedAs, importAttribString); } diff --git a/src/dfmt/formatter.d b/src/dfmt/formatter.d index 59d7ae1..4c4e67a 100644 --- a/src/dfmt/formatter.d +++ b/src/dfmt/formatter.d @@ -518,14 +518,17 @@ private: } else { + if (!isImport) + writeImportLinesFor(0); + while(currentIs(tok!"import")) { // skip to the ending ; of the import statement - while(!currentIs(tok!";")) { index++; } + while(!currentIs(tok!";")) + index++; // skip past the ; index++; } - newline(); } } @@ -624,6 +627,38 @@ private: write(" "); } + void writeImportLinesFor(size_t scopeOrdinal) + { + foreach(importLine;astInformation.importLinesFor(scopeOrdinal)) + { + if (importLine.importString !is null) + { + newline(); + write(importLine.attribString); + write("import "); + if (importLine.renamedAs) + { + write(importLine.renamedAs); + write(" = "); + } + /+ TODO deal with selective imports + if (importLine.selctiveImports) + { + } + +/ + write(importLine.importString); + write(";"); + } + else + { + simpleNewline(); + } + } + + simpleNewline(); + simpleNewline(); + } + void formatColon() { import dfmt.editorconfig : OptionalBoolean; @@ -814,16 +849,7 @@ private: if (writeImports && astInformation.importScopes[scopeOrdinal].length) { - foreach(importString;astInformation.importsFor(scopeOrdinal)) - { - newline(); - if (importString !is null) - { - write("import "); - write(importString); - write(";"); - } - } + writeImportLinesFor(scopeOrdinal); } } } @@ -1472,6 +1498,12 @@ private: void writeToken() { + if (config.dfmt_sort_imports && astInformation.skipTokenLocations.canFindIndex(current.index)) + { + index++; + return ; + } + import std.range : retro; import std.algorithm.searching : countUntil; From 1acd642eb2aeee75184518e66e991a67fd975306 Mon Sep 17 00:00:00 2001 From: Stefan Koch Date: Tue, 28 Nov 2017 13:34:07 +0100 Subject: [PATCH 5/9] fixing by package grouping --- src/dfmt/ast_info.d | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/dfmt/ast_info.d b/src/dfmt/ast_info.d index ceb2197..a771e3f 100644 --- a/src/dfmt/ast_info.d +++ b/src/dfmt/ast_info.d @@ -129,7 +129,7 @@ struct ASTInformation foreach(i, imp;sortedImports) { - if (i > 0 && imp.importStrings.length > 1) + if (i > 0) { const prev = sortedImports[i-1]; if (prev.importStrings.length < 2 @@ -142,7 +142,8 @@ struct ASTInformation result[idx].importString = imp.importStrings.join("."); result[idx].renamedAs = imp.renamedAs; - result[idx++].attribString = imp.attribString; + result[idx].attribString = imp.attribString; + idx++; } result = result[0 .. idx]; From 5f1b1479ebebadc7ca6ee2c06a39dfc5b3416e7e Mon Sep 17 00:00:00 2001 From: Stefan Koch Date: Tue, 28 Nov 2017 14:40:01 +0100 Subject: [PATCH 6/9] move closer to sociomantic spec --- src/dfmt/ast_info.d | 33 +++++++++++++++++++++++++++++---- src/dfmt/formatter.d | 2 -- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/src/dfmt/ast_info.d b/src/dfmt/ast_info.d index a771e3f..4a248db 100644 --- a/src/dfmt/ast_info.d +++ b/src/dfmt/ast_info.d @@ -83,6 +83,20 @@ struct ASTInformation return bestOrdinal; } + /// true if a is closer to t then b + /// with bias towards true + static bool isCloserTo(const string[] a, const string[] b, const string[] t) + { + import std.algorithm : min; + + foreach(i;0 .. min(a.length, b.length, t.length)) + { + + } + + return false; + } + bool importStringLess(const Import a, const Import b) const { bool result; @@ -132,11 +146,22 @@ struct ASTInformation if (i > 0) { const prev = sortedImports[i-1]; - if (prev.importStrings.length < 2 - || imp.importStrings[0 .. $-1] != prev.importStrings[0 .. $-1] - ) + static if (false) { - result[idx++].importString = null; + if (prev.importStrings.length < 2 + || imp.importStrings[0 .. $-1] + != prev.importStrings[0 .. $-1] + ) + { + result[idx++].importString = null; + } + } + else + { + if (imp.importStrings[0] != prev.importStrings[0]) + { + result[idx++].importString = null; + } } } diff --git a/src/dfmt/formatter.d b/src/dfmt/formatter.d index 4c4e67a..c5fc32b 100644 --- a/src/dfmt/formatter.d +++ b/src/dfmt/formatter.d @@ -461,8 +461,6 @@ private: { break; } - else if (currentIs(tok!"{") && config.dfmt_brace_style == BraceStyle.allman) - break; else if (currentIs(tok!"{") && config.dfmt_brace_style == BraceStyle.allman) break; else if (isImport && !currentIs(tok!"import") From 19e6cdd7c56933d304e522f9a8540386ba929898 Mon Sep 17 00:00:00 2001 From: Stefan Koch Date: Tue, 28 Nov 2017 15:37:11 +0100 Subject: [PATCH 7/9] adjust readme --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 7aff4cf..344d0d7 100644 --- a/README.md +++ b/README.md @@ -43,6 +43,7 @@ found in .editorconfig files. * **--outdent_attributes**: See **dfmt_outdent_attributes** below * **--space_after_cast**: See **dfmt_space_after_cast** below * **--space_before_function_parameters**: See **dfmt_space_before_function_parameters** below +* **--sort_imports**: See **sort_imports** below * **--split_operator_at_line_end**: See **dfmt_split_operator_at_line_end** below * **--tab_width**: See **tab_width** below * **--selective_import_space**: See **dfmt_selective_import_space** below @@ -102,6 +103,7 @@ dfmt_split_operator_at_line_end | `true`, `false` | `false` | Place operators on dfmt_space_after_cast | `true`, `false` | `true` | Insert space after the closing paren of a `cast` expression. dfmt_space_after_keywords (Not yet implemented) | `true`, `false` | `true` | Insert space after `if`, `while`, `foreach`, etc, and before the `(`. dfmt_space_before_function_parameters | `true`, `false` | `false` | Insert space before the opening paren of a function parameter list. +dfmt_sort_imports | `true`, `false` | `false` | Sort Imports alphabetically group by root-package dfmt_selective_import_space | `true`, `false` | `true` | Insert space after the module name and before the `:` for selective imports. 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. From fa856430a33d47a0a4b4a033d993ff5ad35d58eb Mon Sep 17 00:00:00 2001 From: Stefan Koch Date: Wed, 29 Nov 2017 15:48:04 +0100 Subject: [PATCH 8/9] remove debug output and add comments --- src/dfmt/ast_info.d | 36 ++++++++++++++++++++++++++++++------ src/dfmt/formatter.d | 8 ++++---- 2 files changed, 34 insertions(+), 10 deletions(-) diff --git a/src/dfmt/ast_info.d b/src/dfmt/ast_info.d index 4a248db..6284466 100644 --- a/src/dfmt/ast_info.d +++ b/src/dfmt/ast_info.d @@ -10,8 +10,11 @@ import dparse.ast; struct Import { + /// the identifier chain of the import string[] importStrings; + /// the lhs of renamed imports string renamedAs; + /// attribs for the import string attribString; } @@ -100,7 +103,30 @@ struct ASTInformation bool importStringLess(const Import a, const Import b) const { bool result; - result = a.importStrings < b.importStrings; + const (string)[] sortKeyA = a.importStrings; + const (string)[] sortKeyB = b.importStrings; +/+ + if (sortKeyA.length > 2) + { + const last = a.importStrings[$ - 1]; + foreach(i,s;a.importStrings[1 .. $-1]) + { + sortKeyA[i + 2] = s; + } + sortKeyA[1] = last; + } + + if (sortKeyB.length > 2) + { + const last = b.importStrings[$ - 1]; + foreach(i,s;sortKeyB[1 .. $-1]) + { + sortKeyB[i + 2] = s; + } + sortKeyB[1] = last; + } ++/ + result = sortKeyA < sortKeyB; /* if (moduleNameStrings.length && isCloserTo(a.importStrings, b.importStrings, moduleNameStrings) { @@ -154,6 +180,7 @@ struct ASTInformation ) { result[idx++].importString = null; + // a null importString means a blank line is inserted } } else @@ -161,6 +188,7 @@ struct ASTInformation if (imp.importStrings[0] != prev.importStrings[0]) { result[idx++].importString = null; + // a null importString means a blank line is inserted } } } @@ -266,10 +294,6 @@ final class FormatVisitor : ASTVisitor void addImport(size_t scopeId, string[] importString, string renamedAs, string importAttribString) { - import std.stdio; - - writeln("addImport(", scopeId, ", ", importString, ", ", renamedAs, ", ", importAttribString, ")"); - astInformation.importScopes[scopeId] ~= Import(importString, renamedAs, importAttribString); } @@ -301,7 +325,7 @@ final class FormatVisitor : ASTVisitor assert (0, "singleImport without identifierChain"); } - + singleImport.accept(this); } override void visit(const ConditionalDeclaration dec) diff --git a/src/dfmt/formatter.d b/src/dfmt/formatter.d index c5fc32b..d28d223 100644 --- a/src/dfmt/formatter.d +++ b/src/dfmt/formatter.d @@ -147,7 +147,7 @@ private: /// Configuration const Config* config; - /// chached end of line string + /// cached end of line string const string eolString; /// Keep track of whether or not an extra newline was just added because of @@ -513,12 +513,12 @@ private: else formatStep(); } + + if (config.dfmt_sort_imports && !isImport) + writeImportLinesFor(0); } else { - if (!isImport) - writeImportLinesFor(0); - while(currentIs(tok!"import")) { // skip to the ending ; of the import statement From eb18a6f6d3c7c0f970d5f2cb0585162ba95caaaf Mon Sep 17 00:00:00 2001 From: Stefan Koch Date: Wed, 29 Nov 2017 16:02:44 +0100 Subject: [PATCH 9/9] fix the handling of module-scope imports --- src/dfmt/ast_info.d | 28 ++-------------------------- src/dfmt/formatter.d | 5 ++++- 2 files changed, 6 insertions(+), 27 deletions(-) diff --git a/src/dfmt/ast_info.d b/src/dfmt/ast_info.d index 6284466..bf1dcca 100644 --- a/src/dfmt/ast_info.d +++ b/src/dfmt/ast_info.d @@ -105,34 +105,10 @@ struct ASTInformation bool result; const (string)[] sortKeyA = a.importStrings; const (string)[] sortKeyB = b.importStrings; -/+ - if (sortKeyA.length > 2) - { - const last = a.importStrings[$ - 1]; - foreach(i,s;a.importStrings[1 .. $-1]) - { - sortKeyA[i + 2] = s; - } - sortKeyA[1] = last; - } - if (sortKeyB.length > 2) - { - const last = b.importStrings[$ - 1]; - foreach(i,s;sortKeyB[1 .. $-1]) - { - sortKeyB[i + 2] = s; - } - sortKeyB[1] = last; - } -+/ result = sortKeyA < sortKeyB; - /* - if (moduleNameStrings.length && isCloserTo(a.importStrings, b.importStrings, moduleNameStrings) - { - - } - */ + + //TODO take the module-name of the module we are formatting into account return result; } diff --git a/src/dfmt/formatter.d b/src/dfmt/formatter.d index d28d223..1faf94d 100644 --- a/src/dfmt/formatter.d +++ b/src/dfmt/formatter.d @@ -631,7 +631,10 @@ private: { if (importLine.importString !is null) { - newline(); + // for some reason newline() creates double + // newlines in module-scope + scopeOrdinal ? newline() : simpleNewline(); + write(importLine.attribString); write("import "); if (importLine.renamedAs)