stylecheck: allow errors to begin with proper nouns

[dominikh]

Example error: "Travis CI blew up, oh noes"

We probably want to maintain a configurable list of proper nouns, similar to our list of initialisms. By default we should add some of the common tech companies that provide APIs. Consider using BigQuery to farm common names.

[dominikh]

If anybody wants to help out with this, they could run a BigQuery query over all Go code, get all error strings (calls to errors.New and fmt.Errorf) and return all the first words of the strings that start with a capital letter. Either an undeduplicated list, or a deduplicated list with counts per word.

[ainar-g]

Do we really want this? I would honestly prefer to live in a world where all logs are all lowercase, all the time. It's more consistent, more greppable (hello -F, goodbye -i), and (arguably) more aesthetically pleasing.

[dominikh]

Do we really want this?

I would say so, yes. The reason error strings in Go should start with a lowercase letter is to allow correct wrapping, so that %s: %s turns into foo went wrong: bar went wrong – the goal is to have proper English sentences. That does, however, mean that proper nouns can, and IMO should, be capitalized correctly. We already do so with initialisms and function names.

In fact, log messages (i.e. log.Print*) commonly start with uppercase letters and no official style guide recommends otherwise.

grep -i seems like a red herring to me. If everyone spelled proper nouns correctly, you'd not need -i. Travis CI would always be Travis CI, GitHub would always be GitHub.

[zx8]

Just got bitten by this today using golangci-lint and ended up having to add a // nolint comment to silence it, so came to raise a feature request to allow configuring proper nouns... and ended up here instead! 😄

I realise it's not a groundbreaking feature (especially since the comment more or less solves the problem anyway), but am just curious if it's still planned to make this configurable?

[dominikh]

It is still planned, yes.

[nirui]

Hi @dominikh, is there any update on this?

The warning error strings should not be capitalized (ST1005) has a completely different meaning than Error strings should not be capitalized (unless beginning with proper nouns or acronyms).

Current behavior of staticcheck is effectively banning (almost) all nouns from appearing at the beginning of an error message, which is not really ideal and don't actually match the review requirements.

Thanks!

[dominikh]

No updates, sorry.

[nirui]

Oh OK. Thanks for the work anyways. Also,

By default we should add some of the common tech companies that provide APIs.

This might be not a good idea since it could create some confusions (for example, on why some words are sanctioned, while not the others). If possible, I would vote for a zero-default approach, that is, by default, there is no noun on the redemption list, user must add in their own words of choice manually as needed. This approach also has the benefit of keeping the initial behavior of staticcheck unchanged, as well as making the implementation simpler.

Just my two cents :)

[dominikh]

If possible, I would vote for a zero-default approach

We already provide a default list of acronyms, so providing a default list of proper nouns seems consistent with that. People don't seem to be confused when their acronym of choice is missing from the list.

This approach also has the benefit of keeping the initial behavior of staticcheck unchanged

We don't strive to maintain old false or noisy positives.

as well as making the implementation simpler

It wouldn't make it any simpler. If we need a configurable list of names, then we can provide a default value for that list. That functionality already exists.