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

checker: add defer at the end of function checker #901

Merged
merged 1 commit into from Jan 20, 2020

Conversation

janisz
Copy link
Contributor

@janisz janisz commented Jan 20, 2020

Detects uneeded defer at the end of function.

cristaloleg
cristaloleg previously approved these changes Jan 20, 2020
Copy link
Member

@cristaloleg cristaloleg left a comment

Choose a reason for hiding this comment

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

SGTM, @quasilyte ?

if fnlit.Body.List == nil {
s += "{}"
} else {
s += "{…}"
Copy link
Member

Choose a reason for hiding this comment

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

TBH I would like to omit Unicode characters as stick to simple ASCII.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I'll fix it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

fnlit, ok := deferStmt.Call.Fun.(*ast.FuncLit)
if ok {
// To avoid long and multi-line warning messages,
// collapse the function literals.
Copy link
Member

Choose a reason for hiding this comment

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

sounds reasonable

}
}()
return
}
Copy link
Member

Choose a reason for hiding this comment

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

Looks good, I'm curious, will it handle defer inside if? :)
Even if no - it can be marked as a TODO comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add a test

Detects uneeded defer at the end of function.
@cristaloleg cristaloleg merged commit 3062653 into go-critic:master Jan 20, 2020
@cristaloleg
Copy link
Member

thanks!

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

3 participants