ronantakizawa/codereview-bench huggingface dataset:
Repository: facebook/react
File: packages/eslint-plugin-react-hooks/src/RulesOfHooks.js (JavaScript)

Code before:
function isInsideComponentOrHook(node) {
  while (node) {
    const functionName = getFunctionName(node);
    if (functionName) {
      if (isComponentName(functionName) || isHook(functionName)) {
        return true;
      }
    }
    if (isForwardRefCallback(node) || isMemoCallback(node)) {
      return true;
    }
    node = node.parent;
  }
  return false;
}

function isInsideDoWhileLoop(node) {
  while (node) {
    if ( node.type === 'DoWhileStatement' ) {
      return true;
    }
    node = node.parent;
  }
  return false;
}

function isUseEffectEventIdentifier(node) {
  if (__EXPERIMENTAL__) {
    return node.type === 'Identifier' && node.name === 'useEffectEvent';
  }
  return false;
}

function isUseIdentifier(node) {
  return isReactFunction(node, 'use');
}

export default {
  meta: {
    type: 'problem',
    docs: {
      description: 'enforces the Rules of Hooks',
      recommended: true,
      url: 'https://reactjs.org/docs/hooks-rules.html',
    },
  },
  create(context) {
    let lastEffect = null;
    const codePathReactHooksMapStack = [];
    const codePathSegmentStack = [];
    const useEffectEventFunctions = new WeakSet();

Review comment (refactor):
Adding an extra utility to traverse up the tree to check if the node is called within a `do/while` loop.

Code after:
function isInsideComponentOrHook(node) {
  while (node) {
    const functionName = getFunctionName(node);
    if (functionName) {
      if (isComponentName(functionName) || isHook(functionName)) {
        return true;
      }
    }
    if (isForwardRefCallback(node) || isMemoCallback(node)) {
      return true;
    }
    node = node.parent;
  }
  return false;
}

function isInsideDoWhileLoop(node) {
  while (node) {
    if (node.type === 'DoWhileStatement') {
      return true;
    }
    node = node.parent;
  }
  return false;
}

function isUseEffectEventIdentifier(node) {
  if (__EXPERIMENTAL__) {
    return node.type === 'Identifier' && node.name === 'useEffectEvent';
  }
  return false;
}

function isUseIdentifier(node) {
  return isReactFunction(node, 'use');
}

export default {
  meta: {
    type: 'problem',
    docs: {
      description: 'enforces the Rules of Hooks',
      recommended: true,
      url: 'https://reactjs.org/docs/hooks-rules.html',
    },
  },
  create(context) {
    let lastEffect = null;
    const codePathReactHooksMapStack = [];
    const codePathSegmentStack = [];
    const useEffectEventFunctions = new WeakSet();
Repository: facebook/react
File: scripts/flags/flags.js (JavaScript)

Code before:
      return 'RN Next Major';
    case 'canary':
      return 'OSS Canary';
    case 'next':
      return 'OSS Next Major';
    case 'experimental':
      return 'OSS Experimental';
    default:
      return arg;
  }
}

const FLAG_CONFIG = {
  'OSS Next Major': getNextMajorFlagValue,
  'OSS Canary': getOSSCanaryFlagValue,
  'OSS Experimental': getOSSExperimentalFlagValue,
  'WWW Classic': getWWWClassicFlagValue,
  'WWW Modern': getWWWModernFlagValue,
  'RN FB': getRNFBFlagValue,
  'RN OSS': getRNOSSFlagValue,
  'RN Next Major': getRNNextMajorFlagValue,
};

const FLAG_COLUMNS = Object.keys(FLAG_CONFIG);

const INTERNAL_VARIANTS = ['WWW Classic', 'WWW Modern', 'RN OSS'];
const OSS_VARIANTS = [
  'OSS Next Major',
  'OSS Canary',
  'OSS Experimental',
  'RN OSS',
  'RN Next Major',
];

// Build the table with the value for each flag.
function buildTable(filterFn) {
  const isDiff = argv.diff != null && argv.diff.length > 1;
  const table = {};
  const filteredFlags = filterFn
    ? allFlagsUniqueFlags.filter(filterFn)
    : allFlagsUniqueFlags;
  // eslint-disable-next-line no-for-of-loops/no-for-of-loops
  for (const flag of filteredFlags) {
    const values = FLAG_COLUMNS.reduce((acc, key) => {
      acc[key] = FLAG_CONFIG[key](flag);
      return acc;
    }, {});

    if (!isDiff) {
      table[flag] = values;
      continue;

Review comment (suggestion):
Should this be RN FB?
```suggestion
const INTERNAL_VARIANTS = ['WWW Classic', 'WWW Modern', 'RN FB'];
```

Code after:
      return 'RN Next Major';
    case 'canary':
      return 'OSS Canary';
    case 'next':
      return 'OSS Next Major';
    case 'experimental':
      return 'OSS Experimental';
    default:
      return arg;
  }
}

const FLAG_CONFIG = {
  'OSS Next Major': getNextMajorFlagValue,
  'OSS Canary': getOSSCanaryFlagValue,
  'OSS Experimental': getOSSExperimentalFlagValue,
  'WWW Classic': getWWWClassicFlagValue,
  'WWW Modern': getWWWModernFlagValue,
  'RN FB': getRNFBFlagValue,
  'RN OSS': getRNOSSFlagValue,
  'RN Next Major': getRNNextMajorFlagValue,
};

const FLAG_COLUMNS = Object.keys(FLAG_CONFIG);

const INTERNAL_VARIANTS = ['WWW Classic', 'WWW Modern', 'RN FB'];
const OSS_VARIANTS = [
  'OSS Next Major',
  'OSS Canary',
  'OSS Experimental',
  'RN OSS',
  'RN Next Major',
];

// Build the table with the value for each flag.
function buildTable(filterFn) {
  const isDiff = argv.diff != null && argv.diff.length > 1;
  const table = {};
  const filteredFlags = filterFn
    ? allFlagsUniqueFlags.filter(filterFn)
    : allFlagsUniqueFlags;
  // eslint-disable-next-line no-for-of-loops/no-for-of-loops
  for (const flag of filteredFlags) {
    const values = FLAG_COLUMNS.reduce((acc, key) => {
      acc[key] = FLAG_CONFIG[key](flag);
      return acc;
    }, {});

    if (!isDiff) {
      table[flag] = values;
      continue;
Repository: facebook/react
File: compiler/apps/playground/__tests__/e2e/page.spec.ts (TypeScript)

Code before:
  const store: Store = {
    source: `export default function TestComponent({ x }) {
      return <Button>{x}</Button>;
    }
    `,
  };
  const hash = encodeStore(store);
  await page.goto(`/#${hash}`, {waitUntil: 'networkidle'});

  // Reset button works
  page.on('dialog', dialog => dialog.accept());
  await page.getByRole('button', {name: 'Reset'}).click();
  await page.screenshot({
    fullPage: true,
    path: 'test-results/02-reset-button-works.png',
  });
  const text =
    (await page.locator('.monaco-editor').nth(1).allInnerTexts()) ?? [];
  const output = await formatPrint(text);

  expect(output).not.toEqual('');
  expect(output).toMatchSnapshot('02-default-output.txt');
});

TEST_CASE_INPUTS.forEach((t, idx) =>
  test.only(`playground compiles: ${t.name}`, async ({page}) => {
    const store: Store = {
      source: t.input,
    };
    const hash = encodeStore(store);
    await page.goto(`/#${hash}`, {waitUntil: 'networkidle'});
    await page.screenshot({
      fullPage: true,
      path: `test-results/03-0${idx}-${t.name}.png`,
    });

    const text =
      (await page.locator('.monaco-editor').nth(1).allInnerTexts()) ?? [];
    let output: string;
    if (t.noFormat) {
      output = text.join('');
    } else {
      output = await formatPrint(text);
    }

    expect(output).not.toEqual('');
    expect(output).toMatchSnapshot(`${t.name}-output.txt`);
  }),
);


Review comment (suggestion):
```suggestion
  test(`playground compiles: ${t.name}`, async ({page}) => {
```

Code after:
  const store: Store = {
    source: `export default function TestComponent({ x }) {
      return <Button>{x}</Button>;
    }
    `,
  };
  const hash = encodeStore(store);
  await page.goto(`/#${hash}`, {waitUntil: 'networkidle'});

  // Reset button works
  page.on('dialog', dialog => dialog.accept());
  await page.getByRole('button', {name: 'Reset'}).click();
  await page.screenshot({
    fullPage: true,
    path: 'test-results/02-reset-button-works.png',
  });
  const text =
    (await page.locator('.monaco-editor').nth(1).allInnerTexts()) ?? [];
  const output = await formatPrint(text);

  expect(output).not.toEqual('');
  expect(output).toMatchSnapshot('02-default-output.txt');
});

TEST_CASE_INPUTS.forEach((t, idx) =>
  test(`playground compiles: ${t.name}`, async ({page}) => {
    const store: Store = {
      source: t.input,
    };
    const hash = encodeStore(store);
    await page.goto(`/#${hash}`, {waitUntil: 'networkidle'});
    await page.screenshot({
      fullPage: true,
      path: `test-results/03-0${idx}-${t.name}.png`,
    });

    const text =
      (await page.locator('.monaco-editor').nth(1).allInnerTexts()) ?? [];
    let output: string;
    if (t.noFormat) {
      output = text.join('');
    } else {
      output = await formatPrint(text);
    }

    expect(output).not.toEqual('');
    expect(output).toMatchSnapshot(`${t.name}-output.txt`);
  }),
);
Repository: facebook/react
File: compiler/packages/eslint-plugin-react-compiler/src/rules/ReactCompilerRule.ts (TypeScript)

Code before:
              ? event.fnLoc
              : detail.loc;
          if (loc != null) {
            context.report({
              message: detail.reason,
              loc,
              suggest,
            });
          }
        }
      },
    };

    try {
      options.environment = validateEnvironmentConfig(
        options.environment ?? {},
      );
    } catch (err) {
      options.logger?.logEvent('', err);
    }

    function hasFlowSuppression(
      nodeLoc: BabelSourceLocation,
      suppression: string,
    ): boolean {
      const sourceCode = context.sourceCode ?? context.getSourceCode();
      const comments = sourceCode.getAllComments();
      const flowSuppressionRegex = new RegExp(
        '\\$FlowFixMe\\[' + suppression + '\\]',
      );
      for (const commentNode of comments) {
        if (
          flowSuppressionRegex.test(commentNode.value) &&
          commentNode.loc!.end.line === nodeLoc.start.line - 1
        ) {
          return true;
        }
      }
      return false;
    }

    let babelAST;
    if (filename.endsWith('.tsx') || filename.endsWith('.ts')) {
      try {
        const {parse: babelParse} = require('@babel/parser');
        babelAST = babelParse(sourceCode, {
          filename,
          sourceType: 'unambiguous',
          plugins: ['typescript', 'jsx'],
        });
      } catch {

Review comment (refactor):
I think you can remove this declaration altogether since this function already closes over `sourceCode` from above.

```suggestion
      const comments = sourceCode.getAllComments();
```

Code after:
              ? event.fnLoc
              : detail.loc;
          if (loc != null) {
            context.report({
              message: detail.reason,
              loc,
              suggest,
            });
          }
        }
      },
    };

    try {
      options.environment = validateEnvironmentConfig(
        options.environment ?? {},
      );
    } catch (err) {
      options.logger?.logEvent('', err);
    }

    function hasFlowSuppression(
      nodeLoc: BabelSourceLocation,
      suppression: string,
    ): boolean {
      const comments = sourceCode.getAllComments();
      const flowSuppressionRegex = new RegExp(
        '\\$FlowFixMe\\[' + suppression + '\\]',
      );
      for (const commentNode of comments) {
        if (
          flowSuppressionRegex.test(commentNode.value) &&
          commentNode.loc!.end.line === nodeLoc.start.line - 1
        ) {
          return true;
        }
      }
      return false;
    }

    let babelAST;
    if (filename.endsWith('.tsx') || filename.endsWith('.ts')) {
      try {
        const {parse: babelParse} = require('@babel/parser');
        babelAST = babelParse(sourceCode.text, {
          filename,
          sourceType: 'unambiguous',
          plugins: ['typescript', 'jsx'],
        });
      } catch {
        /* empty */
Repository: facebook/react
File: packages/react/src/__tests__/ReactChildren-test.js (JavaScript)

Code before:
    }

    const container = document.createElement('div');
    const root = ReactDOMClient.createRoot(container);
    await act(() => {
      root.render(
        <ComponentRenderingFlattenedChildren>
          <div />
          <div />
        </ComponentRenderingFlattenedChildren>,
      );
    });
  });

  // @gate __DEV__
  it('does not throw on children without `_store`', async () => {
    function ComponentRenderingFlattenedChildren({children}) {
      return <div>{React.Children.toArray(children)}</div>;
    }

    const source = <div />;
    const productionElement = {};
    Object.entries(source).forEach(([key, value]) => {
      if (key !== '_owner' && key !== '_store') {
        productionElement[key] = value;
      }
    });
    Object.freeze(productionElement);

    const container = document.createElement('div');
    const root = ReactDOMClient.createRoot(container);
    await act(() => {
      root.render(
        <ComponentRenderingFlattenedChildren>
          {productionElement}
        </ComponentRenderingFlattenedChildren>,
      );
    });
  });

  it('should escape keys', () => {
    const zero = <div key="1" />;
    const one = <div key="1=::=2" />;
    const instance = (
      <div>
        {zero}
        {one}
      </div>
    );
    const mappedChildren = React.Children.map(
      instance.props.children,

Review comment (suggestion):
The way this test case needs to be contrived signals that this mix of environments should really not be supported. We could consider adding build types to the version checks to prevent apps from relying on these setups in the future.

Code after:
    }

    const container = document.createElement('div');
    const root = ReactDOMClient.createRoot(container);
    await act(() => {
      root.render(
        <ComponentRenderingFlattenedChildren>
          <div />
          <div />
        </ComponentRenderingFlattenedChildren>,
      );
    });
  });

  it('does not throw on children without `_store`', async () => {
    function ComponentRenderingFlattenedChildren({children}) {
      return <div>{React.Children.toArray(children)}</div>;
    }

    const source = <div />;
    const productionElement = {};
    Object.entries(source).forEach(([key, value]) => {
      if (key !== '_owner' && key !== '_store') {
        productionElement[key] = value;
      }
    });
    Object.freeze(productionElement);

    const container = document.createElement('div');
    const root = ReactDOMClient.createRoot(container);
    await act(() => {
      root.render(
        <ComponentRenderingFlattenedChildren>
          {productionElement}
        </ComponentRenderingFlattenedChildren>,
      );
    });
  });

  it('should escape keys', () => {
    const zero = <div key="1" />;
    const one = <div key="1=::=2" />;
    const instance = (
      <div>
        {zero}
        {one}
      </div>
    );
    const mappedChildren = React.Children.map(
      instance.props.children,
      kid => kid,
Repository: facebook/react
File: packages/react-native-renderer/src/ReactNativeAttributePayloadFabric.js (JavaScript)

Code before:
    ]: any): AttributeConfiguration);

    if (attributeConfig == null) {
      continue;
    }

    let newValue;

    if (prop === undefined) {
      // Discard the prop if it was previously defined.
      if (payload && payload[propKey] !== undefined) {
        newValue = null;
      } else {
        continue;
      }
    } else if (typeof attributeConfig.process === 'function') {
      // An atomic prop with custom processing.
      newValue = attributeConfig.process(prop);
    } else if (typeof prop === 'function') {
      // A function prop. It represents an event handler. Pass it to native as 'true'.
      newValue = true;
    } else if (typeof attributeConfig !== 'object') {
      // An atomic prop. Doesn't need to be flattened.
      newValue = prop;
    } else if (typeof attributeConfig.diff === 'function') {
      // An atomic prop with custom diffing. We don't need to do diffing when adding props.
      newValue = prop;
    }

    if (newValue !== undefined) {
      if (!payload) {
        payload = ({}: {[string]: $FlowFixMe});
      }
      payload[propKey] = newValue;
      continue;
    }

    payload = fastAddProperties(payload, prop, attributeConfig);
  }

  return payload;
}

/**
 * addProperties adds all the valid props to the payload after being processed.
 */
function addProperties(
  updatePayload: null | Object,
  props: Object,
  validAttributes: AttributeConfiguration,
): null | Object {

Review comment (suggestion):
I'm not sure this is correct. You're assuming now that `attributeConfig` is always going to be an object which you can read `process` function of.

Code after:
      payload = fastAddProperties(payload, props[i], validAttributes);
    }
    return payload;
  }

  for (const propKey in props) {
    const prop = props[propKey];

    const attributeConfig = ((validAttributes[
      propKey
    ]: any): AttributeConfiguration);

    if (attributeConfig == null) {
      continue;
    }

    let newValue;

    if (prop === undefined) {
      // Discard the prop if it was previously defined.
      if (payload && payload[propKey] !== undefined) {
        newValue = null;
      } else {
        continue;
      }
    } else if (typeof attributeConfig === 'object') {
      if (typeof attributeConfig.process === 'function') {
        // An atomic prop with custom processing.
        newValue = attributeConfig.process(prop);
      } else if (typeof attributeConfig.diff === 'function') {
        // An atomic prop with custom diffing. We don't need to do diffing when adding props.
        newValue = prop;
      }
    } else {
      if (typeof prop === 'function') {
        // A function prop. It represents an event handler. Pass it to native as 'true'.
        newValue = true;
      } else {
        // An atomic prop. Doesn't need to be flattened.
        newValue = prop;
      }
    }

    if (newValue !== undefined) {
      if (!payload) {
        payload = ({}: {[string]: $FlowFixMe});
      }
      payload[propKey] = newValue;
      continue;
    }
Repository: facebook/react
File: packages/react-dom-bindings/src/client/validateDOMNesting.js (JavaScript)

Code before:
        parentTag !== 'h6'
      );

    case 'rp':
    case 'rt':
      return impliedEndTags.indexOf(parentTag) === -1;

    case 'caption':
    case 'col':
    case 'colgroup':
    case 'frameset':
    case 'frame':
    case 'tbody':
    case 'td':
    case 'tfoot':
    case 'th':
    case 'thead':
    case 'tr':
      // These tags are only valid with a few parents that have special child
      // parsing rules -- if we're down here, then none of those matched and
      // so we allow it only if we don't know what the parent is, as all other
      // cases are invalid.
      return parentTag == null;
    case 'head':
      // We support rendering <head> in the root when the container is
      // #document, <htm>, or <body>.
      return implicitRootScope || parentTag === null;
    case 'html':
      // We support rendering <html> in the root when the container is
      // #document
      return (
        (implicitRootScope && parentTag === '#document') || parentTag === null
      );
    case 'body':
      // We support rendering <body> in the root when the container is
      // #document or 'html'
      return (
        (implicitRootScope &&
          (parentTag === '#document' || parentTag === 'html')) ||
        parentTag === null
      );
  }

  return true;
}

