ronantakizawa/codereview-bench huggingface dataset:
Repository: freeCodeCamp/freeCodeCamp
File: shared/utils/polyvinyl.ts (TypeScript)

Code before:
    typeof poly.head === 'string' &&
    typeof poly.tail === 'string' &&
    Array.isArray(poly.history);

  return hasProperties(poly) && hasCorrectTypes(poly);
}

function checkPoly(poly: ChallengeFile) {
  invariant(
    isPoly(poly),
    'function should receive a PolyVinyl, but got %s',
    JSON.stringify(poly)
  );
}

// setContent will lose source if not supplied
export function setContent(
  contents: string,
  poly: ChallengeFile,
  source?: string | null
): ChallengeFile {
  checkPoly(poly);
  return {
    ...poly,
    contents,
    source: source ?? null
  };
}

// This is currently only used to add back properties that are not stored in the
// database.
export function regeneratePathAndHistory(file: IncompleteChallengeFile) {
  const newPath = file.name + '.' + file.ext;
  const newFile = {
    ...file,
    path: newPath,
    history: [newPath]
  };
  return newFile;
}

async function clearHeadTail(polyP: Promise<ChallengeFile>) {
  const poly = await polyP;
  checkPoly(poly);
  return {
    ...poly,
    head: '',
    tail: ''
  };
}


Review comment (suggestion):
```suggestion
    source
```

We don't need nullish coalescing for a `string | null` value if the fallback is also `null`.

Code after:
    typeof poly.head === 'string' &&
    typeof poly.tail === 'string' &&
    Array.isArray(poly.history);

  return hasProperties(poly) && hasCorrectTypes(poly);
}

function checkPoly(poly: ChallengeFile) {
  invariant(
    isPoly(poly),
    'function should receive a PolyVinyl, but got %s',
    JSON.stringify(poly)
  );
}

// setContent will lose source if not supplied
export function setContent(
  contents: string,
  poly: ChallengeFile,
  source?: string
): ChallengeFile {
  checkPoly(poly);
  return {
    ...poly,
    contents,
    source
  };
}

// This is currently only used to add back properties that are not stored in the
// database.
export function regeneratePathAndHistory(file: IncompleteChallengeFile) {
  const newPath = file.name + '.' + file.ext;
  const newFile = {
    ...file,
    path: newPath,
    history: [newPath]
  };
  return newFile;
}

async function clearHeadTail(polyP: Promise<ChallengeFile>) {
  const poly = await polyP;
  checkPoly(poly);
  return {
    ...poly,
    head: '',
    tail: ''
  };
}
Repository: freeCodeCamp/freeCodeCamp
File: api/src/plugins/not-found.ts (TypeScript)

Code before:
import type { FastifyPluginCallback } from 'fastify';

import fp from 'fastify-plugin';

import { getRedirectParams } from '../utils/redirection';

/**
 * Plugin for handling missing endpoints.
 *
 * @param fastify The Fastify instance.
 * @param _options Options passed to the plugin via `fastify.register(plugin, options)`.
 * @param done Callback to signal that the logic has completed.
 */
const fourOhFour: FastifyPluginCallback = (fastify, _options, done) => {
  // If the request accepts JSON and does not specifically prefer text/html,
  // this will return a 404 JSON response. Everything else will be redirected.
  fastify.setNotFoundHandler((request, reply) => {
    const logger = fastify.log.child({ request });
    const accepted = request.accepts().type(['json', 'html']);

    logger.debug('User requested path that does not exist: ' + request.url);

    if (accepted == 'json') {
      void reply.code(404).send({ error: 'path not found' });
    } else {
      const { origin } = getRedirectParams(request);
      void reply.status(302);
      void reply.redirectWithMessage(`${origin}/404`, {
        type: 'danger',
        content: `We couldn't find path ${request.url}`
      });
    }
  });
  done();
};

export default fp(fourOhFour, {
  dependencies: ['redirect-with-message', '@fastify/accepts']
});


Review comment (bug):
```suggestion
    logger.debug('User requested path that does not exist');
```

Code after:
import type { FastifyPluginCallback } from 'fastify';

import fp from 'fastify-plugin';

import { getRedirectParams } from '../utils/redirection';

/**
 * Plugin for handling missing endpoints.
 *
 * @param fastify The Fastify instance.
 * @param _options Options passed to the plugin via `fastify.register(plugin, options)`.
 * @param done Callback to signal that the logic has completed.
 */
