Skip to content
This repository has been archived by the owner on May 9, 2021. It is now read-only.

lint: avoid false positives with custom errors-package #360

Closed
wants to merge 4 commits into from

Conversation

leonelquinteros
Copy link
Contributor

@leonelquinteros leonelquinteros commented Dec 5, 2017

When using errors.New(fmt.Sprintf(...)),
lint will alert that you should use fmt.Errorf(...).

Before this patch, this alert was also displayed
when using a custom errors-package.
There are valid use cases to use errors.New(fmt.Sprintf(...))
in a custom errors-package context.

This patch avoids the "false positive" alert
when a custom errors-package is imported in the current file.

Fixes #350

@andybons
Copy link
Member

@googlebot cla?

@gopherbot
Copy link

This PR (HEAD: 63c4d72) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/#/c/lint/+/96091 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link

Message from Gobot Gobot:

Patch Set 1:

Congratulations on opening your first change. Thank you for your contribution!

Next steps:
Within the next week or so, a maintainer will review your change and provide
feedback. See https://golang.org/doc/contribute.html#review for more info and
tips to get your patch through code review.

Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.

During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11, it means that this CL will be reviewed as part of the next development
cycle. See https://golang.org/s/release for more details.


Please don’t reply on this GitHub thread. Visit golang.org/cl/96091.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Andrew Bonventre:

Patch Set 1:

(3 comments)

This should have an associated test


Please don’t reply on this GitHub thread. Visit golang.org/cl/96091.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

This PR (HEAD: 70f7b1a) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/#/c/lint/+/96091 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link

This PR (HEAD: 71ea537) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/#/c/lint/+/96091 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link

Message from Leonel Quinteros:

Patch Set 2:

(3 comments)

Patch Set 1:

(3 comments)

This should have an associated test

Added a test case to cover the allowed cases when importing a custom errors package. The other cases are already covered in the test suite.


Please don’t reply on this GitHub thread. Visit golang.org/cl/96091.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Andrew Bonventre:

Patch Set 3:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/96091.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Leonel Quinteros:

Patch Set 3:

Patch Set 3:

(1 comment)

Sorry, somehow the git commit --amend I did, created a new commit instead of updating the original message: cac6d06

Let me try to fix that.


Please don’t reply on this GitHub thread. Visit golang.org/cl/96091.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

This PR (HEAD: ec9d145) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/#/c/lint/+/96091 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link

Message from Gerrit Bot:

Uploaded patch set 4: Commit message was updated.


Please don’t reply on this GitHub thread. Visit golang.org/cl/96091.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

This PR (HEAD: 053e0ad) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/#/c/lint/+/96091 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link

Message from Gerrit Bot:

Uploaded patch set 5: Commit message was updated.


Please don’t reply on this GitHub thread. Visit golang.org/cl/96091.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Andrew Bonventre:

Patch Set 5:

The commit message for a PR is determined from the title and first comment of the PR, not the commit message itself: https://github.com/golang/go/wiki/GerritBot#how-does-gerritbot-determine-the-final-commit-message

This is because there could be multiple commit messages in a PR, so it’s hard to determine which one should be used.


Please don’t reply on this GitHub thread. Visit golang.org/cl/96091.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Leonel Quinteros:

Patch Set 5:

Patch Set 3:

(1 comment)

OK, now i've rebased the branch, edited the commit messages and force-pushed the changes so it looks better to me.


Please don’t reply on this GitHub thread. Visit golang.org/cl/96091.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Leonel Quinteros:

Patch Set 5:

Patch Set 5:

The commit message for a PR is determined from the title and first comment of the PR, not the commit message itself: https://github.com/golang/go/wiki/GerritBot#how-does-gerritbot-determine-the-final-commit-message

This is because there could be multiple commit messages in a PR, so it’s hard to determine which one should be used.

Oh! Totally missed that. Thanks for the clarification. Quickly-editing right now.


Please don’t reply on this GitHub thread. Visit golang.org/cl/96091.
After addressing review feedback, remember to publish your drafts!

@leonelquinteros leonelquinteros changed the title Fix False positives with custom errors-package and errors.New(fmt.Spr… lint: avoid false positives with custom errors-package Mar 5, 2018
@gopherbot
Copy link

Message from Gerrit Bot:

Uploaded patch set 6: Commit message was updated.


Please don’t reply on this GitHub thread. Visit golang.org/cl/96091.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Andrew Bonventre:

Patch Set 6:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/96091.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Gerrit Bot:

Uploaded patch set 7: Commit message was updated.


Please don’t reply on this GitHub thread. Visit golang.org/cl/96091.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Andrew Bonventre:

Patch Set 7: Code-Review+1

(1 comment)

Ready for you, Alan.


Please don’t reply on this GitHub thread. Visit golang.org/cl/96091.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

This PR (HEAD: ad257d2) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/#/c/lint/+/96091 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link

Message from Andrew Bonventre:

Patch Set 8:

(2 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/96091.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Gerrit Bot:

Uploaded patch set 9: Commit message was updated.


Please don’t reply on this GitHub thread. Visit golang.org/cl/96091.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link

Message from Andrew Bonventre:

Patch Set 9: Code-Review+2


Please don’t reply on this GitHub thread. Visit golang.org/cl/96091.
After addressing review feedback, remember to publish your drafts!

gopherbot pushed a commit that referenced this pull request Mar 7, 2018
When using `errors.New(fmt.Sprintf(...))`,
lint will alert that you should use `fmt.Errorf(...)`.

Before this patch, this alert was also displayed
when using a custom errors-package.
There are valid use cases to use `errors.New(fmt.Sprintf(...))`
in a custom errors-package context.

This patch avoids the "false positive" alert
when a custom errors-package is imported in the current file.

Fixes #350

Change-Id: I7cc82a3435b184f8b4cad0752a75d44f33536dce
GitHub-Last-Rev: ad257d2
GitHub-Pull-Request: #360
Reviewed-on: https://go-review.googlesource.com/96091
Reviewed-by: Andrew Bonventre <andybons@golang.org>
@gopherbot
Copy link

This PR is being closed because golang.org/cl/96091 has been merged.

@gopherbot gopherbot closed this Mar 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants