From 9b6ccd9e5a915c2a9665872b3b45ce07f6b47316 Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Mon, 12 Oct 2026 08:00:28 +0000 Subject: [PATCH 1/2] Check every index read, everywhere: noUncheckedIndexedAccess in tsconfig.json The workspace libraries' last 84 findings handled, so the flag moves from the app's and the embed's own configs into tsconfig.json, and every build -- both libraries, the table web component, the app, the embed -- enforces it. The counted check that held the libraries, its baseline and its CI and preflight steps are removed. reactome-table's store is the spreadsheet behind the GSA form's data entry and had two unit tests. A session test now drives it through generate, import, type, move, select, paste, delete, add and remove rows and columns, and snapshots its state after every step; the snapshots were recorded against the old code, and the rewrite reproduces them exactly. Reads that always threw on a missing row or cell still do, through rowAt/cellAt/maxAt, now saying where. reactome-gsa-form: a parameter type the form does not know, one the GSA service added since, is read as text rather than throwing (parserFor, with a spec); an empty method list or an annotation table with no header row gives an empty value rather than a crash. With the flag in the root config, type-aware lint sees spec and e2e files under it too: three index reads there are guarded. Closes #345. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/tests.yml | 6 - e2e/reacfoam.spec.ts | 4 +- indexed-access-baseline.json | 15 - package.json | 1 - .../tsconfig.app.json | 5 +- .../local-data/local-data.component.ts | 13 +- .../statistical-design.component.ts | 2 +- .../lib/services/analysis-methods.service.ts | 12 +- .../src/lib/services/parse-parameter.spec.ts | 18 + .../dataset-source/dataset-source.effect.ts | 6 +- .../src/lib/state/dataset/dataset.effect.ts | 3 +- .../src/lib/state/dataset/dataset.reducer.ts | 7 +- .../src/lib/state/dataset/dataset.selector.ts | 11 +- .../src/lib/state/method/method.effect.ts | 6 +- .../src/lib/state/method/method.reducer.ts | 3 +- .../src/lib/utilities/species/species.pipe.ts | 3 +- .../lib/component/table/table.component.html | 2 +- .../table.store.session.spec.ts.snap | 1176 +++++++++++++++++ .../src/lib/state/table.store.session.spec.ts | 187 +++ .../src/lib/state/table.store.ts | 146 +- .../src/lib/utils/table.util.ts | 12 +- .../app/search/answer/chat-handoff.spec.ts | 5 +- scripts/check-indexed-access.mjs | 156 --- scripts/preflight.sh | 1 - .../check-indexed-access.spec.mjs | 54 - tsconfig.app.json | 5 +- tsconfig.json | 17 +- 27 files changed, 1542 insertions(+), 334 deletions(-) delete mode 100644 indexed-access-baseline.json create mode 100644 projects/reactome-gsa-form/src/lib/services/parse-parameter.spec.ts create mode 100644 projects/reactome-table/src/lib/state/__snapshots__/table.store.session.spec.ts.snap create mode 100644 projects/reactome-table/src/lib/state/table.store.session.spec.ts delete mode 100644 scripts/check-indexed-access.mjs delete mode 100644 tools/indexed-access-check/check-indexed-access.spec.mjs diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index bd8ed972..16c23cf9 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -87,12 +87,6 @@ jobs: - name: Lint run: npm run check:lint - # noUncheckedIndexedAccess is a compiler flag with no warning level, so it - # is held at a per-file count until it reaches zero and can be enabled - # (#345). Like the lint warnings, a count can only go down. - - name: Unchecked indexed access has not increased - run: npm run check:indexed - # eslint sees unused symbols within a file; knip sees whole files and # exports nothing imports, which was the bigger pile. Also baselined, # because some findings need someone who knows the data model to judge -- diff --git a/e2e/reacfoam.spec.ts b/e2e/reacfoam.spec.ts index 2f533d81..104f3249 100644 --- a/e2e/reacfoam.spec.ts +++ b/e2e/reacfoam.spec.ts @@ -58,7 +58,9 @@ async function centreOf(page: Page, label: string) { function darkInTitleStrip(page: Page) { return page.evaluate(() => { const canvases = [...document.querySelectorAll('cr-reacfoam canvas')]; - const { width, height } = canvases[0]; + const [first] = canvases; + if (!first) throw new Error('no reacfoam canvas'); + const { width, height } = first; const top = Math.floor(height * 0.85); const all = document.createElement('canvas'); all.width = width; diff --git a/indexed-access-baseline.json b/indexed-access-baseline.json deleted file mode 100644 index 09f0bf9b..00000000 --- a/indexed-access-baseline.json +++ /dev/null @@ -1,15 +0,0 @@ -{ - "total": 84, - "files": { - "projects/reactome-gsa-form/src/lib/dataset-form/datasets/select-dataset/dataset-types/local-data/local-data.component.ts": 4, - "projects/reactome-gsa-form/src/lib/state/dataset-source/dataset-source.effect.ts": 2, - "projects/reactome-gsa-form/src/lib/state/dataset/dataset.effect.ts": 1, - "projects/reactome-gsa-form/src/lib/state/dataset/dataset.reducer.ts": 3, - "projects/reactome-gsa-form/src/lib/state/dataset/dataset.selector.ts": 4, - "projects/reactome-gsa-form/src/lib/state/method/method.effect.ts": 2, - "projects/reactome-gsa-form/src/lib/state/method/method.reducer.ts": 1, - "projects/reactome-gsa-form/src/lib/utilities/species/species.pipe.ts": 1, - "projects/reactome-table/src/lib/state/table.store.ts": 63, - "projects/reactome-table/src/lib/utils/table.util.ts": 3 - } -} diff --git a/package.json b/package.json index df104867..07faf186 100644 --- a/package.json +++ b/package.json @@ -40,7 +40,6 @@ "lint": "eslint .", "lint:fix": "eslint . --fix", "check:lint": "node scripts/check-lint.mjs", - "check:indexed": "node scripts/check-indexed-access.mjs", "check:dead": "node scripts/check-dead-code.mjs", "check:lockfile": "node scripts/check-lockfile.mjs", "dead": "knip", diff --git a/projects/reactome-diagram-element/tsconfig.app.json b/projects/reactome-diagram-element/tsconfig.app.json index 88902e4e..28bf5940 100644 --- a/projects/reactome-diagram-element/tsconfig.app.json +++ b/projects/reactome-diagram-element/tsconfig.app.json @@ -2,10 +2,7 @@ "extends": "../../tsconfig.json", "compilerOptions": { "outDir": "../../out-tsc/reactome-diagram-element", - "types": [], - // Every index read checked (#345). Not yet in tsconfig.json: the workspace - // libraries have findings left, held at a count by scripts/check-indexed-access.mjs. - "noUncheckedIndexedAccess": true + "types": [] }, "files": ["src/main.ts"], "include": ["../pathway-browser/src/**/*.d.ts", "../../src/**/*.d.ts"] diff --git a/projects/reactome-gsa-form/src/lib/dataset-form/datasets/select-dataset/dataset-types/local-data/local-data.component.ts b/projects/reactome-gsa-form/src/lib/dataset-form/datasets/select-dataset/dataset-types/local-data/local-data.component.ts index d533cbcb..7c5ea7e6 100644 --- a/projects/reactome-gsa-form/src/lib/dataset-form/datasets/select-dataset/dataset-types/local-data/local-data.component.ts +++ b/projects/reactome-gsa-form/src/lib/dataset-form/datasets/select-dataset/dataset-types/local-data/local-data.component.ts @@ -141,10 +141,9 @@ export class LocalDataComponent { } const lines = text.trim().split('\n'); - for (let i = 1; i < lines.length; i++) { - const columns = lines[i].split('\t'); - for (let j = 1; j < columns.length; j++) { - const value = columns[j].trim(); + for (const line of lines.slice(1)) { + for (const column of line.split('\t').slice(1)) { + const value = column.trim(); if (!Number.isInteger(Number(value))) { return resolve(false); } @@ -166,10 +165,10 @@ export class LocalDataComponent { this.readFile(fileRibo), ]); - const file1FirstLine = file1Content.split('\n')[0].trim(); - const file2FirstLine = file2Content.split('\n')[0].trim(); + const [file1FirstLine = ''] = file1Content.split('\n'); + const [file2FirstLine = ''] = file2Content.split('\n'); - return file1FirstLine === file2FirstLine; + return file1FirstLine.trim() === file2FirstLine.trim(); } catch (error) { return false; } diff --git a/projects/reactome-gsa-form/src/lib/dataset-form/datasets/statistical-design/statistical-design.component.ts b/projects/reactome-gsa-form/src/lib/dataset-form/datasets/statistical-design/statistical-design.component.ts index 9c338f6b..520022d1 100644 --- a/projects/reactome-gsa-form/src/lib/dataset-form/datasets/statistical-design/statistical-design.component.ts +++ b/projects/reactome-gsa-form/src/lib/dataset-form/datasets/statistical-design/statistical-design.component.ts @@ -74,7 +74,7 @@ export class StatisticalDesignComponent implements OnInit { this.store.dispatch(datasetActions.setCovariatesValue({ id: this.datasetId(), value: value })); } - comparisonGroup2Values(groups: string[], group1: string) { + comparisonGroup2Values(groups: string[] | undefined, group1: string) { if (!groups) return []; return groups.filter((group) => group !== group1); } diff --git a/projects/reactome-gsa-form/src/lib/services/analysis-methods.service.ts b/projects/reactome-gsa-form/src/lib/services/analysis-methods.service.ts index a36cedcb..250afffe 100644 --- a/projects/reactome-gsa-form/src/lib/services/analysis-methods.service.ts +++ b/projects/reactome-gsa-form/src/lib/services/analysis-methods.service.ts @@ -6,7 +6,10 @@ import { extractErrorMessage } from '../utilities/utils'; import { MatSnackBar } from '@angular/material/snack-bar'; import { ConfigProvider, REACTOME_GSA_CONFIG } from '../config/gsa-config'; -export const typeToParse: { [p: string]: (value: string) => any } = { +/** Reads a parameter's text as its type: a boolean, a number, or the text itself. */ +type Parser = (value: string) => any; + +const typeToParse: { [p: string]: Parser } = { bool: (value) => value.toLowerCase() === 'true', float: parseFloat, int: parseInt, @@ -14,6 +17,13 @@ export const typeToParse: { [p: string]: (value: string) => any } = { string: (value) => value, }; +/** + * How to read a parameter of this type. A type this form does not know -- one + * the service added since -- is read as text, rather than taking the whole form + * down with it. + */ +export const parserFor = (type: string): Parser => typeToParse[type] ?? ((text: string) => text); + @Injectable({ providedIn: 'root', }) diff --git a/projects/reactome-gsa-form/src/lib/services/parse-parameter.spec.ts b/projects/reactome-gsa-form/src/lib/services/parse-parameter.spec.ts new file mode 100644 index 00000000..d2f69be8 --- /dev/null +++ b/projects/reactome-gsa-form/src/lib/services/parse-parameter.spec.ts @@ -0,0 +1,18 @@ +import { describe, expect, it } from 'vitest'; +import { parserFor } from './analysis-methods.service'; + +// A method's parameters come from the GSA service, typed by name. A type this +// form does not know -- one added since -- is read as text rather than taking +// the form down. +describe('parserFor', () => { + it('reads each known type', () => { + expect(parserFor('bool')('True')).toBe(true); + expect(parserFor('float')('0.05')).toBe(0.05); + expect(parserFor('int')('7')).toBe(7); + expect(parserFor('string')('x')).toBe('x'); + }); + + it('reads a type it does not know as text', () => { + expect(parserFor('ratio')('1:2')).toBe('1:2'); + }); +}); diff --git a/projects/reactome-gsa-form/src/lib/state/dataset-source/dataset-source.effect.ts b/projects/reactome-gsa-form/src/lib/state/dataset-source/dataset-source.effect.ts index 91dbf834..7f54c612 100644 --- a/projects/reactome-gsa-form/src/lib/state/dataset-source/dataset-source.effect.ts +++ b/projects/reactome-gsa-form/src/lib/state/dataset-source/dataset-source.effect.ts @@ -3,7 +3,7 @@ import { Injectable, inject } from '@angular/core'; import { catchError, exhaustMap, map, mergeMap, of, take } from 'rxjs'; import { FetchDatasetService } from '../../services/fetch-dataset.service'; import { datasetSourceActions } from './dataset-source.action'; -import { typeToParse } from '../../services/analysis-methods.service'; +import { parserFor } from '../../services/analysis-methods.service'; @Injectable() export class DatasetSourceEffects { @@ -50,8 +50,8 @@ export class DatasetSourceEffects { externals.forEach((external) => external.parameters.forEach((param) => { // param.id = `${external.id} - ${param.name}`; - param.value = typeToParse[param.type](param.default); - param.default = typeToParse[param.type](param.default); + param.value = parserFor(param.type)(param.default); + param.default = parserFor(param.type)(param.default); }) ); return externals; diff --git a/projects/reactome-gsa-form/src/lib/state/dataset/dataset.effect.ts b/projects/reactome-gsa-form/src/lib/state/dataset/dataset.effect.ts index 60a839e5..9377e617 100644 --- a/projects/reactome-gsa-form/src/lib/state/dataset/dataset.effect.ts +++ b/projects/reactome-gsa-form/src/lib/state/dataset/dataset.effect.ts @@ -211,7 +211,8 @@ export class DatasetEffects { ['', ...summary.sample_metadata.map((col) => col.name)], ...summary.sample_ids.map((sampleId, i) => [ sampleId, - ...summary.sample_metadata!.map((column) => column.values[i]), + // A missing value reads as an empty cell, as the table treats one. + ...summary.sample_metadata!.map((column) => column.values[i] ?? ''), ]), ]; return datasetActions.setAnnotations({ annotations: table, id }); diff --git a/projects/reactome-gsa-form/src/lib/state/dataset/dataset.reducer.ts b/projects/reactome-gsa-form/src/lib/state/dataset/dataset.reducer.ts index 31ebffed..02e8c98c 100644 --- a/projects/reactome-gsa-form/src/lib/state/dataset/dataset.reducer.ts +++ b/projects/reactome-gsa-form/src/lib/state/dataset/dataset.reducer.ts @@ -134,13 +134,14 @@ export const datasetReducer: ActionReducer = createReducer( const annotations = dataset.annotations!; const annotationsWithSimpleIds = cp(annotations); - annotationsWithSimpleIds[0][0] = 'sample_ids'; + const [header] = annotationsWithSimpleIds; + if (header) header[0] = 'sample_ids'; return { id: dataset.id, changes: { annotationColumns: new Map( - transpose(annotationsWithSimpleIds).map((col) => [col[0], col.slice(1)]) + transpose(annotationsWithSimpleIds).map((col) => [col[0] ?? '', col.slice(1)]) ), }, }; @@ -213,7 +214,7 @@ export const datasetReducer: ActionReducer = createReducer( ...dataset.statisticalDesign!, covariances: dataset.annotations?.[0] - .slice(1) //slice the first element, which is the empty string + ?.slice(1) //slice the first element, which is the empty string .filter((name) => name !== dataset.statisticalDesign!.analysisGroup) .map((name) => ({ name, diff --git a/projects/reactome-gsa-form/src/lib/state/dataset/dataset.selector.ts b/projects/reactome-gsa-form/src/lib/state/dataset/dataset.selector.ts index 99628549..d02406ad 100644 --- a/projects/reactome-gsa-form/src/lib/state/dataset/dataset.selector.ts +++ b/projects/reactome-gsa-form/src/lib/state/dataset/dataset.selector.ts @@ -84,7 +84,7 @@ export const datasetFeature = createFeature({ .slice(1) // Remove row ids column .map((column) => ({ // Create groups with distinct values - name: column[0], + name: column[0] ?? '', values: new Set(column.slice(1).filter((s) => s)), })) .filter((group) => group.values.size >= 2) // Filter groups suitable for analysis @@ -115,19 +115,20 @@ export const datasetFeature = createFeature({ .slice(1) // Remove row ids column .map((column) => ({ // Create groups with distinct values - name: column[0], + name: column[0] ?? '', values: column.slice(1), })) .filter((column) => dataset!.statisticalDesign!.analysisGroup !== column.name) // Remove analysis group .filter((column) => { - const group1Values = new Set(); - const group2Values = new Set(); + // A missing value counts as one, as it always has. + const group1Values = new Set(); + const group2Values = new Set(); for (let i = 0; i < analysisColumn.length; i++) { const analysedValue = analysisColumn[i]; for (const [comparisonGroup, values] of [ [dataset!.statisticalDesign!.comparisonGroup1, group1Values], [dataset!.statisticalDesign!.comparisonGroup2, group2Values], - ] as [string, Set][]) { + ] as [string, Set][]) { if (analysedValue === comparisonGroup) { const value = column.values[i]; if (values.has(value)) return false; // remove columns where a "pair" is associated more than once to the same condition diff --git a/projects/reactome-gsa-form/src/lib/state/method/method.effect.ts b/projects/reactome-gsa-form/src/lib/state/method/method.effect.ts index 573ff239..65e181cb 100644 --- a/projects/reactome-gsa-form/src/lib/state/method/method.effect.ts +++ b/projects/reactome-gsa-form/src/lib/state/method/method.effect.ts @@ -1,6 +1,6 @@ import { Actions, createEffect, ofType } from '@ngrx/effects'; import { Injectable, inject } from '@angular/core'; -import { AnalysisMethodsService, typeToParse } from '../../services/analysis-methods.service'; +import { AnalysisMethodsService, parserFor } from '../../services/analysis-methods.service'; import { methodActions } from './method.action'; import { catchError, exhaustMap, map, of } from 'rxjs'; import { ParameterType } from '../../model/methods.model'; @@ -21,8 +21,8 @@ export class MethodEffects { methods.forEach((method) => { method.parameters.forEach((param) => { param.type = param.name === 'email' ? ParameterType.email : param.type; - param.value = typeToParse[param.type](param.default); - param.default = typeToParse[param.type](param.default); + param.value = parserFor(param.type)(param.default); + param.default = parserFor(param.type)(param.default); }); const server = method.parameters.find((param) => param.name === 'reactome_server'); if (server) server.value = this.config().server; diff --git a/projects/reactome-gsa-form/src/lib/state/method/method.reducer.ts b/projects/reactome-gsa-form/src/lib/state/method/method.reducer.ts index 787c8b05..71e33bdf 100644 --- a/projects/reactome-gsa-form/src/lib/state/method/method.reducer.ts +++ b/projects/reactome-gsa-form/src/lib/state/method/method.reducer.ts @@ -15,7 +15,8 @@ export const methodReducer: ActionReducer = createReducer( })), { ...state, - commonParameters: methods[0].parameters.filter((param) => param.scope === 'common'), // They are transferred to global status + // They are transferred to global status; no methods have none. + commonParameters: methods[0]?.parameters.filter((param) => param.scope === 'common') ?? [], } ) ), diff --git a/projects/reactome-gsa-form/src/lib/utilities/species/species.pipe.ts b/projects/reactome-gsa-form/src/lib/utilities/species/species.pipe.ts index 6c319f8c..b5b2b60d 100644 --- a/projects/reactome-gsa-form/src/lib/utilities/species/species.pipe.ts +++ b/projects/reactome-gsa-form/src/lib/utilities/species/species.pipe.ts @@ -4,6 +4,7 @@ import { Pipe, PipeTransform } from '@angular/core'; export class SpeciesPipe implements PipeTransform { transform(value: string): unknown { const words = value.split(' '); - return words[0].charAt(0).toUpperCase() + '. ' + words.slice(1).join(' '); + const [genus = ''] = words; + return genus.charAt(0).toUpperCase() + '. ' + words.slice(1).join(' '); } } diff --git a/projects/reactome-table/src/lib/component/table/table.component.html b/projects/reactome-table/src/lib/component/table/table.component.html index f24d798c..f48dbe61 100644 --- a/projects/reactome-table/src/lib/component/table/table.component.html +++ b/projects/reactome-table/src/lib/component/table/table.component.html @@ -147,7 +147,7 @@ @if (settings.addRow) { ... - +