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

Jump to bottom

x/tools/gopls: do not format broken function call site #53478

Closed
hyangah opened this issue Jun 21, 2022 · 3 comments
Closed

x/tools/gopls: do not format broken function call site #53478

hyangah opened this issue Jun 21, 2022 · 3 comments
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
gopls/unplanned

Comments

@hyangah
Copy link
Contributor

hyangah commented Jun 21, 2022

Save this code:

package pkg

import (
	"net/http"
)

func handler(w http.ResponseWriter, r *http.Request) {
	http.Error(w, ) // incomplete
}

See the "," in http.Error(w, ) // incomplete is removed.

I think we shouldn't format the line with syntax errors.
@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 Jun 21, 2022
@gopherbot gopherbot added this to the Unreleased milestone Jun 21, 2022
@dle8 dle8 modified the milestones: Unreleased, gopls/unplanned Jun 21, 2022
@muirdm
Copy link

muirdm commented Jun 22, 2022

I don't think the trailing comma is a syntax error. To do this we would need to look at the type checker error about missing argument, but that would be a significant departure from gofmt.

@adonovan
Copy link
Member

As Muir explains, formatting is a very shallow operation that works on any valid parse tree. It does not and cannot use types as part of its formatting decisions.

@hyangah
Copy link
Contributor Author

hyangah commented Jun 23, 2022 •
edited

I hoped we could utilize gopls's internal state before handling the format LSP request - so make formatting noop. However, I understand some users prefer formatting code regardless of the presence of type errors.

This case makes me more convinced vscode-go's default behavior that "formats on save" by default is not the right choice. (golang/vscode-go#1815)

@findleyr findleyr closed this as not planned Won't fix, can't repro, duplicate, stale Jul 1, 2022
@golang golang locked and limited conversation to collaborators Jul 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge 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
Milestone
gopls/unplanned
Development

No branches or pull requests
6 participants
@hyangah @adonovan @gopherbot @muirdm @dle8 @findleyr