From 216213c58d29dc8ba2093a1eff82fc67ffa7f3aa Mon Sep 17 00:00:00 2001 From: ccoVeille <3875889+ccoVeille@users.noreply.github.com> Date: Wed, 31 Jul 2024 23:25:57 +0200 Subject: [PATCH 1/2] QF1013: detect strings that could be rewritten with a string literal fix #1566 --- quickfix/analysis.go | 2 + quickfix/qf1013/qf1013.go | 102 ++++++++++++++++++ quickfix/qf1013/qf1013_test.go | 13 +++ .../CheckForStringLiteral/string-literal.go | 25 +++++ .../string-literal.go.golden | 25 +++++ 5 files changed, 167 insertions(+) create mode 100644 quickfix/qf1013/qf1013.go create mode 100644 quickfix/qf1013/qf1013_test.go create mode 100644 quickfix/qf1013/testdata/go1.0/CheckForStringLiteral/string-literal.go create mode 100644 quickfix/qf1013/testdata/go1.0/CheckForStringLiteral/string-literal.go.golden diff --git a/quickfix/analysis.go b/quickfix/analysis.go index b877a9105..60bf5ee2e 100644 --- a/quickfix/analysis.go +++ b/quickfix/analysis.go @@ -16,6 +16,7 @@ import ( "honnef.co/go/tools/quickfix/qf1010" "honnef.co/go/tools/quickfix/qf1011" "honnef.co/go/tools/quickfix/qf1012" + "honnef.co/go/tools/quickfix/qf1013" ) var Analyzers = []*lint.Analyzer{ @@ -31,4 +32,5 @@ var Analyzers = []*lint.Analyzer{ qf1010.SCAnalyzer, qf1011.SCAnalyzer, qf1012.SCAnalyzer, + qf1013.SCAnalyzer, } diff --git a/quickfix/qf1013/qf1013.go b/quickfix/qf1013/qf1013.go new file mode 100644 index 000000000..c1a5fb278 --- /dev/null +++ b/quickfix/qf1013/qf1013.go @@ -0,0 +1,102 @@ +package qf1013 + +import ( + "go/ast" + "go/token" + "strings" + + "honnef.co/go/tools/analysis/code" + "honnef.co/go/tools/analysis/edit" + "honnef.co/go/tools/analysis/lint" + "honnef.co/go/tools/analysis/report" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" +) + +const escapeCharacter = '\\' + +var SCAnalyzer = lint.InitializeAnalyzer(&lint.Analyzer{ + Analyzer: &analysis.Analyzer{ + Name: "QF1013", + Run: run, + Requires: []*analysis.Analyzer{inspect.Analyzer}, + }, + Doc: &lint.RawDocumentation{ + Title: `Simplify string by using raw string literal`, + Since: "2024.8", + Before: `"a string with an escape quote: \""`, + After: "`a string with an escape quote: \"`", + Severity: lint.SeverityHint, + }, +}) + +func run(pass *analysis.Pass) (any, error) { + fn := func(node ast.Node) { + lit, ok := node.(*ast.BasicLit) + if !ok { + return // not a basic lit + } + if lit.Kind != token.STRING { + return // not a string + } + + if strings.HasPrefix(lit.Value, "`") { + // already a raw string + return + } + val := lit.Value + + // in string literals, any character may appear except back quote. + if strings.Contains(val, "`") { + // so, we cannot transform it to a raw string if a back quote appears + return + } + + // this quickfix is intended to write simpler strings by using string literal + // but it is limited to the following use cases: + // - a quote is escaped + // - a backslash is escaped + // - everything else is ignored + if !strings.Contains(val, string(escapeCharacter)) { + // no blackslash in the string + // nothing to do + return + } + + var cleanedVal string + var escapeCharacterFound bool + for _, c := range val[1 : len(val)-1] { + if !escapeCharacterFound { + escapeCharacterFound = (c == escapeCharacter) + if !escapeCharacterFound { + cleanedVal += string(c) + } + continue + } + + // so the previous character was a backslash + // we reset the flag for next character in the string + escapeCharacterFound = false + + switch c { + case escapeCharacter: + // we have an escaped backslash + case '"': + // we have an escaped quote + default: + // currently unsupported + return + } + cleanedVal += string(c) + } + + msg := "Simplify string by using raw string literal" + fix := edit.Fix(msg, edit.ReplaceWithNode(pass.Fset, node, &ast.BasicLit{ + Value: "`" + cleanedVal + "`", + })) + report.Report(pass, node, msg, report.Fixes(fix)) + } + code.Preorder(pass, fn, (*ast.BasicLit)(nil)) + return nil, nil +} diff --git a/quickfix/qf1013/qf1013_test.go b/quickfix/qf1013/qf1013_test.go new file mode 100644 index 000000000..82c1ecc64 --- /dev/null +++ b/quickfix/qf1013/qf1013_test.go @@ -0,0 +1,13 @@ +// Code generated by generate.go. DO NOT EDIT. + +package qf1013 + +import ( + "testing" + + "honnef.co/go/tools/analysis/lint/testutil" +) + +func TestTestdata(t *testing.T) { + testutil.Run(t, SCAnalyzer) +} diff --git a/quickfix/qf1013/testdata/go1.0/CheckForStringLiteral/string-literal.go b/quickfix/qf1013/testdata/go1.0/CheckForStringLiteral/string-literal.go new file mode 100644 index 000000000..c08f0681f --- /dev/null +++ b/quickfix/qf1013/testdata/go1.0/CheckForStringLiteral/string-literal.go @@ -0,0 +1,25 @@ +package pkg + +func fn2(_ string) { +} + +func fn() { + _ = "{\"foo\":true}" //@ diag(`Simplify string by using raw string literal`) + _ = "escaped backslash: \\ foo" //@ diag(`Simplify string by using raw string literal`) + _ = "double escaped new line: \\n" //@ diag(`Simplify string by using raw string literal`) + _ = "triple escaped new line: \\\\n" //@ diag(`Simplify string by using raw string literal`) + + const _ = "escaped double quotes: \"" //@ diag(`Simplify string by using raw string literal`) + fn2("escaped double quotes: \"") //@ diag(`Simplify string by using raw string literal`) + a := "escaped double quotes: \"" //@ diag(`Simplify string by using raw string literal`) + + fn2(a) + const _ = "abc" + _ = "abc" + _ = "escaped backslash \\ plus a back quote `" + _ = "escaped double quotes \" plus a back quote `" + _ = "escaped double quotes \" plus a new line \n" + _ = "escaped double quotes \" plus a tab \t" + + _ = `abc` +} diff --git a/quickfix/qf1013/testdata/go1.0/CheckForStringLiteral/string-literal.go.golden b/quickfix/qf1013/testdata/go1.0/CheckForStringLiteral/string-literal.go.golden new file mode 100644 index 000000000..431144c56 --- /dev/null +++ b/quickfix/qf1013/testdata/go1.0/CheckForStringLiteral/string-literal.go.golden @@ -0,0 +1,25 @@ +package pkg + +func fn2(_ string) { +} + +func fn() { + _ = `{"foo":true}` //@ diag(`Simplify string by using raw string literal`) + _ = `escaped backslash: \ foo` //@ diag(`Simplify string by using raw string literal`) + _ = `double escaped new line: \n` //@ diag(`Simplify string by using raw string literal`) + _ = `triple escaped new line: \\n` //@ diag(`Simplify string by using raw string literal`) + + const _ = `escaped double quotes: "` //@ diag(`Simplify string by using raw string literal`) + fn2(`escaped double quotes: "`) //@ diag(`Simplify string by using raw string literal`) + a := `escaped double quotes: "` //@ diag(`Simplify string by using raw string literal`) + + fn2(a) + const _ = "abc" + _ = "abc" + _ = "escaped backslash \\ plus a back quote `" + _ = "escaped double quotes \" plus a back quote `" + _ = "escaped double quotes \" plus a new line \n" + _ = "escaped double quotes \" plus a tab \t" + + _ = `abc` +} From aa44bce8e44fd8eca18417fa73dbbe18c4de5080 Mon Sep 17 00:00:00 2001 From: ccoVeille <3875889+ccoVeille@users.noreply.github.com> Date: Wed, 31 Jul 2024 23:35:10 +0200 Subject: [PATCH 2/2] wip: proof of concept that shows qf1013 conflicts with s1007 --- .../testdata/go1.0/CheckForStringLiteral/string-literal.go | 5 +++++ .../go1.0/CheckForStringLiteral/string-literal.go.golden | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/quickfix/qf1013/testdata/go1.0/CheckForStringLiteral/string-literal.go b/quickfix/qf1013/testdata/go1.0/CheckForStringLiteral/string-literal.go index c08f0681f..a3f984a0b 100644 --- a/quickfix/qf1013/testdata/go1.0/CheckForStringLiteral/string-literal.go +++ b/quickfix/qf1013/testdata/go1.0/CheckForStringLiteral/string-literal.go @@ -1,5 +1,7 @@ package pkg +import "regexp" + func fn2(_ string) { } @@ -22,4 +24,7 @@ func fn() { _ = "escaped double quotes \" plus a tab \t" _ = `abc` + _ = regexp.MustCompile("escaped backslash: \\ foo") + b := "escaped backslash: \\ foo" + _ = regexp.MustCompile(b) } diff --git a/quickfix/qf1013/testdata/go1.0/CheckForStringLiteral/string-literal.go.golden b/quickfix/qf1013/testdata/go1.0/CheckForStringLiteral/string-literal.go.golden index 431144c56..d829487dd 100644 --- a/quickfix/qf1013/testdata/go1.0/CheckForStringLiteral/string-literal.go.golden +++ b/quickfix/qf1013/testdata/go1.0/CheckForStringLiteral/string-literal.go.golden @@ -1,5 +1,7 @@ package pkg +import "regexp" + func fn2(_ string) { } @@ -22,4 +24,7 @@ func fn() { _ = "escaped double quotes \" plus a tab \t" _ = `abc` + _ = regexp.MustCompile("escaped backslash: \\ foo") + b := "escaped backslash: \\ foo" + _ = regexp.MustCompile(b) }