osmosis icon indicating copy to clipboard operation
osmosis copied to clipboard

Make linter for stable sorting

Open ValarDragon opened this issue 2 years ago • 4 comments

Background

We should make a simple linter to just check for instances of sort.Slice, and replacing it with sort.SliceStable, and sort.Sort -> sort.Stable.

(The application of this change is breaking)

Unstable sort behavior can (validly) change between golang minor releases, so should never be relied upon. Pretty basic issue to get eliminated from our concern list. cref #3762

Ideally this could be in our golangci-linter. If thats hard for whatever reason we can add a custom golang linting step.

Suggested Design

Some basic regex to handle these.

Acceptance Criteria

We have a linter warning for any unstable sort usage.

ValarDragon avatar Jan 14 '23 07:01 ValarDragon

@ValarDragon - I can pick this up

cipherzzz avatar Feb 02 '23 03:02 cipherzzz

Couldn't find a convenient linter for golang that bans certain regex expressions. So to do this, I think we should:

  • [ ] Find the list of golang sorts that are not stable from https://pkg.go.dev/sort
  • [ ] Make a grep or a simple CLI command that will return any usage of these, across all go files in the repo.
  • [ ] Require as a step in make lint running this command, ensuring there are 0 such usages. If there are any usages, have an error directing to use the stable variant of that sort.
  • [ ] Check in CI that this is failing in our repo, and fix the failures.

ValarDragon avatar Mar 03 '23 16:03 ValarDragon

Can I pick this up ?

hoangdv2429 avatar Oct 14 '23 20:10 hoangdv2429

@ValarDragon I think we need AST for this as grep Slice and Sort will not be sufficient. What do you think ?

hoangdv2429 avatar Oct 14 '23 21:10 hoangdv2429