ronantakizawa/codereview-bench huggingface dataset:
Repository: prometheus/prometheus
File: tsdb/errors/errors_test.go (Go)

Code before:

func TestMultiError_As(t *testing.T) {
	tE1 := testError{"error cause 1"}
	tE2 := testError{"error cause 2"}
	var target testError
	testCases := map[string]struct {
		sourceErrors []error
		target       error
		as           bool
	}{
		"MultiError containing only a testError can be cast to that testError": {
			sourceErrors: []error{tE1},
			target:       tE1,
			as:           true,
		},
		"MultiError containing multiple testErrors can be cast to the first testError added": {
			sourceErrors: []error{tE1, tE2},
			target:       tE1,
			as:           true,
		},
		"MultiError containing multiple errors can be cast to the first testError added": {
			sourceErrors: []error{context.Canceled, tE1, context.DeadlineExceeded, tE2},
			target:       tE1,
			as:           true,
		},
		"MultiError non containing a testError cannot be cast to a testError": {
			sourceErrors: []error{context.Canceled, context.DeadlineExceeded},
			as:           false,
		},
	}

	for testName, testCase := range testCases {
		t.Run(testName, func(t *testing.T) {
			mErr := NewMulti(testCase.sourceErrors...).Err()
			if testCase.as {
				require.ErrorAs(t, mErr, &target)
				require.Equal(t, testCase.target, target)
			} else {
				require.NotErrorAs(t, mErr, &target)
			}
		})
	}
}

type testError struct {
	cause string
}

func (e testError) Error() string {
	return fmt.Sprintf("testError[cause: %s]", e.cause)
}

Review comment (suggestion):
[Nit]
```suggestion
		"MultiError not containing a testError cannot be cast to a testError": {
```

Code after:

func TestMultiError_As(t *testing.T) {
	tE1 := testError{"error cause 1"}
	tE2 := testError{"error cause 2"}
	var target testError
	testCases := map[string]struct {
		sourceErrors []error
		target       error
		as           bool
	}{
		"MultiError containing only a testError can be cast to that testError": {
			sourceErrors: []error{tE1},
			target:       tE1,
			as:           true,
		},
		"MultiError containing multiple testErrors can be cast to the first testError added": {
			sourceErrors: []error{tE1, tE2},
			target:       tE1,
			as:           true,
		},
		"MultiError containing multiple errors can be cast to the first testError added": {
			sourceErrors: []error{context.Canceled, tE1, context.DeadlineExceeded, tE2},
			target:       tE1,
			as:           true,
		},
		"MultiError not containing a testError cannot be cast to a testError": {
			sourceErrors: []error{context.Canceled, context.DeadlineExceeded},
			as:           false,
		},
	}

	for testName, testCase := range testCases {
		t.Run(testName, func(t *testing.T) {
			mErr := NewMulti(testCase.sourceErrors...).Err()
			if testCase.as {
				require.ErrorAs(t, mErr, &target)
				require.Equal(t, testCase.target, target)
			} else {
				require.NotErrorAs(t, mErr, &target)
			}
		})
	}
}

type testError struct {
	cause string
}

func (e testError) Error() string {
	return fmt.Sprintf("testError[cause: %s]", e.cause)
}
Repository: prometheus/prometheus
File: promql/parser/ast.go (Go)

Code before:
}

// Inspect traverses an AST in depth-first order: It starts by calling
// f(node, path); node must not be nil. If f returns a nil error, Inspect invokes f
// for all the non-nil children of node, recursively.
// Note: path may be overwritten after f returns; copy path if you need to retain it.
func Inspect(node Node, f inspector) {
	var pathBuf [4]Node        // To reduce allocations during recursion.
	Walk(f, node, pathBuf[:0]) //nolint:errcheck
}

