Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

checkers: add sortSlice checker #929

Merged
merged 4 commits into from May 26, 2020
Merged

Conversation

quasilyte
Copy link
Member

Find suspicious sort slice calls.

Sorting slice argument with a callback that uses
another slice is usually incorrect, as only one
slice will be modified by Slice call inplace.

Signed-off-by: Iskander Sharipov quasilyte@gmail.com

cristaloleg
cristaloleg previously approved these changes May 26, 2020
}
}

func (c *sortSliceChecker) paramIdents(e *ast.FuncType) (*ast.Ident, *ast.Ident) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

fields := e.Params.List
if len(fields) == 2 {
    return fields.Names[0], fields.Names[1]
}
return nil, nil

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't handle the case where both idents are a single field.

i, j int - 1 field with 2 names
i int, j int - 2 fields, each with 1 name

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh! got it, nice.

checkers/testdata/sortSlice/negative_tests.go Show resolved Hide resolved
@@ -10,7 +10,7 @@ require (
github.com/go-toolsmith/astfmt v1.0.0
github.com/go-toolsmith/astp v1.0.0
github.com/go-toolsmith/strparse v1.0.0
github.com/go-toolsmith/typep v1.0.0
github.com/go-toolsmith/typep v1.0.2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<3

Find suspicious sort slice calls.

Sorting `slice` argument with a callback that uses
another slice is usually incorrect, as only one
slice will be modified by Slice call inplace.

Signed-off-by: Iskander Sharipov <quasilyte@gmail.com>
@quasilyte
Copy link
Member Author

PTAL.

@quasilyte quasilyte merged commit 33d100b into master May 26, 2020
@quasilyte quasilyte deleted the quasilyte/addCheck/sortSlice branch May 26, 2020 22:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants