ronantakizawa/codereview-bench huggingface dataset:
Repository: rclone/rclone
File: backend/filelu/filelu_object.go (Go)

Code before:
			reader = struct {
				io.Reader
				io.Closer
			}{
				Reader: io.LimitReader(resp.Body, count),
				Closer: resp.Body,
			}
		} else {
			reader = resp.Body
		}

		return false, nil
	})

	if err != nil {
		return nil, err
	}

	return reader, nil
}

// Update updates the object with new data
func (o *Object) Update(ctx context.Context, in io.Reader, src fs.ObjectInfo, options ...fs.OpenOption) error {
	size := src.Size()

	if size <= 500*1024*1024 {
		err := o.fs.uploadFile(ctx, in, o.remote)
		if err != nil {
			return err
		}
	} else {
		fullPath := path.Join(o.fs.root, o.remote)
		err := o.fs.multipartUpload(ctx, in, fullPath)
		if err != nil {
			return fmt.Errorf("failed to upload file: %w", err)
		}
	}

	o.size = size
	o.modTime = src.ModTime(ctx)
	return nil
}

// Remove deletes the object from FileLu
func (o *Object) Remove(ctx context.Context) error {
	fullPath := "/" + strings.Trim(path.Join(o.fs.root, o.remote), "/")

	err := o.fs.deleteFile(ctx, fullPath)
	if err != nil {
		return err
	}

Review comment (refactor):
This code is duplicated in your `Put` method.

Usually rclone backends call `Update` from `Put`, eg

https://github.com/rclone/rclone/blob/341ce61a2aa535b5c555ade033dfa82b0fab4474/backend/s3/s3.go#L2537-L2545

And all the hard work is done in `Update`

Code after:
		}

		if count > 0 {
			reader = struct {
				io.Reader
				io.Closer
			}{
				Reader: io.LimitReader(resp.Body, count),
				Closer: resp.Body,
			}
		} else {
			reader = resp.Body
		}

		return false, nil
	})
	if err != nil {
		return nil, err
	}

	return reader, nil
}

// Update updates the object with new data
func (o *Object) Update(ctx context.Context, in io.Reader, src fs.ObjectInfo, options ...fs.OpenOption) error {
	size := src.Size()

	if size <= int64(o.fs.opt.UploadCutoff) {
		err := o.fs.uploadFile(ctx, in, o.remote)
		if err != nil {
			return err
		}
	} else {
		fullPath := path.Join(o.fs.root, o.remote)
		err := o.fs.multipartUpload(ctx, in, fullPath)
		if err != nil {
			return fmt.Errorf("failed to upload file: %w", err)
		}
	}

	o.size = size
	o.modTime = src.ModTime(ctx)
	return nil
}

// Remove deletes the object from FileLu
func (o *Object) Remove(ctx context.Context) error {
	fullPath := "/" + strings.Trim(path.Join(o.fs.root, o.remote), "/")

	err := o.fs.deleteFile(ctx, fullPath)
	if err != nil {
Repository: rclone/rclone
File: backend/filelu/filelu_test.go (Go)

Code before:
package filelu_test

import (
	"strings"
	"testing"

	"github.com/rclone/rclone/fstest/fstests"
)

// Custom wrapper to filter out unwanted subtests
func TestIntegration(t *testing.T) {
	// Skip specific encoding tests manually
	if strings.Contains(t.Name(), "FsEncoding/punctuation") {
		t.Skip("Skipping FsEncoding/punctuation — not supported")
	}
	fstests.Run(t, &fstests.Opt{
		RemoteName:      "filelu:",
		NilObject:       nil,
		SkipInvalidUTF8: true,
	})
}


Review comment (suggestion):
Don't change the name of the remote here - you'll break the integration tester.

You can add `-remote filelu:` to your `go test` commands

Code after:
package filelu_test

import (
	"testing"

	"github.com/rclone/rclone/fstest/fstests"
)

// TestIntegration runs integration tests for the FileLu backend
func TestIntegration(t *testing.T) {
	fstests.Run(t, &fstests.Opt{
		RemoteName:      "TestFileLu:",
		NilObject:       nil,
		SkipInvalidUTF8: true,
	})
}
Repository: react-hook-form/react-hook-form
File: src/useForm.ts (TypeScript)

Code before:
        type,
        types,
        message,
        ref: fieldsRef.current[name] ? fieldsRef.current[name]!.ref : {},
      });

      if (shouldRender) {
        reRender();
      }
    }
  };

  function setError(
    name: FieldName<TFieldValues>,
    error:
      | {
          types: MultipleFieldErrors;
        }
      | {
          message?: Message;
          type: string;
        },
  ): void {
    isValidRef.current = false;

    setInternalError({
      name,
      type: '',
      ...error,
      shouldRender: true,
    });
  }

  const watchInternal = React.useCallback(
    (
      fieldNames?: string | string[],
      defaultValue?: unknown,
      watchId?: string,
    ) => {
      const watchFields = watchId
        ? watchFieldsHookRef.current[watchId]
        : watchFieldsRef.current;
      const combinedDefaultValues = isUndefined(defaultValue)
        ? defaultValuesRef.current
        : defaultValue;
      const fieldValues = getFieldsValues<TFieldValues>(
        fieldsRef.current,
        fieldNames,
      );

      if (isString(fieldNames)) {

Review comment (suggestion):
`setInternalError` method is used inside only `setError` method.  I think you can expand `setInternalError` method to `setError` method.

Code after:
        : defaultValue;
      const fieldValues = getFieldsValues<TFieldValues>(
        fieldsRef.current,
        fieldNames,
      );

      if (isString(fieldNames)) {
        return assignWatchFields<TFieldValues>(
          fieldValues,
          fieldNames,
          watchFields,
          isUndefined(defaultValue)
            ? get(combinedDefaultValues, fieldNames)
            : (defaultValue as UnpackNestedValue<DeepPartial<TFieldValues>>),
          true,
        );
      }

      if (isArray(fieldNames)) {
        return fieldNames.reduce(
          (previous, name) => ({
            ...previous,
            [name]: assignWatchFields<TFieldValues>(
              fieldValues,
              name,
              watchFields,
              combinedDefaultValues as UnpackNestedValue<
                DeepPartial<TFieldValues>
              >,
            ),
          }),
          {},
        );
      }

      if (isUndefined(watchId)) {
        isWatchAllRef.current = true;
      }

      return transformToNestObject(
        (!isEmptyObject(fieldValues) && fieldValues) ||
          (combinedDefaultValues as FieldValues),
      );
    },
    [],
  );

  function watch(): UnpackNestedValue<TFieldValues>;
  function watch<
    TFieldName extends string,
    TFieldValue extends TFieldValues[TFieldName]
Repository: react-hook-form/react-hook-form
File: src/useForm.ts (TypeScript)

Code before:
      if (
        field &&
        (!isNameInFieldArray(fieldArrayNamesRef.current, field.ref.name) ||
          forceDelete)
      ) {
        removeFieldEventListener(field, forceDelete);

        if (shouldUnregister) {
          [
            errorsRef,
            touchedFieldsRef,
            dirtyFieldsRef,
            defaultValuesAtRenderRef,
          ].forEach((data) => unset(data.current, field.ref.name));
          [
            fieldsWithValidationRef,
            validFieldsRef,
            watchFieldsRef,
          ].forEach((data) => data.current.delete(field.ref.name));

          if (
            readFormStateRef.current.isValid ||
            readFormStateRef.current.touched
          ) {
            reRender();

            if (resolverRef.current) {
              validateResolver();
            }
          }
        }
      }
    },
    [reRender, validateResolver, removeFieldEventListener, resolverRef],
  );

  function clearErrors(
    name?: FieldName<TFieldValues> | FieldName<TFieldValues>[],
  ): void {
    if (name) {
      (isArray(name) ? name : [name]).forEach((inputName) =>
        unset(errorsRef.current, inputName),
      );
    } else {
      errorsRef.current = {};
    }

    reRender();
  }

  function setError(name: FieldName<TFieldValues>, error: ErrorOption): void {

Review comment (bug):
@bluebill1049 
I'm sorry if I am wrong.
I think it should `!isEmptyObject(dirtyFieldsRef.current)` because this code return true when `dirtyFieldsRef` is empty.

Code after:
      if (
        field &&
        (!isNameInFieldArray(fieldArrayNamesRef.current, field.ref.name) ||
          forceDelete)
      ) {
        removeFieldEventListener(field, forceDelete);

        if (shouldUnregister) {
          [
            errorsRef,
            touchedFieldsRef,
            dirtyFieldsRef,
            defaultValuesAtRenderRef,
          ].forEach((data) => unset(data.current, field.ref.name));
          [
            fieldsWithValidationRef,
            validFieldsRef,
            watchFieldsRef,
          ].forEach((data) => data.current.delete(field.ref.name));

          if (
            readFormStateRef.current.isValid ||
            readFormStateRef.current.touched ||
            readFormStateRef.current.isDirty
          ) {
            isDirtyRef.current = isEmptyObject(dirtyFieldsRef.current);
            reRender();

            if (resolverRef.current) {
              validateResolver();
            }
          }
        }
      }
    },
    [reRender, validateResolver, removeFieldEventListener, resolverRef],
  );

  function clearErrors(
    name?: FieldName<TFieldValues> | FieldName<TFieldValues>[],
  ): void {
    if (name) {
      (isArray(name) ? name : [name]).forEach((inputName) =>
        unset(errorsRef.current, inputName),
      );
    } else {
      errorsRef.current = {};
    }

    reRender();
  }
