Skip to content

Commit

Permalink
Add etc/no-assign-mutated-array rule
Browse files Browse the repository at this point in the history
  • Loading branch information
mattseddon committed Mar 21, 2023
1 parent 0c4be74 commit e2768d3
Show file tree
Hide file tree
Showing 22 changed files with 79 additions and 43 deletions.
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ module.exports = {
camelcase: 'off',
curly: ['error', 'all'],
'etc/no-commented-out-code': 'error',
'etc/no-assign-mutated-array': 'error',
'import/no-unresolved': 'off',
'import/order': [
'error',
Expand Down
3 changes: 2 additions & 1 deletion extension/src/experiments/columns/collect/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
} from '../../../cli/dvc/contract'
import { standardizePath } from '../../../fileSystem/path'
import { timestampColumn } from '../constants'
import { sortCollectedArray } from '../../../util/array'

const collectFromExperiment = (
acc: ColumnAccumulator,
Expand Down Expand Up @@ -72,7 +73,7 @@ export const collectChanges = (data: ExperimentsOutput): string[] => {
collectMetricAndParamChanges(changes, workspace, currentCommit)
collectDepChanges(changes, workspace, currentCommit)

return changes.sort()
return sortCollectedArray(changes)
}

export const collectParamsFiles = (
Expand Down
5 changes: 4 additions & 1 deletion extension/src/experiments/data/collect.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,10 @@ describe('collectFiles', () => {
}
} as ExperimentsOutput

expect(collectFiles(workspace, []).sort()).toStrictEqual([
const files = collectFiles(workspace, [])
files.sort()

expect(files).toStrictEqual([
'further/nested/params.yaml',
'logs.json',
'metrics.json',
Expand Down
8 changes: 6 additions & 2 deletions extension/src/experiments/model/filterBy/tree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,11 @@ import { RegisteredCommands } from '../../../commands/external'
import { InternalCommands } from '../../../commands/internal'
import { sendViewOpenedTelemetryEvent } from '../../../telemetry'
import { EventName } from '../../../telemetry/constants'
import { definedAndNonEmpty, joinTruthyItems } from '../../../util/array'
import {
definedAndNonEmpty,
joinTruthyItems,
sortCollectedArray
} from '../../../util/array'
import { createTreeView, getRootItem } from '../../../tree'
import { Disposable } from '../../../class/dispose'

Expand Down Expand Up @@ -113,7 +117,7 @@ export class ExperimentsFilterByTree
return this.getChildren(onlyRepo)
}

return dvcRoots.sort((a, b) => a.localeCompare(b))
return sortCollectedArray(dvcRoots, (a, b) => a.localeCompare(b))
}

return []
Expand Down
3 changes: 2 additions & 1 deletion extension/src/experiments/model/sortBy/tree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { sendViewOpenedTelemetryEvent } from '../../../telemetry'
import { EventName } from '../../../telemetry/constants'
import { InternalCommands } from '../../../commands/internal'
import { Disposable } from '../../../class/dispose'
import { sortCollectedArray } from '../../../util/array'

export type SortItem = {
dvcRoot: string
Expand Down Expand Up @@ -91,7 +92,7 @@ export class ExperimentsSortByTree
return dvcRoots.some(
dvcRoot => this.experiments.getRepository(dvcRoot).getSorts().length > 0
)
? dvcRoots.sort((a, b) => a.localeCompare(b))
? sortCollectedArray(dvcRoots, (a, b) => a.localeCompare(b))
: []
}

Expand Down
3 changes: 3 additions & 0 deletions extension/src/experiments/model/status/colors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,6 @@ export const colorsList = [
export type Color = (typeof colorsList)[number]

export const copyOriginalColors = (): Color[] => [...colorsList]

export const copyReverseOriginalColors = (): Color[] =>
[...colorsList].reverse()
2 changes: 1 addition & 1 deletion extension/src/experiments/model/status/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const compareTimestamps = (a: Experiment, b: Experiment) =>
getEpoch(b.Created) - getEpoch(a.Created)

export const limitToMaxSelected = (experiments: Experiment[]) =>
experiments
[...experiments]
.sort((a, b) => {
if (a.status === b.status) {
return compareTimestamps(a, b)
Expand Down
5 changes: 3 additions & 2 deletions extension/src/experiments/model/tree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { getDataFromColumnPaths } from './util'
import { WorkspaceExperiments } from '../workspace'
import { sendViewOpenedTelemetryEvent } from '../../telemetry'
import { EventName } from '../../telemetry/constants'
import { definedAndNonEmpty } from '../../util/array'
import { definedAndNonEmpty, sortCollectedArray } from '../../util/array'
import {
createTreeView,
DecoratableTreeItemScheme,
Expand Down Expand Up @@ -176,7 +176,8 @@ export class ExperimentsTree
const [onlyRepo] = dvcRoots
return this.getChildren(onlyRepo)
}
return dvcRoots.sort((a, b) => a.localeCompare(b))

return sortCollectedArray(dvcRoots, (a, b) => a.localeCompare(b))
}

return []
Expand Down
5 changes: 3 additions & 2 deletions extension/src/fileSystem/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
import { load } from 'js-yaml'
import { Uri, workspace, window, commands, ViewColumn } from 'vscode'
import { standardizePath } from './path'
import { definedAndNonEmpty } from '../util/array'
import { definedAndNonEmpty, sortCollectedArray } from '../util/array'
import { Logger } from '../common/logger'
import { gitPath } from '../cli/git/constants'
import { createValidInteger } from '../util/number'
Expand Down Expand Up @@ -66,7 +66,8 @@ export const findDvcRootPaths = async (cwd: string): Promise<string[]> => {
if (definedAndNonEmpty(subRoots)) {
dvcRoots.push(...subRoots)
}
return dvcRoots.sort()

return sortCollectedArray(dvcRoots)
}

export const findAbsoluteDvcRootPath = async (
Expand Down
4 changes: 2 additions & 2 deletions extension/src/path/selection/tree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { WorkspacePlots } from '../../plots/workspace'
import { Resource, ResourceLocator } from '../../resourceLocator'
import { RegisteredCommands } from '../../commands/external'
import { createTreeView } from '../../tree'
import { definedAndNonEmpty } from '../../util/array'
import { definedAndNonEmpty, sortCollectedArray } from '../../util/array'
import { sendViewOpenedTelemetryEvent } from '../../telemetry'
import { ViewOpenedEventName } from '../../telemetry/constants'
import { Disposable } from '../../class/dispose'
Expand Down Expand Up @@ -179,7 +179,7 @@ export abstract class BasePathSelectionTree<
return this.getChildren(onlyRepo)
}

return dvcRoots.sort((a, b) => a.localeCompare(b))
return sortCollectedArray(dvcRoots, (a, b) => a.localeCompare(b))
}

private getChildElements(
Expand Down
13 changes: 8 additions & 5 deletions extension/src/plots/data/collect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {
PlotsOutputOrError
} from '../../cli/dvc/contract'
import { isDvcError } from '../../cli/dvc/reader'
import { uniqueValues } from '../../util/array'
import { sortCollectedArray, uniqueValues } from '../../util/array'
import { isImagePlot, Plot, TemplatePlot } from '../webview/contract'

const collectImageFile = (acc: string[], file: string): void => {
Expand Down Expand Up @@ -64,13 +64,16 @@ export const collectFiles = (
export const collectMetricsFiles = (
data: ExperimentsOutput,
existingFiles: string[]
): string[] =>
uniqueValues([
): string[] => {
const metricsFiles = uniqueValues([
...Object.keys({
...data?.workspace?.baseline?.data?.metrics
}).filter(Boolean),
...existingFiles
]).sort()
])

return sortCollectedArray(metricsFiles)
}

const collectRev = (acc: string[], rev: string): void => {
if (rev !== EXPERIMENT_WORKSPACE_ID && !acc.includes(rev)) {
Expand All @@ -92,5 +95,5 @@ export const collectRevs = (
collectRev(acc, mutableRevision)
}

return [EXPERIMENT_WORKSPACE_ID, ...acc.sort()]
return [EXPERIMENT_WORKSPACE_ID, ...[...acc].sort()]
}
4 changes: 3 additions & 1 deletion extension/src/plots/multiSource/collect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
} from './constants'
import { isImagePlot, Plot, TemplatePlot } from '../webview/contract'
import { PlotsOutput } from '../../cli/dvc/contract'
import { sortCollectedArray } from '../../util/array'

const FIELD_SEPARATOR = '::'

Expand Down Expand Up @@ -163,7 +164,8 @@ const groupVariations = (
lessValuesThanVariations,
expectedOrder
),
valuesMatchVariations: valuesMatchVariations.sort(
valuesMatchVariations: sortCollectedArray(
valuesMatchVariations,
(a, b) => expectedOrder.indexOf(a) - expectedOrder.indexOf(b)
)
}
Expand Down
7 changes: 5 additions & 2 deletions extension/src/repository/model/tree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ export class RepositoriesTree
}

private sortDirectory(contents: PathItem[]) {
return contents.sort((a, b) => {
return [...contents].sort((a, b) => {
const aIsDirectory = a.isDirectory
if (aIsDirectory === b.isDirectory) {
return a.resourceUri.fsPath.localeCompare(b.resourceUri.fsPath)
Expand Down Expand Up @@ -302,7 +302,10 @@ export class RepositoriesTree
)
}

const args = [dvcRoot, ...uniqueValues(tracked).sort()]
const uniqueTracked = uniqueValues(tracked)
uniqueTracked.sort()

const args = [dvcRoot, ...uniqueTracked]

await tryThenMaybeForce(this.internalCommands, commandId, ...args)
}
Expand Down
4 changes: 2 additions & 2 deletions extension/src/setup/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,8 +226,8 @@ export class Setup

public async setRoots() {
const nestedRoots = await this.findWorkspaceDvcRoots()
this.dvcRoots =
this.config.getFocusedProjects() || nestedRoots.flat().sort()
this.dvcRoots = this.config.getFocusedProjects() || nestedRoots.flat()
this.dvcRoots.sort()

void this.sendDataToWebview()
return this.setProjectAvailability()
Expand Down
4 changes: 3 additions & 1 deletion extension/src/setup/quickPick.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { sortCollectedArray } from '../util/array'
import { quickPickManyValues } from '../vscode/quickPick'
import { Title } from '../vscode/title'
import { Toast } from '../vscode/toast'
Expand All @@ -22,5 +23,6 @@ export const pickFocusedProjects = async (
void Toast.showError('Cannot select 0 projects.')
return
}
return values.sort()

return sortCollectedArray(values)
}
5 changes: 4 additions & 1 deletion extension/src/test/e2e/extension.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
} from './util.js'
import { ExperimentsWebview } from './pageObjects/experimentsWebview.js'
import { PlotsWebview } from './pageObjects/plotsWebview.js'
import { sortCollectedArray } from '../../util/array.js'

before('should finish loading the extension', async function () {
this.timeout(240000)
Expand Down Expand Up @@ -190,6 +191,8 @@ describe('Source Control View', function () {
}
)

expect(expectedScmItemLabels.sort()).toStrictEqual(dvcTreeItemLabels.sort())
expect(sortCollectedArray(expectedScmItemLabels)).toStrictEqual(
sortCollectedArray(dvcTreeItemLabels)
)
})
})
4 changes: 2 additions & 2 deletions extension/src/test/suite/experiments/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1165,7 +1165,7 @@ suite('Experiments Test Suite', () => {
.getSelectedRevisions()
.map(({ id }) => id)
.sort()
expect(selectExperimentIds).to.deep.equal(mockExperimentIds.sort())
mockExperimentIds.sort()
expect(selectExperimentIds).to.deep.equal(mockExperimentIds)
}).timeout(WEBVIEW_TEST_TIMEOUT)

Expand Down Expand Up @@ -1207,7 +1207,7 @@ suite('Experiments Test Suite', () => {
.getSelectedRevisions()
.map(({ id }) => id)
.sort()
expect(selectExperimentIds).to.deep.equal(mockExperimentIds.sort())
expect(selectExperimentIds).to.deep.equal([...mockExperimentIds].sort())
expect(mockShowPlots).to.be.calledOnce
expect(mockShowPlots).to.be.calledWith(dvcDemoPath)
}).timeout(WEBVIEW_TEST_TIMEOUT)
Expand Down
4 changes: 3 additions & 1 deletion extension/src/test/suite/plots/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -793,7 +793,9 @@ suite('Plots Test Suite', () => {
expect(confusionMatrixDatapoints.length).to.be.greaterThan(0)

expect(confusionMatrix.revisions?.length).to.equal(4)
expect(confusionMatrix.revisions?.sort()).to.deep.equal(expectedRevisions)
expect([...(confusionMatrix.revisions || [])].sort()).to.deep.equal(
expectedRevisions
)

for (const entry of confusionMatrixDatapoints) {
expect(expectedRevisions).to.include(entry.rev)
Expand Down
8 changes: 8 additions & 0 deletions extension/src/util/array.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,11 @@ export const performSimpleOrderedUpdate = (
])
return [...newOrder]
}

export const sortCollectedArray = <T>(
acc: T[],
callback?: (a: T, b: T) => number
): T[] => {
acc.sort(callback)
return acc
}
22 changes: 10 additions & 12 deletions languageServer/src/textDocument.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,18 +99,16 @@ const symbolScopeAt = (
}
}
})
const symbolStack = (symbolsFound.filter(Boolean) as DocumentSymbol[]).sort(
(a, b) => {
const offA =
textDocument.offsetAt(a.range.end) -
textDocument.offsetAt(a.range.start)
const offB =
textDocument.offsetAt(b.range.end) -
textDocument.offsetAt(b.range.start)

return offB - offA // We want the tighter fits for last, so we can just pop them
}
)
const symbolStack = [
...(symbolsFound.filter(Boolean) as DocumentSymbol[])
].sort((a, b) => {
const offA =
textDocument.offsetAt(a.range.end) - textDocument.offsetAt(a.range.start)
const offB =
textDocument.offsetAt(b.range.end) - textDocument.offsetAt(b.range.start)

return offB - offA // We want the tighter fits for last, so we can just pop them
})

return [...symbolStack]
}
Expand Down
6 changes: 3 additions & 3 deletions webview/src/test/tableDataFixture.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { copyOriginalColors } from 'dvc/src/experiments/model/status/colors'
import { copyReverseOriginalColors } from 'dvc/src/experiments/model/status/colors'
import {
ExperimentStatus,
Row,
Expand Down Expand Up @@ -60,10 +60,10 @@ export const setExperimentsAsSelected = (
fixture: TableData,
labelOrIds: string[]
) => {
let colors = copyOriginalColors().reverse()
let colors = copyReverseOriginalColors()
const nextColor = () => {
if (colors.length === 0) {
colors = copyOriginalColors().reverse()
colors = copyReverseOriginalColors()
}
return colors.pop()
}
Expand Down
2 changes: 1 addition & 1 deletion webview/src/util/ids.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@ export const createIDWithPrefixAndIndex = (
export const getIDWithoutIndex = (id?: string) => id?.split(ID_SEPARATOR)[0]

export const getIDIndex = (id: string) =>
Number.parseInt(id.split(ID_SEPARATOR).reverse()[0], 10)
Number.parseInt(id.split(ID_SEPARATOR).slice(-1)[0], 10)

0 comments on commit e2768d3

Please sign in to comment.