/**
 * Returns whether
 */
function findInvalidAncestorForTag(
  tag: string,

Review comment (suggestion):
```suggestion
      // #document, <html>, or <body>.
```

Code after:
        parentTag !== 'h6'
      );

    case 'rp':
    case 'rt':
      return impliedEndTags.indexOf(parentTag) === -1;

    case 'caption':
    case 'col':
    case 'colgroup':
    case 'frameset':
    case 'frame':
    case 'tbody':
    case 'td':
    case 'tfoot':
    case 'th':
    case 'thead':
    case 'tr':
      // These tags are only valid with a few parents that have special child
      // parsing rules -- if we're down here, then none of those matched and
      // so we allow it only if we don't know what the parent is, as all other
      // cases are invalid.
      return parentTag == null;
    case 'head':
      // We support rendering <head> in the root when the container is
      // #document, <html>, or <body>.
      return implicitRootScope || parentTag === null;
    case 'html':
      // We support rendering <html> in the root when the container is
      // #document
      return (
        (implicitRootScope && parentTag === '#document') || parentTag === null
      );
    case 'body':
      // We support rendering <body> in the root when the container is
      // #document or <html>
      return (
        (implicitRootScope &&
          (parentTag === '#document' || parentTag === 'html')) ||
        parentTag === null
      );
  }

  return true;
}

