ronantakizawa/codereview-bench huggingface dataset:
Repository: spf13/viper
File: errors.go (Go)

Code before:
	"fmt"
)

/* File look-up errors */

// ConfigFileNotFoundError denotes failing to find a configuration file from a search.
//
// Deprecated: This is wrapped by FileNotFoundFromSearchError, which should be used instead.
type ConfigFileNotFoundError struct {
	name      string
	locations []string
}

// Error returns the formatted error.
func (fnfe ConfigFileNotFoundError) Error() string {
	message := fmt.Sprintf("File %q Not Found", fnfe.name)
	if len(fnfe.locations) != 0 {
		message += fmt.Sprintf(" in %v", fnfe.locations)
	}

	return message
}

// Unwraps to FileNotFoundFromSearchError.
func (fnfe ConfigFileNotFoundError) Unwrap() error {
	return FileNotFoundFromSearchError{err: fnfe}
}

// FileNotFoundFromSearchError denotes failing to find a configuration file from a search.
// Wraps ConfigFileNotFoundError.
type FileNotFoundFromSearchError struct {
	err ConfigFileNotFoundError
}

// Error returns the formatted error.
func (fnfe FileNotFoundFromSearchError) Error() string {
	return fnfe.err.Error()
}

// FileNotFoundError denotes failing to find a specific configuration file.
type FileNotFoundError struct {
	path string
}

// Error returns the formatted error.
func (fnfe FileNotFoundError) Error() string {
	return fmt.Sprintf("File %q Not Found", fnfe.path)
}

/* Other error types */


Review comment (suggestion):
Given `ConfigFileNotFoundError` and it wraps the error from search, I'd suggest moving these fields to the new error, and passing that as an unexported field to this error.

Code after:
	"fmt"
)

// FileLookupError is returned when Viper cannot resolve a configuration file.
//
// This is meant to be a common interface for all file look-up errors, occurring either because a
// file does not exist or because it cannot find any file matching finder criteria.
type FileLookupError interface {
	error

	fileLookup()
}

// ConfigFileNotFoundError denotes failing to find a configuration file from a search.
//
// Deprecated: This is error wraps [FileNotFoundFromSearchError], which should be used instead.
type ConfigFileNotFoundError struct {
	locations []string
	name      string
}

// Error returns the formatted error.
func (e ConfigFileNotFoundError) Error() string {
	return e.Unwrap().Error()
}

// Unwraps to FileNotFoundFromSearchError.
func (e ConfigFileNotFoundError) Unwrap() error {
	return FileNotFoundFromSearchError(e)
}

// FileNotFoundFromSearchError denotes failing to find a configuration file from a search.
// Wraps ConfigFileNotFoundError.
type FileNotFoundFromSearchError struct {
	locations []string
	name      string
}

func (e FileNotFoundFromSearchError) fileLookup() {}

// Error returns the formatted error.
func (e FileNotFoundFromSearchError) Error() string {
	message := fmt.Sprintf("File %q not found", e.name)

	if len(e.locations) > 0 {
		message += fmt.Sprintf(" in %v", e.locations)
	}

	return message
}
Repository: spf13/viper
File: errors.go (Go)

Code before:
	locations []string
}

// Error returns the formatted error.
func (fnfe ConfigFileNotFoundError) Error() string {
	message := fmt.Sprintf("File %q Not Found", fnfe.name)
	if len(fnfe.locations) != 0 {
		message += fmt.Sprintf(" in %v", fnfe.locations)
	}

	return message
}

// Unwraps to FileNotFoundFromSearchError.
func (fnfe ConfigFileNotFoundError) Unwrap() error {
	return FileNotFoundFromSearchError{err: fnfe}
}

// FileNotFoundFromSearchError denotes failing to find a configuration file from a search.
// Wraps ConfigFileNotFoundError.
type FileNotFoundFromSearchError struct {
	err ConfigFileNotFoundError
}

