Skip to content

Commit

Permalink
Renaming CheckRequiresForConstructors to CheckMissingAndExtraRequires…
Browse files Browse the repository at this point in the history
… for readability's sake.

Part of verification/adding of ES6 module compatibility to compiler.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=160650967
  • Loading branch information
hjkaria authored and brad4d committed Jul 5, 2017
1 parent 511e40b commit 362d8fe
Show file tree
Hide file tree
Showing 8 changed files with 54 additions and 55 deletions.
Expand Up @@ -37,23 +37,24 @@
import javax.annotation.Nullable;

/**
* This pass walks the AST to create a Collection of 'new' nodes and
* 'goog.require' nodes. It reconciles these Collections, creating a
* warning for each discrepancy.
* Walks the AST looking for usages of qualified names, and 'goog.require's of those names.
* Then, reconciles the two lists, and reports warning for any discrepancies.
*
* <p>The rules on when a warning is reported are: <ul>
* <li>Type is referenced in code → goog.require is required
* (missingRequires check fails if it's not there)
* <li>Type is referenced in an @extends or @implements → goog.require is required
* (missingRequires check fails if it's not there)
* <li>Type is referenced in other JsDoc (@type etc) → goog.require is optional
* (don't warn, regardless of if it is there)
* <li>Type is not referenced at all → goog.require is forbidden
* (extraRequires check fails if it is there)
* <p>The rules on when a warning is reported are:
*
* <ul>
* <li>Type is referenced in code → goog.require is required (missingRequires check fails if it's
* not there)
* <li>Type is referenced in an @extends or @implements → goog.require is required
* (missingRequires check fails if it's not there)
* <li>Type is referenced in other JsDoc (@type etc) → goog.require is optional (don't warn,
* regardless of if it is there)
* <li>Type is not referenced at all → goog.require is forbidden (extraRequires check fails if it
* is there)
* </ul>
*
*/
public class CheckRequiresForConstructors implements HotSwapCompilerPass, NodeTraversal.Callback {
public class CheckMissingAndExtraRequires implements HotSwapCompilerPass, NodeTraversal.Callback {
private final AbstractCompiler compiler;
private final CodingConvention codingConvention;

Expand Down Expand Up @@ -106,7 +107,7 @@ static enum Mode {
ImmutableSet.of(
"goog.testing.asserts", "goog.testing.jsunit", "goog.testing.JsTdTestCaseAdapter");

CheckRequiresForConstructors(AbstractCompiler compiler, Mode mode) {
CheckMissingAndExtraRequires(AbstractCompiler compiler, Mode mode) {
this.compiler = compiler;
this.mode = mode;
this.codingConvention = compiler.getCodingConvention();
Expand Down
6 changes: 3 additions & 3 deletions src/com/google/javascript/jscomp/DefaultPassConfig.java
Expand Up @@ -1171,11 +1171,11 @@ private void assertValidOrderForOptimizations(List<PassFactory> optimizations) {

/** Checks that all constructed classes are goog.require()d. */
private final HotSwapPassFactory checkRequires =
new HotSwapPassFactory("checkRequires", true) {
new HotSwapPassFactory("checkMissingAndExtraRequires", true) {
@Override
protected HotSwapCompilerPass create(AbstractCompiler compiler) {
return new CheckRequiresForConstructors(
compiler, CheckRequiresForConstructors.Mode.FULL_COMPILE);
return new CheckMissingAndExtraRequires(
compiler, CheckMissingAndExtraRequires.Mode.FULL_COMPILE);
}

@Override
Expand Down
24 changes: 13 additions & 11 deletions src/com/google/javascript/jscomp/DiagnosticGroups.java
Expand Up @@ -487,8 +487,8 @@ public DiagnosticGroup forName(String name) {
ClosureRewriteModule.MISSING_MODULE_OR_PROVIDE);

public static final DiagnosticGroup MISSING_REQUIRE =
DiagnosticGroups.registerGroup("missingRequire",
CheckRequiresForConstructors.MISSING_REQUIRE_WARNING);
DiagnosticGroups.registerGroup(
"missingRequire", CheckMissingAndExtraRequires.MISSING_REQUIRE_WARNING);

/**
* A set of diagnostics expected when parsing and type checking partial programs. Useful for clutz
Expand All @@ -511,19 +511,21 @@ public DiagnosticGroup forName(String name) {
DiagnosticGroup.forType(Es6ExternsCheck.MISSING_ES6_EXTERNS));

public static final DiagnosticGroup STRICT_MISSING_REQUIRE =
DiagnosticGroups.registerGroup("strictMissingRequire",
CheckRequiresForConstructors.MISSING_REQUIRE_WARNING,
CheckRequiresForConstructors.MISSING_REQUIRE_FOR_GOOG_SCOPE,
CheckRequiresForConstructors.MISSING_REQUIRE_STRICT_WARNING);
DiagnosticGroups.registerGroup(
"strictMissingRequire",
CheckMissingAndExtraRequires.MISSING_REQUIRE_WARNING,
CheckMissingAndExtraRequires.MISSING_REQUIRE_FOR_GOOG_SCOPE,
CheckMissingAndExtraRequires.MISSING_REQUIRE_STRICT_WARNING);

public static final DiagnosticGroup STRICT_REQUIRES =
DiagnosticGroups.registerGroup("legacyGoogScopeRequire",
CheckRequiresForConstructors.MISSING_REQUIRE_FOR_GOOG_SCOPE,
CheckRequiresForConstructors.EXTRA_REQUIRE_WARNING);
DiagnosticGroups.registerGroup(
"legacyGoogScopeRequire",
CheckMissingAndExtraRequires.MISSING_REQUIRE_FOR_GOOG_SCOPE,
CheckMissingAndExtraRequires.EXTRA_REQUIRE_WARNING);

public static final DiagnosticGroup EXTRA_REQUIRE =
DiagnosticGroups.registerGroup("extraRequire",
CheckRequiresForConstructors.EXTRA_REQUIRE_WARNING);
DiagnosticGroups.registerGroup(
"extraRequire", CheckMissingAndExtraRequires.EXTRA_REQUIRE_WARNING);

@GwtIncompatible("java.util.regex")
public static final DiagnosticGroup MISSING_GETCSSNAME =
Expand Down
4 changes: 2 additions & 2 deletions src/com/google/javascript/jscomp/LintPassConfig.java
Expand Up @@ -67,8 +67,8 @@ protected CompilerPass create(AbstractCompiler compiler) {
new CheckMissingSuper(compiler),
new CheckPrimitiveAsObject(compiler),
new CheckRequiresAndProvidesSorted(compiler),
new CheckRequiresForConstructors(
compiler, CheckRequiresForConstructors.Mode.SINGLE_FILE),
new CheckMissingAndExtraRequires(
compiler, CheckMissingAndExtraRequires.Mode.SINGLE_FILE),
new CheckSideEffects(
compiler, /* report */ true, /* protectSideEffectFreeCode */ false),
new CheckUnusedLabels(compiler),
Expand Down
6 changes: 3 additions & 3 deletions src/com/google/javascript/refactoring/FixingErrorManager.java
Expand Up @@ -16,9 +16,9 @@

package com.google.javascript.refactoring;

import static com.google.javascript.jscomp.CheckRequiresForConstructors.EXTRA_REQUIRE_WARNING;
import static com.google.javascript.jscomp.CheckRequiresForConstructors.MISSING_REQUIRE_STRICT_WARNING;
import static com.google.javascript.jscomp.CheckRequiresForConstructors.MISSING_REQUIRE_WARNING;
import static com.google.javascript.jscomp.CheckMissingAndExtraRequires.EXTRA_REQUIRE_WARNING;
import static com.google.javascript.jscomp.CheckMissingAndExtraRequires.MISSING_REQUIRE_STRICT_WARNING;
import static com.google.javascript.jscomp.CheckMissingAndExtraRequires.MISSING_REQUIRE_WARNING;
import static com.google.javascript.jscomp.ClosureCheckModule.JSDOC_REFERENCE_TO_SHORT_IMPORT_BY_LONG_NAME_INCLUDING_SHORT_NAME;
import static com.google.javascript.jscomp.ClosureCheckModule.REFERENCE_TO_SHORT_IMPORT_BY_LONG_NAME_INCLUDING_SHORT_NAME;

Expand Down
10 changes: 4 additions & 6 deletions test/com/google/javascript/jscomp/ExtraRequireTest.java
Expand Up @@ -16,15 +16,13 @@

package com.google.javascript.jscomp;

import static com.google.javascript.jscomp.CheckRequiresForConstructors.EXTRA_REQUIRE_WARNING;
import static com.google.javascript.jscomp.CheckMissingAndExtraRequires.EXTRA_REQUIRE_WARNING;

import com.google.common.collect.ImmutableList;
import com.google.javascript.jscomp.CompilerOptions.LanguageMode;
import java.util.List;

/**
* Tests for the "extra requires" check in {@link CheckRequiresForConstructors}.
*/
/** Tests for the "extra requires" check in {@link CheckMissingAndExtraRequires}. */
public final class ExtraRequireTest extends CompilerTestCase {
public ExtraRequireTest() {
super();
Expand All @@ -44,8 +42,8 @@ protected CompilerOptions getOptions(CompilerOptions options) {

@Override
protected CompilerPass getProcessor(Compiler compiler) {
return new CheckRequiresForConstructors(compiler,
CheckRequiresForConstructors.Mode.FULL_COMPILE);
return new CheckMissingAndExtraRequires(
compiler, CheckMissingAndExtraRequires.Mode.FULL_COMPILE);
}

public void testNoWarning() {
Expand Down
18 changes: 9 additions & 9 deletions test/com/google/javascript/jscomp/MissingRequireTest.java
Expand Up @@ -17,27 +17,27 @@
package com.google.javascript.jscomp;

import static com.google.common.truth.Truth.assertThat;
import static com.google.javascript.jscomp.CheckRequiresForConstructors.MISSING_REQUIRE_FOR_GOOG_SCOPE;
import static com.google.javascript.jscomp.CheckRequiresForConstructors.MISSING_REQUIRE_STRICT_WARNING;
import static com.google.javascript.jscomp.CheckRequiresForConstructors.MISSING_REQUIRE_WARNING;
import static com.google.javascript.jscomp.CheckMissingAndExtraRequires.MISSING_REQUIRE_FOR_GOOG_SCOPE;
import static com.google.javascript.jscomp.CheckMissingAndExtraRequires.MISSING_REQUIRE_STRICT_WARNING;
import static com.google.javascript.jscomp.CheckMissingAndExtraRequires.MISSING_REQUIRE_WARNING;

import com.google.common.collect.ImmutableList;
import com.google.javascript.jscomp.CompilerOptions.LanguageMode;
import java.util.List;

/**
* Tests for the "missing requires" check in {@link CheckRequiresForConstructors}.
* Tests for the "missing requires" check in {@link CheckMissingAndExtraRequires}.
*
*/

public final class MissingRequireTest extends CompilerTestCase {
private CheckRequiresForConstructors.Mode mode;
private CheckMissingAndExtraRequires.Mode mode;

@Override
protected void setUp() throws Exception {
super.setUp();
setAcceptedLanguage(LanguageMode.ECMASCRIPT_2017);
mode = CheckRequiresForConstructors.Mode.FULL_COMPILE;
mode = CheckMissingAndExtraRequires.Mode.FULL_COMPILE;
}

@Override
Expand All @@ -48,7 +48,7 @@ protected CompilerOptions getOptions(CompilerOptions options) {

@Override
protected CompilerPass getProcessor(Compiler compiler) {
return new CheckRequiresForConstructors(compiler, mode);
return new CheckMissingAndExtraRequires(compiler, mode);
}

private void testMissingRequireStrict(String js, String warningText) {
Expand Down Expand Up @@ -470,7 +470,7 @@ public void testEs6ClassExtendsSomethingInExternsWithNS() {
}

public void testFailConstant() {
mode = CheckRequiresForConstructors.Mode.SINGLE_FILE;
mode = CheckMissingAndExtraRequires.Mode.SINGLE_FILE;
testMissingRequireStrict(
"goog.require('example.Class'); alert(example.Constants.FOO);",
"missing require: 'example.Constants'");
Expand All @@ -480,7 +480,7 @@ public void testFailConstant() {
}

public void testFailGoogArray() {
mode = CheckRequiresForConstructors.Mode.SINGLE_FILE;
mode = CheckMissingAndExtraRequires.Mode.SINGLE_FILE;
testMissingRequireStrict(
"console.log(goog.array.contains([1, 2, 3], 4));",
"missing require: 'goog.array'");
Expand Down
Expand Up @@ -15,14 +15,12 @@
*/
package com.google.javascript.jscomp;

import static com.google.javascript.jscomp.CheckRequiresForConstructors.EXTRA_REQUIRE_WARNING;
import static com.google.javascript.jscomp.CheckRequiresForConstructors.MISSING_REQUIRE_WARNING;
import static com.google.javascript.jscomp.CheckMissingAndExtraRequires.EXTRA_REQUIRE_WARNING;
import static com.google.javascript.jscomp.CheckMissingAndExtraRequires.MISSING_REQUIRE_WARNING;

import com.google.javascript.jscomp.CompilerOptions.LanguageMode;

/**
* Tests for {@link CheckRequiresForConstructors} in single-file mode.
*/
/** Tests for {@link CheckMissingAndExtraRequires} in single-file mode. */
public final class SingleFileCheckRequiresTest extends CompilerTestCase {
@Override
protected void setUp() throws Exception {
Expand All @@ -39,8 +37,8 @@ protected CompilerOptions getOptions(CompilerOptions options) {

@Override
protected CompilerPass getProcessor(final Compiler compiler) {
return new CheckRequiresForConstructors(compiler,
CheckRequiresForConstructors.Mode.SINGLE_FILE);
return new CheckMissingAndExtraRequires(
compiler, CheckMissingAndExtraRequires.Mode.SINGLE_FILE);
}

public void testReferenceToSingleName() {
Expand Down

0 comments on commit 362d8fe

Please sign in to comment.