Don't indent JavaScript IIFEs

[danbeam]

Summary

Because IIFEs[1] are often used like an anonymous namespace around large
sections of JavaScript code, it's useful not to indent to them (which
effectively reduces the column limit by the indent amount needlessly).

It's also common for developers to wrap these around entire files or
libraries. When adopting clang-format, changing the indent entire file
can reduce the usefulness of the blame annotations.

Before:

(function() {
  // clang-format pushes stuff to here
})();

After:

(function() {
// clang-format pushes stuff to here
})();

[1] https://en.wikipedia.org/wiki/Immediately-invoked_function_expression

[mprobst]

lib/Format/UnwrappedLineParser.cpp

2346 ↗

(On Diff #98243)

Why not just a static function?

2351 ↗

(On Diff #98243)

The question is whether we ever expect to find lines starting with (function() { that do not end up being an IIFE. I think that's very unlikely, so I think I'd even drop this FIXME. I wouldn't expect that to be something that needs fixing.

If you wanted to, you could check FormatToken.MatchingParen and then parse from there, but I'm not sure if that pointer is already set up at this point.

2353 ↗

(On Diff #98243)

There's a startsSequenceInternal on Line that might come in handy here?

unittests/Format/FormatTestJS.cpp

371 ↗

(On Diff #98243)

consider adding a test with comments between the tokens (which should work with startsSequence).

374 ↗

(On Diff #98243)

I find it hard to see how these cases are different - can you give them e.g. variable names that call out what's the point of the different cases?

[mprobst]

lib/Format/UnwrappedLineParser.cpp

2362 ↗

(On Diff #98243)

One more - do we want to support IIFEs that take arguments?

(function(global) {
  ...
}(window));

[danbeam]

mprobst@ review

lib/Format/UnwrappedLineParser.cpp

2353 ↗

(On Diff #98243)

it wasn't on Line and unfortunately some of the state that startsSequence relies on isn't set up yet (at least as far as I know, I'm in a foreign land)

2362 ↗

(On Diff #98243)

"Less is more"

unittests/Format/FormatTestJS.cpp

371 ↗

(On Diff #98243)

not done yet, but seems uncommon.

do you want me to add a FIXME?

[mprobst]

unittests/Format/FormatTestJS.cpp

371 ↗

(On Diff #98243)

I think that's OK the way it is.

cfe/trunk/lib/Format/UnwrappedLineParser.cpp

if (I->Tok->isNot(tok::period))
     return false;
   ++I;
   if (I->Tok->TokenText != "scope")
     return false;
   ++I;
   return I->Tok->is(tok::l_paren);
 }
 
+static bool isIIFE(const UnwrappedLine &Line,
+                   const AdditionalKeywords &Keywords) {
+  // Look for the start of an immediately invoked anonymous function.
+  // https://en.wikipedia.org/wiki/Immediately-invoked_function_expression
+  // This is commonly done in JavaScript to create a new, anonymous scope.
+  // Example: (function() { ... })()
+  if (Line.Tokens.size() < 3)
+    return false;
+  auto I = Line.Tokens.begin();
+  if (I->Tok->isNot(tok::l_paren))
+    return false;
+  ++I;
+  if (I->Tok->isNot(Keywords.kw_function))
+    return false;
+  ++I;
+  return I->Tok->is(tok::l_paren);
+}
+
 static bool ShouldBreakBeforeBrace(const FormatStyle &Style,
                                    const FormatToken &InitialToken) {
   if (InitialToken.is(tok::kw_namespace))
     return Style.BraceWrapping.AfterNamespace;
   if (InitialToken.is(tok::kw_class))
     return Style.BraceWrapping.AfterClass;
   if (InitialToken.is(tok::kw_union))
     return Style.BraceWrapping.AfterUnion;
   if (InitialToken.is(tok::kw_struct))
     return Style.BraceWrapping.AfterStruct;
   return false;
 }
 
 void UnwrappedLineParser::parseChildBlock() {
   FormatTok->BlockKind = BK_Block;
   nextToken();
   {
-    bool GoogScope =
-        Style.Language == FormatStyle::LK_JavaScript && isGoogScope(*Line);
+    bool SkipIndent =
+        (Style.Language == FormatStyle::LK_JavaScript &&
+         (isGoogScope(*Line) || isIIFE(*Line, Keywords)));
     ScopedLineState LineState(*this);
     ScopedDeclarationState DeclarationState(*Line, DeclarationScopeStack,
                                             /*MustBeDeclaration=*/false);
-    Line->Level += GoogScope ? 0 : 1;
+    Line->Level += SkipIndent ? 0 : 1;
     parseLevel(/*HasOpeningBrace=*/true);
     flushComments(isOnNewLine(*FormatTok));
-    Line->Level -= GoogScope ? 0 : 1;
+    Line->Level -= SkipIndent ? 0 : 1;
   }
   nextToken();
 }
 
 void UnwrappedLineParser::parsePPDirective() {
   assert(FormatTok->Tok.is(tok::hash) && "'#' expected");
   ScopedMacroState MacroState(*Line, Tokens, FormatTok);
   nextToken();

cfe/trunk/unittests/Format/FormatTestJS.cpp

verifyFormat("goog.scope(function() {\n"
                "});  // goog.scope");
   verifyFormat("goog.scope(function() {\n"
                "// test\n"
                "var x = 0;\n"
                "// test\n"
                "});");
 }
 
+TEST_F(FormatTestJS, IIFEs) {
+  // Internal calling parens; no semi.
+  verifyFormat("(function() {\n"
+               "var a = 1;\n"
+               "}())");
+  // External calling parens; no semi.
+  verifyFormat("(function() {\n"
+               "var b = 2;\n"
+               "})()");
+  // Internal calling parens; with semi.
+  verifyFormat("(function() {\n"
+               "var c = 3;\n"
+               "}());");
+  // External calling parens; with semi.
+  verifyFormat("(function() {\n"
+               "var d = 4;\n"
+               "})();");
+}
+
 TEST_F(FormatTestJS, GoogModules) {
   verifyFormat("goog.module('this.is.really.absurdly.long');",
                getGoogleJSStyleWithColumns(40));
   verifyFormat("goog.require('this.is.really.absurdly.long');",
                getGoogleJSStyleWithColumns(40));
   verifyFormat("goog.provide('this.is.really.absurdly.long');",
                getGoogleJSStyleWithColumns(40));
   verifyFormat("var long = goog.require('this.is.really.absurdly.long');",