// Error returns the formatted error.
func (fnfe FileNotFoundFromSearchError) Error() string {
	return fnfe.err.Error()
}

// FileNotFoundError denotes failing to find a specific configuration file.
type FileNotFoundError struct {
	path string
}

// Error returns the formatted error.
func (fnfe FileNotFoundError) Error() string {
	return fmt.Sprintf("File %q Not Found", fnfe.path)
}

/* Other error types */

// ConfigFileAlreadyExistsError denotes failure to write new configuration file.
type ConfigFileAlreadyExistsError string

// Error returns the formatted error when configuration already exists.
func (faee ConfigFileAlreadyExistsError) Error() string {
	return fmt.Sprintf("Config File %q Already Exists", string(faee))
}

// ConfigMarshalError happens when failing to marshal the configuration.
type ConfigMarshalError struct {

Review comment (suggestion):
```suggestion
func (e FileNotFoundFromSearchError) Error() string {
```

Code after:
	fileLookup()
}

// ConfigFileNotFoundError denotes failing to find a configuration file from a search.
//
// Deprecated: This is error wraps [FileNotFoundFromSearchError], which should be used instead.
type ConfigFileNotFoundError struct {
	locations []string
	name      string
}

// Error returns the formatted error.
func (e ConfigFileNotFoundError) Error() string {
	return e.Unwrap().Error()
}

// Unwraps to FileNotFoundFromSearchError.
func (e ConfigFileNotFoundError) Unwrap() error {
	return FileNotFoundFromSearchError(e)
}

// FileNotFoundFromSearchError denotes failing to find a configuration file from a search.
// Wraps ConfigFileNotFoundError.
type FileNotFoundFromSearchError struct {
	locations []string
	name      string
}

func (e FileNotFoundFromSearchError) fileLookup() {}

// Error returns the formatted error.
func (e FileNotFoundFromSearchError) Error() string {
	message := fmt.Sprintf("File %q not found", e.name)

	if len(e.locations) > 0 {
		message += fmt.Sprintf(" in %v", e.locations)
	}

	return message
}

// FileNotFoundError denotes failing to find a specific configuration file.
type FileNotFoundError struct {
	err  error
	path string
}

func (e FileNotFoundError) fileLookup() {}

// Error returns the formatted error.
func (e FileNotFoundError) Error() string {
Repository: spf13/viper
File: errors.go (Go)

Code before:
	return message
}

// Unwraps to FileNotFoundFromSearchError.
func (fnfe ConfigFileNotFoundError) Unwrap() error {
	return FileNotFoundFromSearchError{err: fnfe}
}

// FileNotFoundFromSearchError denotes failing to find a configuration file from a search.
// Wraps ConfigFileNotFoundError.
type FileNotFoundFromSearchError struct {
	err ConfigFileNotFoundError
}

// Error returns the formatted error.
func (fnfe FileNotFoundFromSearchError) Error() string {
	return fnfe.err.Error()
}

// FileNotFoundError denotes failing to find a specific configuration file.
type FileNotFoundError struct {
	path string
}

// Error returns the formatted error.
func (fnfe FileNotFoundError) Error() string {
	return fmt.Sprintf("File %q Not Found", fnfe.path)
}

/* Other error types */

// ConfigFileAlreadyExistsError denotes failure to write new configuration file.
type ConfigFileAlreadyExistsError string

// Error returns the formatted error when configuration already exists.
func (faee ConfigFileAlreadyExistsError) Error() string {
	return fmt.Sprintf("Config File %q Already Exists", string(faee))
}

// ConfigMarshalError happens when failing to marshal the configuration.
type ConfigMarshalError struct {
	err error
}

// Error returns the formatted configuration error.
func (e ConfigMarshalError) Error() string {
	return fmt.Sprintf("While marshaling config: %s", e.err.Error())
}

// UnsupportedConfigError denotes encountering an unsupported
// configuration filetype.