// ChildrenIter returns an iterator over all child nodes of a syntax tree node.
func ChildrenIter(node Node) func(func(Node) bool) {
	return func(yield func(Node) bool) {
		// According to lore, these switches have significantly better performance than interfaces
		switch n := node.(type) {
		case *EvalStmt:
			yield(n.Expr)
		case Expressions:
			for _, e := range n {
				if !yield(e) {
					return
				}
			}
		case *AggregateExpr:
			if n.Param != nil {
				if !yield(n.Param) {
					return
				}
			}
			if n.Expr != nil {
				yield(n.Expr)
			}
		case *BinaryExpr:
			if !yield(n.LHS) {
				return
			}
			yield(n.RHS)
		case *Call:
			for _, e := range n.Args {
				if !yield(e) {
					return
				}
			}
		case *SubqueryExpr:
			yield(n.Expr)
		case *ParenExpr:
			yield(n.Expr)
		case *UnaryExpr:
			yield(n.Expr)
		case *MatrixSelector:

Review comment (suggestion):
nit, the old default would return  `[]Node{n.Expr, n.Param}`, now we'd yield n.Param first.
Not sure if sth relies on that order.

Code after:
	return f, nil
}

// Inspect traverses an AST in depth-first order: It starts by calling
// f(node, path); node must not be nil. If f returns a nil error, Inspect invokes f
// for all the non-nil children of node, recursively.
// Note: path may be overwritten after f returns; copy path if you need to retain it.
func Inspect(node Node, f inspector) {
	var pathBuf [4]Node        // To reduce allocations during recursion.
	Walk(f, node, pathBuf[:0]) //nolint:errcheck
}

// ChildrenIter returns an iterator over all child nodes of a syntax tree node.
func ChildrenIter(node Node) func(func(Node) bool) {
	return func(yield func(Node) bool) {
		// According to lore, these switches have significantly better performance than interfaces
		switch n := node.(type) {
		case *EvalStmt:
			yield(n.Expr)
		case Expressions:
			for _, e := range n {
				if !yield(e) {
					return
				}
			}
		case *AggregateExpr:
			if n.Expr != nil {
				if !yield(n.Expr) {
					return
				}
			}
			if n.Param != nil {
				yield(n.Param)
			}
		case *BinaryExpr:
			if !yield(n.LHS) {
				return
			}
			yield(n.RHS)
		case *Call:
			for _, e := range n.Args {
				if !yield(e) {
					return
				}
			}
		case *SubqueryExpr:
			yield(n.Expr)
		case *ParenExpr:
			yield(n.Expr)
		case *UnaryExpr:
			yield(n.Expr)
Repository: prometheus/prometheus
File: promql/parser/ast.go (Go)

Code before:
			}
		case *BinaryExpr:
			if !yield(n.LHS) {
				return
			}
			yield(n.RHS)
		case *Call:
			for _, e := range n.Args {
				if !yield(e) {
					return
				}
			}
		case *SubqueryExpr:
			yield(n.Expr)
		case *ParenExpr:
			yield(n.Expr)
		case *UnaryExpr:
			yield(n.Expr)
		case *MatrixSelector:
			yield(n.VectorSelector)
		case *StepInvariantExpr:
			yield(n.Expr)
		case *NumberLiteral, *StringLiteral, *VectorSelector:
			// nothing to do
		default:
			panic(fmt.Errorf("promql.Children: unhandled node type %T", node))
		}
	}
}

// Children returns a list of all child nodes of a syntax tree node.
// Implemented for backwards-compatibility; prefer ChildrenIter().
func Children(node Node) []Node {
	ret := []Node{}
	for e := range ChildrenIter(node) {
		ret = append(ret, e)
	}
	return ret
}

// mergeRanges is a helper function to merge the PositionRanges of two Nodes.
// Note that the arguments must be in the same order as they
// occur in the input string.
func mergeRanges(first, last Node) posrange.PositionRange {
	return posrange.PositionRange{
		Start: first.PositionRange().Start,
		End:   last.PositionRange().End,
	}
}

// PositionRange implements the Node interface.

Review comment (suggestion):
```suggestion
			panic(fmt.Errorf("promql.ChildrenIter: unhandled node type %T", node))
```

Code after:
				yield(n.Param)
			}
		case *BinaryExpr:
			if !yield(n.LHS) {
				return
			}
			yield(n.RHS)
		case *Call:
			for _, e := range n.Args {
				if !yield(e) {
					return
				}
			}
		case *SubqueryExpr:
			yield(n.Expr)
		case *ParenExpr:
			yield(n.Expr)
		case *UnaryExpr:
			yield(n.Expr)
		case *MatrixSelector:
			yield(n.VectorSelector)
		case *StepInvariantExpr:
			yield(n.Expr)
		case *NumberLiteral, *StringLiteral, *VectorSelector:
			// nothing to do
		default:
			panic(fmt.Errorf("promql.ChildrenIter: unhandled node type %T", node))
		}
	}
}

// Children returns a list of all child nodes of a syntax tree node.
// Implemented for backwards-compatibility; prefer ChildrenIter().
func Children(node Node) []Node {
	ret := []Node{}
	for e := range ChildrenIter(node) {
		ret = append(ret, e)
	}
	return ret
}

// mergeRanges is a helper function to merge the PositionRanges of two Nodes.
// Note that the arguments must be in the same order as they
// occur in the input string.
func mergeRanges(first, last Node) posrange.PositionRange {
	return posrange.PositionRange{
		Start: first.PositionRange().Start,
		End:   last.PositionRange().End,
	}
}
Repository: prometheus/prometheus
File: web/ui/mantine-ui/src/pages/AlertsPage.tsx (TypeScript)

Code before:
          value={searchFilter || ""}
          onChange={(event) =>
            setSearchFilter(event.currentTarget.value || null)
          }
        ></TextInput>
      </Group>
      {alertsPageData.groups.length === 0 ? (
        <Alert title="No rules found" icon={<IconInfoCircle />}>
          No rules found.
        </Alert>
      ) : (
        !showEmptyGroups &&
        alertsPageData.groups.length !== shownGroups.length && (
          <Alert
            title="Hiding groups with no matching rules"
            icon={<IconInfoCircle />}
          >
            Hiding {alertsPageData.groups.length - shownGroups.length} empty
            groups due to filters or no rules.
            <Anchor ml="md" fz="1em" onClick={() => setShowEmptyGroups(true)}>
              Show empty groups
            </Anchor>
          </Alert>
        )
      )}
      <Stack>
        <Pagination
          total={totalPageCount}
          value={effectiveActivePage}
          onChange={setActivePage}
          hideWithOnePage
        />
        {renderedPageItems}
      </Stack>
    </Stack>
  );
}


Review comment (suggestion):
Just a quick drive-by cleanup here, getting rid of a superfluous `<Stack>`, that's the only real change in this file 😬

Code after:
      </Group>
      {alertsPageData.groups.length === 0 ? (
        <Alert title="No rules found" icon={<IconInfoCircle />}>
          No rules found.
        </Alert>
      ) : (
        !showEmptyGroups &&
        alertsPageData.groups.length !== shownGroups.length && (
          <Alert
            title="Hiding groups with no matching rules"
            icon={<IconInfoCircle />}
          >
            Hiding {alertsPageData.groups.length - shownGroups.length} empty
            groups due to filters or no rules.
            <Anchor ml="md" fz="1em" onClick={() => setShowEmptyGroups(true)}>
              Show empty groups
            </Anchor>
          </Alert>
        )
      )}
      <Pagination
        total={totalPageCount}
        value={effectiveActivePage}
        onChange={setActivePage}
        hideWithOnePage
      />
      {renderedPageItems}
    </Stack>
  );
}
Repository: prometheus/prometheus
File: template/template.go (Go)

Code before:
				return fmt.Sprintf("%.4g%%", v*100), nil
			},
			"humanizeTimestamp": common_templates.HumanizeTimestamp,
			"toTime": func(i interface{}) (*time.Time, error) {
				v, err := common_templates.ConvertToFloat(i)
				if err != nil {
					return nil, err
				}

				return floatToTime(v)
			},
			"pathPrefix": func() string {
				return externalURL.Path
			},
			"externalURL": func() string {
				return externalURL.String()
			},
			"parseDuration": func(d string) (float64, error) {
				v, err := model.ParseDuration(d)
				if err != nil {
					return 0, err
				}
				return float64(time.Duration(v)) / float64(time.Second), nil
			},
			"parseDurationTime": func(d string) (time.Duration, error) {
				v, err := time.ParseDuration(d)
				if err != nil {
					return 0, err
				}
				return v, nil
			},
		},
		options: options,
	}
}

// AlertTemplateData returns the interface to be used in expanding the template.
func AlertTemplateData(labels, externalLabels map[string]string, externalURL string, smpl promql.Sample) interface{} {
	res := struct {
		Labels         map[string]string
		ExternalLabels map[string]string
		ExternalURL    string
		Value          interface{}
	}{
		Labels:         labels,
		ExternalLabels: externalLabels,
		ExternalURL:    externalURL,
		Value:          smpl.F,
	}

	if smpl.H != nil {

Review comment (suggestion):
Note that the standard library `time.ParseDuration(d)` doesn't understand the PromQL specific durations like `3d` or `1w`. 

Code after:
				return fmt.Sprintf("%.4g%%", v*100), nil
			},
			"humanizeTimestamp": common_templates.HumanizeTimestamp,
			"toTime": func(i interface{}) (*time.Time, error) {
				v, err := common_templates.ConvertToFloat(i)
				if err != nil {
					return nil, err
				}

				return floatToTime(v)
			},
			"toDuration": func(i interface{}) (*time.Duration, error) {
				v, err := common_templates.ConvertToFloat(i)
				if err != nil {
					return nil, err
				}
				d := time.Duration(v * float64(time.Second))
				return &d, nil
			},
			"now": func() float64 {
				return float64(timestamp) / 1000.0
			},
			"pathPrefix": func() string {
				return externalURL.Path
			},
			"externalURL": func() string {
				return externalURL.String()
			},
			"parseDuration": func(d string) (float64, error) {
				v, err := model.ParseDurationAllowNegative(d)
				if err != nil {
					return 0, err
				}
				return float64(time.Duration(v)) / float64(time.Second), nil
			},
		},
		options: options,
	}
}

// AlertTemplateData returns the interface to be used in expanding the template.
func AlertTemplateData(labels, externalLabels map[string]string, externalURL string, smpl promql.Sample) interface{} {
	res := struct {
		Labels         map[string]string
		ExternalLabels map[string]string
		ExternalURL    string
		Value          interface{}
	}{
		Labels:         labels,
		ExternalLabels: externalLabels,
		ExternalURL:    externalURL,
Repository: prometheus/prometheus
File: template/template.go (Go)

Code before:
				}

				return floatToTime(v)
			},
			"nowTime": func() time.Time {
				return time.Now()
			},
			"pathPrefix": func() string {
				return externalURL.Path
			},
			"externalURL": func() string {
				return externalURL.String()
			},
			"parseDuration": func(d string) (float64, error) {
				v, err := model.ParseDuration(d)
				if err != nil {
					return 0, err
				}
				return float64(time.Duration(v)) / float64(time.Second), nil
			},
			"parseGoDuration": func(d string) (time.Duration, error) {
				v, err := time.ParseDuration(d)
				if err != nil {
					return 0, err
				}
				return v, nil
			},
		},
		options: options,
	}
}

