diff --git a/.github/workflows/eslint-monster.lock.yml b/.github/workflows/eslint-monster.lock.yml index ebcb27c84d6..a54f89887d6 100644 --- a/.github/workflows/eslint-monster.lock.yml +++ b/.github/workflows/eslint-monster.lock.yml @@ -541,9 +541,9 @@ jobs: mkdir -p "${RUNNER_TEMP}/gh-aw/safeoutputs" mkdir -p /tmp/gh-aw/safeoutputs mkdir -p /tmp/gh-aw/mcp-logs/safeoutputs - cat > "${RUNNER_TEMP}/gh-aw/safeoutputs/config.json" << 'GH_AW_SAFE_OUTPUTS_CONFIG_eaee126711029267_EOF' - {"assign_to_agent":{"allowed":["copilot"],"max":3,"target":"*"},"close_issue":{"max":10,"required_title_prefix":"[eslint-monster] ","state_reason":"duplicate"},"create_discussion":{"category":"audits","close_older_discussions":true,"expires":48,"fallback_to_issue":true,"max":1,"title_prefix":"[eslint-monster] "},"create_issue":{"expires":168,"labels":["automation","eslint","cookie"],"max":3,"title_prefix":"[eslint-monster] "},"create_report_incomplete_issue":{},"missing_data":{},"missing_tool":{},"noop":{"max":1,"report-as-issue":"true"},"report_incomplete":{},"update_issue":{"allow_body":true,"max":10,"required_title_prefix":"[eslint-monster] ","title_prefix":"[eslint-monster] "}} - GH_AW_SAFE_OUTPUTS_CONFIG_eaee126711029267_EOF + cat > "${RUNNER_TEMP}/gh-aw/safeoutputs/config.json" << 'GH_AW_SAFE_OUTPUTS_CONFIG_a70707033a3aa7b2_EOF' + {"assign_to_agent":{"allowed":["copilot"],"max":3,"target":"*"},"close_issue":{"max":10,"required_title_prefix":"[eslint-monster] ","state_reason":"duplicate"},"create_discussion":{"category":"audits","close_older_discussions":true,"expires":48,"fallback_to_issue":true,"max":1,"title_prefix":"[eslint-monster] "},"create_issue":{"expires":168,"labels":["automation","eslint","cookie"],"max":3,"title_prefix":"[eslint-monster] "},"create_report_incomplete_issue":{},"missing_data":{},"missing_tool":{},"noop":{"max":1,"report-as-issue":"true"},"report_incomplete":{},"update_issue":{"allow_body":true,"max":10,"title_prefix":"[eslint-monster] "}} + GH_AW_SAFE_OUTPUTS_CONFIG_a70707033a3aa7b2_EOF - name: Generate Safe Outputs Tools env: GH_AW_TOOLS_META_JSON: | @@ -1746,7 +1746,7 @@ jobs: GH_AW_ALLOWED_DOMAINS: "*.grafana.net,*.sentry.io,api.githubcopilot.com,api.pi.ai,api.snapcraft.io,archive.ubuntu.com,azure.archive.ubuntu.com,crl.geotrust.com,crl.globalsign.com,crl.identrust.com,crl.sectigo.com,crl.thawte.com,crl.usertrust.com,crl.verisign.com,crl3.digicert.com,crl4.digicert.com,crls.ssl.com,github.com,host.docker.internal,json-schema.org,json.schemastore.org,keyserver.ubuntu.com,ocsp.digicert.com,ocsp.geotrust.com,ocsp.globalsign.com,ocsp.identrust.com,ocsp.sectigo.com,ocsp.ssl.com,ocsp.thawte.com,ocsp.usertrust.com,ocsp.verisign.com,packagecloud.io,packages.cloud.google.com,packages.microsoft.com,ppa.launchpad.net,raw.githubusercontent.com,registry.npmjs.org,s.symcb.com,s.symcd.com,security.ubuntu.com,ts-crl.ws.symantec.com,ts-ocsp.ws.symantec.com,www.googleapis.com" GITHUB_SERVER_URL: ${{ github.server_url }} GITHUB_API_URL: ${{ github.api_url }} - GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: "{\"assign_to_agent\":{\"allowed\":[\"copilot\"],\"max\":3,\"target\":\"*\"},\"close_issue\":{\"max\":10,\"required_title_prefix\":\"[eslint-monster] \",\"state_reason\":\"duplicate\"},\"create_discussion\":{\"category\":\"audits\",\"close_older_discussions\":true,\"expires\":48,\"fallback_to_issue\":true,\"max\":1,\"title_prefix\":\"[eslint-monster] \"},\"create_issue\":{\"expires\":168,\"labels\":[\"automation\",\"eslint\",\"cookie\"],\"max\":3,\"title_prefix\":\"[eslint-monster] \"},\"create_report_incomplete_issue\":{},\"missing_data\":{},\"missing_tool\":{},\"noop\":{\"max\":1,\"report-as-issue\":\"true\"},\"report_incomplete\":{},\"update_issue\":{\"allow_body\":true,\"max\":10,\"required_title_prefix\":\"[eslint-monster] \",\"title_prefix\":\"[eslint-monster] \"}}" + GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: "{\"assign_to_agent\":{\"allowed\":[\"copilot\"],\"max\":3,\"target\":\"*\"},\"close_issue\":{\"max\":10,\"required_title_prefix\":\"[eslint-monster] \",\"state_reason\":\"duplicate\"},\"create_discussion\":{\"category\":\"audits\",\"close_older_discussions\":true,\"expires\":48,\"fallback_to_issue\":true,\"max\":1,\"title_prefix\":\"[eslint-monster] \"},\"create_issue\":{\"expires\":168,\"labels\":[\"automation\",\"eslint\",\"cookie\"],\"max\":3,\"title_prefix\":\"[eslint-monster] \"},\"create_report_incomplete_issue\":{},\"missing_data\":{},\"missing_tool\":{},\"noop\":{\"max\":1,\"report-as-issue\":\"true\"},\"report_incomplete\":{},\"update_issue\":{\"allow_body\":true,\"max\":10,\"title_prefix\":\"[eslint-monster] \"}}" GH_AW_ASSIGN_TO_AGENT_TOKEN: ${{ secrets.GH_AW_AGENT_TOKEN || secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN }} with: github-token: ${{ secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/lint-monster.lock.yml b/.github/workflows/lint-monster.lock.yml index fe74f359198..6bae1adde94 100644 --- a/.github/workflows/lint-monster.lock.yml +++ b/.github/workflows/lint-monster.lock.yml @@ -536,9 +536,9 @@ jobs: mkdir -p "${RUNNER_TEMP}/gh-aw/safeoutputs" mkdir -p /tmp/gh-aw/safeoutputs mkdir -p /tmp/gh-aw/mcp-logs/safeoutputs - cat > "${RUNNER_TEMP}/gh-aw/safeoutputs/config.json" << 'GH_AW_SAFE_OUTPUTS_CONFIG_0caeba2ce3ff905f_EOF' - {"assign_to_agent":{"allowed":["copilot"],"max":3,"target":"*"},"close_issue":{"max":10,"required_title_prefix":"[lint-monster] ","state_reason":"duplicate"},"create_discussion":{"category":"audits","close_older_discussions":true,"expires":48,"fallback_to_issue":true,"max":1,"title_prefix":"[lint-monster] "},"create_issue":{"expires":168,"labels":["automation","lint","cookie"],"max":3,"title_prefix":"[lint-monster] "},"create_report_incomplete_issue":{},"missing_data":{},"missing_tool":{},"noop":{"max":1,"report-as-issue":"true"},"report_incomplete":{},"update_issue":{"allow_body":true,"max":10,"required_title_prefix":"[lint-monster] ","title_prefix":"[lint-monster] "}} - GH_AW_SAFE_OUTPUTS_CONFIG_0caeba2ce3ff905f_EOF + cat > "${RUNNER_TEMP}/gh-aw/safeoutputs/config.json" << 'GH_AW_SAFE_OUTPUTS_CONFIG_77a7106ac1879fb2_EOF' + {"assign_to_agent":{"allowed":["copilot"],"max":3,"target":"*"},"close_issue":{"max":10,"required_title_prefix":"[lint-monster] ","state_reason":"duplicate"},"create_discussion":{"category":"audits","close_older_discussions":true,"expires":48,"fallback_to_issue":true,"max":1,"title_prefix":"[lint-monster] "},"create_issue":{"expires":168,"labels":["automation","lint","cookie"],"max":3,"title_prefix":"[lint-monster] "},"create_report_incomplete_issue":{},"missing_data":{},"missing_tool":{},"noop":{"max":1,"report-as-issue":"true"},"report_incomplete":{},"update_issue":{"allow_body":true,"max":10,"title_prefix":"[lint-monster] "}} + GH_AW_SAFE_OUTPUTS_CONFIG_77a7106ac1879fb2_EOF - name: Generate Safe Outputs Tools env: GH_AW_TOOLS_META_JSON: | @@ -1741,7 +1741,7 @@ jobs: GH_AW_ALLOWED_DOMAINS: "*.grafana.net,*.sentry.io,api.githubcopilot.com,api.pi.ai,api.snapcraft.io,archive.ubuntu.com,azure.archive.ubuntu.com,crl.geotrust.com,crl.globalsign.com,crl.identrust.com,crl.sectigo.com,crl.thawte.com,crl.usertrust.com,crl.verisign.com,crl3.digicert.com,crl4.digicert.com,crls.ssl.com,github.com,host.docker.internal,json-schema.org,json.schemastore.org,keyserver.ubuntu.com,ocsp.digicert.com,ocsp.geotrust.com,ocsp.globalsign.com,ocsp.identrust.com,ocsp.sectigo.com,ocsp.ssl.com,ocsp.thawte.com,ocsp.usertrust.com,ocsp.verisign.com,packagecloud.io,packages.cloud.google.com,packages.microsoft.com,ppa.launchpad.net,raw.githubusercontent.com,registry.npmjs.org,s.symcb.com,s.symcd.com,security.ubuntu.com,ts-crl.ws.symantec.com,ts-ocsp.ws.symantec.com,www.googleapis.com" GITHUB_SERVER_URL: ${{ github.server_url }} GITHUB_API_URL: ${{ github.api_url }} - GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: "{\"assign_to_agent\":{\"allowed\":[\"copilot\"],\"max\":3,\"target\":\"*\"},\"close_issue\":{\"max\":10,\"required_title_prefix\":\"[lint-monster] \",\"state_reason\":\"duplicate\"},\"create_discussion\":{\"category\":\"audits\",\"close_older_discussions\":true,\"expires\":48,\"fallback_to_issue\":true,\"max\":1,\"title_prefix\":\"[lint-monster] \"},\"create_issue\":{\"expires\":168,\"labels\":[\"automation\",\"lint\",\"cookie\"],\"max\":3,\"title_prefix\":\"[lint-monster] \"},\"create_report_incomplete_issue\":{},\"missing_data\":{},\"missing_tool\":{},\"noop\":{\"max\":1,\"report-as-issue\":\"true\"},\"report_incomplete\":{},\"update_issue\":{\"allow_body\":true,\"max\":10,\"required_title_prefix\":\"[lint-monster] \",\"title_prefix\":\"[lint-monster] \"}}" + GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: "{\"assign_to_agent\":{\"allowed\":[\"copilot\"],\"max\":3,\"target\":\"*\"},\"close_issue\":{\"max\":10,\"required_title_prefix\":\"[lint-monster] \",\"state_reason\":\"duplicate\"},\"create_discussion\":{\"category\":\"audits\",\"close_older_discussions\":true,\"expires\":48,\"fallback_to_issue\":true,\"max\":1,\"title_prefix\":\"[lint-monster] \"},\"create_issue\":{\"expires\":168,\"labels\":[\"automation\",\"lint\",\"cookie\"],\"max\":3,\"title_prefix\":\"[lint-monster] \"},\"create_report_incomplete_issue\":{},\"missing_data\":{},\"missing_tool\":{},\"noop\":{\"max\":1,\"report-as-issue\":\"true\"},\"report_incomplete\":{},\"update_issue\":{\"allow_body\":true,\"max\":10,\"title_prefix\":\"[lint-monster] \"}}" GH_AW_ASSIGN_TO_AGENT_TOKEN: ${{ secrets.GH_AW_AGENT_TOKEN || secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN }} with: github-token: ${{ secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN }} diff --git a/pkg/linters/writebytestring/testdata/src/writebytestring/writebytestring.go b/pkg/linters/writebytestring/testdata/src/writebytestring/writebytestring.go index fafdaa60ec6..5d9b1223c1b 100644 --- a/pkg/linters/writebytestring/testdata/src/writebytestring/writebytestring.go +++ b/pkg/linters/writebytestring/testdata/src/writebytestring/writebytestring.go @@ -23,7 +23,7 @@ type myString string func badNamedString() { var buf bytes.Buffer s := myString("hello") - buf.Write([]byte(s)) // want `buf\.Write\(\[\]byte\(s\)\) can be replaced with io\.WriteString\(&buf, s\) to potentially avoid a \[\]byte allocation if the writer implements io\.StringWriter` + buf.Write([]byte(s)) // want `buf\.Write\(\[\]byte\(s\)\) can be replaced with io\.WriteString\(&buf, string\(s\)\) to potentially avoid a \[\]byte allocation if the writer implements io\.StringWriter` } func badFile() { diff --git a/pkg/linters/writebytestring/testdata/src/writebytestring/writebytestring.go.golden b/pkg/linters/writebytestring/testdata/src/writebytestring/writebytestring.go.golden new file mode 100644 index 00000000000..5b00554a551 --- /dev/null +++ b/pkg/linters/writebytestring/testdata/src/writebytestring/writebytestring.go.golden @@ -0,0 +1,74 @@ +package writebytestring + +import ( + "bytes" + "io" + "os" +) + +type customWriter struct{} + +func (c *customWriter) Write(p []byte) (int, error) { return len(p), nil } + +func bad() { + var buf bytes.Buffer + s := "hello" + io.WriteString(&buf, s) // want `buf\.Write\(\[\]byte\(s\)\) can be replaced with io\.WriteString\(&buf, s\) to potentially avoid a \[\]byte allocation if the writer implements io\.StringWriter` + + io.WriteString(&buf, "world") // want `buf\.Write\(\[\]byte\("world"\)\) can be replaced with io\.WriteString\(&buf, "world"\) to potentially avoid a \[\]byte allocation if the writer implements io\.StringWriter` +} + +type myString string + +func badNamedString() { + var buf bytes.Buffer + s := myString("hello") + io.WriteString(&buf, string(s)) // want `buf\.Write\(\[\]byte\(s\)\) can be replaced with io\.WriteString\(&buf, string\(s\)\) to potentially avoid a \[\]byte allocation if the writer implements io\.StringWriter` +} + +func badFile() { + f, err := os.Create("/tmp/test.txt") + if err != nil { + return + } + defer f.Close() + msg := "hello" + io.WriteString(f, msg) // want `f\.Write\(\[\]byte\(msg\)\) can be replaced with io\.WriteString\(f, msg\) to potentially avoid a \[\]byte allocation if the writer implements io\.StringWriter` +} + +func badCustomWriter() { + w := &customWriter{} + s := "hello" + io.WriteString(w, s) // want `w\.Write\(\[\]byte\(s\)\) can be replaced with io\.WriteString\(w, s\) to potentially avoid a \[\]byte allocation if the writer implements io\.StringWriter` +} + +func badInterfaceWriter(w io.Writer) { + s := "hello" + io.WriteString(w, s) // want `w\.Write\(\[\]byte\(s\)\) can be replaced with io\.WriteString\(w, s\) to potentially avoid a \[\]byte allocation if the writer implements io\.StringWriter` +} + +func goodBytes() { + var buf bytes.Buffer + b := []byte("hello") + buf.Write(b) // already []byte — no conversion +} + +func goodWriteString() { + // Using io.WriteString is the idiomatic form. + var buf bytes.Buffer + s := "hello" + buf.WriteString(s) +} + +func goodNotString() { + var buf bytes.Buffer + n := 42 + buf.Write([]byte{byte(n)}) +} + +func suppressed() { + var buf bytes.Buffer + s := "hello" + //nolint:writebytestring + buf.Write([]byte(s)) +} diff --git a/pkg/linters/writebytestring/writebytestring.go b/pkg/linters/writebytestring/writebytestring.go index 932d3c0b200..6e0eca04088 100644 --- a/pkg/linters/writebytestring/writebytestring.go +++ b/pkg/linters/writebytestring/writebytestring.go @@ -104,6 +104,14 @@ func run(pass *analysis.Pass) (any, error) { return } + // io.WriteString requires an exact predeclared string argument. If the + // argument is a named string type (e.g. type MyStr string), wrap it with + // string(...) so the emitted fix compiles. + sExpr := sText + if st := pass.TypesInfo.TypeOf(strArg); st != nil && !isExactString(st) { + sExpr = "string(" + sText + ")" + } + // When the receiver is an addressable value whose Write method lives on // the pointer type (e.g. var buf bytes.Buffer), io.WriteString requires // the pointer form so that the interface conversion compiles. @@ -117,8 +125,8 @@ func run(pass *analysis.Pass) (any, error) { pass.Report(analysis.Diagnostic{ Pos: call.Pos(), End: call.End(), - Message: fmt.Sprintf("%s.Write([]byte(%s)) can be replaced with io.WriteString(%s, %s) to potentially avoid a []byte allocation if the writer implements io.StringWriter", wText, sText, writerArg, sText), - SuggestedFixes: buildFix(call, writerArg, sText), + Message: fmt.Sprintf("%s.Write([]byte(%s)) can be replaced with io.WriteString(%s, %s) to potentially avoid a []byte allocation if the writer implements io.StringWriter", wText, sText, writerArg, sExpr), + SuggestedFixes: buildFix(call, writerArg, sExpr), }) }) @@ -158,6 +166,15 @@ func isStringType(pass *analysis.Pass, expr ast.Expr) bool { return ok && basic.Kind() == types.String } +// isExactString reports whether t is the predeclared string type, not a named +// type whose underlying type is string. io.WriteString(w Writer, s string) +// requires a predeclared string; named string types need an explicit string(...) +// conversion to satisfy the parameter type. +func isExactString(t types.Type) bool { + b, ok := t.(*types.Basic) + return ok && b.Kind() == types.String +} + // implementsWriter reports whether expr's type implements io.Writer. // It uses types.Implements against a synthetic io.Writer interface so the check // is idiomatic and avoids manually re-implementing the signature comparison. diff --git a/pkg/linters/writebytestring/writebytestring_test.go b/pkg/linters/writebytestring/writebytestring_test.go index 5e2f458c301..f025a5fea9d 100644 --- a/pkg/linters/writebytestring/writebytestring_test.go +++ b/pkg/linters/writebytestring/writebytestring_test.go @@ -12,5 +12,5 @@ import ( func TestAnalyzer(t *testing.T) { testdata := analysistest.TestData() - analysistest.Run(t, testdata, writebytestring.Analyzer, "writebytestring") + analysistest.RunWithSuggestedFixes(t, testdata, writebytestring.Analyzer, "writebytestring") }