clang-format: [JS] Introduce WrapJavaScriptImports option.

[mprobst]

Summary

When turned on, clang-format wraps JavaScript imports (and importing exports),
instead of forcing the entire import statement onto one line.

[djasper]

include/clang/Format/Format.h

631 ↗

(On Diff #60484)

Wondering whether we should call this JavaScriptWrapImports so that the JS-specific options stay together. What do you think?

[mprobst]

• rename to JavaScriptWrapImports
• re-alphabetize options list

[djasper]

lib/Format/TokenAnnotator.cpp

789 ↗

(On Diff #60519)

I think it is a bit dodgy to not mark the line as import statement based on this style option. Would it be a lot of work to pull the decision of whether or not to format this out a level higher?

[mprobst]

• classify as LT_ImportStatement, re-enable wrapping later
• no early exist in TokenAnnotator: while loop is required for consumeToken().

[djasper]

Looks good.

cfe/trunk/include/clang/Format/Format.h

struct FormatStyle {
 
   /// \brief The number of columns to use for indentation.
   unsigned IndentWidth;
 
   /// \brief Indent if a function definition or declaration is wrapped after the
   /// type.
   bool IndentWrappedFunctionNames;
 
+  /// \brief Quotation styles for JavaScript strings. Does not affect template
+  /// strings.
+  enum JavaScriptQuoteStyle {
+    /// Leave string quotes as they are.
+    JSQS_Leave,
+    /// Always use single quotes.
+    JSQS_Single,
+    /// Always use double quotes.
+    JSQS_Double
+  };
+
+  /// \brief The JavaScriptQuoteStyle to use for JavaScript strings.
+  JavaScriptQuoteStyle JavaScriptQuotes;
+
+  /// \brief Whether to wrap JavaScript import/export statements.
+  bool JavaScriptWrapImports;
+
   /// \brief If true, empty lines at the start of blocks are kept.
   bool KeepEmptyLinesAtTheStartOfBlocks;
 
   /// \brief Supported languages.
   ///
   /// When stored in a configuration file, specifies the language, that the
   /// configuration targets. When passed to the ``reformat()`` function, enables
   /// syntax features specific to the language.
enum UseTabStyle {
     /// Use tabs whenever we need to fill whitespace that spans at least from
     /// one tab stop to the next one.
     UT_Always
   };
 
   /// \brief The way to use tab characters in the resulting file.
   UseTabStyle UseTab;
 
-  /// \brief Quotation styles for JavaScript strings. Does not affect template
-  /// strings.
-  enum JavaScriptQuoteStyle {
-    /// Leave string quotes as they are.
-    JSQS_Leave,
-    /// Always use single quotes.
-    JSQS_Single,
-    /// Always use double quotes.
-    JSQS_Double
-  };
-
-  /// \brief The JavaScriptQuoteStyle to use for JavaScript strings.
-  JavaScriptQuoteStyle JavaScriptQuotes;
-
   bool operator==(const FormatStyle &R) const {
     return AccessModifierOffset == R.AccessModifierOffset &&
            AlignAfterOpenBracket == R.AlignAfterOpenBracket &&
            AlignConsecutiveAssignments == R.AlignConsecutiveAssignments &&
            AlignConsecutiveDeclarations == R.AlignConsecutiveDeclarations &&
            AlignEscapedNewlinesLeft == R.AlignEscapedNewlinesLeft &&
            AlignOperands == R.AlignOperands &&
            AlignTrailingComments == R.AlignTrailingComments &&
return AccessModifierOffset == R.AccessModifierOffset &&
            DisableFormat == R.DisableFormat &&
            ExperimentalAutoDetectBinPacking ==
                R.ExperimentalAutoDetectBinPacking &&
            ForEachMacros == R.ForEachMacros &&
            IncludeCategories == R.IncludeCategories &&
            IndentCaseLabels == R.IndentCaseLabels &&
            IndentWidth == R.IndentWidth && Language == R.Language &&
            IndentWrappedFunctionNames == R.IndentWrappedFunctionNames &&
+           JavaScriptQuotes == R.JavaScriptQuotes &&
+           JavaScriptWrapImports == R.JavaScriptWrapImports &&
            KeepEmptyLinesAtTheStartOfBlocks ==
                R.KeepEmptyLinesAtTheStartOfBlocks &&
            MacroBlockBegin == R.MacroBlockBegin &&
            MacroBlockEnd == R.MacroBlockEnd &&
            MaxEmptyLinesToKeep == R.MaxEmptyLinesToKeep &&
            NamespaceIndentation == R.NamespaceIndentation &&
            ObjCBlockIndentWidth == R.ObjCBlockIndentWidth &&
            ObjCSpaceAfterProperty == R.ObjCSpaceAfterProperty &&
return AccessModifierOffset == R.AccessModifierOffset &&
            SpaceInEmptyParentheses == R.SpaceInEmptyParentheses &&
            SpacesBeforeTrailingComments == R.SpacesBeforeTrailingComments &&
            SpacesInAngles == R.SpacesInAngles &&
            SpacesInContainerLiterals == R.SpacesInContainerLiterals &&
            SpacesInCStyleCastParentheses == R.SpacesInCStyleCastParentheses &&
            SpacesInParentheses == R.SpacesInParentheses &&
            SpacesInSquareBrackets == R.SpacesInSquareBrackets &&
            Standard == R.Standard && TabWidth == R.TabWidth &&
-           UseTab == R.UseTab &&
-           JavaScriptQuotes == R.JavaScriptQuotes;
+           UseTab == R.UseTab;
   }
 };
 
 /// \brief Returns a format style complying with the LLVM coding standards:
 /// http://llvm.org/docs/CodingStandards.html.
 FormatStyle getLLVMStyle();
 
 /// \brief Returns a format style complying with one of Google's style guides:

cfe/trunk/lib/Format/Format.cpp

IO.mapOptional("ExperimentalAutoDetectBinPacking",
                    Style.ExperimentalAutoDetectBinPacking);
     IO.mapOptional("ForEachMacros", Style.ForEachMacros);
     IO.mapOptional("IncludeCategories", Style.IncludeCategories);
     IO.mapOptional("IncludeIsMainRegex", Style.IncludeIsMainRegex);
     IO.mapOptional("IndentCaseLabels", Style.IndentCaseLabels);
     IO.mapOptional("IndentWidth", Style.IndentWidth);
     IO.mapOptional("IndentWrappedFunctionNames",
                    Style.IndentWrappedFunctionNames);
+    IO.mapOptional("JavaScriptQuotes", Style.JavaScriptQuotes);
+    IO.mapOptional("JavaScriptWrapImports", Style.JavaScriptWrapImports);
     IO.mapOptional("KeepEmptyLinesAtTheStartOfBlocks",
                    Style.KeepEmptyLinesAtTheStartOfBlocks);
     IO.mapOptional("MacroBlockBegin", Style.MacroBlockBegin);
     IO.mapOptional("MacroBlockEnd", Style.MacroBlockEnd);
     IO.mapOptional("MaxEmptyLinesToKeep", Style.MaxEmptyLinesToKeep);
     IO.mapOptional("NamespaceIndentation", Style.NamespaceIndentation);
     IO.mapOptional("ObjCBlockIndentWidth", Style.ObjCBlockIndentWidth);
     IO.mapOptional("ObjCSpaceAfterProperty", Style.ObjCSpaceAfterProperty);
IO.mapOptional("SpacesInContainerLiterals",
                    Style.SpacesInContainerLiterals);
     IO.mapOptional("SpacesInCStyleCastParentheses",
                    Style.SpacesInCStyleCastParentheses);
     IO.mapOptional("SpacesInParentheses", Style.SpacesInParentheses);
     IO.mapOptional("SpacesInSquareBrackets", Style.SpacesInSquareBrackets);
     IO.mapOptional("Standard", Style.Standard);
     IO.mapOptional("TabWidth", Style.TabWidth);
     IO.mapOptional("UseTab", Style.UseTab);
-    IO.mapOptional("JavaScriptQuotes", Style.JavaScriptQuotes);
   }
 };
 
 template <> struct MappingTraits<FormatStyle::BraceWrappingFlags> {
   static void mapping(IO &IO, FormatStyle::BraceWrappingFlags &Wrapping) {
     IO.mapOptional("AfterClass", Wrapping.AfterClass);
     IO.mapOptional("AfterControlStatement", Wrapping.AfterControlStatement);
     IO.mapOptional("AfterEnum", Wrapping.AfterEnum);
if (Language == FormatStyle::LK_Java) {
     GoogleStyle.AlignOperands = false;
     GoogleStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
     GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
     GoogleStyle.BreakBeforeTernaryOperators = false;
     GoogleStyle.CommentPragmas = "@(export|return|see|visibility) ";
     GoogleStyle.MaxEmptyLinesToKeep = 3;
     GoogleStyle.SpacesInContainerLiterals = false;
     GoogleStyle.JavaScriptQuotes = FormatStyle::JSQS_Single;
+    GoogleStyle.JavaScriptWrapImports = false;
   } else if (Language == FormatStyle::LK_Proto) {
     GoogleStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
     GoogleStyle.SpacesInContainerLiterals = false;
   }
 
   return GoogleStyle;
 }
 

cfe/trunk/lib/Format/TokenAnnotator.cpp

LineType parseLine() {
     // and should not be line-wrapped.
     if (Style.Language == FormatStyle::LK_Proto && Line.Level == 0 &&
         CurrentToken->is(Keywords.kw_option)) {
       next();
       if (CurrentToken && CurrentToken->is(tok::identifier))
         return LT_ImportStatement;
     }
 
+    bool KeywordVirtualFound = false;
+    bool ImportStatement = false;
+
     // import {...} from '...';
     if (Style.Language == FormatStyle::LK_JavaScript &&
         CurrentToken->is(Keywords.kw_import))
-      return LT_ImportStatement;
+      ImportStatement = true;
 
-    bool KeywordVirtualFound = false;
-    bool ImportStatement = false;
     while (CurrentToken) {
       if (CurrentToken->is(tok::kw_virtual))
         KeywordVirtualFound = true;
       if (Style.Language == FormatStyle::LK_JavaScript) {
         // export {...} from '...';
         // An export followed by "from 'some string';" is a re-export from
         // another module identified by a URI and is treated as a
         // LT_ImportStatement (i.e. prevent wraps on it for long URIs).

cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp

if (ShouldFormat && TheLine.Type != LT_Invalid) {
       if (!DryRun)
         formatFirstToken(*TheLine.First, PreviousLine, TheLine.Level, Indent,
                          TheLine.InPPDirective);
 
       NextLine = Joiner.getNextMergedLine(DryRun, IndentTracker);
       unsigned ColumnLimit = getColumnLimit(TheLine.InPPDirective, NextLine);
       bool FitsIntoOneLine =
           TheLine.Last->TotalLength + Indent <= ColumnLimit ||
-          TheLine.Type == LT_ImportStatement;
+          (TheLine.Type == LT_ImportStatement &&
+           (Style.Language != FormatStyle::LK_JavaScript ||
+            !Style.JavaScriptWrapImports));
 
       if (Style.ColumnLimit == 0)
         NoColumnLimitLineFormatter(Indenter, Whitespaces, Style, this)
             .formatLine(TheLine, Indent, DryRun);
       else if (FitsIntoOneLine)
         Penalty += NoLineBreakFormatter(Indenter, Whitespaces, Style, this)
                        .formatLine(TheLine, Indent, DryRun);
       else

cfe/trunk/unittests/Format/FormatTestJS.cpp

verifyFormat("type X = {\n"
                "};\n"
                "class C {}");
 }
 
 TEST_F(FormatTestJS, Modules) {
   verifyFormat("import SomeThing from 'some/module.js';");
   verifyFormat("import {X, Y} from 'some/module.js';");
   verifyFormat("import a, {X, Y} from 'some/module.js';");
-  verifyFormat("import {VeryLongImportsAreAnnoying, VeryLongImportsAreAnnoying,"
-               " VeryLongImportsAreAnnoying, VeryLongImportsAreAnnoying"
-               "} from 'some/module.js';");
   verifyFormat("import {X, Y,} from 'some/module.js';");
   verifyFormat("import {X as myLocalX, Y as myLocalY} from 'some/module.js';");
   // Ensure Automatic Semicolon Insertion does not break on "as\n".
   verifyFormat("import {X as myX} from 'm';", "import {X as\n"
                                               " myX} from 'm';");
   verifyFormat("import * as lib from 'some/module.js';");
   verifyFormat("var x = {import: 1};\nx.import = 2;");
 
TEST_F(FormatTestJS, Modules) {
   verifyFormat("export default [];");
   verifyFormat("export default () => {};");
   verifyFormat("export interface Foo { foo: number; }\n"
                "export class Bar {\n"
                "  blah(): string { return this.blah; };\n"
                "}");
 }
 
+TEST_F(FormatTestJS, ImportWrapping) {
+  verifyFormat("import {VeryLongImportsAreAnnoying, VeryLongImportsAreAnnoying,"
+               " VeryLongImportsAreAnnoying, VeryLongImportsAreAnnoying"
+               "} from 'some/module.js';");
+  FormatStyle Style = getGoogleJSStyleWithColumns(80);
+  Style.JavaScriptWrapImports = true;
+  verifyFormat("import {\n"
+               "  VeryLongImportsAreAnnoying,\n"
+               "  VeryLongImportsAreAnnoying,\n"
+               "  VeryLongImportsAreAnnoying,\n"
+               "} from 'some/module.js';",
+               Style);
+  verifyFormat("import {\n"
+               "  A,\n"
+               "  A,\n"
+               "} from 'some/module.js';",
+               Style);
+  verifyFormat("export {\n"
+               "  A,\n"
+               "  A,\n"
+               "} from 'some/module.js';",
+               Style);
+}
+
 TEST_F(FormatTestJS, TemplateStrings) {
   // Keeps any whitespace/indentation within the template string.
   verifyFormat("var x = `hello\n"
             "     ${  name    }\n"
             "  !`;",
             "var x    =    `hello\n"
                    "     ${  name    }\n"
                    "  !`;");

cfe/trunk/unittests/Format/SortImportsTestJS.cpp

verifySort("import {sym} from 'a';"
              "\n"
              "let x = 1;\n",
              "import {sym} from 'a'; let x = 1;\n");
 }
 
 TEST_F(SortImportsTestJS, Comments) {
   verifySort("/** @fileoverview This is a great file. */\n"
              "// A very important import follows.\n"
-             "import {sym} from 'a'; /* more comments */\n"
-             "import {sym} from 'b'; // from //foo:bar\n",
+             "import {sym} from 'a';  /* more comments */\n"
+             "import {sym} from 'b';  // from //foo:bar\n",
              "/** @fileoverview This is a great file. */\n"
-             "import {sym} from 'b'; // from //foo:bar\n"
+             "import {sym} from 'b';  // from //foo:bar\n"
              "// A very important import follows.\n"
-             "import {sym} from 'a'; /* more comments */\n");
+             "import {sym} from 'a';  /* more comments */\n");
 }
 
 TEST_F(SortImportsTestJS, SortStar) {
   verifySort("import * as foo from 'a';\n"
              "import {sym} from 'a';\n"
              "import * as bar from 'b';\n",
              "import {sym} from 'a';\n"
              "import * as foo from 'a';\n"