Skip to content

Commit dc6a5cf

Browse files
griesemergopherbot
griesemer
authored and
gopherbot
committed
go/types, types2: quote user-supplied names in error messages
Use `' quotes (as in `foo') to differentiate from Go quotes. Quoting prevents confusion when user-supplied names alter the meaning of the error message. For instance, report duplicate method `wanted' rather than duplicate method wanted Exceptions: - don't quote _: `_' is ugly and not necessary - don't quote after a ":": undefined name: foo - don't quote if the name is used correctly in a statement: goto L jumps over variable declaration Quoting is done with a helper function and can be centrally adjusted and fine-tuned as needed. Adjusted some test cases to explicitly include the quoted names. Fixes #65790. Change-Id: Icce667215f303ab8685d3e5cb00d540a2fd372ca Reviewed-on: https://go-review.googlesource.com/c/go/+/571396 TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Robert Griesemer <[email protected]> Reviewed-by: Robert Findley <[email protected]> Run-TryBot: Emmanuel Odeke <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Auto-Submit: Robert Griesemer <[email protected]>
1 parent 2e8d84f commit dc6a5cf

33 files changed

+174
-103
lines changed

‎src/cmd/compile/internal/types2/builtins.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ func (check *Checker) builtin(x *operand, call *syntax.CallExpr, id builtinId) (
2525
if hasDots(call) && id != _Append {
2626
check.errorf(dddErrPos(call),
2727
InvalidDotDotDot,
28-
invalidOp+"invalid use of ... with built-in %s", bin.name)
28+
invalidOp+"invalid use of ... with built-in %s", quote(bin.name))
2929
check.use(argList...)
3030
return
3131
}
@@ -210,7 +210,7 @@ func (check *Checker) builtin(x *operand, call *syntax.CallExpr, id builtinId) (
210210
if id == _Len {
211211
code = InvalidLen
212212
}
213-
check.errorf(x, code, invalidArg+"%s for %s", x, bin.name)
213+
check.errorf(x, code, invalidArg+"%s for %s", x, quote(bin.name))
214214
}
215215
return
216216
}
@@ -533,7 +533,7 @@ func (check *Checker) builtin(x *operand, call *syntax.CallExpr, id builtinId) (
533533
case _Max, _Min:
534534
// max(x, ...)
535535
// min(x, ...)
536-
check.verifyVersionf(call.Fun, go1_21, bin.name)
536+
check.verifyVersionf(call.Fun, go1_21, quote(bin.name))
537537

538538
op := token.LSS
539539
if id == _Max {
@@ -576,7 +576,7 @@ func (check *Checker) builtin(x *operand, call *syntax.CallExpr, id builtinId) (
576576
if x.mode != constant_ {
577577
x.mode = value
578578
// A value must not be untyped.
579-
check.assignment(x, &emptyInterface, "argument to "+bin.name)
579+
check.assignment(x, &emptyInterface, "argument to "+quote(bin.name))
580580
if x.mode == invalid {
581581
return
582582
}
@@ -641,7 +641,7 @@ func (check *Checker) builtin(x *operand, call *syntax.CallExpr, id builtinId) (
641641
if nargs > 0 {
642642
params = make([]Type, nargs)
643643
for i, a := range args {
644-
check.assignment(a, nil, "argument to "+predeclaredFuncs[id].name)
644+
check.assignment(a, nil, "argument to "+quote(predeclaredFuncs[id].name))
645645
if a.mode == invalid {
646646
return
647647
}
@@ -992,7 +992,7 @@ func (check *Checker) applyTypeFunc(f func(Type) Type, x *operand, id builtinId)
992992
default:
993993
panic("unreachable")
994994
}
995-
check.softErrorf(x, code, "%s not supported as argument to %s for go1.18 (see go.dev/issue/50937)", x, predeclaredFuncs[id].name)
995+
check.softErrorf(x, code, "%s not supported as argument to %s for go1.18 (see go.dev/issue/50937)", x, quote(predeclaredFuncs[id].name))
996996

997997
// Construct a suitable new type parameter for the result type.
998998
// The type parameter is placed in the current package so export/import

‎src/cmd/compile/internal/types2/call.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -719,7 +719,7 @@ func (check *Checker) selector(x *operand, e *syntax.SelectorExpr, def *TypeName
719719
goto Error
720720
}
721721
if !exp.Exported() {
722-
check.errorf(e.Sel, UnexportedName, "%s not exported by package %s", sel, pkg.name)
722+
check.errorf(e.Sel, UnexportedName, "%s not exported by package %s", quote(sel), quote(pkg.name))
723723
// ok to continue
724724
}
725725
}

‎src/cmd/compile/internal/types2/check.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,7 @@ func (check *Checker) initFiles(files []*syntax.File) {
298298
check.files = append(check.files, file)
299299

300300
default:
301-
check.errorf(file, MismatchedPkgName, "package %s; expected %s", name, pkg.name)
301+
check.errorf(file, MismatchedPkgName, "package %s; expected %s", quote(name), quote(pkg.name))
302302
// ignore this file
303303
}
304304
}

‎src/cmd/compile/internal/types2/decl.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -733,7 +733,7 @@ func (check *Checker) checkFieldUniqueness(base *Named) {
733733
// For historical consistency, we report the primary error on the
734734
// method, and the alt decl on the field.
735735
err := check.newError(DuplicateFieldAndMethod)
736-
err.addf(alt, "field and method with the same name %s", fld.name)
736+
err.addf(alt, "field and method with the same name %s", quote(fld.name))
737737
err.addAltDecl(fld)
738738
err.report()
739739
}

‎src/cmd/compile/internal/types2/format.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,39 @@ import (
1414
"strings"
1515
)
1616

17+
// quote encloses s in `' quotes, as in `foo', except for _,
18+
// which is left alone.
19+
//
20+
// Use to prevent confusion when user supplied names alter the
21+
// meaning of an error message.
22+
//
23+
// For instance, report
24+
//
25+
// duplicate method `wanted'
26+
//
27+
// rather than
28+
//
29+
// duplicate method wanted
30+
//
31+
// Exceptions:
32+
//
33+
// - don't quote _:
34+
// `_' is ugly and not necessary
35+
// - don't quote after a ":" as there's no need for it:
36+
// undefined name: foo
37+
// - don't quote if the name is used correctly in a statement:
38+
// goto L jumps over variable declaration
39+
//
40+
// quote encloses s in `' quotes, as in `foo',
41+
// except for _ which is left alone.
42+
func quote(s string) string {
43+
if s == "_" {
44+
// `_' is ugly and not necessary
45+
return s
46+
}
47+
return "`" + s + "'"
48+
}
49+
1750
func sprintf(qf Qualifier, tpSubscripts bool, format string, args ...any) string {
1851
for i, arg := range args {
1952
switch a := arg.(type) {

‎src/cmd/compile/internal/types2/issues_test.go

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -257,13 +257,12 @@ func TestIssue22525(t *testing.T) {
257257
got := "\n"
258258
conf := Config{Error: func(err error) { got += err.Error() + "\n" }}
259259
typecheck(src, &conf, nil) // do not crash
260-
want := `
261-
p:1:27: a declared and not used
262-
p:1:30: b declared and not used
263-
p:1:33: c declared and not used
264-
p:1:36: d declared and not used
265-
p:1:39: e declared and not used
266-
`
260+
want := "\n" +
261+
"p:1:27: `a' declared and not used\n" +
262+
"p:1:30: `b' declared and not used\n" +
263+
"p:1:33: `c' declared and not used\n" +
264+
"p:1:36: `d' declared and not used\n" +
265+
"p:1:39: `e' declared and not used\n"
267266
if got != want {
268267
t.Errorf("got: %swant: %s", got, want)
269268
}

‎src/cmd/compile/internal/types2/labels.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,11 @@ func (check *Checker) labels(body *syntax.BlockStmt) {
2828
msg = "goto %s jumps into block"
2929
alt.(*Label).used = true // avoid another error
3030
code = JumpIntoBlock
31+
// don't quote name here because "goto L" matches the code
3132
} else {
3233
msg = "label %s not declared"
3334
code = UndeclaredLabel
35+
name = quote(name)
3436
}
3537
check.errorf(jmp.Label, code, msg, name)
3638
}
@@ -39,7 +41,7 @@ func (check *Checker) labels(body *syntax.BlockStmt) {
3941
for name, obj := range all.elems {
4042
obj = resolve(name, obj)
4143
if lbl := obj.(*Label); !lbl.used {
42-
check.softErrorf(lbl.pos, UnusedLabel, "label %s declared and not used", lbl.name)
44+
check.softErrorf(lbl.pos, UnusedLabel, "label %s declared and not used", quote(lbl.name))
4345
}
4446
}
4547
}
@@ -135,7 +137,7 @@ func (check *Checker) blockBranches(all *Scope, parent *block, lstmt *syntax.Lab
135137
if alt := all.Insert(lbl); alt != nil {
136138
err := check.newError(DuplicateLabel)
137139
err.soft = true
138-
err.addf(lbl.pos, "label %s already declared", name)
140+
err.addf(lbl.pos, "label %s already declared", quote(name))
139141
err.addAltDecl(alt)
140142
err.report()
141143
// ok to continue
@@ -191,7 +193,7 @@ func (check *Checker) blockBranches(all *Scope, parent *block, lstmt *syntax.Lab
191193
}
192194
}
193195
if !valid {
194-
check.errorf(s.Label, MisplacedLabel, "invalid break label %s", name)
196+
check.errorf(s.Label, MisplacedLabel, "invalid break label %s", quote(name))
195197
return
196198
}
197199

@@ -206,7 +208,7 @@ func (check *Checker) blockBranches(all *Scope, parent *block, lstmt *syntax.Lab
206208
}
207209
}
208210
if !valid {
209-
check.errorf(s.Label, MisplacedLabel, "invalid continue label %s", name)
211+
check.errorf(s.Label, MisplacedLabel, "invalid continue label %s", quote(name))
210212
return
211213
}
212214

‎src/cmd/compile/internal/types2/stmt.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ func (check *Checker) usage(scope *Scope) {
6464
return cmpPos(unused[i].pos, unused[j].pos) < 0
6565
})
6666
for _, v := range unused {
67-
check.softErrorf(v.pos, UnusedVar, "%s declared and not used", v.name)
67+
check.softErrorf(v.pos, UnusedVar, "%s declared and not used", quote(v.name))
6868
}
6969

7070
for _, scope := range scope.children {
@@ -496,7 +496,7 @@ func (check *Checker) stmt(ctxt stmtContext, s syntax.Stmt) {
496496
for _, obj := range res.vars {
497497
if alt := check.lookup(obj.name); alt != nil && alt != obj {
498498
err := check.newError(OutOfScopeResult)
499-
err.addf(s, "result parameter %s not in scope at return", obj.name)
499+
err.addf(s, "result parameter %s not in scope at return", quote(obj.name))
500500
err.addf(alt, "inner declaration of %s", obj)
501501
err.report()
502502
// ok to continue

‎src/cmd/compile/internal/types2/typeset.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -226,8 +226,8 @@ func computeInterfaceTypeSet(check *Checker, pos syntax.Pos, ityp *Interface) *_
226226
case explicit:
227227
if check != nil {
228228
err := check.newError(DuplicateDecl)
229-
err.addf(atPos(pos), "duplicate method %s", m.name)
230-
err.addf(atPos(mpos[other.(*Func)]), "other declaration of %s", m.name)
229+
err.addf(atPos(pos), "duplicate method %s", quote(m.name))
230+
err.addf(atPos(mpos[other.(*Func)]), "other declaration of %s", quote(m.name))
231231
err.report()
232232
}
233233
default:
@@ -240,8 +240,8 @@ func computeInterfaceTypeSet(check *Checker, pos syntax.Pos, ityp *Interface) *_
240240
check.later(func() {
241241
if pos.IsKnown() && !check.allowVersion(atPos(pos), go1_14) || !Identical(m.typ, other.Type()) {
242242
err := check.newError(DuplicateDecl)
243-
err.addf(atPos(pos), "duplicate method %s", m.name)
244-
err.addf(atPos(mpos[other.(*Func)]), "other declaration of %s", m.name)
243+
err.addf(atPos(pos), "duplicate method %s", quote(m.name))
244+
err.addf(atPos(mpos[other.(*Func)]), "other declaration of %s", quote(m.name))
245245
err.report()
246246
}
247247
}).describef(atPos(pos), "duplicate method check for %s", m.name)

‎src/cmd/compile/internal/types2/typexpr.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ func (check *Checker) ident(x *operand, e *syntax.Name, def *TypeName, wantType
8787

8888
switch obj := obj.(type) {
8989
case *PkgName:
90-
check.errorf(e, InvalidPkgUse, "use of package %s not in selector", obj.name)
90+
check.errorf(e, InvalidPkgUse, "use of package %s not in selector", quote(obj.name))
9191
return
9292

9393
case *Const:
@@ -109,7 +109,7 @@ func (check *Checker) ident(x *operand, e *syntax.Name, def *TypeName, wantType
109109

110110
case *TypeName:
111111
if !check.enableAlias && check.isBrokenAlias(obj) {
112-
check.errorf(e, InvalidDeclCycle, "invalid use of type alias %s in recursive type (see go.dev/issue/50729)", obj.name)
112+
check.errorf(e, InvalidDeclCycle, "invalid use of type alias %s in recursive type (see go.dev/issue/50729)", quote(obj.name))
113113
return
114114
}
115115
x.mode = typexpr

‎src/go/types/builtins.go

Lines changed: 6 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/go/types/call.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -722,7 +722,7 @@ func (check *Checker) selector(x *operand, e *ast.SelectorExpr, def *TypeName, w
722722
goto Error
723723
}
724724
if !exp.Exported() {
725-
check.errorf(e.Sel, UnexportedName, "%s not exported by package %s", sel, pkg.name)
725+
check.errorf(e.Sel, UnexportedName, "%s not exported by package %s", quote(sel), quote(pkg.name))
726726
// ok to continue
727727
}
728728
}

‎src/go/types/check.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ func (check *Checker) initFiles(files []*ast.File) {
303303
check.files = append(check.files, file)
304304

305305
default:
306-
check.errorf(atPos(file.Package), MismatchedPkgName, "package %s; expected %s", name, pkg.name)
306+
check.errorf(atPos(file.Package), MismatchedPkgName, "package %s; expected %s", quote(name), quote(pkg.name))
307307
// ignore this file
308308
}
309309
}

‎src/go/types/decl.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -825,7 +825,7 @@ func (check *Checker) checkFieldUniqueness(base *Named) {
825825
// For historical consistency, we report the primary error on the
826826
// method, and the alt decl on the field.
827827
err := check.newError(DuplicateFieldAndMethod)
828-
err.addf(alt, "field and method with the same name %s", fld.name)
828+
err.addf(alt, "field and method with the same name %s", quote(fld.name))
829829
err.addAltDecl(fld)
830830
err.report()
831831
}

‎src/go/types/format.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,39 @@ import (
1515
"strings"
1616
)
1717

18+
// quote encloses s in `' quotes, as in `foo', except for _,
19+
// which is left alone.
20+
//
21+
// Use to prevent confusion when user supplied names alter the
22+
// meaning of an error message.
23+
//
24+
// For instance, report
25+
//
26+
// duplicate method `wanted'
27+
//
28+
// rather than
29+
//
30+
// duplicate method wanted
31+
//
32+
// Exceptions:
33+
//
34+
// - don't quote _:
35+
// `_' is ugly and not necessary
36+
// - don't quote after a ":" as there's no need for it:
37+
// undefined name: foo
38+
// - don't quote if the name is used correctly in a statement:
39+
// goto L jumps over variable declaration
40+
//
41+
// quote encloses s in `' quotes, as in `foo',
42+
// except for _ which is left alone.
43+
func quote(s string) string {
44+
if s == "_" {
45+
// `_' is ugly and not necessary
46+
return s
47+
}
48+
return "`" + s + "'"
49+
}
50+
1851
func sprintf(fset *token.FileSet, qf Qualifier, tpSubscripts bool, format string, args ...any) string {
1952
for i, arg := range args {
2053
switch a := arg.(type) {

‎src/go/types/issues_test.go

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -265,13 +265,12 @@ func TestIssue22525(t *testing.T) {
265265
got := "\n"
266266
conf := Config{Error: func(err error) { got += err.Error() + "\n" }}
267267
typecheck(src, &conf, nil) // do not crash
268-
want := `
269-
p:1:27: a declared and not used
270-
p:1:30: b declared and not used
271-
p:1:33: c declared and not used
272-
p:1:36: d declared and not used
273-
p:1:39: e declared and not used
274-
`
268+
want := "\n" +
269+
"p:1:27: `a' declared and not used\n" +
270+
"p:1:30: `b' declared and not used\n" +
271+
"p:1:33: `c' declared and not used\n" +
272+
"p:1:36: `d' declared and not used\n" +
273+
"p:1:39: `e' declared and not used\n"
275274
if got != want {
276275
t.Errorf("got: %swant: %s", got, want)
277276
}

0 commit comments

Comments
 (0)