ronantakizawa/codereview-bench huggingface dataset:
Repository: atom/atom
File: src/main-process/atom-window.js (JavaScript)

Code before:
  }

  reload() {
    this.loadedPromise = new Promise(resolve => {
      this.resolveLoadedPromise = resolve;
    });
    this.prepareToUnload().then(canUnload => {
      if (canUnload) this.browserWindow.reload();
    });
    return this.loadedPromise;
  }

  showSaveDialog(options, callback) {
    options = Object.assign(
      {
        title: 'Save File',
        defaultPath: this.projectRoots[0]
      },
      options
    );

    if (typeof callback === 'function') {
      // Async
      dialog.showSaveDialog(this.browserWindow, options, callback);
    } else {
      // Sync
      return dialog.showSaveDialog(this.browserWindow, options);
    }
  }

  toggleDevTools() {
    return this.browserWindow.toggleDevTools();
  }

  openDevTools() {
    return this.browserWindow.openDevTools();
  }

  closeDevTools() {
    return this.browserWindow.closeDevTools();
  }

  setDocumentEdited(documentEdited) {
    return this.browserWindow.setDocumentEdited(documentEdited);
  }

  setRepresentedFilename(representedFilename) {
    return this.browserWindow.setRepresentedFilename(representedFilename);
  }

  setProjectRoots(projectRootPaths) {

Review comment (bug):
For the case when there is a callback to run, maybe we should log a deprecation notice. (Although I would think it would be good to have feedback from Atom's official maintainers for that?)

Or we could support callbacks in Atom's code indefinitely if we want to. There's nothing strictly wrong with callbacks other than that they can be hard for humans to read, as far as I know.

Code after:
  }

  reload() {
    this.loadedPromise = new Promise(resolve => {
      this.resolveLoadedPromise = resolve;
    });
    this.prepareToUnload().then(canUnload => {
      if (canUnload) this.browserWindow.reload();
    });
    return this.loadedPromise;
  }

  showSaveDialog(options, callback) {
    options = Object.assign(
      {
        title: 'Save File',
        defaultPath: this.projectRoots[0]
      },
      options
    );

    let promise = dialog.showSaveDialog(this.browserWindow, options);
    if (typeof callback === 'function') {
      promise = promise.then(({ filePath, bookmark }) => {
        callback(filePath, bookmark);
      });
    }
    return promise;
  }

  toggleDevTools() {
    return this.browserWindow.toggleDevTools();
  }

  openDevTools() {
    return this.browserWindow.openDevTools();
  }

  closeDevTools() {
    return this.browserWindow.closeDevTools();
  }

  setDocumentEdited(documentEdited) {
    return this.browserWindow.setDocumentEdited(documentEdited);
  }

  setRepresentedFilename(representedFilename) {
    return this.browserWindow.setRepresentedFilename(representedFilename);
  }

  setProjectRoots(projectRootPaths) {
Repository: atom/atom
File: src/compile-cache.js (JavaScript)

Code before:
'use strict';

// For now, we're not using babel or ES6 features like `let` and `const` in
// this file, because `apm` requires this file directly in order to pre-warm
// Atom's compile-cache when installing or updating packages, using an older
// version of node.js

const path = require('path');
const fs = require('fs-plus');
const sourceMapSupport = require('@atom/source-map-support');

const PackageTranspilationRegistry = require('./package-transpilation-registry');
let CSON = null;

const packageTranspilationRegistry = new PackageTranspilationRegistry();

const COMPILERS = {
  '.js': packageTranspilationRegistry.wrapTranspiler(require('./babel')),
  '.ts': packageTranspilationRegistry.wrapTranspiler(require('./typescript')),
  '.tsx': packageTranspilationRegistry.wrapTranspiler(require('./typescript')),
  '.coffee': packageTranspilationRegistry.wrapTranspiler(
    require('./coffee-script')
  )
};

exports.addTranspilerConfigForPath = function(
  packagePath,
  packageName,
  packageMeta,
  config
) {

Review comment (performance):
```suggestion
// Atom's compile-cache when installing or updating packages, using apm's Node-js
```

Code after:
'use strict';

// Atom's compile-cache when installing or updating packages, called by apm's Node-js

const path = require('path');
const fs = require('fs-plus');
const sourceMapSupport = require('@atom/source-map-support');

const PackageTranspilationRegistry = require('./package-transpilation-registry');
let CSON = null;

const packageTranspilationRegistry = new PackageTranspilationRegistry();

const COMPILERS = {
  '.js': packageTranspilationRegistry.wrapTranspiler(require('./babel')),
  '.ts': packageTranspilationRegistry.wrapTranspiler(require('./typescript')),
  '.tsx': packageTranspilationRegistry.wrapTranspiler(require('./typescript')),
  '.coffee': packageTranspilationRegistry.wrapTranspiler(
    require('./coffee-script')
  )
};

exports.addTranspilerConfigForPath = function(
  packagePath,
  packageName,
  packageMeta,
  config
) {
  packagePath = fs.realpathSync(packagePath);
  packageTranspilationRegistry.addTranspilerConfigForPath(
    packagePath,
Repository: atom/atom
File: src/state-store.js (JavaScript)

Code before:
'use strict';

module.exports = class StateStore {
  constructor(databaseName, version) {
    this.connected = false;
    this.databaseName = databaseName;
    this.version = version;
  }

  get dbPromise() {
    if (!this._dbPromise) {
      this._dbPromise = new Promise(resolve => {
        const dbOpenRequest = indexedDB.open(this.databaseName, this.version);
        dbOpenRequest.onupgradeneeded = event => {
          let db = event.target.result;
          db.onerror = error => {
            atom.notifications.addError('Error loading database', {
              stack: new Error('Error loading database').stack,
              dismissable: true
            });
            console.error('Error loading database', error);
          };
          db.createObjectStore('states');
        };
        dbOpenRequest.onsuccess = () => {
          this.connected = true;
          resolve(dbOpenRequest.result);
        };
        dbOpenRequest.onerror = error => {
          atom.notifications.addError('Could not connect to indexedDB', {
            stack: new Error('Could not connect to indexedDB').stack,
            dismissable: true
          });
          console.error('Could not connect to indexedDB', error);
          this.connected = false;
          resolve(null);
        };
      });
    }

    return this._dbPromise;
  }

Review comment (suggestion):
```suggestion
            atom.notifications.addFatalError('Error loading database', {
```

Code after:
'use strict';

module.exports = class StateStore {
  constructor(databaseName, version) {
    this.connected = false;
    this.databaseName = databaseName;
    this.version = version;
  }

  get dbPromise() {
    if (!this._dbPromise) {
      this._dbPromise = new Promise(resolve => {
        const dbOpenRequest = indexedDB.open(this.databaseName, this.version);
        dbOpenRequest.onupgradeneeded = event => {
          let db = event.target.result;
          db.onerror = error => {
            atom.notifications.addFatalError('Error loading database', {
              stack: new Error('Error loading database').stack,
              dismissable: true
            });
            console.error('Error loading database', error);
          };
          db.createObjectStore('states');
        };
        dbOpenRequest.onsuccess = () => {
          this.connected = true;
          resolve(dbOpenRequest.result);
        };
        dbOpenRequest.onerror = error => {
          atom.notifications.addFatalError('Could not connect to indexedDB', {
            stack: new Error('Could not connect to indexedDB').stack,
            dismissable: true
          });
          console.error('Could not connect to indexedDB', error);
          this.connected = false;
          resolve(null);
        };
      });
    }

    return this._dbPromise;
  }
Repository: atom/atom
File: src/path-watcher.js (JavaScript)

Code before:
          (await isOpenInEditor(realOldPath))
        )
          return;

        this.onEvents([
          { action: 'renamed', path: realNewPath, oldPath: realOldPath }
        ]);
      })
    );
  }
}

// Private: Implement a native watcher by translating events from an NSFW watcher.
class NSFWNativeWatcher extends NativeWatcher {
  async doStart(rootPath, eventCallback, errorCallback) {
    const handler = events => {
      this.onEvents(
        events.map(event => {
          const action =
            ACTION_MAP.get(event.action) || `unexpected (${event.action})`;
          const payload = { action };

          if (event.file) {
            payload.path = path.join(event.directory, event.file);
          } else {
            payload.oldPath = path.join(event.directory, event.oldFile??'');
            payload.path = path.join(event.directory, event.newFile??'');
          }

          return payload;
        })
      );
    };

    this.watcher = await nsfw(this.normalizedPath, handler, {
      debounceMS: 100,
      errorCallback: this.onError
    });

    await this.watcher.start();
  }

  doStop() {
    return this.watcher.stop();
  }
}

// Extended: Manage a subscription to filesystem events that occur beneath a root directory. Construct these by
// calling `watchPath`. To watch for events within active project directories, use {Project::onDidChangeFiles}
// instead.
//

Review comment (suggestion):
See comment about nullish coalescing not being supported
```suggestion
            payload.oldPath = path.join(event.directory, event.oldFile == undefined ? '' : event.oldFile);
```

Code after:
          (await isOpenInEditor(realOldPath))
        )
          return;

        this.onEvents([
          { action: 'renamed', path: realNewPath, oldPath: realOldPath }
        ]);
      })
    );
  }
}

// Private: Implement a native watcher by translating events from an NSFW watcher.
class NSFWNativeWatcher extends NativeWatcher {
  async doStart(rootPath, eventCallback, errorCallback) {
    const handler = events => {
      this.onEvents(
        events.map(event => {
          const action =
            ACTION_MAP.get(event.action) || `unexpected (${event.action})`;
          const payload = { action };

          if (event.file) {
            payload.path = path.join(event.directory, event.file);
          } else {
            payload.oldPath = path.join(
              event.directory,
              typeof event.oldFile === 'undefined' ? '' : event.oldFile
            );
            payload.path = path.join(
              event.directory,
              typeof event.newFile === 'undefined' ? '' : event.newFile
            );
          }

          return payload;
        })
      );
    };

    this.watcher = await nsfw(this.normalizedPath, handler, {
      debounceMS: 100,
      errorCallback: this.onError
    });

    await this.watcher.start();
  }

  doStop() {
    return this.watcher.stop();
  }
Repository: atom/atom
File: src/path-watcher.js (JavaScript)

Code before:
        )
          return;

        this.onEvents([
          { action: 'renamed', path: realNewPath, oldPath: realOldPath }
        ]);
      })
    );
  }
}

// Private: Implement a native watcher by translating events from an NSFW watcher.
class NSFWNativeWatcher extends NativeWatcher {
  async doStart(rootPath, eventCallback, errorCallback) {
    const handler = events => {
      this.onEvents(
        events.map(event => {
          const action =
            ACTION_MAP.get(event.action) || `unexpected (${event.action})`;
          const payload = { action };

          if (event.file) {
            payload.path = path.join(event.directory, event.file);
          } else {
            payload.oldPath = path.join(event.directory, event.oldFile??'');
            payload.path = path.join(event.directory, event.newFile??'');
          }

          return payload;
        })
      );
    };

    this.watcher = await nsfw(this.normalizedPath, handler, {
      debounceMS: 100,
      errorCallback: this.onError
    });

    await this.watcher.start();
  }

