Re-enable skylint checking
Updated skylint to latest, which requires fixing deprecated + operator on depset.
Closes #290
PiperOrigin-RevId: 215215883
diff --git a/.circleci/config.yml b/.circleci/config.yml
index 83d2cd0..91dfabf 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -112,10 +112,7 @@
- run: .circleci/setup_cache.sh
- run: sudo cp .circleci/bazel.rc /etc/bazel.bazelrc
- *setup-bazel-remote-cache
- # Disable skylint for now as we're on an older version that is not
- # compatible with Bazel 0.17.1 and the newer version has deprecated checks
- # that fail in this repo that can't be turned off
- # - run: yarn skylint
+ - run: yarn skylint
workflows:
version: 2
diff --git a/internal/build_defs.bzl b/internal/build_defs.bzl
index f398b10..ea980cf 100644
--- a/internal/build_defs.bzl
+++ b/internal/build_defs.bzl
@@ -66,6 +66,7 @@
def _compile_action(ctx, inputs, outputs, tsconfig_file, node_opts, description = "prodmode"):
externs_files = []
+ action_inputs = []
action_outputs = []
for output in outputs:
if output.basename.endswith(".externs.js"):
@@ -84,19 +85,18 @@
if not action_outputs:
return struct()
- node_module_inputs = _filter_ts_inputs(ctx.files.node_modules)
+ action_inputs.extend(_filter_ts_inputs(ctx.files.node_modules))
# Also include files from npm fine grained deps as action_inputs.
# These deps are identified by the NodeModuleInfo provider.
for d in ctx.attr.deps:
if NodeModuleInfo in d:
- node_module_inputs.extend(_filter_ts_inputs(d.files))
- action_inputs = depset(node_module_inputs, transitive = [inputs])
+ action_inputs.extend(_filter_ts_inputs(d.files))
if ctx.file.tsconfig:
- action_inputs += [ctx.file.tsconfig]
+ action_inputs.append(ctx.file.tsconfig)
if TsConfigInfo in ctx.attr.tsconfig:
- action_inputs += ctx.attr.tsconfig[TsConfigInfo].deps
+ action_inputs.extend(ctx.attr.tsconfig[TsConfigInfo].deps)
# Pass actual options for the node binary in the special "--node_options" argument.
arguments = ["--node_options=%s" % opt for opt in node_opts]
@@ -115,7 +115,7 @@
ctx.actions.run(
progress_message = "Compiling TypeScript (%s) %s" % (description, ctx.label),
mnemonic = mnemonic,
- inputs = action_inputs,
+ inputs = depset(action_inputs, transitive = [inputs]),
outputs = action_outputs,
# Use the built-in shell environment
# Allow for users who set a custom shell that can locate standard binaries like tr and uname
@@ -132,7 +132,7 @@
return struct(
label = ctx.label,
tsconfig = tsconfig_file,
- inputs = action_inputs,
+ inputs = depset(action_inputs, transitive = [inputs]),
outputs = action_outputs,
compiler = ctx.executable.compiler,
)
diff --git a/internal/common/compilation.bzl b/internal/common/compilation.bzl
index 1d4c592..b811c78 100644
--- a/internal/common/compilation.bzl
+++ b/internal/common/compilation.bzl
@@ -20,6 +20,8 @@
BASE_ATTRIBUTES = dict()
+_DEBUG = False
+
DEPS_ASPECTS = [
module_mappings_aspect,
]
@@ -69,6 +71,34 @@
fail("%s is neither a TypeScript nor a JS producing rule.\n%s\n" % (dep.label, allowed_deps_msg))
+_DEPSET_TYPE = type(depset())
+
+def _check_ts_provider(dep):
+ """Verifies the type shape of the typescript provider in dep, if it has one.
+ """
+
+ # Under Bazel, some third parties have created typescript providers which may not be compatible.
+ # Rather than users getting an obscure error later, explicitly check them and point to the
+ # target that created the bad provider.
+ # TODO(alexeagle): remove this after some transition period, maybe mid-2019
+ if hasattr(dep, "typescript"):
+ if type(dep.typescript.declarations) != _DEPSET_TYPE:
+ fail("typescript provider in %s defined declarations as a %s rather than a depset" % (
+ dep.label,
+ type(dep.typescript.declarations),
+ ))
+ if type(dep.typescript.transitive_declarations) != _DEPSET_TYPE:
+ fail("typescript provider in %s defined transitive_declarations as a %s rather than a depset" % (
+ dep.label,
+ type(dep.typescript.transitive_declarations),
+ ))
+ if type(dep.typescript.type_blacklisted_declarations) != _DEPSET_TYPE:
+ fail("typescript provider in %s defined type_blacklisted_declarations as a %s rather than a depset" % (
+ dep.label,
+ type(dep.typescript.type_blacklisted_declarations),
+ ))
+ return dep
+
def _collect_dep_declarations(ctx, deps):
"""Collects .d.ts files from typescript and javascript dependencies.
@@ -88,20 +118,37 @@
# .d.ts files whose types tsickle will not emit (used for ts_declaration(generate_externs=False).
type_blacklisted_declarations = depset()
+ deps_and_helpers = [
+ _check_ts_provider(dep)
+ for dep in deps + getattr(ctx.attr, "_helpers", [])
+ if hasattr(dep, "typescript")
+ ]
- for dep in deps + getattr(ctx.attr, "_helpers", []):
- if hasattr(dep, "typescript"):
- direct_deps_declarations += dep.typescript.declarations
- transitive_deps_declarations += dep.typescript.transitive_declarations
- type_blacklisted_declarations += dep.typescript.type_blacklisted_declarations
+ # .d.ts files from direct dependencies, ok for strict deps
+ direct_deps_declarations = [dep.typescript.declarations for dep in deps_and_helpers]
+
+ # all reachable .d.ts files from dependencies.
+ transitive_deps_declarations = [
+ dep.typescript.transitive_declarations
+ for dep in deps_and_helpers
+ ]
+
+ # .d.ts files whose types tsickle will not emit (used for ts_declaration(generate_externs=False).
+ type_blacklisted_declarations = [
+ dep.typescript.type_blacklisted_declarations
+ for dep in deps_and_helpers
+ ]
# If a tool like github.com/angular/clutz can create .d.ts from type annotated .js
# its output will be collected here.
return struct(
- direct = direct_deps_declarations,
- transitive = transitive_deps_declarations,
- type_blacklisted = type_blacklisted_declarations,
+ direct = depset(transitive = direct_deps_declarations),
+ transitive = depset(
+ [extra for extra in ctx.files._additional_d_ts],
+ transitive = transitive_deps_declarations,
+ ),
+ type_blacklisted = depset(transitive = type_blacklisted_declarations),
)
def _outputs(ctx, label, srcs_files = []):
@@ -213,9 +260,9 @@
if not is_library and not ctx.attr.generate_externs:
type_blacklisted_declarations += srcs_files
- # The list of output files. These are the files that are always built
+ # The depsets of output files. These are the files that are always built
# (including e.g. if you "blaze build :the_target" directly).
- files = depset()
+ files_depsets = []
# A manifest listing the order of this rule's *.ts files (non-transitive)
# Only generated if the rule has any sources.
@@ -232,13 +279,12 @@
tsickle_externs_path = tsickle_externs[0] if tsickle_externs else None
# Calculate allowed dependencies for strict deps enforcement.
- allowed_deps = depset()
-
- # A target's sources may depend on each other,
- allowed_deps += srcs_files[:]
-
- # or on a .d.ts from a direct dependency
- allowed_deps += dep_declarations.direct
+ allowed_deps = depset(
+ # A target's sources may depend on each other,
+ srcs_files,
+ # or on a .d.ts from a direct dependency
+ transitive = [dep_declarations.direct],
+ )
tsconfig_es6 = tsc_wrapped_tsconfig(
ctx,
@@ -273,7 +319,7 @@
]
outputs.append(profile_file)
- files += [perf_trace_file, profile_file]
+ files_depsets.append(depset([perf_trace_file, profile_file]))
ctx.actions.write(
output = ctx.outputs.tsconfig,
@@ -324,7 +370,7 @@
]
outputs.append(profile_file)
- files += [perf_trace_file, profile_file]
+ files_depsets.append(depset([perf_trace_file, profile_file]))
ctx.actions.write(output = tsconfig_json_es5, content = json_marshal(
tsconfig_es5,
@@ -358,22 +404,20 @@
devmode_manifest = None
# Downstream rules see the .d.ts files produced or declared by this rule.
- declarations = depset()
- declarations += gen_declarations
- declarations += src_declarations
+ declarations_depsets = [depset(gen_declarations + src_declarations)]
if not srcs_files:
# Re-export sources from deps.
# TODO(b/30018387): introduce an "exports" attribute.
for dep in deps:
if hasattr(dep, "typescript"):
- declarations += dep.typescript.declarations
- files += declarations
+ declarations_depsets.append(dep.typescript.declarations)
+ files_depsets.extend(declarations_depsets)
# If this is a ts_declaration, add tsickle_externs to the outputs list to
# force compilation of d.ts files. (tsickle externs are produced by running a
# compilation over the d.ts file and extracting type information.)
if not is_library:
- files += depset(tsickle_externs)
+ files_depsets.append(depset(tsickle_externs))
transitive_es5_sources = depset()
transitive_es6_sources = depset()
@@ -391,7 +435,7 @@
transitive_es6_sources = depset(transitive = [transitive_es6_sources, es6_sources])
return {
- "files": files,
+ "files": depset(transitive = files_depsets),
"output_groups": {
"es6_sources": es6_sources,
"es5_sources": es5_sources,
@@ -405,7 +449,7 @@
),
# TODO(martinprobst): Prune transitive deps, only re-export what's needed.
"typescript": {
- "declarations": declarations,
+ "declarations": depset(transitive = declarations_depsets),
"transitive_declarations": transitive_decls,
"es6_sources": es6_sources,
"transitive_es6_sources": transitive_es6_sources,
diff --git a/internal/devserver/ts_devserver.bzl b/internal/devserver/ts_devserver.bzl
index ba29312..0b732c9 100644
--- a/internal/devserver/ts_devserver.bzl
+++ b/internal/devserver/ts_devserver.bzl
@@ -27,9 +27,9 @@
files = depset()
for d in ctx.attr.deps:
if hasattr(d, "node_sources"):
- files += d.node_sources
+ files = depset(transitive = [files, d.node_sources])
elif hasattr(d, "files"):
- files += d.files
+ files = depset(transitive = [files, d.files])
if ctx.label.workspace_root:
# We need the workspace_name for the target being visited.
@@ -112,7 +112,7 @@
files = devserver_runfiles,
# We don't expect executable targets to depend on the devserver, but if they do,
# they can see the JavaScript code.
- transitive_files = depset(ctx.files.data) + files,
+ transitive_files = depset(ctx.files.data, transitive = [files]),
collect_data = True,
collect_default = True,
),
diff --git a/internal/karma/ts_web_test.bzl b/internal/karma/ts_web_test.bzl
index 27df24e..5c584e6 100644
--- a/internal/karma/ts_web_test.bzl
+++ b/internal/karma/ts_web_test.bzl
@@ -34,9 +34,9 @@
files = depset(ctx.files.srcs)
for d in ctx.attr.deps:
if hasattr(d, "node_sources"):
- files += d.node_sources
+ files = depset(transitive = [files, d.node_sources])
elif hasattr(d, "files"):
- files += d.files
+ files = depset(transitive = [files, d.files])
# Write the AMD names shim bootstrap file
amd_names_shim = ctx.actions.declare_file(
diff --git a/internal/protobufjs/ts_proto_library.bzl b/internal/protobufjs/ts_proto_library.bzl
index d65ebbc..8abb241 100644
--- a/internal/protobufjs/ts_proto_library.bzl
+++ b/internal/protobufjs/ts_proto_library.bzl
@@ -105,9 +105,9 @@
return struct(
files = depset([dts]),
typescript = struct(
- declarations = [dts],
- transitive_declarations = [dts],
- type_blacklisted_declarations = [],
+ declarations = depset([dts]),
+ transitive_declarations = depset([dts]),
+ type_blacklisted_declarations = depset(),
es5_sources = depset([js_es5]),
es6_sources = depset([js_es6]),
transitive_es5_sources = depset(),
diff --git a/package.bzl b/package.bzl
index bcc1e1b..a79d50f 100644
--- a/package.bzl
+++ b/package.bzl
@@ -105,8 +105,7 @@
_maybe(
http_archive,
name = "io_bazel",
- urls = ["https://github.com/bazelbuild/bazel/releases/download/0.9.0/bazel-0.9.0-dist.zip"],
- sha256 = "efb28fed4ffcfaee653e0657f6500fc4cbac61e32104f4208da385676e76312a",
+ urls = ["https://github.com/bazelbuild/bazel/releases/download/0.17.1/bazel-0.17.1-dist.zip"],
)
#############################################
diff --git a/package.json b/package.json
index d453cc4..fac1c21 100644
--- a/package.json
+++ b/package.json
@@ -50,7 +50,8 @@
"e2e-package_typescript_karma": "cd internal/e2e/package_typescript_karma; bazel test ...",
"e2e-ts_auto_deps": "cd internal/e2e/ts_auto_deps; bazel run @build_bazel_rules_typescript//ts_auto_deps -- -recursive && bazel build simple",
"preskylint": "bazel build --noshow_progress @io_bazel//src/tools/skylark/java/com/google/devtools/skylark/skylint:Skylint",
- "skylint": "find . -type f -name \"*.bzl\" ! -path \"*/node_modules/*\" | xargs $(bazel info bazel-bin)/external/io_bazel/src/tools/skylark/java/com/google/devtools/skylark/skylint/Skylint",
+ "//": "disable deprecated-api skylint check because we must make legacy-style providers for TS interop in google3",
+ "skylint": "find . -type f -name \"*.bzl\" ! -path \"*/node_modules/*\" | xargs $(bazel info bazel-bin)/external/io_bazel/src/tools/skylark/java/com/google/devtools/skylark/skylint/Skylint --disable-checks=deprecated-api",
"skydoc": "bazel build //docs && unzip -o -d docs/api bazel-bin/docs/docs-skydoc.zip",
"version": "node ./on-version.js && git stage package.bzl"
}
