Skip to content

Commit cc95d85

Browse files
griesemergopherbot
griesemer
authored and
gopherbot
committed
cmd/compile: remove quoting in favor of clearer prose in error messages
In an attempt to address issue #65790 (confusing error messages), quoting of names was introduced for some (but not all) names used in error messages. That CL solved the issue at hand at the cost of extra punctuation (the quotes) plus some inconsistency (not all names were quoted). This CL removes the quoting again in favor or adding a qualifying noun (such as "name", "label", "package", "built-in" etc.) before a user- specified name where needed. For instance, instead of invalid argument to `max' we now say invalid argument to built-in max There's still a chance for confusion. For instance, before an error might have been `sadly' not exported by package X and now it would be name sadly not exported by package X but adverbs (such as "sadly") seem unlikely names in programs. This change touches a lot of files but only affects error messages. Fixes #67685. Change-Id: I95435b388f92cade316e2844d59ecf6953b178bc Reviewed-on: https://go-review.googlesource.com/c/go/+/589118 Auto-Submit: Robert Griesemer <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Robert Findley <[email protected]> Reviewed-by: Robert Griesemer <[email protected]>
1 parent f103918 commit cc95d85

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+117
-187
lines changed

‎src/cmd/compile/internal/syntax/parser.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ func (p *parser) syntaxErrorAt(pos Pos, msg string) {
268268
var tok string
269269
switch p.tok {
270270
case _Name:
271-
tok = "`" + p.lit + "'"
271+
tok = "name " + p.lit
272272
case _Semi:
273273
tok = p.lit
274274
case _Literal:

‎src/cmd/compile/internal/syntax/testdata/issue20789.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,4 @@
66
// Line 9 must end in EOF for this test (no newline).
77

88
package e
9-
func([<-chan<-[func /* ERROR unexpected `u' */ u){go
9+
func([<-chan<-[func /* ERROR unexpected name u */ u){go

‎src/cmd/compile/internal/syntax/testdata/issue47704.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ package p
77
func _() {
88
_ = m[] // ERROR expected operand
99
_ = m[x,]
10-
_ = m[x /* ERROR unexpected `a' */ a b c d]
10+
_ = m[x /* ERROR unexpected name a */ a b c d]
1111
}
1212

1313
// test case from the issue

‎src/cmd/compile/internal/syntax/testdata/issue49205.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ package p
77
// test case from issue
88

99
type _ interface{
10-
m /* ERROR unexpected `int' in interface type; possibly missing semicolon or newline or } */ int
10+
m /* ERROR unexpected name int in interface type; possibly missing semicolon or newline or } */ int
1111
}
1212

1313
// other cases where the fix for this issue affects the error message
@@ -16,12 +16,12 @@ const (
1616
x int = 10 /* ERROR unexpected literal "foo" in grouped declaration; possibly missing semicolon or newline or \) */ "foo"
1717
)
1818

19-
var _ = []int{1, 2, 3 /* ERROR unexpected `int' in composite literal; possibly missing comma or } */ int }
19+
var _ = []int{1, 2, 3 /* ERROR unexpected name int in composite literal; possibly missing comma or } */ int }
2020

2121
type _ struct {
2222
x y /* ERROR syntax error: unexpected comma in struct type; possibly missing semicolon or newline or } */ ,
2323
}
2424

25-
func f(a, b c /* ERROR unexpected `d' in parameter list; possibly missing comma or \) */ d) {
26-
f(a, b, c /* ERROR unexpected `d' in argument list; possibly missing comma or \) */ d)
25+
func f(a, b c /* ERROR unexpected name d in parameter list; possibly missing comma or \) */ d) {
26+
f(a, b, c /* ERROR unexpected name d in argument list; possibly missing comma or \) */ d)
2727
}

‎src/cmd/compile/internal/syntax/testdata/issue52391.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,5 +13,5 @@ type _ interface {
1313
(int) | (string)
1414
(int) | ~(string)
1515
(/* ERROR unexpected ~ */ ~int)
16-
(int /* ERROR unexpected \| */ | /* ERROR unexpected `string' */ string /* ERROR unexpected \) */ )
16+
(int /* ERROR unexpected \| */ | /* ERROR unexpected name string */ string /* ERROR unexpected \) */ )
1717
}

‎src/cmd/compile/internal/syntax/testdata/issue65790.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,6 @@ import (
99
)
1010

1111
func f() {
12-
int status // ERROR syntax error: unexpected `status' at end of statement
12+
int status // ERROR syntax error: unexpected name status at end of statement
1313
fmt.Println(status)
1414
}

‎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", quote(bin.name))
28+
invalidOp+"invalid use of ... with built-in %s", 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, quote(bin.name))
213+
check.errorf(x, code, invalidArg+"%s for built-in %s", x, 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, "%s", quote(bin.name))
536+
check.verifyVersionf(call.Fun, go1_21, "built-in %s", 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 "+quote(bin.name))
579+
check.assignment(x, &emptyInterface, "argument to built-in "+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 "+quote(predeclaredFuncs[id].name))
644+
check.assignment(a, nil, "argument to built-in"+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, quote(predeclaredFuncs[id].name))
995+
check.softErrorf(x, code, "%s not supported as argument to built-in %s for go1.18 (see go.dev/issue/50937)", x, 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", quote(sel), quote(pkg.name))
722+
check.errorf(e.Sel, UnexportedName, "name %s not exported by package %s", sel, 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
@@ -310,7 +310,7 @@ func (check *Checker) initFiles(files []*syntax.File) {
310310
check.files = append(check.files, file)
311311

312312
default:
313-
check.errorf(file, MismatchedPkgName, "package %s; expected %s", quote(name), quote(pkg.name))
313+
check.errorf(file, MismatchedPkgName, "package %s; expected package %s", name, pkg.name)
314314
// ignore this file
315315
}
316316
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -743,7 +743,7 @@ func (check *Checker) checkFieldUniqueness(base *Named) {
743743
// For historical consistency, we report the primary error on the
744744
// method, and the alt decl on the field.
745745
err := check.newError(DuplicateFieldAndMethod)
746-
err.addf(alt, "field and method with the same name %s", quote(fld.name))
746+
err.addf(alt, "field and method with the same name %s", fld.name)
747747
err.addAltDecl(fld)
748748
err.report()
749749
}

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

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -14,39 +14,6 @@ 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-
5017
func sprintf(qf Qualifier, tpSubscripts bool, format string, args ...any) string {
5118
for i, arg := range args {
5219
switch a := arg.(type) {

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -258,11 +258,11 @@ func TestIssue22525(t *testing.T) {
258258
conf := Config{Error: func(err error) { got += err.Error() + "\n" }}
259259
typecheck(src, &conf, nil) // do not crash
260260
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"
261+
"p:1:27: declared and not used: a\n" +
262+
"p:1:30: declared and not used: b\n" +
263+
"p:1:33: declared and not used: c\n" +
264+
"p:1:36: declared and not used: d\n" +
265+
"p:1:39: declared and not used: e\n"
266266
if got != want {
267267
t.Errorf("got: %swant: %s", got, want)
268268
}

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

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,11 @@ func (check *Checker) labels(body *syntax.BlockStmt) {
2626
name := jmp.Label.Value
2727
if alt := all.Lookup(name); alt != nil {
2828
msg = "goto %s jumps into block"
29-
alt.(*Label).used = true // avoid another error
3029
code = JumpIntoBlock
31-
// don't quote name here because "goto L" matches the code
30+
alt.(*Label).used = true // avoid another error
3231
} else {
3332
msg = "label %s not declared"
3433
code = UndeclaredLabel
35-
name = quote(name)
3634
}
3735
check.errorf(jmp.Label, code, msg, name)
3836
}
@@ -41,7 +39,7 @@ func (check *Checker) labels(body *syntax.BlockStmt) {
4139
for name, obj := range all.elems {
4240
obj = resolve(name, obj)
4341
if lbl := obj.(*Label); !lbl.used {
44-
check.softErrorf(lbl.pos, UnusedLabel, "label %s declared and not used", quote(lbl.name))
42+
check.softErrorf(lbl.pos, UnusedLabel, "label %s declared and not used", lbl.name)
4543
}
4644
}
4745
}
@@ -137,7 +135,7 @@ func (check *Checker) blockBranches(all *Scope, parent *block, lstmt *syntax.Lab
137135
if alt := all.Insert(lbl); alt != nil {
138136
err := check.newError(DuplicateLabel)
139137
err.soft = true
140-
err.addf(lbl.pos, "label %s already declared", quote(name))
138+
err.addf(lbl.pos, "label %s already declared", name)
141139
err.addAltDecl(alt)
142140
err.report()
143141
// ok to continue
@@ -193,7 +191,7 @@ func (check *Checker) blockBranches(all *Scope, parent *block, lstmt *syntax.Lab
193191
}
194192
}
195193
if !valid {
196-
check.errorf(s.Label, MisplacedLabel, "invalid break label %s", quote(name))
194+
check.errorf(s.Label, MisplacedLabel, "invalid break label %s", name)
197195
return
198196
}
199197

@@ -208,7 +206,7 @@ func (check *Checker) blockBranches(all *Scope, parent *block, lstmt *syntax.Lab
208206
}
209207
}
210208
if !valid {
211-
check.errorf(s.Label, MisplacedLabel, "invalid continue label %s", quote(name))
209+
check.errorf(s.Label, MisplacedLabel, "invalid continue label %s", name)
212210
return
213211
}
214212

‎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", quote(v.name))
67+
check.softErrorf(v.pos, UnusedVar, "declared and not used: %s", 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", quote(obj.name))
499+
err.addf(s, "result parameter %s not in scope at return", 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", quote(m.name))
230-
err.addf(atPos(mpos[other.(*Func)]), "other declaration of %s", quote(m.name))
229+
err.addf(atPos(pos), "duplicate method %s", m.name)
230+
err.addf(atPos(mpos[other.(*Func)]), "other declaration of method %s", 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", quote(m.name))
244-
err.addf(atPos(mpos[other.(*Func)]), "other declaration of %s", quote(m.name))
243+
err.addf(atPos(pos), "duplicate method %s", m.name)
244+
err.addf(atPos(mpos[other.(*Func)]), "other declaration of method %s", 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
@@ -95,7 +95,7 @@ func (check *Checker) ident(x *operand, e *syntax.Name, def *TypeName, wantType
9595

9696
switch obj := obj.(type) {
9797
case *PkgName:
98-
check.errorf(e, InvalidPkgUse, "use of package %s not in selector", quote(obj.name))
98+
check.errorf(e, InvalidPkgUse, "use of package %s not in selector", obj.name)
9999
return
100100

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

118118
case *TypeName:
119119
if !check.conf.EnableAlias && check.isBrokenAlias(obj) {
120-
check.errorf(e, InvalidDeclCycle, "invalid use of type alias %s in recursive type (see go.dev/issue/50729)", quote(obj.name))
120+
check.errorf(e, InvalidDeclCycle, "invalid use of type alias %s in recursive type (see go.dev/issue/50729)", obj.name)
121121
return
122122
}
123123
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", quote(sel), quote(pkg.name))
725+
check.errorf(e.Sel, UnexportedName, "name %s not exported by package %s", sel, 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
@@ -332,7 +332,7 @@ func (check *Checker) initFiles(files []*ast.File) {
332332
check.files = append(check.files, file)
333333

334334
default:
335-
check.errorf(atPos(file.Package), MismatchedPkgName, "package %s; expected %s", quote(name), quote(pkg.name))
335+
check.errorf(atPos(file.Package), MismatchedPkgName, "package %s; expected package %s", name, pkg.name)
336336
// ignore this file
337337
}
338338
}

‎src/go/types/decl.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -842,7 +842,7 @@ func (check *Checker) checkFieldUniqueness(base *Named) {
842842
// For historical consistency, we report the primary error on the
843843
// method, and the alt decl on the field.
844844
err := check.newError(DuplicateFieldAndMethod)
845-
err.addf(alt, "field and method with the same name %s", quote(fld.name))
845+
err.addf(alt, "field and method with the same name %s", fld.name)
846846
err.addAltDecl(fld)
847847
err.report()
848848
}

0 commit comments

Comments
 (0)