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

Variable name only replaced once #18

Closed
silverwind opened this issue Apr 18, 2024 · 8 comments
Closed

Variable name only replaced once #18

silverwind opened this issue Apr 18, 2024 · 8 comments
Assignees
Labels
bug Something isn't working wontfix This will not be worked on

Comments

@silverwind
Copy link

Running with the -dict option and passing commiter,committer, I see the variable only got replaced once on the first occurence, instead of all occurences in go code. I think it should have replaced on line 248 too here:

Screenshot 2024-04-18 at 20 02 09

Is it expected?

Ref: go-gitea/gitea#30573

@ldez
Copy link
Member

ldez commented Apr 18, 2024

First, it's not a new behavior.

I have a minimal reproducible example:

package temp

import (
	"fmt"
	"os"
)

func foo() {
	smoe, _ := os.Open("test") // Detected
	defer smoe.Close() // Not detected

	fmt.Println(smoe) // Detected
	fmt.Println(smoe.Name()) // Not detected
}
$ go run ./cmd/misspell ./temp
temp/temp.go:9:1: "smoe" is a misspelling of "some"
temp/temp.go:12:13: "smoe" is a misspelling of "some"

@ldez
Copy link
Member

ldez commented Apr 18, 2024

It's a side effect/false negative of the host parsing.

@silverwind
Copy link
Author

Ah yes, then it's an existing bug that wasn't noticed so far.

@ldez ldez added the bug Something isn't working label Apr 18, 2024
@ldez
Copy link
Member

ldez commented Apr 18, 2024

This has been the same behavior for the past 8 years, the goal being to avoid false positives.
Then those false negatives are expected.

@ldez ldez added the wontfix This will not be worked on label Apr 18, 2024
@ldez
Copy link
Member

ldez commented Apr 18, 2024

I can improve a bit by following some basic domain name rules, but it's impossible to fully fix that without creating a regression.

@silverwind
Copy link
Author

Fine with me, those cases are easy enough to correct manually.

@ldez
Copy link
Member

ldez commented Apr 19, 2024

I improved hostname detection as I explained before.

Now, defer smoe.Close() will be detected.
But defer smoe.close() is still not detected.

I created a v0.5.1

As the problem cannot be fully fixed, I will close this issue.

@ldez ldez closed this as completed Apr 19, 2024
@silverwind
Copy link
Author

Thanks, I can confirm it new replaced these missing cases, even without having reset the diff :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants