ronantakizawa/codereview-bench huggingface dataset:
Repository: chartjs/Chart.js
File: src/core/core.adapters.js (JavaScript)

Code before:
 * @private
 */

/**
 * @return {*}
 */
function abstract() {
  throw new Error('This method is not implemented: Check that a complete date adapter is provided.');
}

/**
 * Date adapter (current used by the time scale)
 * @namespace Chart._adapters._date
 * @memberof Chart._adapters
 * @private
 */

/**
 * Currently supported unit string values.
 * @typedef {('millisecond'|'second'|'minute'|'hour'|'day'|'week'|'month'|'quarter'|'year')} Unit
 * @memberof Chart._adapters._date
 */

export class DateAdapter {

  constructor(options, chartOptions) {
    this.options = options || {};
    this.chartOptions = chartOptions || {};
  }

  /**
	 * Returns a map of time formats for the supported formatting units defined
	 * in Unit as well as 'datetime' representing a detailed date/time string.
	 * @returns {{string: string}}
	 */
  formats() {
    return abstract();
  }

  /**
	 * Parses the given `value` and return the associated timestamp.
	 * @param {any} value - the value to parse (usually comes from the data)
	 * @param {string} [format] - the expected data format
	 * @returns {(number|null)}
	 */
  parse(value, format) { // eslint-disable-line no-unused-vars
    return abstract();
  }

  /**
	 * Returns the formatted date in the specified `format` for a given `timestamp`.

Review comment (suggestion):
Maby `localeOptions` might be a better name, chartOptions feels a bit misleading, get the feeling like all the chart opts are passed with that name but that is not the case 

Code after:
 * @private
 */

/**
 * @typedef { import("../../types/index.esm").ChartOptions } ChartOptions
 */

/**
 * @return {*}
 */
function abstract() {
  throw new Error('This method is not implemented: Check that a complete date adapter is provided.');
}

/**
 * Date adapter (current used by the time scale)
 * @namespace Chart._adapters._date
 * @memberof Chart._adapters
 * @private
 */

/**
 * Currently supported unit string values.
 * @typedef {('millisecond'|'second'|'minute'|'hour'|'day'|'week'|'month'|'quarter'|'year')} Unit
 * @memberof Chart._adapters._date
 */

export class DateAdapter {

  constructor(options) {
    this.options = options || {};
  }

  /**
   * Will called with chart options after adapter creation.
   * @param {ChartOptions} chartOptions
   */
  // eslint-disable-next-line no-unused-vars
  init(chartOptions) {}

  /**
	 * Returns a map of time formats for the supported formatting units defined
	 * in Unit as well as 'datetime' representing a detailed date/time string.
	 * @returns {{string: string}}
	 */
  formats() {
    return abstract();
  }

  /**
	 * Parses the given `value` and return the associated timestamp.
Repository: chartjs/Chart.js
File: types/index.d.ts (TypeScript)

Code before:
     */
    filter(item: LegendItem, data: ChartData): boolean;

    /**
     * Sorts the legend items
     */
    sort(a: LegendItem, b: LegendItem, data: ChartData): number;

    /**
     * Override point style for the legend. Only applies if usePointStyle is true
     */
    pointStyle: PointStyle;

    /**
     * Text alignment
     */
    textAlign?: TextAlign;

    /**
     * Label style will match corresponding point style (size is based on the minimum value between boxWidth and font.size).
     * @default false
     */
    usePointStyle: boolean;

    /**
     * Label borderRadius will match coresponding borderRadius.
     * @default false
     */
    useBorderRadius: boolean;

    /**
     * Override the borderRadius to use.
     * @default undefined
     */
    borderRadius: number;
  };
  /**
   * true for rendering the legends from right to left.
   */
  rtl: boolean;
  /**
   * This will force the text direction 'rtl' or 'ltr' on the canvas for rendering the legend, regardless of the css specified on the canvas
   * @default canvas' default
   */
  textDirection: string;

