String() should not mutate object

[Gattermeier]

String() mutating the object can cause problems of a doubly query escaped location string. For instance in this example:

cfg := mysql.LoadConfigFromEnv()
log.Println(cfg)

db, err := cfg.DB()
if err != nil {
	panic(err)
}

String() called by the logger will mutate m.Location with query escape. The subsequent String() call via cfg.DB() will lead to the doubly query escaped string. If there is no specific reason as to why String() should have these side effects it might be good not to mutate the object?

cc @bajh

taking another look

[fsouza]

Change looks good. Can you add tests?

[coveralls]

Coverage remained the same at 51.665% when pulling f67a293 on Gattermeier:string-mutation into accd7fe on NYTimes:master.

[jprobinson]

🔇