Skip to content

Commit

Permalink
Fix incremental type checking for goog modules (#615)
Browse files Browse the repository at this point in the history
 - Add CheckTypeSummaryWarningsGuard to ignore errors from ijs files.
 - Wrap modules in goog.modules for ijs files and sort them.
 - Add input delimiters to help with debugging.
  • Loading branch information
gkdn authored May 14, 2024
1 parent 78563e2 commit 26321bf
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 2 deletions.
29 changes: 27 additions & 2 deletions closure/compiler/test/linting/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@ load("//closure:defs.bzl", "closure_js_library")

closure_js_library(
name = "love",
srcs = ["love.js"],
srcs = [
"love.js",
"somemodule.js",
],
internal_expect_failure = True,
)

Expand All @@ -31,6 +34,12 @@ file_test(
regexp = "JSC_MISSING_JSDOC",
)

file_test(
name = "checksGoogModule",
file = "love-stderr.txt",
regexp = "somemodule.js",
)

closure_js_library(
name = "superfluousSuppress",
srcs = ["love.js"],
Expand All @@ -47,7 +56,10 @@ file_test(

closure_js_library(
name = "lenient_love",
srcs = ["love.js"],
srcs = [
"love.js",
"somemodule.js",
],
lenient = True,
)

Expand All @@ -63,3 +75,16 @@ file_test(
# This is a dummy file to trigger per library type check
file = "lenient_love_typecheck",
)

closure_js_library(
name = "depends_lenient",
srcs = ["dummy.js"],
deps = [":lenient_love"],
)

file_test(
name = "depends_lenient_no_type_check_errors",
content = "",
# This is a dummy file to trigger per library type check
file = "depends_lenient_typecheck",
)
Empty file.
23 changes: 23 additions & 0 deletions closure/compiler/test/linting/somemodule.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright 2024 The Closure Rules Authors. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

goog.module('somemodule');


// I'm not documented!
function foo(a, b) {
return a + b;
}

exports = foo;
8 changes: 8 additions & 0 deletions java/com/google/javascript/jscomp/JsCheckerPassConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ final class JsCheckerPassConfig extends PassConfig.PassConfigDelegate {
this.checks.maybeAdd(closureRewriteClass());
this.checks.maybeAdd(lateLintChecks());
this.checks.maybeAdd(ijsGeneration());
this.checks.maybeAdd(whitespaceWrapGoogModules());
}

@Override
Expand Down Expand Up @@ -137,4 +138,11 @@ private PassFactory ijsGeneration() {
.setInternalFactory((compiler) -> new ConvertToTypedInterface(compiler))
.build();
}

private PassFactory whitespaceWrapGoogModules() {
return PassFactory.builder()
.setName("whitespaceWrapGoogModules")
.setInternalFactory(WhitespaceWrapGoogModules::new)
.build();
}
}
2 changes: 2 additions & 0 deletions java/com/google/javascript/jscomp/JsCompilerRunner.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import com.google.common.collect.Iterables;
import com.google.javascript.jscomp.deps.ModuleLoader;
import com.google.javascript.jscomp.ijs.CheckTypeSummaryWarningsGuard;
import java.io.IOException;

final class JsCompilerRunner extends CommandLineRunner {
Expand Down Expand Up @@ -56,6 +57,7 @@ protected Compiler createCompiler() {
protected CompilerOptions createOptions() {
CompilerOptions options = super.createOptions();
options.setExportTestFunctions(exportTestFunctions);
options.addWarningsGuard(new CheckTypeSummaryWarningsGuard(CheckLevel.OFF));
options.addWarningsGuard(warnings);
options.setModuleResolutionMode(ModuleLoader.ResolutionMode.NODE);
return options;
Expand Down

0 comments on commit 26321bf

Please sign in to comment.