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

ST1020: allow comments to start with deprecation comment #1549

Closed
wants to merge 1 commit into from

Conversation

arp242
Copy link
Contributor

@arp242 arp242 commented May 27, 2024

Fixes #1378

@@ -55,6 +58,10 @@ func run(pass *analysis.Pass) (interface{}, error) {
if !ast.IsExported(decl.Name.Name) {
return
}
if _, ok := depr.Objects[pass.TypesInfo.ObjectOf(decl.Name)]; ok && strings.HasPrefix(text, "Deprecated:") {
Copy link
Owner

Choose a reason for hiding this comment

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

When would a comment have the right prefix but not be marked as deprecated by deprecated.Analyzer?

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 put that strings.HasPrefix in so that this:

// wrong comment yo. //@diag (`comment on exported function`)
//
// Deprecated: don't use.
func F6() {}

Still gets flagged, and it only skips if the Deprecated: ... comment is the only comment.

Copy link
Owner

Choose a reason for hiding this comment

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

I'm not confused about the strings.HasPrefix, but about the use of depr. Isn't the strings.HasPrefix call enough?

Copy link
Contributor Author

@arp242 arp242 Jun 1, 2024

Choose a reason for hiding this comment

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

Oh right. Yeah, I guess that is rather redundant and pointless; just seemed like the "proper way" to do it. I removed it.

This pull request was closed.
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.

ST1020: ignore deprecation comment
2 participants