Repository: react-hook-form/react-hook-form
File: src/utils/isSameError.ts (TypeScript)

Code before:
import isObject from './isObject';
import { FieldError } from '../types/form';

export default (
  error: FieldError | undefined,
  { type, types, message }: FieldError,
): boolean =>
  isObject(error) &&
  error.type === type &&
  error.message === message &&
  Object.entries(error.types || {}).every(
    ([key, value]) => (types || {})[key] === value,
  ) &&
  Object.keys(error.types || {}).length === Object.keys(types || {}).length;


Review comment (suggestion):
If I don't do this, it will be impossible to cover this on the test because `Object.keys(error.types || {}).length === Object.keys(types).length` will prevent that from happening.

Code after:
import isObject from './isObject';
import { FieldError } from '../types/form';

export default (
  error: FieldError | undefined,
  { type, types = {}, message }: FieldError,
): boolean =>
  isObject(error) &&
  error.type === type &&
  error.message === message &&
  Object.keys(error.types || {}).length === Object.keys(types).length &&
  Object.entries(error.types || {}).every(
    ([key, value]) => types[key] === value,
  );
Repository: react-hook-form/react-hook-form
File: src/useForm.test.tsx (TypeScript)

Code before:

    it('should set value of multiple select correctly', async () => {
      const { result } = renderHook(() => useForm<{ test: string }>());

      result.current.register({
        name: 'test',
        type: 'select-multiple',
        value: '1',
        options: [{ value: '1', selected: true }] as any,
      });

      result.current.setValue('test', '1');

      await act(async () => {
        await result.current.handleSubmit((data) => {
          expect(data).toEqual({
            test: ['1'],
          });
        })({
          preventDefault: () => {},
          persist: () => {},
        } as React.SyntheticEvent);
      });
    });

    it('should not set unmountFieldsState value when shouldUnregister is set to false', async () => {
      const { result } = renderHook(() =>
        useForm<{ test: string }>({
          shouldUnregister: false,
        }),
      );

      result.current.register({
        name: 'test',
        type: 'select-multiple',
        value: '1',
        options: [{ value: '1', selected: true }] as any,
      });

      result.current.setValue('test', '1');

      expect(result.current.control.unmountFieldsStateRef.current).toEqual({});
    });

    it('should set nested value correctly ', () => {
      const { result } = renderHook(() =>
        useForm<{
          test1: NestedValue<string[]>;
          test2: NestedValue<{
            key1: string;
            key2: number;

Review comment (suggestion):
this test has not been able to test correctly too.
You should check `unmountFieldsStateRef` after executed `unmount()` or `unregister()`.

Code after:
        attributes: { value: '1' },
      });

      result.current.setValue('test', '1');

      await act(async () => {
        await result.current.handleSubmit((data) => {
          expect(data).toEqual({
            test: '1',
          });
        })({
          preventDefault: () => {},
          persist: () => {},
        } as React.SyntheticEvent);
      });
    });

    it('should set value of multiple select correctly', async () => {
      const { result } = renderHook(() => useForm<{ test: string }>());

      result.current.register({
        name: 'test',
        type: 'select-multiple',
        value: '1',
        options: [{ value: '1', selected: true }] as any,
      });

      result.current.setValue('test', '1');

      await act(async () => {
        await result.current.handleSubmit((data) => {
          expect(data).toEqual({
            test: ['1'],
          });
        })({
          preventDefault: () => {},
          persist: () => {},
        } as React.SyntheticEvent);
      });
    });

    it('should set unmountFieldsState value when shouldUnregister is set to false', async () => {
      const { result } = renderHook(() =>
        useForm<{ test: string; checkbox: string[] }>({
          shouldUnregister: false,
        }),
      );

      act(() => {
        result.current.setValue('test', '1');
        result.current.setValue('checkbox', ['1', '2']);
Repository: react-hook-form/react-hook-form
File: src/utils/insert.test.ts (TypeScript)

Code before:
      ),
    ).toEqual([
      {
        firstName: '1',
        lastName: 'Luo',
        id: '75309979-e340-49eb-8016-5f67bfb56c1c',
      },
      {
        firstName: '2',
        lastName: 'Luo',
        id: '75309979-e340-49eb-8016-5f67bfb56c1c',
      },
      {
        firstName: '3',
        lastName: 'Luo',
        id: '75309979-e340-49eb-8016-5f67bfb56c1c',
      },
      {
        firstName: '4',
        lastName: 'Luo',
        id: '75309979-e340-49eb-8016-5f67bfb56c1c',
      },
    ]);
  });

  it('should insert null as value when value to be inserted is falsy', () => {
    expect(insert([1, 2, 4], 2)).toEqual([1, 2, undefined, 4]);
    expect(insert([1, 2, 4], 2, 0)).toEqual([1, 2, undefined, 4]);
    expect(insert([1, 2, 4], 2, false as any)).toEqual([1, 2, undefined, 4]);
    expect(insert([1, 2, 4], 2, '' as any)).toEqual([1, 2, undefined, 4]);
    expect(insert([1, 2, 4], 2, undefined as any)).toEqual([
      1,
      2,
      undefined,
      4,
    ]);
  });

  it('should spread value when it is an array at one deep-level', () => {
    expect(insert([1, 2], 2, [3, 4])).toEqual([1, 2, 3, 4]);
    expect(insert([1, 2], 2, [3, [4]])).toEqual([1, 2, 3, [4]]);
  });
});