// AlertTemplateData returns the interface to be used in expanding the template.
func AlertTemplateData(labels, externalLabels map[string]string, externalURL string, smpl promql.Sample) interface{} {
	res := struct {
		Labels         map[string]string
		ExternalLabels map[string]string
		ExternalURL    string
		Value          interface{}
	}{
		Labels:         labels,
		ExternalLabels: externalLabels,
		ExternalURL:    externalURL,
		Value:          smpl.F,
	}

	if smpl.H != nil {
		res.Value = smpl.H
	}

	return res

Review comment (suggestion):
```suggestion
				v, err := model.ParseDuration(d)
				if err != nil {
					return 0, err
				}
				return time.Duration(v), nil
```

Code after:
				}

				return floatToTime(v)
			},
			"toDuration": func(i interface{}) (*time.Duration, error) {
				v, err := common_templates.ConvertToFloat(i)
				if err != nil {
					return nil, err
				}
				d := time.Duration(v * float64(time.Second))
				return &d, nil
			},
			"now": func() float64 {
				return float64(timestamp) / 1000.0
			},
			"pathPrefix": func() string {
				return externalURL.Path
			},
			"externalURL": func() string {
				return externalURL.String()
			},
			"parseDuration": func(d string) (float64, error) {
				v, err := model.ParseDurationAllowNegative(d)
				if err != nil {
					return 0, err
				}
				return float64(time.Duration(v)) / float64(time.Second), nil
			},
		},
		options: options,
	}
}

// AlertTemplateData returns the interface to be used in expanding the template.
func AlertTemplateData(labels, externalLabels map[string]string, externalURL string, smpl promql.Sample) interface{} {
	res := struct {
		Labels         map[string]string
		ExternalLabels map[string]string
		ExternalURL    string
		Value          interface{}
	}{
		Labels:         labels,
		ExternalLabels: externalLabels,
		ExternalURL:    externalURL,
		Value:          smpl.F,
	}

	if smpl.H != nil {
		res.Value = smpl.H
	}
Repository: prometheus/prometheus
File: template/template.go (Go)

Code before:
					}
					prefix = p
					v /= 1024
				}
				return fmt.Sprintf("%.4g%s", v, prefix), nil
			},
			"humanizeDuration": common_templates.HumanizeDuration,
			"humanizePercentage": func(i interface{}) (string, error) {
				v, err := common_templates.ConvertToFloat(i)
				if err != nil {
					return "", err
				}
				return fmt.Sprintf("%.4g%%", v*100), nil
			},
			"humanizeTimestamp": common_templates.HumanizeTimestamp,
			"toTime": func(i interface{}) (*time.Time, error) {
				v, err := common_templates.ConvertToFloat(i)
				if err != nil {
					return nil, err
				}

				return floatToTime(v)
			},
			"now": func() time.Time {
				return time.Now()
			},
			"pathPrefix": func() string {
				return externalURL.Path
			},
			"externalURL": func() string {
				return externalURL.String()
			},
			"parseDuration": func(d string) (float64, error) {
				v, err := model.ParseDuration(d)
				if err != nil {
					return 0, err
				}
				return float64(time.Duration(v)) / float64(time.Second), nil
			},
			"parseGoDuration": func(d string) (time.Duration, error) {
				v, err := model.ParseDuration(d)
				if err != nil {
					return 0, err
				}
				return time.Duration(v), nil
			},
		},
		options: options,
	}
}


Review comment (suggestion):
To stay consistent with the other functions, let's return a "standard Prometheus timestamp here, i.e. 

```suggestion
			"now": model.Now,
```

The result can be converted to a Go time with the existing `toTime`.

