Rules_nodejs: Design: run dev process tools like a linter

[pweaver]

I am trying to write a test rules which uses a nodejs binary. My rule currently looks like the following error:

/private/var/tmp/_bazel_pweaver/b8912c475ba6eed2e1170c9c188c1211/bazel-sandbox/3295380895294116077/execroot/abstract/bazel-out/darwin_x86_64-fastbuild/bin/app/app_tslint_test.runfiles/__main__/bazel_rules/tslint: line 93: /private/var/tmp/_bazel_pweaver/b8912c475ba6eed2e1170c9c188c1211/bazel-sandbox/3295380895294116077/execroot/abstract/bazel-out/darwin_x86_64-fastbuild/bin/app/app_tslint_test.runfiles/__main__/external/io_bazel_rules_typescript_node/bin/node: No such file or directory

When I add a transitive runfile declaration on the nodejs binary I get a target (rule class of 'nodejs_binary') doesn't have provider 'runfiles'. But shouldn't the nodejs_binary be exporting it's runfiles?

def _tslint_test_impl(ctx):
  """
  Implementation for tslint__test Skylark rule.
  """

  srcs = ctx.files.srcs
  tslint = ctx.attr.tslint.files.to_list()[0]
  tslint_bin = ctx.executable.tslint_bin
  exe = ctx.outputs.executable

  args = ["#!/bin/sh\nexec", tslint_bin.short_path,
         "-c ", tslint.short_path]
  args += [src.short_path for src in srcs]

  ctx.file_action(
      executable=True,
      output=exe,
      content=" ".join(args))
  return struct(
      files=set([exe]),
      runfiles=ctx.runfiles(
        files=[exe, tslint] + srcs,
        transitive_files=ctx.attr.tslint_bin.runfiles,
        )
      )

[alexeagle]

nodejs_binary doesn't expose its runfiles - I think it ought to be treated as a root in the action graph. I think nodejs_test is the right answer here (probably using jasmine)

[alexeagle]

I'm curious about the use case here - I wouldn't imagine running tslint as part of the build tool. It doesn't produce an output that you'd request to build.

[pweaver]

I wanted to run tslint as a lint test on the .ts files in the project. I wasn't entirely sure how to go about doing that. It seems to me that tslint should be treated as the test binary and the .ts files as the test data files. (I am pretty new to bazel so I might also be missing some basic concepts and workflows).

[alexeagle]

At google, we don't use Bazel for running developer workflow tasks that touch original sources like lint, format, etc. We only use it to produce build artifacts.
Ideally those workflow tools have awareness of the delta between your client state and some baseline (eg. if you have a GitHub PR, we know the base commit and the HEAD of the branch to merge). The tools tell you something about the quality of your change. Bazel doesn't have any idea what has changed, because it can't know what branch you intend to merge your changes against (you could imagine it's typically upstream/master but not in some cases).
Probably you should just run tslint from a script in the package.json, or using some task runner like gulp.
That said, you could imagine that running a linter under bazel has some advantages - you'd only re-lint packages where a file has changed, which would make incremental linting a lot faster. If you want to pursue it, we could chat design on this thread.

[pweaver]

That makes sense.

From my perspective, it would would make the developer workflow simpler if we treated all tests (ones that touch source files or otherwise) the same. The biggest advantage we have had to switching to Bazel is that setup, dependency management, and execution of everything in the projects I work are is greatly simplified and I can imagine that other people outside Google feel the same, so I think I would like to figure out how to design this.

That being said, for lint style tests on smaller projects mostly I don't care when we run them across the full project because they tend to be fast enough that all lints can be run in a few seconds but that may change as the projects grow larger.

[alexeagle]

By the way @pweaver we do have a jasmine_node_test rule now.
Also, I'm experimenting with running tslint under Bazel, with the thought we might do this for googlers as well. Type-checked rules need a ts.Program which means they should be run under exactly the same compilation context as a regular build. That makes it hard to get exact results from running them in the tslint command line. (see #54)

[alexeagle]

I did an experiment with this long ago
https://github.com/bazelbuild/rules_typescript/compare/master...alexeagle:tslint

with the announcement that tslint is EOL we have some work to do wiring in ESLint, so we should think about doing it this way under both Bazel and Blaze (google internal)

[dsculptor]

At google, we don't use Bazel for running developer workflow tasks that touch original sources like lint, format, etc. We only use it to produce build artifacts.
Ideally those workflow tools have awareness of the delta between your client state and some baseline (eg. if you have a GitHub PR, we know the base commit and the HEAD of the branch to merge). The tools tell you something about the quality of your change. Bazel doesn't have any idea what has changed, because it can't know what branch you intend to merge your changes against (you could imagine it's typically upstream/master but not in some cases).
Probably you should just run tslint from a script in the package.json, or using some task runner like gulp.
That said, you could imagine that running a linter under bazel has some advantages - you'd only re-lint packages where a file has changed, which would make incremental linting a lot faster. If you want to pursue it, we could chat design on this thread.

Just checking in, if there has been innovation here to support the known linters like pylint and bazel's own buildifier!! Any prototype solutions appreciated!

On the note of bazel giving first class support for linting, I believe these low hanging specs can make a difference:

1. Internally implemented Flag: Have a bazel internal setting, which could be triggered by "--lint-as-well".
2. Setup lint-chain: The compatible lint tools like pylint, buildifier, "your-own-linter with some linter-like specifications" could be set via toolchain mechanism.
3. No more boilerplate: Lint rules should NOT be explicitly written by users.
4. Simple git-based file resolution: bazel lint COMMIT-ish/un-commited

But then, why should bazel support something which is so much of a separate system to bazel? Perhaps, it can enable the 5, 6 which can be of bazel's interest:

---

1. Optionally support: bazel lint "package" -> find all the src files which transitive closure of all inputs for the pkg -> run lint.
2. No. 5 should be one step. Whose output could be percolated to internal bazel targets' representation & see granular impact.

The impact on packages could be good to see via follow up query to bazel on:
list of bad files -> list of targets marked dirty

[thundergolfer]

Any prototype solutions appreciated!

@donraj I'm working on a prototype over here -> https://github.com/thundergolfer/bazel-linting-system. Any feedback would be appreciated 🙂 👍