const fourOhFour: FastifyPluginCallback = (fastify, _options, done) => {
  // If the request accepts JSON and does not specifically prefer text/html,
  // this will return a 404 JSON response. Everything else will be redirected.
  fastify.setNotFoundHandler((req, reply) => {
    const logger = fastify.log.child({ req });
    const accepted = req.accepts().type(['json', 'html']);

    logger.debug('User requested path that does not exist');

    if (accepted == 'json') {
      void reply.code(404).send({ error: 'path not found' });
    } else {
      const { origin } = getRedirectParams(req);
      void reply.status(302);
      void reply.redirectWithMessage(`${origin}/404`, {
        type: 'danger',
        content: `We couldn't find path ${req.url}`
      });
    }
  });
  done();
};

export default fp(fourOhFour, {
  dependencies: ['redirect-with-message', '@fastify/accepts']
});
Repository: freeCodeCamp/freeCodeCamp
File: api/src/plugins/not-found.ts (TypeScript)

Code before:
import type { FastifyPluginCallback } from 'fastify';

import fp from 'fastify-plugin';

import { getRedirectParams } from '../utils/redirection';

/**
 * Plugin for handling missing endpoints.
 *
 * @param fastify The Fastify instance.
 * @param _options Options passed to the plugin via `fastify.register(plugin, options)`.
 * @param done Callback to signal that the logic has completed.
 */
const fourOhFour: FastifyPluginCallback = (fastify, _options, done) => {
  // If the request accepts JSON and does not specifically prefer text/html,
  // this will return a 404 JSON response. Everything else will be redirected.
  fastify.setNotFoundHandler((request, reply) => {
    const logger = fastify.log.child({ request });
    const accepted = request.accepts().type(['json', 'html']);

    logger.debug('User requested path that does not exist: ' + request.url);

    if (accepted == 'json') {
      void reply.code(404).send({ error: 'path not found' });
    } else {
      const { origin } = getRedirectParams(request);
      void reply.status(302);
      void reply.redirectWithMessage(`${origin}/404`, {
        type: 'danger',
        content: `We couldn't find path ${request.url}`
      });
    }
  });
  done();
};

export default fp(fourOhFour, {
  dependencies: ['redirect-with-message', '@fastify/accepts']
});


Review comment (suggestion):
I forgot: The serializer only handles it if the object contains exactly `{ req }`

```suggestion
  fastify.setNotFoundHandler((req, reply) => {
    const logger = fastify.log.child({ req });
    const accepted = req.accepts().type(['json', 'html']);
```

Code after:
import type { FastifyPluginCallback } from 'fastify';

import fp from 'fastify-plugin';

import { getRedirectParams } from '../utils/redirection';

/**
 * Plugin for handling missing endpoints.
 *
 * @param fastify The Fastify instance.
 * @param _options Options passed to the plugin via `fastify.register(plugin, options)`.
 * @param done Callback to signal that the logic has completed.
 */
const fourOhFour: FastifyPluginCallback = (fastify, _options, done) => {
  // If the request accepts JSON and does not specifically prefer text/html,
  // this will return a 404 JSON response. Everything else will be redirected.
  fastify.setNotFoundHandler((req, reply) => {
    const logger = fastify.log.child({ req });
    const accepted = req.accepts().type(['json', 'html']);

    logger.debug('User requested path that does not exist');

    if (accepted == 'json') {
      void reply.code(404).send({ error: 'path not found' });
    } else {
      const { origin } = getRedirectParams(req);
      void reply.status(302);
      void reply.redirectWithMessage(`${origin}/404`, {
        type: 'danger',
        content: `We couldn't find path ${req.url}`
      });
    }
  });
  done();
};

export default fp(fourOhFour, {
  dependencies: ['redirect-with-message', '@fastify/accepts']
});
Repository: freeCodeCamp/freeCodeCamp
File: shared/config/curriculum.ts (TypeScript)

Code before:
}

/*
 * SuperBlockStages.Upcoming = SHOW_UPCOMING_CHANGES === 'true'
 * 'Upcoming' is for development -> not shown on stag or prod anywhere
 *
 * SuperBlockStages.Next = deployed, but only shown if the Growthbook feature
 * is enabled.
 *
 */
export enum SuperBlockStage {
  Core,
  English,
  Professional,
  Extra,
  Legacy,
  Upcoming,
  Next,
  NextEnglish
}

const defaultStageOrder = [
  SuperBlockStage.Next,
  SuperBlockStage.Core,
  SuperBlockStage.English,
  SuperBlockStage.NextEnglish,
  SuperBlockStage.Professional,
  SuperBlockStage.Extra,
  SuperBlockStage.Legacy
];