  doStop() {
    return this.watcher.stop();
  }
}

// Extended: Manage a subscription to filesystem events that occur beneath a root directory. Construct these by
// calling `watchPath`. To watch for events within active project directories, use {Project::onDidChangeFiles}
// instead.
//
// Multiple PathWatchers may be backed by a single native watcher to conserve operation system resources.

Review comment (suggestion):
```suggestion
            payload.path = path.join(event.directory, event.newFile == undefined ? '' : event.newFile);
```

Code after:
        )
          return;

        this.onEvents([
          { action: 'renamed', path: realNewPath, oldPath: realOldPath }
        ]);
      })
    );
  }
}

// Private: Implement a native watcher by translating events from an NSFW watcher.
class NSFWNativeWatcher extends NativeWatcher {
  async doStart(rootPath, eventCallback, errorCallback) {
    const handler = events => {
      this.onEvents(
        events.map(event => {
          const action =
            ACTION_MAP.get(event.action) || `unexpected (${event.action})`;
          const payload = { action };

          if (event.file) {
            payload.path = path.join(event.directory, event.file);
          } else {
            payload.oldPath = path.join(
              event.directory,
              typeof event.oldFile === 'undefined' ? '' : event.oldFile
            );
            payload.path = path.join(
              event.directory,
              typeof event.newFile === 'undefined' ? '' : event.newFile
            );
          }

          return payload;
        })
      );
    };

    this.watcher = await nsfw(this.normalizedPath, handler, {
      debounceMS: 100,
      errorCallback: this.onError
    });

    await this.watcher.start();
  }

