stringsbuilder.go

     1  // Copyright 2024 The Go Authors. All rights reserved.
     2  // Use of this source code is governed by a BSD-style
     3  // license that can be found in the LICENSE file.
     4  
     5  package modernize
     6  
     7  import (
     8  	"cmp"
     9  	"fmt"
    10  	"go/ast"
    11  	"go/constant"
    12  	"go/token"
    13  	"go/types"
    14  	"maps"
    15  	"slices"
    16  	"strings"
    17  
    18  	"golang.org/x/tools/go/analysis"
    19  	"golang.org/x/tools/go/analysis/passes/inspect"
    20  	"golang.org/x/tools/go/ast/edge"
    21  	"golang.org/x/tools/go/ast/inspector"
    22  	"golang.org/x/tools/internal/analysis/analyzerutil"
    23  	typeindexanalyzer "golang.org/x/tools/internal/analysis/typeindex"
    24  	"golang.org/x/tools/internal/astutil"
    25  	"golang.org/x/tools/internal/refactor"
    26  	"golang.org/x/tools/internal/typesinternal/typeindex"
    27  )
    28  
    29  var StringsBuilderAnalyzer = &analysis.Analyzer{
    30  	Name: "stringsbuilder",
    31  	Doc:  analyzerutil.MustExtractDoc(doc, "stringsbuilder"),
    32  	Requires: []*analysis.Analyzer{
    33  		inspect.Analyzer,
    34  		typeindexanalyzer.Analyzer,
    35  	},
    36  	Run: stringsbuilder,
    37  	URL: "https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/modernize#stringbuilder",
    38  }
    39  
    40  // stringsbuilder replaces string += string in a loop by strings.Builder.
    41  func stringsbuilder(pass *analysis.Pass) (any, error) {
    42  	// Skip the analyzer in packages where its
    43  	// fixes would create an import cycle.
    44  	if within(pass, "strings", "runtime") {
    45  		return nil, nil
    46  	}
    47  
    48  	var (
    49  		inspect = pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
    50  		index   = pass.ResultOf[typeindexanalyzer.Analyzer].(*typeindex.Index)
    51  		info    = pass.TypesInfo
    52  	)
    53  
    54  	// Gather all local string variables that appear on the
    55  	// LHS of some string += string assignment.
    56  	candidates := make(map[*types.Var]bool)
    57  	for curAssign := range inspect.Root().Preorder((*ast.AssignStmt)(nil)) {
    58  		assign := curAssign.Node().(*ast.AssignStmt)
    59  		if assign.Tok == token.ADD_ASSIGN && is[*ast.Ident](assign.Lhs[0]) {
    60  			if v, ok := info.Uses[assign.Lhs[0].(*ast.Ident)].(*types.Var); ok &&
    61  				v.Kind() == types.LocalVar &&
    62  				types.Identical(v.Type(), builtinString.Type()) {
    63  				candidates[v] = true
    64  			}
    65  		}
    66  	}
    67  
    68  	lexicalOrder := func(x, y *types.Var) int { return cmp.Compare(x.Pos(), y.Pos()) }
    69  
    70  	// File and Pos of last fix edit,
    71  	// for overlapping fix span detection.
    72  	var (
    73  		lastEditFile *ast.File
    74  		lastEditEnd  token.Pos
    75  	)
    76  
    77  	// Now check each candidate variable's decl and uses.
    78  nextcand:
    79  	for _, v := range slices.SortedFunc(maps.Keys(candidates), lexicalOrder) {
    80  		var edits, postEdits []analysis.TextEdit // postEdits are emitted last
    81  
    82  		// Check declaration of s has one of these forms:
    83  		//
    84  		//    s := expr
    85  		//    var s [string] [= expr]
    86  		//    var ( ...; s [string] [= expr] )			(s is last)
    87  		//
    88  		// and transform to one of:
    89  		//
    90  		//    var   s strings.Builder ; s.WriteString(expr)
    91  		//    var ( s strings.Builder); s.WriteString(expr)
    92  		//
    93  		def, ok := index.Def(v)
    94  		if !ok {
    95  			continue
    96  		}
    97  
    98  		// To avoid semantic conflicts, do not offer a fix if its edit
    99  		// range (ignoring import edits) overlaps a previous fix.
   100  		// This fixes #76983 and is an ad-hoc mitigation of #76476.
   101  		file := astutil.EnclosingFile(def)
   102  		if file == lastEditFile && v.Pos() < lastEditEnd {
   103  			continue
   104  		}
   105  		filename := pass.Fset.File(file.FileStart).Name()
   106  		// Suppress diagnostics in test files, where suggested fixes may increase
   107  		// verbosity, and performance doesn't matter as much.
   108  		// See https://go.dev/issue/78613
   109  		if strings.HasSuffix(filename, "_test.go") {
   110  			continue
   111  		}
   112  
   113  		ek := def.ParentEdgeKind()
   114  		if ek == edge.AssignStmt_Lhs &&
   115  			len(def.Parent().Node().(*ast.AssignStmt).Lhs) == 1 {
   116  			// Have: s := expr
   117  			// => var s strings.Builder; s.WriteString(expr)
   118  
   119  			assign := def.Parent().Node().(*ast.AssignStmt)
   120  
   121  			// Reject "if s := f(); ..." since in that context
   122  			// we can't replace the assign with two statements.
   123  			switch def.Parent().Parent().Node().(type) {
   124  			case *ast.BlockStmt, *ast.CaseClause, *ast.CommClause:
   125  				// OK: these are the parts of syntax that
   126  				// allow unrestricted statement lists.
   127  			default:
   128  				continue
   129  			}
   130  
   131  			// Add strings import.
   132  			prefix, importEdits := refactor.AddImport(
   133  				info, astutil.EnclosingFile(def), "strings", "strings", "Builder", v.Pos())
   134  			edits = append(edits, importEdits...)
   135  
   136  			if isEmptyString(info, assign.Rhs[0]) {
   137  				// s := ""
   138  				// ---------------------
   139  				// var s strings.Builder
   140  				edits = append(edits, analysis.TextEdit{
   141  					Pos:     assign.Pos(),
   142  					End:     assign.End(),
   143  					NewText: fmt.Appendf(nil, "var %[1]s %[2]sBuilder", v.Name(), prefix),
   144  				})
   145  
   146  			} else {
   147  				// s :=                                 expr
   148  				// -------------------------------------    -
   149  				// var s strings.Builder; s.WriteString(expr)
   150  				edits = append(edits, []analysis.TextEdit{
   151  					{
   152  						Pos:     assign.Pos(),
   153  						End:     assign.Rhs[0].Pos(),
   154  						NewText: fmt.Appendf(nil, "var %[1]s %[2]sBuilder; %[1]s.WriteString(", v.Name(), prefix),
   155  					},
   156  					{
   157  						Pos:     assign.End(),
   158  						End:     assign.End(),
   159  						NewText: []byte(")"),
   160  					},
   161  				}...)
   162  
   163  			}
   164  
   165  		} else if ek == edge.ValueSpec_Names &&
   166  			len(def.Parent().Node().(*ast.ValueSpec).Names) == 1 &&
   167  			first(def.Parent().Parent().LastChild()) == def.Parent() {
   168  			// Have: var   s [string] [= expr]
   169  			//   or: var ( s [string] [= expr] )
   170  			// => var s strings.Builder; s.WriteString(expr)
   171  			//
   172  			// The LastChild check rejects this case:
   173  			//   var ( s [string] [= expr]; others... )
   174  			// =>
   175  			//   var ( s strings.Builder; others... ); s.WriteString(expr)
   176  			// since it moves 'expr' across 'others', requiring
   177  			// reformatting of syntax, which in general is lossy
   178  			// of comments and vertical space.
   179  			// We expect this to be rare.
   180  
   181  			// Add strings import.
   182  			prefix, importEdits := refactor.AddImport(
   183  				info, astutil.EnclosingFile(def), "strings", "strings", "Builder", v.Pos())
   184  			edits = append(edits, importEdits...)
   185  
   186  			spec := def.Parent().Node().(*ast.ValueSpec)
   187  			decl := def.Parent().Parent().Node().(*ast.GenDecl)
   188  
   189  			init := spec.Names[0].End() // start of " = expr"
   190  			if spec.Type != nil {
   191  				init = spec.Type.End()
   192  			}
   193  
   194  			// Replace (possibly absent) type:
   195  			//
   196  			// var s [string]
   197  			//      ----------------
   198  			// var s strings.Builder
   199  			edits = append(edits, analysis.TextEdit{
   200  				Pos:     spec.Names[0].End(),
   201  				End:     init,
   202  				NewText: fmt.Appendf(nil, " %sBuilder", prefix),
   203  			})
   204  
   205  			if len(spec.Values) > 0 && !isEmptyString(info, spec.Values[0]) {
   206  				if decl.Rparen.IsValid() {
   207  					// var decl with explicit parens:
   208  					//
   209  					// var ( ...  =               expr )
   210  					//           -                     -
   211  					// var ( ... ); s.WriteString(expr)
   212  					edits = append(edits, []analysis.TextEdit{
   213  						{
   214  							Pos:     init,
   215  							End:     init,
   216  							NewText: []byte(")"),
   217  						},
   218  						{
   219  							Pos: spec.Values[0].End(),
   220  							End: decl.End(),
   221  						},
   222  					}...)
   223  				}
   224  
   225  				// =               expr
   226  				// ----------------    -
   227  				// ; s.WriteString(expr)
   228  				edits = append(edits, []analysis.TextEdit{
   229  					{
   230  						Pos:     init,
   231  						End:     spec.Values[0].Pos(),
   232  						NewText: fmt.Appendf(nil, "; %s.WriteString(", v.Name()),
   233  					},
   234  					{
   235  						Pos:     spec.Values[0].End(),
   236  						End:     spec.Values[0].End(),
   237  						NewText: []byte(")"),
   238  					},
   239  				}...)
   240  			} else {
   241  				// delete "= expr"
   242  				edits = append(edits, analysis.TextEdit{
   243  					Pos: init,
   244  					End: spec.End(),
   245  				})
   246  			}
   247  
   248  		} else {
   249  			continue
   250  		}
   251  
   252  		// Check uses of s.
   253  		//
   254  		// - All uses of s except the final one must be of the form
   255  		//
   256  		//    s += expr
   257  		//
   258  		//   Each of these will become s.WriteString(expr).
   259  		//   At least one of them must be in an intervening loop
   260  		//   w.r.t. the declaration of s:
   261  		//
   262  		//    var s string
   263  		//    for ... { s += expr }
   264  		//
   265  		// - All uses of s after the last += must be rvalue uses (e.g. use(s), not &s).
   266  		//   Each of these will become s.String().
   267  		//
   268  		//   Perhaps surprisingly, it is fine for there to be an
   269  		//   intervening loop or lambda w.r.t. the declaration of s:
   270  		//
   271  		//    var s strings.Builder
   272  		//    for range kSmall { s.WriteString(expr) }
   273  		//    for range kLarge { use(s.String()) } // called repeatedly
   274  		//
   275  		//   Even though that might cause the s.String() operation to be
   276  		//   executed repeatedly, this is not a deoptimization because,
   277  		//   by design, (*strings.Builder).String does not allocate.
   278  		var (
   279  			numLoopAssigns int             // number of += assignments within a loop
   280  			loopAssign     *ast.AssignStmt // first += assignment within a loop
   281  			seenRvalueUse  bool            // => we've seen at least one rvalue use of s
   282  		)
   283  		for curUse := range index.Uses(v) {
   284  			// Strip enclosing parens around Ident.
   285  			curUse = astutil.UnparenEnclosingCursor(curUse)
   286  			ek := curUse.ParentEdgeKind()
   287  
   288  			// intervening reports whether cur has an ancestor of
   289  			// one of the given types that is within the scope of v.
   290  			intervening := func(types ...ast.Node) bool {
   291  				for cur := range curUse.Enclosing(types...) {
   292  					if v.Pos() <= cur.Node().Pos() { // in scope of v
   293  						return true
   294  					}
   295  				}
   296  				return false
   297  			}
   298  
   299  			if ek == edge.AssignStmt_Lhs {
   300  				// After an rvalue use, no more assignments are allowed.
   301  				if seenRvalueUse {
   302  					continue nextcand
   303  				}
   304  
   305  				assign := curUse.Parent().Node().(*ast.AssignStmt)
   306  				if assign.Tok != token.ADD_ASSIGN {
   307  					continue nextcand
   308  				}
   309  				// Have: s += expr
   310  
   311  				// At least one of the += operations
   312  				// must appear within a loop.
   313  				// relative to the declaration of s.
   314  				if intervening((*ast.ForStmt)(nil), (*ast.RangeStmt)(nil)) {
   315  					numLoopAssigns++
   316  					if loopAssign == nil {
   317  						loopAssign = assign
   318  					}
   319  				}
   320  
   321  				// s +=          expr
   322  				//  -------------    -
   323  				// s.WriteString(expr)
   324  				edits = append(edits, analysis.TextEdit{
   325  					// replace " += " with ".WriteString("
   326  					Pos:     assign.Lhs[0].End(),
   327  					End:     assign.Rhs[0].Pos(),
   328  					NewText: []byte(".WriteString("),
   329  				})
   330  
   331  				// Delay inserting the closing parenthesis, in case it overlaps with a
   332  				// .String() edit, since it would need to come after.
   333  				postEdits = append(postEdits, analysis.TextEdit{
   334  					// insert ")"
   335  					Pos:     assign.End(),
   336  					End:     assign.End(),
   337  					NewText: []byte(")"),
   338  				})
   339  
   340  			} else if ek == edge.UnaryExpr_X &&
   341  				curUse.Parent().Node().(*ast.UnaryExpr).Op == token.AND {
   342  				// Have: use(&s)
   343  				continue nextcand // s is used as an lvalue; reject
   344  
   345  			} else {
   346  				// The only possible l-value uses of a string variable
   347  				// are assignments (s=expr, s+=expr, etc) and &s.
   348  				// (For strings, we can ignore method calls s.m().)
   349  				// All other uses are r-values.
   350  				seenRvalueUse = true
   351  
   352  				edits = append(edits, analysis.TextEdit{
   353  					// insert ".String()"
   354  					Pos:     curUse.Node().End(),
   355  					End:     curUse.Node().End(),
   356  					NewText: []byte(".String()"),
   357  				})
   358  			}
   359  		}
   360  		if !seenRvalueUse {
   361  			continue nextcand // no rvalue use; reject
   362  		}
   363  		if numLoopAssigns == 0 {
   364  			continue nextcand // no += in a loop; reject
   365  		}
   366  
   367  		edits = append(edits, postEdits...)
   368  
   369  		lastEditFile = file
   370  		lastEditEnd = edits[len(edits)-1].End
   371  
   372  		pass.Report(analysis.Diagnostic{
   373  			Pos:     loopAssign.Pos(),
   374  			End:     loopAssign.End(),
   375  			Message: "using string += string in a loop is inefficient",
   376  			SuggestedFixes: []analysis.SuggestedFix{{
   377  				Message:   "Replace string += string with strings.Builder",
   378  				TextEdits: edits,
   379  			}},
   380  		})
   381  	}
   382  
   383  	return nil, nil
   384  }
   385  
   386  // isEmptyString reports whether e (a string-typed expression) has constant value "".
   387  func isEmptyString(info *types.Info, e ast.Expr) bool {
   388  	tv, ok := info.Types[e]
   389  	return ok && tv.Value != nil && constant.StringVal(tv.Value) == ""
   390  }
   391
View as plain text