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: `unusedparams` should ignore unused params in functions satisfying the `http.HandlerFunc` type #44152

Open
PSalant726 opened this issue Feb 7, 2021 · 2 comments
Labels

Comments

@PSalant726
Copy link

@PSalant726 PSalant726 commented Feb 7, 2021

What version of Go, VS Code & VS Code Go extension are you using?

  • Run go version to get version of Go from the VS Code integrated terminal.
    • 1.15.7 darwin/amd64
  • Run gopls -v version to get version of Gopls from the VS Code integrated terminal.
    • 0.6.5
  • Run code -v or code-insiders -v to get version of VS Code or VS Code Insiders.
    • 1.53.0
  • Check your installed extensions to get the version of the VS Code Go extension
    • 0.22.0

Share the Go related settings you have added/edited

"[go]": {
        "editor.codeActionsOnSave": { "source.organizeImports": true },
        "editor.formatOnSave": true
},
"[go.mod]": {
        "editor.codeActionsOnSave": { "source.organizeImports": true },
        "editor.formatOnSave": true
},
"go.coverOnTestPackage": false,
"go.formatTool": "goimports",
"go.lintFlags": ["--fast", "--max-same-issues=0"],
"go.lintTool": "golangci-lint",
"go.playground": { "run": false },
"go.testFlags": ["-count=1", "-cover"],
"go.toolsManagement.autoUpdate": true,
"gopls": {
        "ui.codelenses": { "generate": false, "test": true },
        "ui.completion.usePlaceholders": true,
        "ui.diagnostic.analyses": { "unusedparams": true }
}

Describe the bug

With code like the below example, where a function (previewHandler) satisfies the http.HanderlFunc type, but does not use or modify the http.Request (second argument), the unusedparams analyzer included with gopls should not warn on the issue.

package main

import (
  "log"
  "net/http"
  "text/template"

  "github.com/gorilla/mux"
)

func NewRouterWithRoutes() *mux.Router {
  var (
    router = mux.NewRouter()
    get    = router.Methods(http.MethodGet).Subrouter()
  )

  get.HandleFunc(PathPreview, previewHandler)

  return router
}

func previewHandler(w http.ResponseWriter, r *http.Request) {  // WARNING: potentially unused parameter: 'r' (unusedparams)
  if err := templates.ExecuteTemplate(w, "preview.html", &Page{}); err != nil {
    http.Error(w, err.Error(), http.StatusInternalServerError)
    log.Println(err)
  }
}
@stamblerre stamblerre transferred this issue from golang/vscode-go Feb 7, 2021
@hyangah
Copy link
Contributor

@hyangah hyangah commented Feb 7, 2021

Don't people use _ or unnamed params to make it clear the params aren't used in the body?
The unusedparams analyzer already ignores parameters that do not have a name or are underscored.

cc @stamblerre

@PSalant726
Copy link
Author

@PSalant726 PSalant726 commented Feb 7, 2021

Don't people use _ or unnamed params to make it clear the params aren't used in the body?

Yes, some do, but this isn't strictly enforced by the compiler. I don't believe that gopls should warn on functions that implement such a commonly used type.

@hyangah hyangah changed the title `unusedparams` should ignore unused params in functions satisfying the `http.HandlerFunc` type x/tools/gopls: `unusedparams` should ignore unused params in functions satisfying the `http.HandlerFunc` type Feb 7, 2021
@gopherbot gopherbot added this to the Unreleased milestone Feb 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants