Resolve #69: Support struct literal comment directive

[navijation]

Summary

Support comment directives of the form //exhaustruct:enforce and //exhaustruct:ignore in order to override exclusion
and inclusion flag rules on particular struct literals.

Details

• Support a new -use-directives boolean flag to turn on struct literal level enforcement
• When an enforcement directive exists for a struct literal, use that alone to decide whether to process the literal
  • Use ast.CommentMap to perform comment traversal, caching one within the analyzer for each file
  • For ease of use, directive comments can be attached either to struct literal nodes or their direct parent nodes,
    which handles some basic cases like directives over assignment lines and field assignment lines
• Update README with usage information and examples
• Add some comments in i test package to make benchmark fairer

Benchmarks

• On my machine, directive scanning adds less than a 5% slowdown when run on code with no directives

[xobotyi]

The main issue with this PR - it also does refactoring of analyzer which is beyond the scope of described change - it should have beentwo separate PR's

[navijation]

Fair enough. I'll revert the refactor

[navijation]

Okay @xobotyi should now be limited to what's required for this change.

[xobotyi]

Regarding your email: no need to 'threaten' with fork of package - you're always free to do so if you like to. Yes i dont have a lot of free time to support all of my OSS projects nowadays, but i do dedicate time for it, sadly feature you requested is not an easy one, that can be solved in a matter of hours, therefore requires crapton of time to investigate it.

Regarding this pull request: I've been digging this one recently and the way it is done basycally has no sense.
In explicit mode you should enable what structures to check with comment nearby structure definition, not usage as you've done here, since linter itself looses its main purpose - offloading brain with need to check smth.

//exhastruct:enforce
t := T{}

Above is a bad solution which will esentially make linter useless and everyone will forget to enforce linter where it is used. Essentially you can achieve the same with disabling linter where you don't need it - same amount of hustle, no changes to linter required. Thught your PR is not completely gone wrong way, your approach still allows to scan for ignore comment that should occur in place of usage.

Overall it should look as described below.

//exhastruct:enforce
type T struct{}

t1 := T{} // <- this one checked
//exhaustruct:ignore
t3 := T{} // <- not checked as disabled by comment

type T2 struct{}

t2 := T2{} // <- this one not

Hardest part is to access AST of definiton file, suddenly i don't have a robust solution for this, yet i'm trying to figure this ot on the free time.
Sadly I'm not able to assure you with any kind of ETA on this issue as i already spend well above 16 hours investigating the way to access comments of type definition AST leaf.

[dtext]

@xobotyi maybe have a look at the issue this resolves again. The use case presented there is very much valid and present around 100 times in a codebase I'm working on.

[navijation]

@xobotyi Regarding my email, I was not threatening you at all. I was simply explaining why I would prefer merging the code into your repo rather than integrating my own fork into my company's repo: forking the repo and then integrating it with golangci-lint (either by maintaining a golangci-lint fork or including a shared private object everywhere in our infra setup) is annoying. The reality is that no fork will ever be as popular because this was first to be included in golangci-lint.

In explicit mode you should enable what structures to check with comment nearby structure definition, not usage as you've done here, since linter itself looses its main purpose - offloading brain with need to check smth.

For context, the underlying issue is analogous nishanths/exhaustive#36 and the solution to nishanths/exhaustive#39. Many teams do not want to have a struct forced to be exhaustive in every single call site by default and to be forced to use ignore directives everywhere to remove enforcement. For some types this makes sense: typically "parameter types" that wrap a function's arguments should always be enforced. For many types this does not make sense, e.g. a "sparse" struct that usually does not get more than a few top-level fields filled out.

However, even for such "sparse" structs, there are a few cases where we do want exhaustive field enforcement. In particular, functions that "transform" data type A1 to a similar data type A2 are great candidates for exhaustiveness checks because it is usually necessary to propagate new fields added to A1 to A2.

type InternalStatement struct {
	Balance        int
	OverdueBalance int  // newly added field
	StartDate      time.Time
	EndDate        time.Time
}

type ExternalStatement struct {
	Balance        money.Amount `json:"balance"`
	OverdueBalance int          `json:"overdue_balance"` // newly added field
	StartDate      string       `json:"start_date"`
	EndDate        string       `json:"end_date"`
}

func ToExternalStatement(statement *InternalStatement) *ExternalStatement {
	//exhaustruct:enforce
	return &ExternalStatement{
		Balance:   money.FromCents(statement.Balance),
		// oops, forgot to map OverdueBalance! But exhaustruct catches this
		StartDate: formatMonthDay(statement.StartDate),
		EndDate:   formatMonthDay(statement.EndDate),
	}
}

sadly feature you requested is not an easy one, that can be solved in a matter of hours, therefore requires crapton of time to investigate it.

Regarding struct definition enforce / ignore comments (which is a separate issue, #37), I agree that is harder to support and incurs a performance hit of analayzer fact storage / lookup. It is fortunate that, at least for my company's project and @dtext's, this feature seems to be more valuable in practice, and it is both simpler and more performant to include.

[navijation]

Closing in favor of #88