  title: {
    /**
     * Is the legend title displayed.
     * @default false
     */

Review comment (suggestion):
```suggestion
     * Label borderRadius will match corresponding borderRadius.
```

Code after:
     */
    filter(item: LegendItem, data: ChartData): boolean;

    /**
     * Sorts the legend items
     */
    sort(a: LegendItem, b: LegendItem, data: ChartData): number;

    /**
     * Override point style for the legend. Only applies if usePointStyle is true
     */
    pointStyle: PointStyle;

    /**
     * Text alignment
     */
    textAlign?: TextAlign;

    /**
     * Label style will match corresponding point style (size is based on the minimum value between boxWidth and font.size).
     * @default false
     */
    usePointStyle: boolean;

    /**
     * Label borderRadius will match corresponding borderRadius.
     * @default false
     */
    useBorderRadius: boolean;

    /**
     * Override the borderRadius to use.
     * @default undefined
     */
    borderRadius: number;
  };
  /**
   * true for rendering the legends from right to left.
   */
  rtl: boolean;
  /**
   * This will force the text direction 'rtl' or 'ltr' on the canvas for rendering the legend, regardless of the css specified on the canvas
   * @default canvas' default
   */
  textDirection: string;

  title: {
    /**
     * Is the legend title displayed.
     * @default false
     */
Repository: chartjs/Chart.js
File: types/index.d.ts (TypeScript)

Code before:
  afterBody: string[];

  // Title
  // lines of text that form the title
  title: string[];

  // Footer
  // lines of text that form the footer
  footer: string[];

  // Styles to render for each item in body[]. This is the styling of the squares in the tooltip
  labelColors: TooltipLabelStyle[];
  labelTextColors: Color[];
  labelPointStyles: { pointStyle: PointStyle; rotation: number }[];

  // 0 opacity is a hidden tooltip
  opacity: number;

  // tooltip options
  options: TooltipOptions<TType>;

  getActiveElements(): ActiveElement[];
  setActiveElements(active: ActiveDataPoint[], eventPosition: Point): void;
}

export interface TooltipPosition extends Point{
  xAlign?: TooltipXAlignment;
  yAlign?: TooltipYAlignment;
}

export type TooltipPositionerFunction<TType extends ChartType> = (
  this: TooltipModel<TType>,
  items: readonly ActiveElement[],
  eventPosition: Point
) => TooltipPosition | false;

export interface TooltipPositionerMap {
  average: TooltipPositionerFunction<ChartType>;
  nearest: TooltipPositionerFunction<ChartType>;
}

export type TooltipPositioner = keyof TooltipPositionerMap;

export interface Tooltip extends Plugin {
  readonly positioners: TooltipPositionerMap;
}

export const Tooltip: Tooltip;

export interface TooltipCallbacks<
  TType extends ChartType,

Review comment (suggestion):
```suggestion
export interface TooltipPosition extends Point {
```

Code after:
  afterBody: string[];

  // Title
  // lines of text that form the title
  title: string[];

  // Footer
  // lines of text that form the footer
  footer: string[];

  // Styles to render for each item in body[]. This is the styling of the squares in the tooltip
  labelColors: TooltipLabelStyle[];
  labelTextColors: Color[];
  labelPointStyles: { pointStyle: PointStyle; rotation: number }[];

  // 0 opacity is a hidden tooltip
  opacity: number;

  // tooltip options
  options: TooltipOptions<TType>;

  getActiveElements(): ActiveElement[];
  setActiveElements(active: ActiveDataPoint[], eventPosition: Point): void;
}

export interface TooltipPosition extends Point {
  xAlign?: TooltipXAlignment;
  yAlign?: TooltipYAlignment;
}

export type TooltipPositionerFunction<TType extends ChartType> = (
  this: TooltipModel<TType>,
  items: readonly ActiveElement[],
  eventPosition: Point
) => TooltipPosition | false;

export interface TooltipPositionerMap {
  average: TooltipPositionerFunction<ChartType>;
  nearest: TooltipPositionerFunction<ChartType>;
}