Code after:
					}
					prefix = p
					v /= 1024
				}
				return fmt.Sprintf("%.4g%s", v, prefix), nil
			},
			"humanizeDuration": common_templates.HumanizeDuration,
			"humanizePercentage": func(i interface{}) (string, error) {
				v, err := common_templates.ConvertToFloat(i)
				if err != nil {
					return "", err
				}
				return fmt.Sprintf("%.4g%%", v*100), nil
			},
			"humanizeTimestamp": common_templates.HumanizeTimestamp,
			"toTime": func(i interface{}) (*time.Time, error) {
				v, err := common_templates.ConvertToFloat(i)
				if err != nil {
					return nil, err
				}

				return floatToTime(v)
			},
			"toDuration": func(i interface{}) (*time.Duration, error) {
				v, err := common_templates.ConvertToFloat(i)
				if err != nil {
					return nil, err
				}
				d := time.Duration(v * float64(time.Second))
				return &d, nil
			},
			"now": func() float64 {
				return float64(timestamp) / 1000.0
			},
			"pathPrefix": func() string {
				return externalURL.Path
			},
			"externalURL": func() string {
				return externalURL.String()
			},
			"parseDuration": func(d string) (float64, error) {
				v, err := model.ParseDurationAllowNegative(d)
				if err != nil {
					return 0, err
				}
				return float64(time.Duration(v)) / float64(time.Second), nil
			},
		},
		options: options,
	}
}
Repository: prometheus/prometheus
File: template/template.go (Go)

Code before:

				return floatToTime(v)
			},
			"now": func() time.Time {
				return time.Now()
			},
			"pathPrefix": func() string {
				return externalURL.Path
			},
			"externalURL": func() string {
				return externalURL.String()
			},
			"parseDuration": func(d string) (float64, error) {
				v, err := model.ParseDuration(d)
				if err != nil {
					return 0, err
				}
				return float64(time.Duration(v)) / float64(time.Second), nil
			},
			"parseGoDuration": func(d string) (time.Duration, error) {
				v, err := model.ParseDuration(d)
				if err != nil {
					return 0, err
				}
				return time.Duration(v), nil
			},
		},
		options: options,
	}
}

// AlertTemplateData returns the interface to be used in expanding the template.
func AlertTemplateData(labels, externalLabels map[string]string, externalURL string, smpl promql.Sample) interface{} {
	res := struct {
		Labels         map[string]string
		ExternalLabels map[string]string
		ExternalURL    string
		Value          interface{}
	}{
		Labels:         labels,
		ExternalLabels: externalLabels,
		ExternalURL:    externalURL,
		Value:          smpl.F,
	}

	if smpl.H != nil {
		res.Value = smpl.H
	}

	return res
}

Review comment (suggestion):
To stay more in line with the other functions (i.e. deal with the Prometheus types everywhere but offer a conversion functions for Go types), let's simply enable negative durations above in `parseDuration` and instead of `parseGoDuration`, offer a `toDuration` functions similar to the existing `toTime` function.

Code after:

				return floatToTime(v)
			},
			"toDuration": func(i interface{}) (*time.Duration, error) {
				v, err := common_templates.ConvertToFloat(i)
				if err != nil {
					return nil, err
				}
				d := time.Duration(v * float64(time.Second))
				return &d, nil
			},
			"now": func() float64 {
				return float64(timestamp) / 1000.0
			},
			"pathPrefix": func() string {
				return externalURL.Path
			},
			"externalURL": func() string {
				return externalURL.String()
			},
			"parseDuration": func(d string) (float64, error) {
				v, err := model.ParseDurationAllowNegative(d)
				if err != nil {
					return 0, err
				}
				return float64(time.Duration(v)) / float64(time.Second), nil
			},
		},
		options: options,
	}
}

// AlertTemplateData returns the interface to be used in expanding the template.
func AlertTemplateData(labels, externalLabels map[string]string, externalURL string, smpl promql.Sample) interface{} {
	res := struct {
		Labels         map[string]string
		ExternalLabels map[string]string
		ExternalURL    string
		Value          interface{}
	}{
		Labels:         labels,
		ExternalLabels: externalLabels,
		ExternalURL:    externalURL,
		Value:          smpl.F,
	}

	if smpl.H != nil {
		res.Value = smpl.H
	}

	return res