QF1012 : b.WriteString(x + y) -> b.WriteString(s); b.WriteString(y) #1684#1697
QF1012 : b.WriteString(x + y) -> b.WriteString(s); b.WriteString(y) #1684#1697IlyasYOY wants to merge 3 commits intodominikh:masterfrom
Conversation
Add pattern `checkWriteStringConcatQ` to detect `WriteString(x + y)` calls and suggest splitting them into separate `WriteString` calls. Introduce `ReplaceWithStatements` in analysis/edit to replace a range with multiple statements. Update quickfix logic to use the new helper and extend the matcher list to include the new pattern.
Capture the full concat expression, add unwrapRight to recursively collect string literals, generate a WriteString statement for each literal, and extend tests and golden files to cover nested and multi‑literal concatenations.
… clarity Rename the helper function to better express its purpose and replace all its usages accordingly. This improves code readability and maintainability.
IlyasYOY
left a comment
There was a problem hiding this comment.
I'd be glad to finish this PR, if you think I'm heading in the right direction and you want to see this feature here.
| func run(pass *analysis.Pass) (any, error) { | ||
| fn := func(node ast.Node) { | ||
| getRecv := func(m *pattern.Matcher) (ast.Expr, types.Type) { | ||
| getRecv := func(m *pattern.Matcher) (ast.Expr, ast.Expr, types.Type) { |
There was a problem hiding this comment.
I've added a new return type, because I need original value to use it in WriteString calls. Now the logic might seem strange, but I didn't want remove & right after we added it.
I think I might:
- document it here or
- return struct/named arguments.
| report.Report(pass, node, msg, report.Fixes(fix)) | ||
| } else if m, ok := code.Match(pass, checkWriteStringConcatQ, node); ok { | ||
| _, recv, recvT := getRecv(m) | ||
| // TODO(IlyasYOY): add support for []bytes, similar to the block above. |
There was a problem hiding this comment.
I will definitely need to add Write method handling here.
| return all | ||
| } | ||
|
|
||
| // TODO(IlyasYOY): handle case for any expression returning string. |
There was a problem hiding this comment.
I will refactor this logic to handle more cases. For now, I support only literal ones.
Maybe you have an idea which cases I might beware the most?
I think all of them must be strings as long code compiles.
| }) | ||
| } | ||
|
|
||
| const msg = "Replace WriteString(x + y) with WriteString(x); WriteString(y)" |
There was a problem hiding this comment.
I think we might want to change the name. I could not think of the better option.
I was worried that the message might become too big.
|
I'll try to take a look at this once I get the chance, but it might be a while. |
Adds support for a simple string concat into QF1012 as mentioned in #1684.
I'd be glad to provide the fixes you think are necessary.