gopherjs

gopherjs copied to clipboard

Background

I just built a trivial linter during a recently 2-hour livestream on my YouTube channel. During my next livestream, I intend to start building a real/useful linter. And I had the idea of creating one for gopherjs.

So I'm opening this issue to get feedback and to solicit rule ideas to include.

Proposal

Create a new repo, github.com/gopherjs/gopherjsvet, which will contain a linter that can be embedded into golangci-lint, as well as compatible with go vet, and usable as a standalone binary.

Linter rules

My proposed linter rules (pending feasability, of course) so far are listed here. I'm soliciting suggestions for others. They should be issues that apply only to GopherJS code, as any other issues should go in a general-purpose linter. Most of these come from JavaScript Tips and Gotchas

• [ ] Do not use items or fields of type js.Object directly
• [x] *js.Object must always be a pointer
• [ ] A struct that embeds a *js.Object must always be a pointer.
• [x] Ensure that whenever js tags are present, a *js.Object field is embedded as the first struct field.
• [ ] Ensure that struct literals for struct types that embed *js.Object do not also initialize other fields.
• [x] Ensure no non-scalar types in structs that embed *js.Object.
• [ ] Complain about trying to json.Unmarshal into mixed Go/JS data structures.
• [ ] Complain about blocking function calls in callbacks (not sure how easy this is actually to detect conclusively, but I'm sure we can get somewhere)
• [ ] Ensure that proper build tags are set on any files that import github.com/gopherjs/gopherjs/js and syscall/js
• [ ] Warn about use of the net/http package
• [ ] (Optionally) warn about the use of encoding/json package when only unmarshaling.

Other Considerations

My immediate interest in this is mostly educational. So whether this proposal is accepted or not, I do intend to go forward (I just may do so in a private project, rather than one hosted under the gopherjs project).

Beyond that immediate goal, if this proves to be both feasable and of general interest, once the linter reaches a certain level of usefulness/maturity (criteria TBD), I would like to:

• Publish pre-compiled binaries with goreleaser for common OSes (Linux, Mac, Windows probably)
• Submit a PR to golangci-lint to include this linter

I like this idea, I think it would help avoiding a lot of possible pitfalls (aka know bugs :) ). I would also propose to integrate it into the gopherjs tool itself as a vet verb.

I would also propose to integrate it into the gopherjs tool itself as a vet verb

Ah yes, good (and obvoius) suggestion.

This makes me wonder, though... should the tool still live in a separate repo, or should it live here, where it can track with GopherJS releases more easily?

I think because it would also be a standalone tool, it makes most sense to keep it in a separate repository, but I could be convinced otherwise.

I would also propose to integrate it into the gopherjs tool itself as a vet verb.

I attempted to work on this today, but it's not quite as easy as one might hope. The go/analysis/multichecker library is expected to be called directly from main(), and handles its own command line parsing. This gives us two options, as far as I can see:

1. Simply call gopherjsvet as an external command. That means gopherjsvet has to be installed separately, which is a bit annoying.
2. Copy-paste a bunch of code from golang.org/x/go/analysis/internal into GopherJS to tweak the behavior we need.

A third possibility, which I tried a bit, with little luck, would be to mangle os.Args before calling multichecker.Main. I'm not sure if this will give us all the flexibility we need, particularly with respect to build tags. I think we'll end up duplicating significant portions of the internal logic anyway. But maybe this approach would end up being less intrusive than option 2 above.

Might be worth asking in the gopher slack #tools channel, but I feel like option #1 is the only one really viable. It is really unfortunate that multichecker doesn't optionally accept something like flagset.

So we have the same problem with option #1... gopherjsvet ignores the js and ecmascript build tags by default. Which means that I'll need to re-implement multichecker.Main anyway, unless we want to purely integrate with golangci-lint, which has its own init, naturally.

I'll work on a custom implementation, to see how ugly it gets.