Using upstream maligned and updating error message

[iwankgb]

It seems that we can swap our maligned fork for the upstream version, but messages from upstream are slightly different from those that have been used so far. I'm not sure how big issue it can be and I would like to discuss it.

https://github.com/golangci/maligned/commits/master
https://github.com/mdempsky/maligned/commits/master

[Nivl]

The new error message might cause an issue for the people that have set up rules in the config file, but my assumption is that if you have a rule to exclude this message then the whole linter becomes useless. That being said maybe someone can prove me wrong on that. I'm assuming it's also expected to have some messages change when we update a linter since we usually don't own the code and are bound to whatever happens upstream.

The biggest issue is, IMO, the fact that we're fully losing the "suggestion" that we get with -v. Without the suggestion, you know that the struct is not optimized but that's it, it's your job to figure out how to reorder everything to make them fit. With the suggestion, it actually gives you the solution to have a proper memory-aligned struct. There's a PR open to add support for that, but the owner of maligned never merged it and then recently refactored the codebase which added a bunch of breaking changes to the PR.

We gonna need more people to weigh on if we want to merge this PR not. We could also have someone send a new PR to maligned to add support for -v. cc @golangci/team

[iwankgb]

@golangci/team, can more of your express an opinion?

[bombsimon]

I don't see any problem changing message. Similar thing happened when we bumped gochecknoglobals f.ex. It's easy to pin a version of golangci-lint, in fact it's probably harder to auto update. As long as peoples CI doesn't break because we make changes I think we should prefer keeping the repo up to date with all linters and dependencies.

If I remember correctly, one of the reasons why golangci-lint doesn't provide much of a public interface to execute the tool from code is to be able to somewhat diverge from semver and be able to update any version of a linter (major, minor or patch) without having to do the same for golangci-lint.

[ldez]

I shared the opinion of @Nivl about suggestions:

The biggest issue is, IMO, the fact that we're fully losing the "suggestion" that we get with -v. Without the suggestion, you know that the struct is not optimized but that's it, it's your job to figure out how to reorder everything to make them fit. With the suggestion, it actually gives you the solution to have a proper memory-aligned struct. There's a PR open to add support for that, but the owner of maligned never merged it and then recently refactored the codebase which added a bunch of breaking changes to the PR.

[iwankgb]

There is no consensus therefore it won't be merged.

[Nivl]

Note that as of last week, maligned has been deprecated. The recommandation is to switch to https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/fieldalignment