export function getStageOrder({
  showUpcomingChanges
}: Config): SuperBlockStage[] {
  const stageOrder = [...defaultStageOrder];

  if (showUpcomingChanges) stageOrder.push(SuperBlockStage.Upcoming);
  return stageOrder;
}

export type StageMap = {
  [key in SuperBlockStage]: SuperBlocks[];
};

// Groups of superblocks in learn map. This should include all superblocks.
export const superBlockStages: StageMap = {
  [SuperBlockStage.Next]: [SuperBlocks.FullStackDeveloper],
  [SuperBlockStage.Core]: [
    SuperBlocks.RespWebDesignNew,
    SuperBlocks.JsAlgoDataStructNew,
    SuperBlocks.FrontEndDevLibs,

Review comment (suggestion):
I think `Professional` is supposed to be above `Legacy`?

```suggestion
  SuperBlockStage.Professional,
  SuperBlockStage.Legacy,
```

Code after:
}

/*
 * SuperBlockStages.Upcoming = SHOW_UPCOMING_CHANGES === 'true'
 * 'Upcoming' is for development -> not shown on stag or prod anywhere
 *
 * SuperBlockStages.Next = deployed, but only shown if the Growthbook feature
 * is enabled.
 *
 */
export enum SuperBlockStage {
  Core,
  English,
  Professional,
  Extra,
  Legacy,
  Upcoming,
  Next
}

const defaultStageOrder = [
  SuperBlockStage.Core,
  SuperBlockStage.English,
  SuperBlockStage.Extra,
  SuperBlockStage.Legacy,
  SuperBlockStage.Professional,
  SuperBlockStage.Next
];

export function getStageOrder({
  showUpcomingChanges
}: Config): SuperBlockStage[] {
  const stageOrder = [...defaultStageOrder];

  if (showUpcomingChanges) stageOrder.push(SuperBlockStage.Upcoming);
  return stageOrder;
}

export type StageMap = {
  [key in SuperBlockStage]: SuperBlocks[];
};

// Groups of superblocks in learn map. This should include all superblocks.
export const superBlockStages: StageMap = {
  [SuperBlockStage.Core]: [SuperBlocks.FullStackDeveloper],

  [SuperBlockStage.English]: [SuperBlocks.A2English, SuperBlocks.B1English],
  [SuperBlockStage.Professional]: [SuperBlocks.FoundationalCSharp],
  [SuperBlockStage.Extra]: [
    SuperBlocks.TheOdinProject,
    SuperBlocks.CodingInterviewPrep,
Repository: freeCodeCamp/freeCodeCamp
File: tools/challenge-parser/parser/plugins/add-video-question.js (JavaScript)

Code before:
const { root } = require('mdast-builder');
const find = require('unist-util-find');
const { getSection } = require('./utils/get-section');
const getAllBefore = require('./utils/before-heading');
const mdastToHtml = require('./utils/mdast-to-html');
const { getParagraphContent } = require('./utils/get-paragraph-content');

const { splitOnThematicBreak } = require('./utils/split-on-thematic-break');

function plugin() {
  return transformer;
  function transformer(tree, file) {
    const description = getSection(tree, '--desc--');

    if (getSection(description, '--transcript--').length > 0) {
      throw Error('transcript should not be nested inside of the description');
    }

    const allQuestionNodes = getSection(tree, '--questions--');

    if (allQuestionNodes.length > 0) {
      const questions = [];
      const questionTrees = [];

      allQuestionNodes.forEach(questionNode => {
        const isStartOfQuestion =
          questionNode.children?.[0]?.value === '--text--';
        if (isStartOfQuestion) {
          questionTrees.push([questionNode]);
        } else {
          questionTrees[questionTrees.length - 1].push(questionNode);
        }
      });

      questionTrees.forEach(questionNodes => {
        const questionTree = root(questionNodes);

        const textNodes = getSection(questionTree, '--text--');

Review comment (suggestion):
```suggestion
    const description = getSection(tree, '--description--');
```

Code after:
const { root } = require('mdast-builder');
const find = require('unist-util-find');
const { getSection } = require('./utils/get-section');
const getAllBefore = require('./utils/before-heading');
const mdastToHtml = require('./utils/mdast-to-html');
const { getParagraphContent } = require('./utils/get-paragraph-content');

const { splitOnThematicBreak } = require('./utils/split-on-thematic-break');

function plugin() {
  return transformer;
  function transformer(tree, file) {
    const allQuestionNodes = getSection(tree, '--questions--');

    if (allQuestionNodes.length > 0) {
      const questions = [];
      const questionTrees = [];

      allQuestionNodes.forEach(questionNode => {
        const isStartOfQuestion =
          questionNode.children?.[0]?.value === '--text--';
        if (isStartOfQuestion) {
          questionTrees.push([questionNode]);
        } else {
          questionTrees[questionTrees.length - 1].push(questionNode);
        }
      });

      questionTrees.forEach(questionNodes => {
        const questionTree = root(questionNodes);

        const textNodes = getSection(questionTree, '--text--');
        const answersNodes = getSection(questionTree, '--answers--');
        const solutionNodes = getSection(questionTree, '--video-solution--');

        questions.push(getQuestion(textNodes, answersNodes, solutionNodes));
      });
Repository: freeCodeCamp/freeCodeCamp
File: tools/challenge-parser/parser/plugins/utils/get-section.js (JavaScript)

Code before:
const find = require('unist-util-find');
const findAfter = require('unist-util-find-after');
const findAllAfter = require('unist-util-find-all-after');
const between = require('unist-util-find-all-between');
const { findAll } = require('./find-all');

const isMarker = node => {
  if (node.children && node.children[0]) {
    const child = node.children[0];
    return (
      child.type === 'text' &&
      child.value.startsWith('--') &&
      child.value.endsWith('--') &&
      node.type === 'heading'
    );
  } else {
    return false;
  }
};

function _getSection(tree) {
  return start => {
    if (!start) return [];

    const isEnd = node => {
      return node.depth <= start.depth && isMarker(node);
    };

    const end = findAfter(tree, start, isEnd);

    const targetNodes = end
      ? between(tree, start, end)
      : findAllAfter(tree, start);
    return targetNodes;
  };
}

const startNode = marker => ({
  type: 'heading',
  children: [
    {
      type: 'text',
      value: marker

Review comment (suggestion):
```suggestion
  if (node.children && node.children[0]) {
    const child = node.children[0];
    return (
      child.type === 'text' &&
      child.value.startsWith('--') &&
      child.value.endsWith('--') &&
      node.type === 'heading'
    );
  };
  return false;
```

`if` statements which end in a return do not need `else`.

Code after:
const find = require('unist-util-find');
const findAfter = require('unist-util-find-after');
const findAllAfter = require('unist-util-find-all-after');
const between = require('unist-util-find-all-between');
const { findAll } = require('./find-all');

const isMarker = node => {
  if (node.children && node.children[0]) {
    const child = node.children[0];
    return (
      child.type === 'text' &&
      child.value.startsWith('--') &&
      child.value.endsWith('--') &&
      node.type === 'heading'
    );
  }
  return false;
};

function _getSection(tree) {
  return start => {
    if (!start) return [];

    const isEnd = node => {
      return node.depth <= start.depth && isMarker(node);
    };

    const end = findAfter(tree, start, isEnd);

    const targetNodes = end
      ? between(tree, start, end)
      : findAllAfter(tree, start);
    return targetNodes;
  };
}

const startNode = marker => ({
  type: 'heading',
  children: [
    {
      type: 'text',
      value: marker
    }
Repository: freeCodeCamp/freeCodeCamp
File: tools/client-plugins/browser-scripts/typescript-worker.ts (TypeScript)

Code before:
  data: {
    type: 'compile';
    code: string;
  };
}

interface TSCompiledMessage {
  type: 'compiled';
  value: string;
  error: string;
}

interface CheckIsReadyRequestEvent extends MessageEvent {
  data: {
    type: 'check-is-ready';
  };
}

interface CancelEvent extends MessageEvent {
  data: {
    type: 'cancel';
    value: number;
  };
}

const TS_VERSION = '5.7.3'; // hardcoding for now, in the future this may be dynamic. Must be fully specified version

let tsEnv: VirtualTypeScriptEnvironment | null = null;
let compilerHost: CompilerHost | null = null;
let cachedVersion: string | null = null;

// NOTE: vfs.globals must only be imported once, otherwise it will throw.
importScripts(
  'https://cdn.jsdelivr.net/npm/@typescript/vfs@1.6.0/dist/vfs.globals.js'
);

function importTS(version: string) {
  if (cachedVersion == version) return;
  importScripts(
    `https://cdnjs.cloudflare.com/ajax/libs/typescript/${version}/typescript.min.js`
  );
  cachedVersion = version;
}

async function setupTypeScript() {
  importTS(TS_VERSION);
  const compilerOptions: CompilerOptions = {
    target: ts.ScriptTarget.ES2015,
    skipLibCheck: true // TODO: look into why this is needed. Are we doing something wrong? Could it be that it's not "synced"  with this TS version?
    // from the docs: "Note: it's possible for this list to get out of
    // sync with TypeScript over time. It was last synced with TypeScript

Review comment (suggestion):
```suggestion
// Pin at the latest TS version available as cdnjs doesn't support version range.
const TS_VERSION = '5.7.3';
```

Code after:
  data: {
    type: 'compile';
    code: string;
  };
}

interface TSCompiledMessage {
  type: 'compiled';
  value: string;
  error: string;
}

interface CheckIsReadyRequestEvent extends MessageEvent {
  data: {
    type: 'check-is-ready';
  };
}

interface CancelEvent extends MessageEvent {
  data: {
    type: 'cancel';
    value: number;
  };
}

// Pin at the latest TS version available as cdnjs doesn't support version range.
const TS_VERSION = '5.7.3';

let tsEnv: VirtualTypeScriptEnvironment | null = null;
let compilerHost: CompilerHost | null = null;
let cachedVersion: string | null = null;

// NOTE: vfs.globals must only be imported once, otherwise it will throw.
importScripts(
  'https://cdn.jsdelivr.net/npm/@typescript/vfs@1.6.0/dist/vfs.globals.js'
);

function importTS(version: string) {
  if (cachedVersion == version) return;
  importScripts(
    `https://cdnjs.cloudflare.com/ajax/libs/typescript/${version}/typescript.min.js`
  );
  cachedVersion = version;
}

async function setupTypeScript() {
  importTS(TS_VERSION);
  const compilerOptions: CompilerOptions = {
    target: ts.ScriptTarget.ES2015,
    skipLibCheck: true // TODO: look into why this is needed. Are we doing something wrong? Could it be that it's not "synced"  with this TS version?
    // from the docs: "Note: it's possible for this list to get out of
Repository: freeCodeCamp/freeCodeCamp
File: tools/challenge-helper-scripts/helpers/get-base-meta.ts (TypeScript)

Code before:
  superBlock: '',
  order: 42,
  helpCategory: '',
  challengeOrder: [
    {
      id: '',
      title: ''
    }
  ]
};

const stepMeta = {
  ...baseMeta,
  usesMultifileEditor: true,
  hasEditableBoundaries: true
};

// Manually extract order from the list of properties for full stack projects
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const { order, ...orderLessStep } = stepMeta;

const fullStackStepMeta = {
  ...orderLessStep,
  blockType: '',
  blockLayout: ''
};

const quizMeta = {
  ...orderLessStep,
  blockType: 'quiz',
  blockLayout: 'link'
};

const languageMeta = {
  ...baseMeta,
  blockLayout: 'dialogue-grid'
};

export const getBaseMeta = {
  Step: stepMeta,
  Quiz: quizMeta,
  Language: languageMeta,
  FullStack: fullStackStepMeta,
  Default: stepMeta
};


Review comment (suggestion):
Rather than starting with `order` in the base and then removing it, we can just start without it.

If you declare the type

```ts
interface Meta {
  name: string;
  isUpcomingChange: boolean;
  dashedName: string;
  superBlock: string;
  helpCategory: string;
  challengeOrder: Array<{
    id: string;
    title: string;
  }>;
  usesMultifileEditor?: boolean;
  hasEditableBoundaries?: boolean;
  blockType?: string;
  blockLayout?: string;
  order?: number;
}
```

and make the map from strings to metas a `Record<string, Meta>` then all the types work out.

The order should never have been 42, it was joke (I'm not looking at git blame in case I made it...) and makes it seem like the default value was 42 for some reason. As far as I can see it's never used and it just serves to set the type to `number`, but we can do that separately.

Code after:
  superBlock: string;
  helpCategory: string;
  challengeOrder: Array<{
    id: string;
    title: string;
  }>;
  usesMultifileEditor?: boolean;
  hasEditableBoundaries?: boolean;
  blockType?: string;
  blockLayout?: string;
  order?: number;
}

const baseMeta: Meta = {
  name: '',
  superBlock: '',
  isUpcomingChange: true,
  dashedName: '',
  helpCategory: '',
  blockLayout: 'legacy-challenge-list',
  challengeOrder: [
    {
      id: '',
      title: ''
    }
  ]
};

const stepMeta = {
  ...baseMeta,
  usesMultifileEditor: true,
  hasEditableBoundaries: true
};

const fullStackStepMeta = {
  ...baseMeta,
  blockType: '',
  blockLayout: '',
  usesMultifileEditor: true
};

const quizMeta = {
  ...baseMeta,
  blockType: 'quiz',
  blockLayout: 'link'
};

const languageMeta = {
  ...baseMeta,
  blockLayout: 'dialogue-grid'
};