feat: add gocheckcompilerdirectives

[leighmcculloch]

Add gocheckcompilerdirectives that checks that compiler directives (//go: comments) are valid, and catches easy mistakes.

https://github.com/leighmcculloch/gocheckcompilerdirectives

Compiler directives are comments in the form of //go: that provide an instruction to the compiler. For example, //go:generate, //go:embed, //go:build, etc.

Compiler directives are easy to make mistakes with. The linter will detect the following mistakes.

1. Adding a space in between the comment bars and the first character, e.g. // go:, will cause the compiler to silently ignore the comment.

2. Mistyping a directives name, e.g. //go:embod, will cause the compiler to silently ignore the comment.

[boring-cyborg]

Hey, thank you for opening your first Pull Request !

[CLAassistant]

All committers have signed the CLA.

[ldez]

In order for a pull request adding a linter to be reviewed, the linter and the PR must follow some requirements.

Pull Request Description

• It must have a link to the linter repository.
• It must provide a short description of the linter.

Linter

• It must not be a duplicate of another linter or a rule of a linter. (the team will help to verify that)
• It must have a valid license (AGPL is not allowed) and the file must contain the required information by the license, ex: author, year, etc.
• The linter repository must have a CI and tests.
• It must use go/analysis.
• It must have a valid tag, ex: v1.0.0, v0.1.0.
• It must not contain init().
• It must not contain panic(), log.fatal(), os.exit(), or similar.
• It must not have false positives/negatives. (the team will help to verify that)
• It must have tests inside golangci-lint.

The Linter Tests Inside Golangci-lint

• They must have at least one std lib import.
• They must work with T=<name of the linter test file>.go make test_linters. (the team will help to verify that)

.golangci.reference.yml

• The linter must be added to the list of available linters (alphabetical case-insensitive order).
  • enable and disable options
• If the linter has a configuration, the exhaustive configuration of the linter must be added (alphabetical case-insensitive order)
  • The values must be different from the default ones.
  • The default values must be defined in a comment.
  • The option must have a short description.

Others Requirements

• The files (tests and linter) inside golangci-lint must have the same name as the linter.
• The .golangci.yml of golangci-lint itself must not be edited and the linter must not be added to this file.
• The linters must be sorted in the alphabetical order (case-insensitive) in the Manager.GetAllSupportedLinterConfigs(...) and .golangci.reference.yml.
• The load mode (WithLoadMode(...)):
  • if the linter doesn't use types: goanalysis.LoadModeSyntax
  • goanalysis.LoadModeTypesInfo required WithLoadForGoAnalysis() in the Manager.GetAllSupportedLinterConfigs(...)
• The version in WithSince(...) must be the next minor version (v1.X.0) of golangci-lint.

Recommendations

• The linter should not use SSA. (currently, SSA does not support generics)
• The linter repository should have a readme and linting.
• The linter should be published as a binary. (useful to diagnose bug origins)

---

The golangci-lint team will edit this comment to check the boxes before and during the review.

This checklist does not imply that we will accept the linter.

[leighmcculloch]

@ldez Thanks for posting the list of instructions. I've reviewed the list and after making a couple changes to the PR I believe it satisfies the requirements.

[ldez]

I would like to suggest another name for your linter because "directives" is a generic term (for example replace or require in a go.mod are directives).
So I suggest something like checkgocompilerdirectives

https://pkg.go.dev/cmd/compile#hdr-Compiler_Directives

[leighmcculloch]

That's a good idea, I'd be happy to rename it. I'll rename it to gocheckcompilerdirectives. Does that work for you?

[ldez]

gocheck is an existing tool.

it can be gocompilerdirectiveslint

[leighmcculloch]

gocheck is an existing tool

I also maintain gochecknoglobals, gochecknoinits. That's the pattern I name my linters. I don't think I can rename the others at this point.

[ldez]

That's the pattern I name my linters. I don't think I can rename the others at this point.

It's always possible to change or at least to use a new pattern 😉

[leighmcculloch]

I've pushed a change to this pull request updating the name of the linter to gocheckcompilerdirectives to more accurately reflect what is being checked. I kept the gocheck prefix because that's what my small suite of linters all use and I'd like to keep them consistently named. As far as I know this has not caused any confusion so far and gochecknoglobals and gochecknoinits have been in golangci-lint for sometime.

[ldez]

the linter position in the manager is not right (alphabetically ordered)

Edit: misread

[leighmcculloch]

@ldez Thanks for re-reviewing. I've merged the two suggested changes removing the config. Thanks, I didn't realize I could simply pass nil when no config was available.

[ldez]

LGTM