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

Why not just a static function?

2351

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

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

unittests/Format/FormatTestJS.cpp

371

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

374

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

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

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

[danbeam]

mprobst@ review

lib/Format/UnwrappedLineParser.cpp

2353

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

"Less is more"

unittests/Format/FormatTestJS.cpp

371

not done yet, but seems uncommon.

do you want me to add a FIXME?

[mprobst]

unittests/Format/FormatTestJS.cpp

371

I think that's OK the way it is.

lib/Format/UnwrappedLineParser.h

private:
   bool tryToParseLambda();
   bool tryToParseLambdaIntroducer();
   void tryToParseJSFunction();
   void addUnwrappedLine();
   bool eof() const;
   void nextToken();
   const FormatToken *getPreviousToken();
   void readToken();
+  bool isIIFE() const;
 
   // Decides which comment tokens should be added to the current line and which
   // should be added as comments before the next token.
   //
   // Comments specifies the sequence of comment tokens to analyze. They get
   // either pushed to the current line or added to the comments before the next
   // token.
   //

lib/Format/UnwrappedLineParser.cpp

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());
     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();

 void UnwrappedLineParser::pushToken(FormatToken *Tok) {
   Line->Tokens.push_back(UnwrappedLineNode(Tok));
   if (MustBreakBeforeNextToken) {
     Line->Tokens.back().Tok->MustBreakBefore = true;
     MustBreakBeforeNextToken = false;
   }
 }
 
+bool UnwrappedLineParser::isIIFE() const {
+  // 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() { ... })()
+  // FIXME: check for the end invocation or alternate ways to start function
+  // expressions?
+  if (Line->Tokens.size() < 5)
+    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;
+  if (I->Tok->isNot(tok::l_paren))
+    return false;
+  ++I;
+  if (I->Tok->isNot(tok::r_paren))
+    return false;
+  ++I;
+  return I->Tok->is(tok::l_brace);
+}
+
 } // end namespace format
 } // end namespace clang

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, IIFE) {
+  verifyFormat("(function() {\n"
+               "var a = 1;\n"
+               "}())");
+  verifyFormat("(function() {\n"
+               "var b = 2;\n"
+               "})()");
+  verifyFormat("(function() {\n"
+               "var c = 3;\n"
+               "})();");
+  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');",