  doStop() {
    return this.watcher.stop();
  }
}
Repository: atom/atom
File: packages/one-dark-ui/lib/main.js (JavaScript)

Code before:
const root = document.documentElement;
const themeName = 'one-dark-ui';

module.exports = {
  activate(state) {
    atom.config.observe(`${themeName}.fontSize`, setFontSize);
    atom.config.observe(`${themeName}.tabSizing`, setTabSizing);
    atom.config.observe(`${themeName}.tabCloseButton`, setTabCloseButton);
    atom.config.observe(`${themeName}.hideDockButtons`, setHideDockButtons);
    atom.config.observe(`${themeName}.stickyHeaders`, setStickyHeaders);

  deactivate() {
    unsetFontSize();
    unsetTabSizing();
    unsetTabCloseButton();
    unsetHideDockButtons();
    unsetStickyHeaders();
  }
};

// Font Size -----------------------

function setFontSize(currentFontSize) {
  root.style.fontSize = `${currentFontSize}px`;
}

function unsetFontSize() {
  root.style.fontSize = '';
}

// Tab Sizing -----------------------

function setTabSizing(tabSizing) {
  root.setAttribute(`theme-${themeName}-tabsizing`, tabSizing.toLowerCase());
}

function unsetTabSizing() {

Review comment (suggestion):
You accidentally deleted a brace
```suggestion
  },

```

Code after:
const root = document.documentElement;
const themeName = 'one-dark-ui';

module.exports = {
  activate(state) {
    atom.config.observe(`${themeName}.fontSize`, setFontSize);
    atom.config.observe(`${themeName}.tabSizing`, setTabSizing);
    atom.config.observe(`${themeName}.tabCloseButton`, setTabCloseButton);
    atom.config.observe(`${themeName}.hideDockButtons`, setHideDockButtons);
    atom.config.observe(`${themeName}.stickyHeaders`, setStickyHeaders);
  },

  deactivate() {
    unsetFontSize();
    unsetTabSizing();
    unsetTabCloseButton();
    unsetHideDockButtons();
    unsetStickyHeaders();
  }
};

// Font Size -----------------------

function setFontSize(currentFontSize) {
  root.style.fontSize = `${currentFontSize}px`;
}

function unsetFontSize() {
  root.style.fontSize = '';
}

// Tab Sizing -----------------------

function setTabSizing(tabSizing) {
  root.setAttribute(`theme-${themeName}-tabsizing`, tabSizing.toLowerCase());
}
Repository: atom/atom
File: script/lib/check-chromedriver-version.js (JavaScript)

Code before:
  // Mksnapshot should be at least v9.0.2
  const chromedriverVer = buildMetadata.dependencies['electron-chromedriver'];
  const mksnapshotVer = buildMetadata.dependencies['electron-mksnapshot'];
  const chromedriverActualVer = chromedriverMetadataPath.version;
  const mksnapshotActualVer = mksnapshotMetadataPath.version;

  // Always use caret on electron-chromedriver so that it can pick up the best minor/patch versions
  if (!chromedriverVer.startsWith('^')) {
    throw new Error(
      `electron-chromedriver version in script/package.json should start with a caret to match latest patch version.`
    );
  }

  if (!mksnapshotVer.startsWith('^')) {
    throw new Error(
      `electron-mksnapshot version in script/package.json should start with a caret to match latest patch version.`
    );
  }

  if (!semver.satisfies(chromedriverActualVer, '>=9.0.0')) {
    throw new Error(
      `electron-chromedriver should be at least v9.0.0 to support the ELECTRON_CUSTOM_VERSION environment variable.`
    );
  }

  if (!semver.satisfies(mksnapshotActualVer, '>=9.0.2')) {
    throw new Error(
      `electron-mksnapshot should be at least v9.0.2 to support the ELECTRON_CUSTOM_VERSION environment variable.`
    );
  }
};


Review comment (suggestion):
```suggestion
  if (semver.lt(chromedriverActualVer, majorElectronVersion)) {
```

Code after:
  process.env.ELECTRON_CUSTOM_VERSION ||
    require('../config').appMetadata.electronVersion
);

module.exports = function() {
  // Chromedriver should be at least v9.0.0
  // Mksnapshot should be at least v9.0.2
  const chromedriverVer = buildMetadata.dependencies['electron-chromedriver'];
  const mksnapshotVer = buildMetadata.dependencies['electron-mksnapshot'];
  const chromedriverActualVer = chromedriverMetadataPath.version;
  const mksnapshotActualVer = mksnapshotMetadataPath.version;

  // Always use caret on electron-chromedriver so that it can pick up the best minor/patch versions
  if (!chromedriverVer.startsWith('^')) {
    throw new Error(
      `electron-chromedriver version in script/package.json should start with a caret to match latest patch version.`
    );
  }

  if (!mksnapshotVer.startsWith('^')) {
    throw new Error(
      `electron-mksnapshot version in script/package.json should start with a caret to match latest patch version.`
    );
  }

  if (!semver.satisfies(chromedriverActualVer, `>=${majorElectronVersion}`)) {
    throw new Error(
      `electron-chromedriver should be at least v${majorElectronVersion} to support the ELECTRON_CUSTOM_VERSION environment variable.`
    );
  }

  if (!semver.satisfies(mksnapshotActualVer, `>=${majorElectronVersion}`)) {
    throw new Error(
      `electron-mksnapshot should be at least v${majorElectronVersion} to support the ELECTRON_CUSTOM_VERSION environment variable.`
    );
  }
};
Repository: atom/atom
File: script/lib/check-chromedriver-version.js (JavaScript)

Code before:
  // Always use caret on electron-chromedriver so that it can pick up the best minor/patch versions
  if (!chromedriverVer.startsWith('^')) {
    throw new Error(
      `electron-chromedriver version in script/package.json should start with a caret to match latest patch version.`
    );
  }

  if (!mksnapshotVer.startsWith('^')) {
    throw new Error(
      `electron-mksnapshot version in script/package.json should start with a caret to match latest patch version.`
    );
  }

  if (!semver.satisfies(chromedriverActualVer, '>=9.0.0')) {
    throw new Error(
      `electron-chromedriver should be at least v9.0.0 to support the ELECTRON_CUSTOM_VERSION environment variable.`
    );
  }

  if (!semver.satisfies(mksnapshotActualVer, '>=9.0.2')) {
    throw new Error(
      `electron-mksnapshot should be at least v9.0.2 to support the ELECTRON_CUSTOM_VERSION environment variable.`
    );
  }
};


Review comment (suggestion):
```suggestion
  if (semver.lt(mksnapshotActualVer, majorElectronVersion)) {
```

