Skip to content

Commit

Permalink
[NTI] Suppress CheckAccessControls const warnings when run with NTI.
Browse files Browse the repository at this point in the history
Users opting into NTI will likely prefer the stricter const checks that
NTI provides over those of CheckAccessControls.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=128632773
  • Loading branch information
aravind-pg authored and blickly committed Jul 28, 2016
1 parent b3018d9 commit 9cf80e1
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 0 deletions.
3 changes: 3 additions & 0 deletions src/com/google/javascript/jscomp/Compiler.java
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,9 @@ protected void reconcileOptionsWithGuards() {
// But we do not want to see the warnings from OTI.
if (options.getNewTypeInference() && options.getRunOTIAfterNTI()) {
options.checkTypes = true;
// Supress warnings from the const checks of CheckAccessControls so as to avoid
// duplication.
options.setWarningLevel(DiagnosticGroups.ACCESS_CONTROLS_CONST, CheckLevel.OFF);
if (!options.reportOTIErrorsUnderNTI) {
options.setWarningLevel(
DiagnosticGroups.OLD_CHECK_TYPES,
Expand Down
8 changes: 8 additions & 0 deletions src/com/google/javascript/jscomp/CompilerOptions.java
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,14 @@ public boolean shouldGenerateTypedExterns() {

private boolean useNewTypeInference;

/**
* When this flag is enabled, we run OTI after NTI so that the AST is annotated with the old types
* instead of the new and susbsequent passes that use type information continue to get it from
* the old types. When disabled, we don't run OTI at all, so the AST stays annotated
* with the new types, and that is what subsequent passes use. It should only ever be disabled in
* tests that want to verify that a pass works with the new types.
* TODO(aravindpg): move this flag into CompilerTestCase in some form.
*/
private boolean runOTIAfterNTI = true;

/**
Expand Down
5 changes: 5 additions & 0 deletions src/com/google/javascript/jscomp/DiagnosticGroups.java
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,11 @@ public DiagnosticGroup forName(String name) {
NewTypeInference.CONST_PROPERTY_REASSIGNED,
NewTypeInference.CONST_PROPERTY_DELETED);

static final DiagnosticGroup ACCESS_CONTROLS_CONST =
DiagnosticGroups.registerGroup("accessControlsConst",
CheckAccessControls.CONST_PROPERTY_DELETED,
CheckAccessControls.CONST_PROPERTY_REASSIGNED_VALUE);

public static final DiagnosticGroup CONSTANT_PROPERTY =
DiagnosticGroups.registerGroup("constantProperty",
CheckAccessControls.CONST_PROPERTY_DELETED,
Expand Down
13 changes: 13 additions & 0 deletions test/com/google/javascript/jscomp/IntegrationTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -778,6 +778,19 @@ public void testBothTypeCheckersRunNoDupWarning() {
"})();"));
}

public void testNTIConstWarningsOverrideAccessControls() {
CompilerOptions options = createCompilerOptions();
options.setCheckTypes(true);
String js =
LINE_JOINER.join(
"/** @constructor */ function Foo(name) {}",
"/** @const */ Foo.prop = 1;",
"Foo.prop = 2;");
test(options, js, CheckAccessControls.CONST_PROPERTY_REASSIGNED_VALUE);
options.setNewTypeInference(true);
test(options, js, NewTypeInference.CONST_PROPERTY_REASSIGNED);
}

public void testNTInoMaskTypeParseError() {
CompilerOptions options = createCompilerOptions();
options.setCheckTypes(true);
Expand Down

0 comments on commit 9cf80e1

Please sign in to comment.