Fix detection of goog.js.

We need to:
- Use something other than "var COMPILED = false;" to find base.js.
  Inside base.js.i.js it will start with "var COMPILED;" @provideGoog seems reasonable.
- Scan externs for base.js and / or goog.js for incremental type checking.
- Check for a file that provides "goog", rather than a file that provides only "goog"
  (base.js will also have a synthetic provide for its path).

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=216644571

Author: johnplaisted

src/com/google/javascript/jscomp/RewriteGoogJsImports.java

@@ -88,7 +88,7 @@ public enum Mode {
     LINT_AND_REWRITE
   }
 
-  private static final ImmutableList<String> EXPECTED_BASE_PROVIDES = ImmutableList.of("goog");
+  private static final String EXPECTED_BASE_PROVIDE = "goog";
 
   private final Mode mode;
   private final AbstractCompiler compiler;
@@ -326,7 +326,7 @@ private Node findGoogJsScriptNode(Node root) {
     // Find Closure's base.js file. goog.js should be right next to it.
     for (Node script : root.children()) {
       ImmutableList<String> provides = compiler.getInput(script.getInputId()).getProvides();
-      if (EXPECTED_BASE_PROVIDES.equals(provides)) {
+      if (provides.contains(EXPECTED_BASE_PROVIDE)) {
         // Use resolveModuleAsPath as if it is not part of the input we don't want to report an
         // error.
         expectedGoogPath =
@@ -365,6 +365,11 @@ public void process(Node externs, Node root) {
 
     Node googJsScriptNode = findGoogJsScriptNode(root);
 
+    if (googJsScriptNode == null) {
+      // Potentially in externs if library level type checking.
+      googJsScriptNode = findGoogJsScriptNode(externs);
+    }
+
     if (mode == Mode.LINT_AND_REWRITE) {
       if (googJsScriptNode != null) {
         exportsFinder = new FindExports(googJsScriptNode);

src/com/google/javascript/jscomp/deps/JsFileLineParser.java

@@ -136,12 +136,15 @@ void doParse(String filePath, Reader fileContents) {
         ++lineNum;
         try {
           String revisedLine = line;
+          String revisedBlockCommentLine = "";
           if (inMultilineComment) {
             int endOfComment = revisedLine.indexOf("*/");
             if (endOfComment != -1) {
+              revisedBlockCommentLine = revisedLine.substring(0, endOfComment + 2);
               revisedLine = revisedLine.substring(endOfComment + 2);
               inMultilineComment = false;
             } else {
+              revisedBlockCommentLine = line;
               revisedLine = "";
             }
           }
@@ -156,14 +159,18 @@ void doParse(String filePath, Reader fileContents) {
                 revisedLine = revisedLine.substring(0, startOfLineComment);
                 break;
               } else if (startOfMultilineComment != -1) {
-                int endOfMultilineComment = revisedLine.indexOf("*/",
-                    startOfMultilineComment + 2);
+                int endOfMultilineComment = revisedLine.indexOf("*/", startOfMultilineComment + 2);
                 if (endOfMultilineComment == -1) {
-                  revisedLine = revisedLine.substring(
-                      0, startOfMultilineComment);
+                  revisedBlockCommentLine = revisedLine.substring(startOfMultilineComment);
+                  revisedLine = revisedLine.substring(0, startOfMultilineComment);
                   inMultilineComment = true;
                   break;
                 } else {
+                  if (!parseBlockCommentLine(
+                          revisedLine.substring(startOfMultilineComment, endOfMultilineComment + 2))
+                      && shortcutMode) {
+                    break;
+                  }
                   revisedLine =
                       revisedLine.substring(0, startOfMultilineComment) +
                       revisedLine.substring(endOfMultilineComment + 2);
@@ -174,6 +181,12 @@ void doParse(String filePath, Reader fileContents) {
             }
           }
 
+          if (!revisedBlockCommentLine.isEmpty()) {
+            if (!parseBlockCommentLine(revisedBlockCommentLine) && shortcutMode) {
+              break;
+            }
+          }
+
           if (!revisedLine.isEmpty()) {
             // This check for shortcut mode should be redundant, but
             // it's done for safety reasons.
@@ -208,12 +221,15 @@ void doParse(String filePath, Reader fileContents) {
    */
   abstract boolean parseLine(String line) throws ParseException;
 
+  boolean parseBlockCommentLine(String line) {
+    return true;
+  }
+
   /**
    * Parses a JS string literal.
    *
    * @param jsStringLiteral The literal. Must look like "asdf" or 'asdf'
-   * @throws ParseException Thrown if there is a string literal that cannot be
-   *     parsed.
+   * @throws ParseException Thrown if there is a string literal that cannot be parsed.
    */
   String parseJsString(String jsStringLiteral) throws ParseException {
     valueMatcher.reset(jsStringLiteral);

src/com/google/javascript/jscomp/deps/JsFileParser.java

@@ -87,8 +87,8 @@ public final class JsFileParser extends JsFileLineParser {
    */
   private static final Pattern ES6_EXPORT_PATTERN = Pattern.compile("^export\\b");
 
-  /** The first non-comment line of base.js */
-  private static final String BASE_JS_START = "var COMPILED = false;";
+  /** Line in comment indicating that the file is Closure's base.js. */
+  private static final String PROVIDES_GOOG_COMMENT = "@provideGoog";
 
   /** The start of a bundled goog.module, i.e. one that is wrapped in a goog.loadModule call */
   private static final String BUNDLED_GOOG_MODULE_START = "goog.loadModule(function(";
@@ -245,10 +245,16 @@ private void setModuleType(ModuleType type) {
     moduleType = type;
   }
 
-  /**
-   * Parses a line of JavaScript, extracting goog.provide and goog.require
-   * information.
-   */
+  @Override
+  protected boolean parseBlockCommentLine(String line) {
+    if (includeGoogBase && line.contains(PROVIDES_GOOG_COMMENT)) {
+      provides.add("goog");
+      return false;
+    }
+    return true;
+  }
+
+  /** Parses a line of JavaScript, extracting goog.provide and goog.require information. */
   @Override
   protected boolean parseLine(String line) throws ParseException {
     boolean lineHasProvidesOrRequires = false;
@@ -308,12 +314,6 @@ protected boolean parseLine(String line) throws ParseException {
           }
         }
       }
-    } else if (includeGoogBase && line.startsWith(BASE_JS_START) &&
-               provides.isEmpty() && requires.isEmpty()) {
-      provides.add("goog");
-
-      // base.js can't provide or require anything else.
-      return false;
     }
 
     if (line.startsWith("import") || line.startsWith("export")) {

test/com/google/javascript/jscomp/CommandLineRunnerTest.java

@@ -813,10 +813,9 @@ public void testSourceSortingOn() {
   public void testSourceSortingOn2() {
     test(
         new String[] {
-          "goog.provide('a');",
-          "goog.require('a');\n/** This is base.js */\nvar COMPILED = false;",
+          "goog.provide('a');", "goog.require('a');",
         },
-        new String[] {"var a={};", "var COMPILED=!1"});
+        new String[] {"var a={};", ""});
   }
 
   @Test
@@ -826,7 +825,7 @@ public void testSourceSortingOn3() {
     test(
         new String[] {
           "goog.addDependency('sym', [], []);\nvar x = 3;",
-          "/** This is base.js */\nvar COMPILED = false;",
+          "/** This is base.js @provideGoog */ var COMPILED = false;",
         },
         new String[] {"var COMPILED = !1;", "var x = 3;"});
   }
@@ -940,7 +939,7 @@ public void testSourcePruningOn7() {
     args.add("--dependency_mode=LOOSE");
     test(
         new String[] {
-          "/** This is base.js */\nvar COMPILED = false;",
+          "/** This is base.js @provideGoog */ var COMPILED = false;",
         },
         new String[] {
           "var COMPILED = !1;",
@@ -1055,19 +1054,20 @@ public void testOnlyClosureDependenciesEmptyEntryPoints() throws Exception {
   public void testOnlyClosureDependenciesOneEntryPoint() {
     args.add("--dependency_mode=STRICT");
     args.add("--entry_point=goog:beer");
-    test(new String[] {
+    test(
+        new String[] {
           "goog.require('beer'); var beerRequired = 1;",
           "goog.provide('beer');\ngoog.require('hops');\nvar beerProvided = 1;",
           "goog.provide('hops'); var hopsProvided = 1;",
           "goog.provide('scotch'); var scotchProvided = 1;",
           "goog.require('scotch');\nvar includeFileWithoutProvides = 1;",
-          "/** This is base.js */\nvar COMPILED = false;",
-         },
-         new String[] {
-           "var COMPILED = !1;",
-           "var hops = {}, hopsProvided = 1;",
-           "var beer = {}, beerProvided = 1;"
-         });
+          "/** This is base.js @provideGoog */ var COMPILED = false;",
+        },
+        new String[] {
+          "var COMPILED = !1;",
+          "var hops = {}, hopsProvided = 1;",
+          "var beer = {}, beerProvided = 1;"
+        });
   }
 
   @Test

test/com/google/javascript/jscomp/CompilerTestCase.java

@@ -2236,6 +2236,21 @@ protected static Externs externs(String... srcTexts) {
     return new Externs(createSources("externs", srcTexts));
   }
 
+  protected static Externs externs(SourceFile... externs) {
+    // Copy SourceFile objects to prevent the externs bit from polluting tests.
+    return new Externs(
+        Arrays.stream(externs)
+            .map(
+                f -> {
+                  try {
+                    return SourceFile.fromCode(f.getName(), f.getCode());
+                  } catch (IOException e) {
+                    throw new RuntimeException(e);
+                  }
+                })
+            .collect(ImmutableList.toImmutableList()));
+  }
+
   protected static Externs externs(List<SourceFile> files) {
     return new Externs(files);
   }

test/com/google/javascript/jscomp/RewriteGoogJsImportsTest.java

@@ -30,9 +30,9 @@
 
 @RunWith(JUnit4.class)
 public final class RewriteGoogJsImportsTest extends CompilerTestCase {
-  // JsFileParser determines if this file is base.js by looking at the first line of the file.
+  // JsFileParser determines if this file is base.js by looking at the first comment of the file.
   private static final SourceFile BASE =
-      SourceFile.fromCode("/closure/base.js", "var COMPILED = false;");
+      SourceFile.fromCode("/closure/base.js", "/** @provideGoog */");
 
   private static final SourceFile GOOG =
       SourceFile.fromCode(
@@ -102,6 +102,24 @@ public void testImportStar() {
                     "use(goog.require, goog.foo, goog.MyClass, goog.constant);"))));
   }
 
+  @Test
+  public void testGoogAndBaseInExterns() {
+    test(
+        externs(BASE, GOOG),
+        srcs(
+            SourceFile.fromCode(
+                "testcode",
+                lines(
+                    "import * as goog from './closure/goog.js';",
+                    "use(goog.require, goog.foo, goog.MyClass, goog.constant);"))),
+        expected(
+            SourceFile.fromCode(
+                "testcode",
+                lines(
+                    "import './closure/goog.js';",
+                    "use(goog.require, goog.foo, goog.MyClass, goog.constant);"))));
+  }
+
   @Test
   public void testBadPropertyAccess() {
     test(

test/com/google/javascript/jscomp/deps/JsFileLineParserTest.java

@@ -22,7 +22,6 @@
 import com.google.javascript.jscomp.PrintStreamErrorManager;
 import java.io.StringReader;
 import junit.framework.TestCase;
-import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -35,16 +34,6 @@
 @RunWith(JUnit4.class)
 public final class JsFileLineParserTest extends TestCase {
 
-  TestParser parser;
-  private ErrorManager errorManager;
-
-  @Override
-  @Before
-  public void setUp() {
-    errorManager = new PrintStreamErrorManager(System.err);
-    parser = new TestParser(errorManager);
-  }
-
   @Test
   public void testSingleLine1() {
     assertStrip("2", "// 1\n2");
@@ -95,13 +84,40 @@ public void testMixedLine2() {
     assertStrip("1 34", "1/** // 2 **/ 3\n4");
   }
 
+  @Test
+  public void testBlockComment() {
+    assertBlocks("/** one line */", "/** one line */");
+  }
+
+  @Test
+  public void testInlineBlockComment() {
+    assertBlocks("/** one line */", "var x; /** one line */");
+    assertBlocks("/** one line */", "/** one line */ var y;");
+    assertBlocks("/** one line */", "var x; /** one line */ var y;");
+  }
+
+  @Test
+  public void testMultipleBlockComments() {
+    assertBlocks("/** first *//** * second */", "/** first */\n/**\n * second\n */");
+  }
+
   private void assertStrip(String expected, String input) {
+    ErrorManager errorManager = new PrintStreamErrorManager(System.err);
+    TestParser parser = new TestParser(errorManager);
     parser.doParse("file", new StringReader(input));
     assertThat(parser.toString()).isEqualTo(expected);
   }
 
+  private void assertBlocks(String expected, String input) {
+    ErrorManager errorManager = new PrintStreamErrorManager(System.err);
+    TestParser parser = new TestParser(errorManager);
+    parser.doParse("file", new StringReader(input));
+    assertThat(parser.comments.toString()).isEqualTo(expected);
+  }
+
   private static class TestParser extends JsFileLineParser {
     StringBuilder sb = new StringBuilder();
+    StringBuilder comments = new StringBuilder();
 
     TestParser(ErrorManager errorManager) {
       super(errorManager);
@@ -113,6 +129,12 @@ boolean parseLine(String line) {
       return true;
     }
 
+    @Override
+    boolean parseBlockCommentLine(String line) {
+      comments.append(line);
+      return true;
+    }
+
     @Override
     public String toString() {
       return sb.toString();

test/com/google/javascript/jscomp/deps/JsFileParserTest.java

@@ -498,10 +498,7 @@ public void testShortcutMode3() {
 
   @Test
   public void testIncludeGoog1() {
-    String contents = "/**\n" +
-        " * the first constant in base.js\n" +
-        " */\n" +
-        "var COMPILED = false;\n";
+    String contents = "/**\n" + " * @provideGoog\n" + " */\n";
 
     DependencyInfo expected = SimpleDependencyInfo.builder(CLOSURE_PATH, SRC_PATH)
         .setProvides(ImmutableList.of("goog"))