Skip to content

x/tools/go/analysis/passes/modernize: stringsbuilder: suppress diagnostics in tests, where accidentally quadratic is not a DoS risk #78613

@arp242

Description

@arp242

The modernisers in go fix are fairly uncontroversial: the "modern" way is obviously better. Good stuff.

The stringsbuilder moderniser is the exception, as the replacement code is more verbose, albeit faster. This is a meaningful improvement in some cases, but in many cases the performance gain is completely meaningless as the code just isn't run often enough: something like printing the output of a CLI program or concatenating a string in a test case are common examples. go fix makes the code worse here, for no measurable benefit.

I've run go fix on a whole bunch of projects in the last few months, and it has never made the code faster and only replaced cases where += was intentionally used because it's more convenient and the performance doesn't matter.

I don't know if this ship has sailed by now, but personally I would not be in favour of including it at all in the standard go fix run as it seems far too opinionated and context-dependent on whether it's better, which makes it stand out from all the other modernisers.

Alternatively, excluding this fix from test files would alleviate the worst of it.

Metadata

Metadata

Assignees

No one assigned

    Labels

    AnalysisIssues related to static analysis (vet, x/tools/go/analysis)NeedsInvestigationSomeone must examine and confirm this is a valid issue and not a duplicate of an existing one.

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions