Hey, thank you for opening your first Pull Request !

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign ourContributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let usrecheck it.}

I'm having some issue running the tests that I can't seem to resolve, or even find the cause of:

go run ./cmd/golangci-lint/ run --no-config --disable-all --enable=responsewriterlint ./test/testdata/responsewriterlint.gotest/testdata/responsewriterlint.go:5:2: could not import errors (Config.Importer.Import(errors) returned nil but no error) (typecheck)"errors"^test/testdata/responsewriterlint.go:6:2: could not import fmt (Config.Importer.Import(fmt) returned nil but no error) (typecheck)"fmt"^test/testdata/responsewriterlint.go:7:2: could not import net/http (Config.Importer.Import(net/http) returned nil but no error) (typecheck)"net/http"^exit status 1

UPDATE: this was fixed by using theWithLoadForGoAnalysis method in the loader

In order for a pull request adding a linter to be reviewed, the linter and the PR must follow some requirements.

Pull Request Description

• It must have a link to the linter repository.
• It must provide a short description of the linter.

Linter

• It must not be a duplicate of another linter or a rule of a linter. (the team will help to verify that)
• It must have a valid license (AGPL is not allowed) and the file must contain the required information by the license, ex: author, year, etc.
• The linter repository must have a CI and tests.
• It must usego/analysis.
• It must have a valid tag, ex:v1.0.0,v0.1.0.
• It must not containinit().
• It must not containpanic(),log.fatal(),os.exit(), or similar.
• It must not have false positives/negatives. (the team will help to verify that)
• It must have tests inside golangci-lint.

The Linter Tests Inside Golangci-lint

• They must have at least one std lib import.
• They must work withT=<name of the linter test file>.go make test_linters. (the team will help to verify that)

.golangci.reference.yml

• The linter must be added to the list of available linters (alphabetical case-insensitive order).
  • enable anddisable options
• If the linter has a configuration, the exhaustive configuration of the linter must be added (alphabetical case-insensitive order)
  • The values must be different from the default ones.
  • The default values must be defined in a comment.
  • The option must have a short description.

Others Requirements

• The files (tests and linter) inside golangci-lint must have the same name as the linter.
• The.golangci.yml of golangci-lint itself must not be edited and the linter must not be added to this file.
• The linters must be sorted in the alphabetical order (case-insensitive) in theManager.GetAllSupportedLinterConfigs(...) and.golangci.reference.yml.
• The load mode (WithLoadMode(...)):
  • if the linter doesn't use types:goanalysis.LoadModeSyntax
  • goanalysis.LoadModeTypesInfo requiredWithLoadForGoAnalysis() in theManager.GetAllSupportedLinterConfigs(...)
• The version inWithSince(...) must be the next minor version (v1.X.0) of golangci-lint.

Recommendations

• The linter should not use SSA. (currently, SSA does not support generics)
• The linter repository should have a readme and linting.
• The linter should be published as a binary. (useful to diagnose bug origins)

The golangci-lint team will edit this comment to check the boxes before and during the review.

If the author of the PR is a member of the golangci-lint team, he should not edit this message.

This checklist does not imply that we will accept the linter.

Can someone explain to me whythis code generates theresponsewriter-lint issue?

funcserveDepsJS(rw http.ResponseWriter,req*http.Request,dirstring) {...b,err:=closure.GenDeps(root)iferr!=nil {log.Print(err)http.Error(rw,"Server error",500)return}rw.Header().Set("Content-Type","text/javascript; charset=utf-8")rw.Write([]byte("// auto-generated from perkeepd\n"))rw.Write(b)}

github.com/perkeep/perkeep/pkg/server/ui.go:700:5: function serveDepsJS: Multiple calls to http.ResponseWriter.Write in the same function body. This is most probably a bug.github.com/perkeep/perkeep/pkg/server/ui.go:701:5: function serveDepsJS: Multiple calls to http.ResponseWriter.Write in the same function body. This is most probably a bug.

@alexandear I'm unsure which one is your question:

