Skip to content

Commit 98789f3

Browse files
jnthntatumjnthntatum
authored
Address non-const format string lint findings (#1096)
* Lint fixes for non-constant format string * Fix non-const format string errors.
1 parent a108e9e commit 98789f3

File tree

15 files changed

+42
-33
lines changed

15 files changed

+42
-33
lines changed

‎cel/env.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ func (e *Env) Check(ast *Ast) (*Ast, *Issues) {
217217
chk, err := e.initChecker()
218218
if err != nil {
219219
errs := common.NewErrors(ast.Source())
220-
errs.ReportError(common.NoLocation, err.Error())
220+
errs.ReportErrorString(common.NoLocation, err.Error())
221221
return nil, NewIssuesWithSourceInfo(errs, ast.NativeRep().SourceInfo())
222222
}
223223

‎cel/library.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -731,15 +731,15 @@ var (
731731
func timestampGetFullYear(ts, tz ref.Val) ref.Val {
732732
t, err := inTimeZone(ts, tz)
733733
if err != nil {
734-
return types.NewErr(err.Error())
734+
return types.NewErrFromString(err.Error())
735735
}
736736
return types.Int(t.Year())
737737
}
738738

739739
func timestampGetMonth(ts, tz ref.Val) ref.Val {
740740
t, err := inTimeZone(ts, tz)
741741
if err != nil {
742-
return types.NewErr(err.Error())
742+
return types.NewErrFromString(err.Error())
743743
}
744744
// CEL spec indicates that the month should be 0-based, but the Time value
745745
// for Month() is 1-based.
@@ -749,63 +749,63 @@ func timestampGetMonth(ts, tz ref.Val) ref.Val {
749749
func timestampGetDayOfYear(ts, tz ref.Val) ref.Val {
750750
t, err := inTimeZone(ts, tz)
751751
if err != nil {
752-
return types.NewErr(err.Error())
752+
return types.NewErrFromString(err.Error())
753753
}
754754
return types.Int(t.YearDay() - 1)
755755
}
756756

757757
func timestampGetDayOfMonthZeroBased(ts, tz ref.Val) ref.Val {
758758
t, err := inTimeZone(ts, tz)
759759
if err != nil {
760-
return types.NewErr(err.Error())
760+
return types.NewErrFromString(err.Error())
761761
}
762762
return types.Int(t.Day() - 1)
763763
}
764764

765765
func timestampGetDayOfMonthOneBased(ts, tz ref.Val) ref.Val {
766766
t, err := inTimeZone(ts, tz)
767767
if err != nil {
768-
return types.NewErr(err.Error())
768+
return types.NewErrFromString(err.Error())
769769
}
770770
return types.Int(t.Day())
771771
}
772772

773773
func timestampGetDayOfWeek(ts, tz ref.Val) ref.Val {
774774
t, err := inTimeZone(ts, tz)
775775
if err != nil {
776-
return types.NewErr(err.Error())
776+
return types.NewErrFromString(err.Error())
777777
}
778778
return types.Int(t.Weekday())
779779
}
780780

781781
func timestampGetHours(ts, tz ref.Val) ref.Val {
782782
t, err := inTimeZone(ts, tz)
783783
if err != nil {
784-
return types.NewErr(err.Error())
784+
return types.NewErrFromString(err.Error())
785785
}
786786
return types.Int(t.Hour())
787787
}
788788

789789
func timestampGetMinutes(ts, tz ref.Val) ref.Val {
790790
t, err := inTimeZone(ts, tz)
791791
if err != nil {
792-
return types.NewErr(err.Error())
792+
return types.NewErrFromString(err.Error())
793793
}
794794
return types.Int(t.Minute())
795795
}
796796

797797
func timestampGetSeconds(ts, tz ref.Val) ref.Val {
798798
t, err := inTimeZone(ts, tz)
799799
if err != nil {
800-
return types.NewErr(err.Error())
800+
return types.NewErrFromString(err.Error())
801801
}
802802
return types.Int(t.Second())
803803
}
804804

805805
func timestampGetMilliseconds(ts, tz ref.Val) ref.Val {
806806
t, err := inTimeZone(ts, tz)
807807
if err != nil {
808-
return types.NewErr(err.Error())
808+
return types.NewErrFromString(err.Error())
809809
}
810810
return types.Int(t.Nanosecond() / 1000000)
811811
}

‎common/errors.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,11 @@ func (e *Errors) ReportError(l Location, format string, args ...any) {
4646
e.ReportErrorAtID(0, l, format, args...)
4747
}
4848

49+
// ReportErrorString records an error at a source location.
50+
func (e *Errors) ReportErrorString(l Location, message string) {
51+
e.ReportErrorAtID(0, l, "%s", message)
52+
}
53+
4954
// ReportErrorAtID records an error at a source location and expression id.
5055
func (e *Errors) ReportErrorAtID(id int64, l Location, format string, args ...any) {
5156
e.numErrors++

‎common/types/err.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,12 @@ func NewErr(format string, args ...any) ref.Val {
6262
return &Err{error: fmt.Errorf(format, args...)}
6363
}
6464

65+
// NewErr creates a new Err with the provided message.
66+
// TODO: Audit the use of this function and standardize the error messages and codes.
67+
func NewErrFromString(message string) ref.Val {
68+
return &Err{error: errors.New(message)}
69+
}
70+
6571
// NewErrWithNodeID creates a new Err described by the format string and args.
6672
// TODO: Audit the use of this function and standardize the error messages and codes.
6773
func NewErrWithNodeID(id int64, format string, args ...any) ref.Val {

‎common/types/list.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ func (l *baseList) Equal(other ref.Val) ref.Val {
243243
func (l *baseList) Get(index ref.Val) ref.Val {
244244
ind, err := IndexOrError(index)
245245
if err != nil {
246-
return ValOrErr(index, err.Error())
246+
return ValOrErr(index, "%v", err)
247247
}
248248
if ind < 0 || ind >= l.size {
249249
return NewErr("index '%d' out of range in list size '%d'", ind, l.Size())
@@ -427,7 +427,7 @@ func (l *concatList) Equal(other ref.Val) ref.Val {
427427
func (l *concatList) Get(index ref.Val) ref.Val {
428428
ind, err := IndexOrError(index)
429429
if err != nil {
430-
return ValOrErr(index, err.Error())
430+
return ValOrErr(index, "%v", err)
431431
}
432432
i := Int(ind)
433433
if i < l.prevList.Size().(Int) {

‎common/types/object.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ func (o *protoObj) Get(index ref.Val) ref.Val {
151151
}
152152
fv, err := fd.GetFrom(o.value)
153153
if err != nil {
154-
return NewErr(err.Error())
154+
return NewErrFromString(err.Error())
155155
}
156156
return o.NativeToValue(fv)
157157
}

‎ext/formatting.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -434,7 +434,7 @@ func (stringFormatValidator) Validate(env *cel.Env, _ cel.ValidatorConfig, a *as
434434
// use a placeholder locale, since locale doesn't affect syntax
435435
_, err := parseFormatString(formatStr, formatCheck, formatCheck, "en_US")
436436
if err != nil {
437-
iss.ReportErrorAtID(getErrorExprID(e.ID(), err), err.Error())
437+
iss.ReportErrorAtID(getErrorExprID(e.ID(), err), "%v", err)
438438
continue
439439
}
440440
seenArgs := formatCheck.argsRequested

‎ext/guards.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,28 +24,28 @@ import (
2424

2525
func intOrError(i int64, err error) ref.Val {
2626
if err != nil {
27-
return types.NewErr(err.Error())
27+
return types.NewErrFromString(err.Error())
2828
}
2929
return types.Int(i)
3030
}
3131

3232
func bytesOrError(bytes []byte, err error) ref.Val {
3333
if err != nil {
34-
return types.NewErr(err.Error())
34+
return types.NewErrFromString(err.Error())
3535
}
3636
return types.Bytes(bytes)
3737
}
3838

3939
func stringOrError(str string, err error) ref.Val {
4040
if err != nil {
41-
return types.NewErr(err.Error())
41+
return types.NewErrFromString(err.Error())
4242
}
4343
return types.String(str)
4444
}
4545

4646
func listStringOrError(strs []string, err error) ref.Val {
4747
if err != nil {
48-
return types.NewErr(err.Error())
48+
return types.NewErrFromString(err.Error())
4949
}
5050
return types.DefaultTypeAdapter.NativeToValue(strs)
5151
}

‎ext/native.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,7 @@ func (tp *nativeTypeProvider) NewValue(typeName string, fields map[string]ref.Va
343343
}
344344
fieldVal, err := val.ConvertToNative(refFieldDef.Type)
345345
if err != nil {
346-
return types.NewErr(err.Error())
346+
return types.NewErrFromString(err.Error())
347347
}
348348
refField := refVal.FieldByIndex(refFieldDef.Index)
349349
refFieldVal := reflect.ValueOf(fieldVal)
@@ -450,7 +450,7 @@ func convertToCelType(refType reflect.Type) (*cel.Type, bool) {
450450
func (tp *nativeTypeProvider) newNativeObject(val any, refValue reflect.Value) ref.Val {
451451
valType, err := newNativeType(tp.options.fieldNameHandler, refValue.Type())
452452
if err != nil {
453-
return types.NewErr(err.Error())
453+
return types.NewErrFromString(err.Error())
454454
}
455455
return &nativeObj{
456456
Adapter: tp,

‎interpreter/attributes_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1120,7 +1120,7 @@ func TestAttributeStateTracking(t *testing.T) {
11201120
}
11211121
parsed, errors := p.Parse(src)
11221122
if len(errors.GetErrors()) != 0 {
1123-
t.Fatalf(errors.ToDisplayString())
1123+
t.Fatal(errors.ToDisplayString())
11241124
}
11251125
cont := containers.DefaultContainer
11261126
reg := newTestRegistry(t)
@@ -1135,7 +1135,7 @@ func TestAttributeStateTracking(t *testing.T) {
11351135
}
11361136
checked, errors := checker.Check(parsed, src, env)
11371137
if len(errors.GetErrors()) != 0 {
1138-
t.Fatalf(errors.ToDisplayString())
1138+
t.Fatal(errors.ToDisplayString())
11391139
}
11401140
in, err := NewActivation(tc.in)
11411141
if err != nil {

‎interpreter/interpreter_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1780,7 +1780,7 @@ func TestInterpreter_SetProto2PrimitiveFields(t *testing.T) {
17801780
types.NewObjectType("google.expr.proto2.test.TestAllTypes")))
17811781
checked, errors := checker.Check(parsed, src, env)
17821782
if len(errors.GetErrors()) != 0 {
1783-
t.Errorf(errors.ToDisplayString())
1783+
t.Error(errors.ToDisplayString())
17841784
}
17851785

17861786
attrs := NewAttributeFactory(cont, reg, reg)
@@ -1829,7 +1829,7 @@ func TestInterpreter_MissingIdentInSelect(t *testing.T) {
18291829
env.AddIdents(decls.NewVariable("a.b", types.DynType))
18301830
checked, errors := checker.Check(parsed, src, env)
18311831
if len(errors.GetErrors()) != 0 {
1832-
t.Fatalf(errors.ToDisplayString())
1832+
t.Fatal(errors.ToDisplayString())
18331833
}
18341834

18351835
attrs := NewPartialAttributeFactory(cont, reg, reg)
@@ -1885,7 +1885,7 @@ func TestInterpreter_TypeConversionOpt(t *testing.T) {
18851885
env := newTestEnv(t, cont, reg)
18861886
checked, errors := checker.Check(parsed, src, env)
18871887
if len(errors.GetErrors()) != 0 {
1888-
t.Fatalf(errors.ToDisplayString())
1888+
t.Fatal(errors.ToDisplayString())
18891889
}
18901890
attrs := NewAttributeFactory(cont, reg, reg)
18911891
interp := newStandardInterpreter(t, cont, reg, reg, attrs)

‎parser/errors.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@
1515
package parser
1616

1717
import (
18-
"fmt"
19-
2018
"github.com/google/cel-go/common"
2119
)
2220

@@ -31,11 +29,11 @@ func (e *parseErrors) errorCount() int {
3129
}
3230

3331
func (e *parseErrors) internalError(message string) {
34-
e.errs.ReportErrorAtID(0, common.NoLocation, message)
32+
e.errs.ReportErrorAtID(0, common.NoLocation, "%s", message)
3533
}
3634

3735
func (e *parseErrors) syntaxError(l common.Location, message string) {
38-
e.errs.ReportErrorAtID(0, l, fmt.Sprintf("Syntax error: %s", message))
36+
e.errs.ReportErrorAtID(0, l, "Syntax error: %s", message)
3937
}
4038

4139
func (e *parseErrors) reportErrorAtID(id int64, l common.Location, message string, args ...any) {

‎parser/parser.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -970,7 +970,7 @@ func (p *parser) expandMacro(exprID int64, function string, target ast.Expr, arg
970970
loc = p.helper.getLocation(exprID)
971971
}
972972
p.helper.deleteID(exprID)
973-
return p.reportError(loc, err.Message), true
973+
return p.reportError(loc, "%s", err.Message), true
974974
}
975975
// A nil value from the macro indicates that the macro implementation decided that
976976
// an expansion should not be performed.

‎parser/parser_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2039,7 +2039,7 @@ func TestParse(t *testing.T) {
20392039
if tc.E == "" {
20402040
t.Fatalf("Unexpected errors: %v", actualErr)
20412041
} else if !test.Compare(actualErr, tc.E) {
2042-
t.Fatalf(test.DiffMessage("Error mismatch", actualErr, tc.E))
2042+
t.Fatal(test.DiffMessage("Error mismatch", actualErr, tc.E))
20432043
}
20442044
return
20452045
} else if tc.E != "" {

‎repl/typefmt.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ func ParseType(t string) (*exprpb.Type, error) {
312312
for i, e := range errs {
313313
msgs[i] = e.Error()
314314
}
315-
err = fmt.Errorf("errors parsing type:\n" + strings.Join(msgs, "\n"))
315+
err = fmt.Errorf("errors parsing type:\n%s", strings.Join(msgs, "\n"))
316316
}
317317

318318
return result, err

0 commit comments

Comments
 (0)