Review comment (suggestion):
```suggestion
func (e FileNotFoundError) Error() string {
```

Code after:

// Error returns the formatted error.
func (e ConfigFileNotFoundError) Error() string {
	return e.Unwrap().Error()
}

// Unwraps to FileNotFoundFromSearchError.
func (e ConfigFileNotFoundError) Unwrap() error {
	return FileNotFoundFromSearchError(e)
}

// FileNotFoundFromSearchError denotes failing to find a configuration file from a search.
// Wraps ConfigFileNotFoundError.
type FileNotFoundFromSearchError struct {
	locations []string
	name      string
}

func (e FileNotFoundFromSearchError) fileLookup() {}

// Error returns the formatted error.
func (e FileNotFoundFromSearchError) Error() string {
	message := fmt.Sprintf("File %q not found", e.name)

	if len(e.locations) > 0 {
		message += fmt.Sprintf(" in %v", e.locations)
	}

	return message
}

// FileNotFoundError denotes failing to find a specific configuration file.
type FileNotFoundError struct {
	err  error
	path string
}

func (e FileNotFoundError) fileLookup() {}

// Error returns the formatted error.
func (e FileNotFoundError) Error() string {
	return fmt.Sprintf("file not found: %s", e.path)
}

// ConfigFileAlreadyExistsError denotes failure to write new configuration file.
type ConfigFileAlreadyExistsError string

// Error returns the formatted error when configuration already exists.
func (e ConfigFileAlreadyExistsError) Error() string {
	return fmt.Sprintf("Config File %q Already Exists", string(e))
}
Repository: spf13/viper
File: errors.go (Go)

Code before:
}

// Unwraps to FileNotFoundFromSearchError.
func (fnfe ConfigFileNotFoundError) Unwrap() error {
	return FileNotFoundFromSearchError{err: fnfe}
}

// FileNotFoundFromSearchError denotes failing to find a configuration file from a search.
// Wraps ConfigFileNotFoundError.
type FileNotFoundFromSearchError struct {
	err ConfigFileNotFoundError
}

// Error returns the formatted error.
func (fnfe FileNotFoundFromSearchError) Error() string {
	return fnfe.err.Error()
}

// FileNotFoundError denotes failing to find a specific configuration file.
type FileNotFoundError struct {
	path string
}

// Error returns the formatted error.
func (fnfe FileNotFoundError) Error() string {
	return fmt.Sprintf("File %q Not Found", fnfe.path)
}

/* Other error types */

// ConfigFileAlreadyExistsError denotes failure to write new configuration file.
type ConfigFileAlreadyExistsError string

// Error returns the formatted error when configuration already exists.
func (faee ConfigFileAlreadyExistsError) Error() string {
	return fmt.Sprintf("Config File %q Already Exists", string(faee))
}

// ConfigMarshalError happens when failing to marshal the configuration.
type ConfigMarshalError struct {
	err error
}

// Error returns the formatted configuration error.
func (e ConfigMarshalError) Error() string {
	return fmt.Sprintf("While marshaling config: %s", e.err.Error())
}

// UnsupportedConfigError denotes encountering an unsupported
// configuration filetype.
type UnsupportedConfigError string

Review comment (suggestion):
```suggestion
	return fmt.Sprintf("file not found: %s", fnfe.path)
```

Code after:
// Error returns the formatted error.
func (e ConfigFileNotFoundError) Error() string {
	return e.Unwrap().Error()
}

// Unwraps to FileNotFoundFromSearchError.
func (e ConfigFileNotFoundError) Unwrap() error {
	return FileNotFoundFromSearchError(e)
}

// FileNotFoundFromSearchError denotes failing to find a configuration file from a search.
// Wraps ConfigFileNotFoundError.
type FileNotFoundFromSearchError struct {
	locations []string
	name      string
}

func (e FileNotFoundFromSearchError) fileLookup() {}

