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

fix: don't report issue related to invalid position #22

Merged
merged 9 commits into from
Mar 23, 2024

Conversation

ldez
Copy link
Contributor

@ldez ldez commented Mar 20, 2024

Fixes #21

@ldez ldez force-pushed the fix/report-without-filepath branch from 1cd092f to d442b7c Compare March 20, 2024 16:41
contextcheck.go Outdated Show resolved Hide resolved
@kkHAIKE
Copy link
Owner

kkHAIKE commented Mar 21, 2024

I really want to understand what kind of problem this is because in content check, each issue is quite important; otherwise, they cannot be connected.

Also, I hope this fix preferably doesn't alter the condition logic; just skipping Reportf will suffice.

func (r *runner) Reportf(pos token.Pos, format string, args ...interface{}) {
    if pos..IsValid() {
        r.pass.Reportf(pos, format, args...)
    }
}

@ldez ldez force-pushed the fix/report-without-filepath branch from 087e6b2 to 66970b8 Compare March 21, 2024 15:10
@ldez ldez force-pushed the fix/report-without-filepath branch from 66970b8 to 7fdfd4d Compare March 21, 2024 15:15
@ldez
Copy link
Contributor Author

ldez commented Mar 22, 2024

I improved the PR to try to find the closest valid position.
I think that with this PR, 100% of reports will have a position (at least the closest possible to the problem).

@ldez ldez force-pushed the fix/report-without-filepath branch from f4311de to b08c735 Compare March 22, 2024 14:49
@kkHAIKE
Copy link
Owner

kkHAIKE commented Mar 23, 2024

LGTM

@kkHAIKE kkHAIKE merged commit 82fe695 into kkHAIKE:master Mar 23, 2024
@ldez ldez deleted the fix/report-without-filepath branch March 23, 2024 03:25
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.

Issue found in/using golangci-lint, about warnings not processing properly
3 participants