• why is the responsewriter-lint linting code it shouldn't be linting? As far as I understand the way golangci-lint and its own testing, each linter should only deal with its own testdata files, but not the others, so I can't answer that. I don't even know if my assumption here is correct or not Misunderstood, that's in a different codebase, not a different linter. You seem to be using the version of golangci-lint that's in this branch with everything enabled? This linter is not in yet, there are some issues I need to work through it. It's probably best not to use this until such time that I get this into a position where it would be accepted and merged into it, and even then only turned on if you are okay with what it's testing.

The PR itself has a link to the linter's own repository, where the readme has info on what it aims to do and why. I wrote it such that if I was in a situation where I wanted to find out what it's for, the readme would answer it.

Let me know if you have additional questions.

@javorszky, I apologize if my question comes across as rude - as a non-native speaker, I want to make sure I understand how this linter can be helpful in finding bugs.

I'm checking the linter requirementIt must not have false positives/negatives. from thecomment above.

What I did:

• Download and install linter by running the command (this is a standard way to install Go programs, not every system hasmake, e.g. Windows):

go install github.com/javorszky/go-responsewriter-lint/cmd/responsewriter-lint@latest

• Run the linter on some big repo, e.g.perkeep:

responsewriter-lint ./...

perkeep was created by the former [maintainer] of Go'snet/http standard library, and therefore, it's very unlikely there are bugs withhttp.ResponseWriter.

• Get results, a false positives:

/Users/redko.o/src/github.com/perkeep/perkeep/internal/httputil/httputil.go:85:5: function ReturnJSONCode: Multiple calls to http.ResponseWriter.Write in the same function body. This is most probably a bug./Users/redko.o/src/github.com/perkeep/perkeep/internal/httputil/httputil.go:86:5: function ReturnJSONCode: Multiple calls to http.ResponseWriter.Write in the same function body. This is most probably a bug./Users/redko.o/src/github.com/perkeep/perkeep/pkg/server/ui.go:700:5: function serveDepsJS: Multiple calls to http.ResponseWriter.Write in the same function body. This is most probably a bug./Users/redko.o/src/github.com/perkeep/perkeep/pkg/server/ui.go:701:5: function serveDepsJS: Multiple calls to http.ResponseWriter.Write in the same function body. This is most probably a bug.

My understanding is that when you created the PR to addresponsewriter-lint togolangci-lint, the linter was considered complete and ready for production use.

I am curious to know if you have analyzed any open-source projects withresponsewriter-lint and foundhttp.ResponseWriter bugs. If so, could you provide links to the results? The examples in thetestdata folder seem artificial and may not represent real-world scenarios.

@alexandear that context makes a lot more sense, thank you. When I made the linter I followed this tutorial:https://disaev.me/p/writing-useful-go-analysis-linter/#run-on-the-real-and-large-repositories, and ran the linter on the Go source (1.19.2 at the time). That one did not flag up any issues.

With false positives / negatives, I agree with you. In this case I don'treally count it as a false positive:

• does it flag up those two calls to Write in the same function? yes
• do I expect the linter to flag them up? also yes
• is this a bug in the code with the two Write calls? probably not

I wrote in the description that there are times when multiple calls or out of order calls to those methods are desirable. The purpose of this linter is to get developers to be purposeful in calling them out of order or multiple times.

As an example, there's thegochecknoinit linter which flags up code that has aninit function that introduces side effects. A fair number of database drivers rely on there beinginit functions to work. I don't think that linter complaining about the init functions in those libraries would be false positive either.

The tests are what I used to validate the linter so that it triggers when I want it to, and does not trigger when I do not want it to. It's copy pasted from the linter's own repository. Tests there pass. Testing is written as per the tutorial.

It seems like the way golangci-lint wraps the analysis tests is somewhat different. Does it bail on the first found issue?

I would like some help with figuring out the differences between my own testing, and golangci-lint's way of doing it.

Good point in adding instructions / blurb to installation methods. I do not have access to a Windows computer, but will do my best to accommodate users of other OSs.

Maybe the flagging of multiple successive Write calls should be made optional and default to off. That would allow zero false positives and still allow to enable that check.

Multiple write calls are common in API responses that stream data and want to send out some initial data as soon as possible (e.g. a. CSV header in a CSV response). I would not like to flag all these in the future to make a linter happy.