// Error returns the formatted error.
func (e FileNotFoundFromSearchError) Error() string {
	message := fmt.Sprintf("File %q not found", e.name)

	if len(e.locations) > 0 {
		message += fmt.Sprintf(" in %v", e.locations)
	}

	return message
}

// FileNotFoundError denotes failing to find a specific configuration file.
type FileNotFoundError struct {
	err  error
	path string
}

func (e FileNotFoundError) fileLookup() {}

// Error returns the formatted error.
func (e FileNotFoundError) Error() string {
	return fmt.Sprintf("file not found: %s", e.path)
}

// ConfigFileAlreadyExistsError denotes failure to write new configuration file.
type ConfigFileAlreadyExistsError string

// Error returns the formatted error when configuration already exists.
func (e ConfigFileAlreadyExistsError) Error() string {
	return fmt.Sprintf("Config File %q Already Exists", string(e))
}
Repository: spf13/viper
File: errors.go (Go)

Code before:
package viper

import (
	"fmt"
)

/* File look-up errors */

// FileLookupError is returned when Viper cannot resolve a configuration file.
//
// This is meant to be a common interface for all file look-up errors, occurring either because a
// file does not exist or because it cannot find any file matching finder criteria.
type FileLookupError interface {
	error

	fileLookup()
}

// ConfigFileNotFoundError denotes failing to find a configuration file from a search.
//
// Deprecated: This is wrapped by FileNotFoundFromSearchError, which should be used instead.
type ConfigFileNotFoundError struct {
	locations []string
	name      string
}

// Error returns the formatted error.
func (e ConfigFileNotFoundError) Error() string {
	message := fmt.Sprintf("File %q Not Found", e.name)
	if len(e.locations) != 0 {
		message += fmt.Sprintf(" in %v", e.locations)
	}

	return message
}