export type TooltipPositioner = keyof TooltipPositionerMap;

export interface Tooltip extends Plugin {
  readonly positioners: TooltipPositionerMap;
}

export const Tooltip: Tooltip;

export interface TooltipCallbacks<
  TType extends ChartType,
Repository: chartjs/Chart.js
File: types/index.d.ts (TypeScript)

Code before:
interface BarParsedData extends CartesianParsedData {
  // Only specified if floating bars are show
  _custom?: {
    barStart: number;
    barEnd: number;
    start: number;
    end: number;
    min: number;
    max: number;
  }
}

interface BubbleParsedData extends CartesianParsedData {
  // The bubble radius value
  _custom: number;
}

interface RadialParsedData {
  r: number;
}

export interface ChartTypeRegistry {
  bar: {
    chartOptions: BarControllerChartOptions;
    datasetOptions: BarControllerDatasetOptions;
    defaultDataPoint: number | number[] | null;
    metaExtensions: {};
    parsedDataType: BarParsedData,
    scales: keyof CartesianScaleTypeRegistry;
  };
  line: {
    chartOptions: LineControllerChartOptions;
    datasetOptions: LineControllerDatasetOptions & FillerControllerDatasetOptions;
    defaultDataPoint: ScatterDataPoint | number | null;
    metaExtensions: {};
    parsedDataType: CartesianParsedData;
    scales: keyof CartesianScaleTypeRegistry;
  };
  scatter: {
    chartOptions: ScatterControllerChartOptions;
    datasetOptions: ScatterControllerDatasetOptions;
    defaultDataPoint: ScatterDataPoint | number | null;
    metaExtensions: {};
    parsedDataType: CartesianParsedData;
    scales: keyof CartesianScaleTypeRegistry;
  };
  bubble: {
    chartOptions: unknown;
    datasetOptions: BubbleControllerDatasetOptions;
    defaultDataPoint: BubbleDataPoint;
    metaExtensions: {};

Review comment (suggestion):
Since it always uses 2 entries this is a safer and more descriptive option
```suggestion
    defaultDataPoint: number | [number, number] | null;
```

Code after:
interface BarParsedData extends CartesianParsedData {
  // Only specified if floating bars are show
  _custom?: {
    barStart: number;
    barEnd: number;
    start: number;
    end: number;
    min: number;
    max: number;
  }
}

interface BubbleParsedData extends CartesianParsedData {
  // The bubble radius value
  _custom: number;
}

interface RadialParsedData {
  r: number;
}

export interface ChartTypeRegistry {
  bar: {
    chartOptions: BarControllerChartOptions;
    datasetOptions: BarControllerDatasetOptions;
    defaultDataPoint: number | [number, number] | null;
    metaExtensions: {};
    parsedDataType: BarParsedData,
    scales: keyof CartesianScaleTypeRegistry;
  };
  line: {
    chartOptions: LineControllerChartOptions;
    datasetOptions: LineControllerDatasetOptions & FillerControllerDatasetOptions;
    defaultDataPoint: ScatterDataPoint | number | null;
    metaExtensions: {};
    parsedDataType: CartesianParsedData;
    scales: keyof CartesianScaleTypeRegistry;
  };
  scatter: {
    chartOptions: ScatterControllerChartOptions;
    datasetOptions: ScatterControllerDatasetOptions;
    defaultDataPoint: ScatterDataPoint | number | null;
    metaExtensions: {};
    parsedDataType: CartesianParsedData;
    scales: keyof CartesianScaleTypeRegistry;
  };
  bubble: {
    chartOptions: unknown;
    datasetOptions: BubbleControllerDatasetOptions;
    defaultDataPoint: BubbleDataPoint;
    metaExtensions: {};
Repository: chartjs/Chart.js
File: src/core/core.controller.js (JavaScript)

Code before:
 * @param {boolean} inChartArea
 * @param {boolean} isClick
 * @returns {ChartEvent|null}
 */
function determineLastEvent(e, lastEvent, inChartArea, isClick) {
  if (!inChartArea || e.type === 'mouseout') {
    return null;
  }
  if (isClick) {
    return lastEvent;
  }
  return e;
}

function getDatasetArea(meta) {
  const {iScale, vScale} = meta;
  if (iScale && vScale) {
    const horScale = iScale.isHorizontal() ? iScale : vScale;
    const verScale = iScale.isHorizontal() ? vScale : iScale;
    return {
      left: horScale.left,
      right: horScale.right,
      top: verScale.top,
      bottom: verScale.bottom
    };
  }
}

class Chart {

  static defaults = defaults;
  static instances = instances;
  static overrides = overrides;
  static registry = registry;
  static version = version;
  static getChart = getChart;

  static register(...items) {
    registry.add(...items);
    invalidatePlugins();
  }

  static unregister(...items) {
    registry.remove(...items);
    invalidatePlugins();
  }

  // eslint-disable-next-line max-statements
  constructor(item, userConfig) {
    const config = this.config = new Config(userConfig);
    const initialCanvas = getCanvas(item);

Review comment (suggestion):
This is what I was suggesting: 

```suggestion
  const {xScale, yScale} = meta;
  if (xScale && yScale) {
    return {
      left: xScale.left,
      right: xScale.right,
      top: yScale.top,
      bottom: yScale.bottom
    };
  }
```

Code after:
 * @param {boolean} inChartArea
 * @param {boolean} isClick
 * @returns {ChartEvent|null}
 */
function determineLastEvent(e, lastEvent, inChartArea, isClick) {
  if (!inChartArea || e.type === 'mouseout') {
    return null;
  }
  if (isClick) {
    return lastEvent;
  }
  return e;
}

function getDatasetArea(meta) {
  const {xScale, yScale} = meta;
  if (xScale && yScale) {
    return {
      left: xScale.left,
      right: xScale.right,
      top: yScale.top,
      bottom: yScale.bottom
    };
  }
}

class Chart {

  static defaults = defaults;
  static instances = instances;
  static overrides = overrides;
  static registry = registry;
  static version = version;
  static getChart = getChart;

  static register(...items) {
    registry.add(...items);
    invalidatePlugins();
  }

  static unregister(...items) {
    registry.remove(...items);
    invalidatePlugins();
  }

  // eslint-disable-next-line max-statements
  constructor(item, userConfig) {
    const config = this.config = new Config(userConfig);
    const initialCanvas = getCanvas(item);
    const existingChart = getChart(initialCanvas);
    if (existingChart) {
Repository: chartjs/Chart.js
File: src/helpers/helpers.collection.ts (TypeScript)

Code before:
import {_capitalize} from './helpers.core';

/**
 * Binary search
 * @param table - the table search. must be sorted!
 * @param value - value to find
 * @param cmp
 * @private
 */
export function _lookup(
  table: unknown[],
  value: number,
  cmp?: (value: number) => boolean
) {
  cmp = cmp || ((index) => table[index] < value);
  let hi = table.length - 1;
  let lo = 0;
  let mid: number;

  while (hi - lo > 1) {
    mid = (lo + hi) >> 1;
    if (cmp(mid)) {
      lo = mid;
    } else {
      hi = mid;
    }
  }

  return {lo, hi};
}

/**
 * Binary search
 * @param table - the table search. must be sorted!
 * @param key - property name for the value in each entry
 * @param value - value to find
 * @param last - lookup last index
 * @private
 */
export const _lookupByKey = (
  table: Record<string, number>[],
  key: string,
  value: number,
  last?: boolean
) =>
  _lookup(table, value, last

Review comment (suggestion):
I think it might be good to change this definition slightly to

```ts
export function _lookup<T>(
  table: T[],
  value: number,
  cmp?: (value: T) => boolean
)
```

My thinking is that we default to assuming `T` is a number on line 15 so this will guard against someone passing a complex object but forgetting to pass the `cmp` function.

We could also do this with an override so there is one version where you pass `number[]` and no `cmp` and then if you pass an object, `cmp` becomes required

Code after:
import {_capitalize} from './helpers.core';

/**
 * Binary search
 * @param table - the table search. must be sorted!
 * @param value - value to find
 * @param cmp
 * @private
 */
export function _lookup(
  table: number[],
  value: number,
  cmp?: (value: number) => boolean
): {lo: number, hi: number};
export function _lookup<T>(
  table: T[],
  value: number,
  cmp: (value: number) => boolean
): {lo: number, hi: number};
export function _lookup(
  table: unknown[],
  value: number,
  cmp?: (value: number) => boolean
) {
  cmp = cmp || ((index) => table[index] < value);
  let hi = table.length - 1;
  let lo = 0;
  let mid: number;

  while (hi - lo > 1) {
    mid = (lo + hi) >> 1;
    if (cmp(mid)) {
      lo = mid;
    } else {
      hi = mid;
    }
  }

  return {lo, hi};
}

/**
 * Binary search
 * @param table - the table search. must be sorted!
 * @param key - property name for the value in each entry
 * @param value - value to find
Repository: chartjs/Chart.js
File: src/helpers/helpers.curve.ts (TypeScript)

Code before:
import {almostEquals, distanceBetweenPoints, sign} from './helpers.math';
import {_isPointInArea} from './helpers.canvas';
import {ChartArea} from '../../types';

export interface SplinePoint {
  x: number;
  y: number;
  skip?: boolean;

  // Both Bezier and monotone interpolations have these fields
  // but they are added in different spots
  cp1x?: number;
  cp1y?: number;
  cp2x?: number;
  cp2y?: number;
}

/**
 * Keep for backwards compatibility
 */
export type MonotoneSplinePoint = SplinePoint

const EPSILON = Number.EPSILON || 1e-14;

type OptionalSplinePoint = SplinePoint | false
const getPoint = (points: SplinePoint[], i: number): OptionalSplinePoint => i < points.length && !points[i].skip && points[i];
const getValueAxis = (indexAxis: 'x' | 'y') => indexAxis === 'x' ? 'y' : 'x';

export function splineCurve(
  firstPoint: SplinePoint,
  middlePoint: SplinePoint,
  afterPoint: SplinePoint,
  t: number
): {
    previous: SplinePoint
    next: SplinePoint
  } {
  // Props to Rob Spencer at scaled innovation for his post on splining between points
  // http://scaledinnovation.com/analytics/splines/aboutSplines.html

  // This function must also respect "skipped" points

  const previous = firstPoint.skip ? middlePoint : firstPoint;
  const current = middlePoint;
  const next = afterPoint.skip ? middlePoint : afterPoint;
  const d01 = distanceBetweenPoints(current, previous);

Review comment (suggestion):
If we want to squeeze this into v4, then I can remove the `MonotoneSplinePoint` type entirely otherwise this has to wait until v5

Code after:
import {almostEquals, distanceBetweenPoints, sign} from './helpers.math';
import {_isPointInArea} from './helpers.canvas';
import {ChartArea} from '../../types';

export interface SplinePoint {
  x: number;
  y: number;
  skip?: boolean;

  // Both Bezier and monotone interpolations have these fields
  // but they are added in different spots
  cp1x?: number;
  cp1y?: number;
  cp2x?: number;
  cp2y?: number;
}

const EPSILON = Number.EPSILON || 1e-14;

type OptionalSplinePoint = SplinePoint | false
const getPoint = (points: SplinePoint[], i: number): OptionalSplinePoint => i < points.length && !points[i].skip && points[i];
const getValueAxis = (indexAxis: 'x' | 'y') => indexAxis === 'x' ? 'y' : 'x';

export function splineCurve(
  firstPoint: SplinePoint,
  middlePoint: SplinePoint,
  afterPoint: SplinePoint,
  t: number
): {
    previous: SplinePoint
    next: SplinePoint
  } {
  // Props to Rob Spencer at scaled innovation for his post on splining between points
  // http://scaledinnovation.com/analytics/splines/aboutSplines.html

  // This function must also respect "skipped" points

  const previous = firstPoint.skip ? middlePoint : firstPoint;
  const current = middlePoint;
  const next = afterPoint.skip ? middlePoint : afterPoint;
  const d01 = distanceBetweenPoints(current, previous);
  const d12 = distanceBetweenPoints(next, current);

  let s01 = d01 / (d01 + d12);
  let s12 = d12 / (d01 + d12);
Repository: chartjs/Chart.js
File: src/helpers/helpers.dom.ts (TypeScript)

Code before:
 * @private
 */
function parseMaxStyle(styleValue, node, parentProperty) {
  let valueInPixels;
  if (typeof styleValue === 'string') {
    valueInPixels = parseInt(styleValue, 10);

    if (styleValue.indexOf('%') !== -1) {
      // percentage * size in dimension
      valueInPixels = (valueInPixels / 100) * node.parentNode[parentProperty];
    }
  } else {
    valueInPixels = styleValue;
  }

  return valueInPixels;
}

const getComputedStyle = (element: HTMLElement) =>
  element.ownerDocument.defaultView.getComputedStyle(element, null);

export function getStyle(el: HTMLElement, property: string): string {
  return getComputedStyle(el).getPropertyValue(property);
}

type ChartType = {
  currentDevicePixelRatio: number;
  readonly canvas: HTMLCanvasElement;
  width: number;
  height: number;
  readonly ctx: CanvasRenderingContext2D;
};

type PositionedStyle = {
  top: number;
  right: number;
  bottom: number;
  left: number;
  width: number;
  height: number;
};

const positions = ['top', 'right', 'bottom', 'left'];
function getPositionedStyle(styles, style, suffix): PositionedStyle {
  const result = {} as PositionedStyle;
  suffix = suffix ? '-' + suffix : '';
  for (let i = 0; i < 4; i++) {
    const pos = positions[i];
    result[pos] = parseFloat(styles[style + '-' + pos + suffix]) || 0;
  }
  result.width = result.left + result.right;

Review comment (suggestion):
I think `import { type Chart } from '../../types` is more correct here

Code after:
/**
 * convert max-width/max-height values that may be percentages into a number
 * @private
 */

function parseMaxStyle(styleValue: string | number, node: HTMLElement, parentProperty: string) {
  let valueInPixels: number;
  if (typeof styleValue === 'string') {
    valueInPixels = parseInt(styleValue, 10);

    if (styleValue.indexOf('%') !== -1) {
      // percentage * size in dimension
      valueInPixels = (valueInPixels / 100) * node.parentNode[parentProperty];
    }
  } else {
    valueInPixels = styleValue;
  }

  return valueInPixels;
}

const getComputedStyle = (element: HTMLElement): CSSStyleDeclaration =>
  element.ownerDocument.defaultView.getComputedStyle(element, null);

export function getStyle(el: HTMLElement, property: string): string {
  return getComputedStyle(el).getPropertyValue(property);
}

const positions = ['top', 'right', 'bottom', 'left'];
function getPositionedStyle(styles: CSSStyleDeclaration, style: string, suffix?: string): ChartArea {
  const result = {} as ChartArea;
  suffix = suffix ? '-' + suffix : '';
  for (let i = 0; i < 4; i++) {
    const pos = positions[i];
    result[pos] = parseFloat(styles[style + '-' + pos + suffix]) || 0;
  }
  result.width = result.left + result.right;
  result.height = result.top + result.bottom;
  return result;
}

const useOffsetPos = (x: number, y: number, target: HTMLElement | EventTarget) =>
  (x > 0 || y > 0) && (!target || !(target as HTMLElement).shadowRoot);

/**
 * @param e
 * @param canvas
 * @returns Canvas position
 */
function getCanvasPosition(
  e: Event | TouchEvent | MouseEvent,