/**
 * Returns whether
 */
function findInvalidAncestorForTag(
  tag: string,
Repository: facebook/react
File: packages/react-dom-bindings/src/client/validateDOMNesting.js (JavaScript)

Code before:
    case 'frameset':
    case 'frame':
    case 'tbody':
    case 'td':
    case 'tfoot':
    case 'th':
    case 'thead':
    case 'tr':
      // These tags are only valid with a few parents that have special child
      // parsing rules -- if we're down here, then none of those matched and
      // so we allow it only if we don't know what the parent is, as all other
      // cases are invalid.
      return parentTag == null;
    case 'head':
      // We support rendering <head> in the root when the container is
      // #document, <htm>, or <body>.
      return implicitRootScope || parentTag === null;
    case 'html':
      // We support rendering <html> in the root when the container is
      // #document
      return (
        (implicitRootScope && parentTag === '#document') || parentTag === null
      );
    case 'body':
      // We support rendering <body> in the root when the container is
      // #document or 'html'
      return (
        (implicitRootScope &&
          (parentTag === '#document' || parentTag === 'html')) ||
        parentTag === null
      );
  }

  return true;
}

/**
 * Returns whether
 */
function findInvalidAncestorForTag(
  tag: string,
  ancestorInfo: AncestorInfoDev,
): ?Info {
  switch (tag) {
    case 'address':
    case 'article':
    case 'aside':
    case 'blockquote':
    case 'center':
    case 'details':
    case 'dialog':

Review comment (suggestion):
To make grepping easier.
```suggestion
      // #document or '<html>'
```

Code after:
    case 'frameset':
    case 'frame':
    case 'tbody':
    case 'td':
    case 'tfoot':
    case 'th':
    case 'thead':
    case 'tr':
      // These tags are only valid with a few parents that have special child
      // parsing rules -- if we're down here, then none of those matched and
      // so we allow it only if we don't know what the parent is, as all other
      // cases are invalid.
      return parentTag == null;
    case 'head':
      // We support rendering <head> in the root when the container is
      // #document, <html>, or <body>.
      return implicitRootScope || parentTag === null;
    case 'html':
      // We support rendering <html> in the root when the container is
      // #document
      return (
        (implicitRootScope && parentTag === '#document') || parentTag === null
      );
    case 'body':
      // We support rendering <body> in the root when the container is
      // #document or <html>
      return (
        (implicitRootScope &&
          (parentTag === '#document' || parentTag === 'html')) ||
        parentTag === null
      );
  }

  return true;
}

/**
 * Returns whether
 */
function findInvalidAncestorForTag(
  tag: string,
  ancestorInfo: AncestorInfoDev,
): ?Info {
  switch (tag) {
    case 'address':
    case 'article':
    case 'aside':
    case 'blockquote':
    case 'center':
    case 'details':
    case 'dialog':