Review comment (suggestion):
Should you change this word ?

```suggestion
  it('should insert undefined as value when value to be inserted is falsy', () => {
```

Code after:
      ),
    ).toEqual([
      {
        firstName: '1',
        lastName: 'Luo',
        id: '75309979-e340-49eb-8016-5f67bfb56c1c',
      },
      {
        firstName: '2',
        lastName: 'Luo',
        id: '75309979-e340-49eb-8016-5f67bfb56c1c',
      },
      {
        firstName: '3',
        lastName: 'Luo',
        id: '75309979-e340-49eb-8016-5f67bfb56c1c',
      },
      {
        firstName: '4',
        lastName: 'Luo',
        id: '75309979-e340-49eb-8016-5f67bfb56c1c',
      },
    ]);
  });

  it('should insert undefined as value when value to be inserted is falsy', () => {
    expect(insert([1, 2, 4], 2)).toEqual([1, 2, undefined, 4]);
    expect(insert([1, 2, 4], 2, 0)).toEqual([1, 2, undefined, 4]);
    expect(insert([1, 2, 4], 2, false as any)).toEqual([1, 2, undefined, 4]);
    expect(insert([1, 2, 4], 2, '' as any)).toEqual([1, 2, undefined, 4]);
    expect(insert([1, 2, 4], 2, undefined as any)).toEqual([
      1,
      2,
      undefined,
      4,
    ]);
  });

  it('should spread value when it is an array at one deep-level', () => {
    expect(insert([1, 2], 2, [3, 4])).toEqual([1, 2, 3, 4]);
    expect(insert([1, 2], 2, [3, [4]])).toEqual([1, 2, 3, [4]]);
  });
});
Repository: react-hook-form/react-hook-form
File: src/useWatch.test.tsx (TypeScript)

Code before:

      expect(parentCount).toBe(1);
      expect(childCount).toBe(2);

      parentCount = 0;
      childCount = 0;

      await actComponent(async () => {
        await fireEvent.submit(screen.getByRole('button', { name: /submit/i }));
      });

      expect(parentCount).toBe(2);
      expect(childCount).toBe(2);

      parentCount = 0;
      childCount = 0;

      await actComponent(async () => {
        await fireEvent.input(childInput, { target: { value: 'test1' } });
      });

      expect(parentCount).toBe(0);
      expect(childCount).toBe(1);
    });

    it("should not external re-render when field name don't match", async () => {
      let key = 1;

      (generateId as any).mockImplementation(() => key++);

      let renderCount = 0;

      const Child = ({ control }: { control: Control }) => {
        useWatch({ name: 'test2', control });

        return <div />;
      };

      const Parent = () => {
        const { register, control } = useForm();
        useWatch({ name: 'test1', control });

        renderCount++;

        return (
          <form>
            <input type="text" name="test1" ref={register} />
            <input type="text" name="test2" ref={register} />
            <Child control={control} />
          </form>
        );

Review comment (suggestion):
```suggestion
    it("should not re-render external component when field name don't match", async () => {
```

Code after:

      expect(parentCount).toBe(1);
      expect(childCount).toBe(2);

      parentCount = 0;
      childCount = 0;

      await actComponent(async () => {
        await fireEvent.submit(screen.getByRole('button', { name: /submit/i }));
      });

      expect(parentCount).toBe(2);
      expect(childCount).toBe(2);

      parentCount = 0;
      childCount = 0;

      await actComponent(async () => {
        await fireEvent.input(childInput, { target: { value: 'test1' } });
      });

      expect(parentCount).toBe(0);
      expect(childCount).toBe(1);
    });

    it("should not re-render external component when field name don't match", async () => {
      let key = 1;

      (generateId as any).mockImplementation(() => key++);

      let renderCount = 0;

      const Child = ({ control }: { control: Control }) => {
        useWatch({ name: 'test2', control });

        return <div />;
      };

      const Parent = () => {
        const { register, control } = useForm();
        useWatch({ name: 'test1', control });

        renderCount++;

        return (
          <form>
            <input type="text" name="test1" ref={register} />
            <input type="text" name="test2" ref={register} />
            <Child control={control} />
          </form>
        );
Repository: react-hook-form/react-hook-form
File: src/useForm.test.tsx (TypeScript)

Code before:
      expect(mockReset).not.toHaveBeenCalled();
    });

    it('should reset correct isValid formState after reset with valid value', async () => {
      const { result } = renderHook(() =>
        useForm<{ input: string; issue: string }>({
          mode: VALIDATION_MODE.onChange,
        }),
      );

      result.current.formState.isValid;

      await act(async () =>
        result.current.register(
          { name: 'issue', value: '' },
          { required: true },
        ),
      );

      expect(result.current.formState.isValid).toBeFalsy();

      await act(async () =>
        result.current.reset({
          issue: 'test',
        }),
      );

      expect(result.current.formState.isValid).toBeTruthy();
    });

    it('should set default value if values is specified to first argument', async () => {
      const { result } = renderHook(() => useForm());

      result.current.register('test');

      act(() => result.current.reset({ test: 'test' }));

      expect(result.current.control.defaultValuesRef.current).toEqual({
        test: 'test',
      });
    });

    it('should reset unmountFieldsState value when shouldUnregister set to false', () => {
      const { result } = renderHook(() =>
        useForm({
          shouldUnregister: false,
        }),
      );

      result.current.register('test');


Review comment (suggestion):
```suggestion
      act(() =>
        result.current.reset({
          issue: 'test',
        }),
      );
```

I think here is not asynchronous.

Code after:
      expect(mockReset).not.toHaveBeenCalled();
    });

    it('should reset correct isValid formState after reset with valid value', async () => {
      const { result } = renderHook(() =>
        useForm<{ input: string; issue: string }>({
          mode: VALIDATION_MODE.onChange,
        }),
      );

      result.current.formState.isValid;

      await act(async () =>
        result.current.register(
          { name: 'issue', value: '' },
          { required: true },
        ),
      );

      expect(result.current.formState.isValid).toBeFalsy();

      act(() =>
        result.current.reset({
          issue: 'test',
        }),
      );

      expect(result.current.formState.isValid).toBeTruthy();
    });

    it('should set default value if values is specified to first argument', async () => {
      const { result } = renderHook(() => useForm());

      result.current.register('test');

      act(() => result.current.reset({ test: 'test' }));

      expect(result.current.control.defaultValuesRef.current).toEqual({
        test: 'test',
      });
    });

    it('should reset unmountFieldsState value when shouldUnregister set to false', () => {
      const { result } = renderHook(() =>
        useForm({
          shouldUnregister: false,
        }),
      );

      result.current.register('test');