// Unwraps to FileNotFoundFromSearchError.
func (e ConfigFileNotFoundError) Unwrap() error {
	return FileNotFoundFromSearchError{err: e, locations: e.locations, name: e.name}

Review comment (suggestion):
@sagikazarmark I don't really know what the vision is for `fileLookup` other than to differentiate it from a normal error, let me know what you might have wanted here more explicitly.

Code after:
package viper

import (
	"fmt"
)

// FileLookupError is returned when Viper cannot resolve a configuration file.
//
// This is meant to be a common interface for all file look-up errors, occurring either because a
// file does not exist or because it cannot find any file matching finder criteria.
type FileLookupError interface {
	error

	fileLookup()
}

// ConfigFileNotFoundError denotes failing to find a configuration file from a search.
//
// Deprecated: This is error wraps [FileNotFoundFromSearchError], which should be used instead.
type ConfigFileNotFoundError struct {
	locations []string
	name      string
}

// Error returns the formatted error.
func (e ConfigFileNotFoundError) Error() string {
	return e.Unwrap().Error()
}

// Unwraps to FileNotFoundFromSearchError.
func (e ConfigFileNotFoundError) Unwrap() error {
	return FileNotFoundFromSearchError(e)
}

// FileNotFoundFromSearchError denotes failing to find a configuration file from a search.
// Wraps ConfigFileNotFoundError.
type FileNotFoundFromSearchError struct {
	locations []string
	name      string
Repository: spf13/viper
File: viper.go (Go)

Code before:

	// set innermost value
	deepestMap[lastKey] = value
}

// ReadInConfig will discover and load the configuration file from disk
// and key/value stores, searching in one of the defined paths.
func ReadInConfig() error { return v.ReadInConfig() }

func (v *Viper) ReadInConfig() error {
	var pathError *fs.PathError

	v.logger.Info("attempting to read in config file")
	filename, err := v.getConfigFile()
	if err != nil {
		return err
	}

	if !slices.Contains(SupportedExts, v.getConfigType()) {
		return UnsupportedConfigError(v.getConfigType())
	}

	v.logger.Debug("reading file", "file", filename)
	file, err := afero.ReadFile(v.fs, filename)
	if err != nil {
		if errors.As(err, &pathError) {
			// The specified config file is missing
			return FileNotFoundError{err: err, path: filename}
		} else {
			// We hit some other error from the filesystem that isn't a missing file
			return err
		}
	}

	config := make(map[string]any)

	err = v.unmarshalReader(bytes.NewReader(file), config)
	if err != nil {
		return err
	}

	v.config = config
	return nil
}

// MergeInConfig merges a new configuration with an existing config.
func MergeInConfig() error { return v.MergeInConfig() }

func (v *Viper) MergeInConfig() error {
	v.logger.Info("attempting to merge in config file")
	filename, err := v.getConfigFile()

Review comment (suggestion):
@sagikazarmark This avoids the extra `fs.Stat` syscall, but I'm worried about this match being overly broad. I think normally you'd check for specific error code from `syscall`, but I'm worried about making a change that won't work across OSes.

Code after:
	value = toCaseInsensitiveValue(value)

	path := strings.Split(key, v.keyDelim)
	lastKey := strings.ToLower(path[len(path)-1])
	deepestMap := deepSearch(v.override, path[0:len(path)-1])

	// set innermost value
	deepestMap[lastKey] = value
}

// ReadInConfig will discover and load the configuration file from disk
// and key/value stores, searching in one of the defined paths.
func ReadInConfig() error { return v.ReadInConfig() }

func (v *Viper) ReadInConfig() error {
	v.logger.Info("attempting to read in config file")
	filename, err := v.getConfigFile()
	if err != nil {
		return err
	}

	if !slices.Contains(SupportedExts, v.getConfigType()) {
		return UnsupportedConfigError(v.getConfigType())
	}

	v.logger.Debug("reading file", "file", filename)

	file, err := afero.ReadFile(v.fs, filename)
	if errors.Is(err, fs.ErrNotExist) {
		// The specified config file is missing
		return FileNotFoundError{err: err, path: filename}
	} else if err != nil {
		// We hit some other error from the filesystem that isn't a missing file
		return err
	}

	config := make(map[string]any)

	err = v.unmarshalReader(bytes.NewReader(file), config)
	if err != nil {
		return err
	}

	v.config = config
	return nil
}

// MergeInConfig merges a new configuration with an existing config.
func MergeInConfig() error { return v.MergeInConfig() }

func (v *Viper) MergeInConfig() error {
Repository: spf13/viper
File: errors.go (Go)

Code before:
// Error returns the formatted error.
func (e ConfigFileNotFoundError) Error() string {
	message := fmt.Sprintf("File %q Not Found", e.name)
	if len(e.locations) != 0 {
		message += fmt.Sprintf(" in %v", e.locations)
	}

	return message
}

// Unwraps to FileNotFoundFromSearchError.
func (e ConfigFileNotFoundError) Unwrap() error {
	return FileNotFoundFromSearchError{err: e, locations: e.locations, name: e.name}
}

// FileNotFoundFromSearchError denotes failing to find a configuration file from a search.
// Wraps ConfigFileNotFoundError.
type FileNotFoundFromSearchError struct {
	err       ConfigFileNotFoundError
	locations []string
	name      string
}

func (e FileNotFoundFromSearchError) fileLookup() {
	return
}

// Error returns the formatted error.
func (e FileNotFoundFromSearchError) Error() string {
	return e.err.Error()
}

// FileNotFoundError denotes failing to find a specific configuration file.
type FileNotFoundError struct {
	err  error
	path string
}

func (e FileNotFoundError) fileLookup() {
	return
}

// Error returns the formatted error.
func (e FileNotFoundError) Error() string {
	return fmt.Sprintf("file not found: %s", e.path)
}

/* Other error types */

// ConfigFileAlreadyExistsError denotes failure to write new configuration file.
type ConfigFileAlreadyExistsError string

Review comment (suggestion):
```suggestion
func (e FileNotFoundFromSearchError) fileLookup() {}
```

Code after:
	return e.Unwrap().Error()
}

// Unwraps to FileNotFoundFromSearchError.
func (e ConfigFileNotFoundError) Unwrap() error {
	return FileNotFoundFromSearchError(e)
}

// FileNotFoundFromSearchError denotes failing to find a configuration file from a search.
// Wraps ConfigFileNotFoundError.
type FileNotFoundFromSearchError struct {
	locations []string
	name      string
}

func (e FileNotFoundFromSearchError) fileLookup() {}

// Error returns the formatted error.
func (e FileNotFoundFromSearchError) Error() string {
	message := fmt.Sprintf("File %q not found", e.name)

	if len(e.locations) > 0 {
		message += fmt.Sprintf(" in %v", e.locations)
	}

	return message
}

// FileNotFoundError denotes failing to find a specific configuration file.
type FileNotFoundError struct {
	err  error
	path string
}

func (e FileNotFoundError) fileLookup() {}

// Error returns the formatted error.
func (e FileNotFoundError) Error() string {
	return fmt.Sprintf("file not found: %s", e.path)
}

// ConfigFileAlreadyExistsError denotes failure to write new configuration file.
type ConfigFileAlreadyExistsError string

// Error returns the formatted error when configuration already exists.
func (e ConfigFileAlreadyExistsError) Error() string {
	return fmt.Sprintf("Config File %q Already Exists", string(e))
}

// ConfigMarshalError happens when failing to marshal the configuration.
type ConfigMarshalError struct {
Repository: square/retrofit
File: retrofit/src/main/java/retrofit2/Invocation.java (Java)

Code before:
   * This will never be null when created by Retrofit. Null will only be returned when created
   * by {@link #of(Method, List)}.
   */
  @Nullable
  public Object instance() {
    return instance;
  }

  public Method method() {
    return method;
  }

  public List<?> arguments() {
    return arguments;
  }

  /**
   * The URL from the annotation like {@link retrofit2.http.GET @GET} or
   * {@link retrofit2.http.POST @POST}.
   * <p>
   * If the method uses the {@link retrofit2.http.Path @Path} annotation, this is the template URL
   * before path substitution, as it occurs in source code.
   * <p>
   * This value will be null if one of the method's parameter is annotated
   * {@link retrofit2.http.Url @Url}. It will also be null if this Invocation was created using one
   * of the {@link #of factory-functions} that don't have this value.
   */
  @Nullable
  public String annotationUrl() {
    return annotationUrl;
  }

  @Override
  public String toString() {
    return String.format("%s.%s() %s", service.getName(), method.getName(), arguments);
  }
}


Review comment (suggestion):
```suggestion
   * of the {@linkplain #of factory-functions} that don't have this value.
```

Code after:
   * This will never be null when created by Retrofit. Null will only be returned when created
   * by {@link #of(Method, List)}.
   */
  @Nullable
  public Object instance() {
    return instance;
  }

  public Method method() {
    return method;
  }

  public List<?> arguments() {
    return arguments;
  }

  /**
   * The URL from the annotation like {@link retrofit2.http.GET @GET} or
   * {@link retrofit2.http.POST @POST}.
   * <p>
   * If the method uses the {@link retrofit2.http.Path @Path} annotation, this is the template URL
   * before path substitution, as it occurs in source code.
   * <p>
   * This value will be null if one of the method's parameter is annotated
   * {@link retrofit2.http.Url @Url}. It will also be null if this Invocation was created using one
   * of the {@linkplain #of factory-functions} that don't have this value.
   */
  @Nullable
  public String annotationUrl() {
    return annotationUrl;
  }

  @Override
  public String toString() {
    return String.format("%s.%s() %s", service.getName(), method.getName(), arguments);
  }
}