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

x/tools/gopls: use more specific quick fix description for undefined name #65087

Closed
hyangah opened this issue Jan 12, 2024 · 5 comments
Closed
Assignees
Labels
gopls/analysis Issues related to running analysis in gopls gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@hyangah
Copy link
Contributor

hyangah commented Jan 12, 2024

When the function is not defined, gopls provides a quickfix with the description "undefined"
Screenshot 2024-01-12 at 5 58 03 PM

It's unclear what it does. Use more specific description e.g. "add missing function" like TS language server
Screenshot 2024-01-12 at 6 04 14 PM

@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Jan 12, 2024
@gopherbot gopherbot added this to the Unreleased milestone Jan 12, 2024
@raghvenders
Copy link
Contributor

@hyangah -It looks like actionable message. I am making changes for this. Kindly assign this to me.
Even for undeclared name too, we can change into an actionable message "Add missing declaration ..."

By the way, as this message got changed go-staticcheck uses the same string undefined and those bring out warning. Kindly suggest.

@adonovan
Copy link
Member

Looking at the logic in gopls that creates this patch--in the undeclared analyzer--it seems that it already gives a reasonable name to the fix, but this must be getting lost somewhere along the plumbing between the analyzer and the LSP:

https://cs.opensource.google/go/x/tools/+/refs/tags/gopls/v0.15.0-pre.1:gopls/internal/analysis/undeclaredname/undeclared.go;l=146-156

	return fset, &analysis.SuggestedFix{
		Message: fmt.Sprintf("Create function \"%s\"", ident.Name),
		TextEdits: []analysis.TextEdit{{
			Pos:     pos,
			End:     pos,
			NewText: b.Bytes(),
		}},
	}, nil

@raghvenders
Copy link
Contributor

@alandonovan , Thanks. The place you are referring comes in while apply Fix, correct me if I can understand this better.

I tried applying the fix locally and it goes in runForError() and change error message for functions undeclared

@hyangah Is this expected ?

pass.Report(analysis.Diagnostic{
		Pos:     err.Pos,
		End:     end,
		Message: err.Msg,
	})

Screenshot 2024-01-16 160316
Screenshot 2024-01-16 160303

@hyangah
Copy link
Contributor Author

hyangah commented Jan 17, 2024

Thanks @raghvenders
Create function "%s" as proposed by the analyzer seems more concise.

@hyangah hyangah modified the milestones: Unreleased, gopls/v0.16.0 Jan 17, 2024
@hyangah hyangah added the gopls/analysis Issues related to running analysis in gopls label Jan 17, 2024
@gopherbot
Copy link

Change https://go.dev/cl/556755 mentions this issue: gopls/internal/lsp/source: fix Fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gopls/analysis Issues related to running analysis in gopls gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

5 participants