Code after:
  // Mksnapshot should be at least v9.0.2
  const chromedriverVer = buildMetadata.dependencies['electron-chromedriver'];
  const mksnapshotVer = buildMetadata.dependencies['electron-mksnapshot'];
  const chromedriverActualVer = chromedriverMetadataPath.version;
  const mksnapshotActualVer = mksnapshotMetadataPath.version;

  // Always use caret on electron-chromedriver so that it can pick up the best minor/patch versions
  if (!chromedriverVer.startsWith('^')) {
    throw new Error(
      `electron-chromedriver version in script/package.json should start with a caret to match latest patch version.`
    );
  }

  if (!mksnapshotVer.startsWith('^')) {
    throw new Error(
      `electron-mksnapshot version in script/package.json should start with a caret to match latest patch version.`
    );
  }

  if (!semver.satisfies(chromedriverActualVer, `>=${majorElectronVersion}`)) {
    throw new Error(
      `electron-chromedriver should be at least v${majorElectronVersion} to support the ELECTRON_CUSTOM_VERSION environment variable.`
    );
  }

  if (!semver.satisfies(mksnapshotActualVer, `>=${majorElectronVersion}`)) {
    throw new Error(
      `electron-mksnapshot should be at least v${majorElectronVersion} to support the ELECTRON_CUSTOM_VERSION environment variable.`
    );
  }
};
Repository: axios/axios
File: test/specs/helpers/formDataToJSON.spec.js (JavaScript)

Code before:
import formDataToJSON from '../../../lib/helpers/formDataToJSON';

describe('formDataToJSON', function () {
  it('should convert a FormData Object to JSON Object', function () {
    const formData = new FormData();

    formData.append('foo[bar][baz]', '123');

    expect(formDataToJSON(formData)).toEqual({
      foo: {
        bar: {
          baz: '123'
        }
      }
    });
  });

  it('should convert repeatable values as an array', function () {
    const formData = new FormData();

    formData.append('foo', '1');
    formData.append('foo', '2');

    expect(formDataToJSON(formData)).toEqual({
      foo: ['1', '2']
    });
  });

  it('should convert props with empty brackets to arrays', function () {
    const formData = new FormData();

    formData.append('foo[]', '1');
    formData.append('foo[]', '2');

    expect(formDataToJSON(formData)).toEqual({
      foo: ['1', '2']
    });
  });

  it('should supported indexed arrays', function () {
    const formData = new FormData();

    formData.append('foo[0]', '1');
    formData.append('foo[1]', '2');

    expect(formDataToJSON(formData)).toEqual({
      foo: ['1', '2']
    });
  });
});


Review comment (security):
@DigitalBrainJS I think I may have something that could be interesting for you in order to detect these problems before they got reported to you.

Here is an introduction to the pattern I was thinking of: https://fast-check.dev/blog/2023/09/21/detect-prototype-pollution-automatically/. If you think it can be useful, I can try to first plug it on formDataToJSON and then with your pointers extend its usage to other potentially vulnerable parts. (side-note: Im' the author of fast-check)

Code after:
    formData.append('foo[]', '1');
    formData.append('foo[]', '2');

    expect(formDataToJSON(formData)).toEqual({
      foo: ['1', '2']
    });
  });

  it('should supported indexed arrays', function () {
    const formData = new FormData();

    formData.append('foo[0]', '1');
    formData.append('foo[1]', '2');

    expect(formDataToJSON(formData)).toEqual({
      foo: ['1', '2']
    });
  });

  it('should resist prototype pollution CVE', () => {
    const formData = new FormData();

    formData.append('foo[0]', '1');
    formData.append('foo[1]', '2');
    formData.append('__proto__.x', 'hack');
    formData.append('constructor.prototype.y', 'value');

    expect(formDataToJSON(formData)).toEqual({
      foo: ['1', '2'],
      constructor: {
        prototype: {
          y: 'value'
        }
      }
    });

    expect({}.x).toEqual(undefined);
    expect({}.y).toEqual(undefined);
  });
});