Skip to content

Commit

Permalink
Use noUncheckedIndexedAccess as TS configuration
Browse files Browse the repository at this point in the history
This makes the code more strict even if it requires some redundant
validations.

Change-type: patch
  • Loading branch information
pipex committed Aug 22, 2024
1 parent f40e388 commit 49e25e5
Show file tree
Hide file tree
Showing 11 changed files with 51 additions and 34 deletions.
2 changes: 1 addition & 1 deletion lib/agent/runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ export class Runtime<TState> {
// If the parent does not exist or the key does not exist
// then delete the sensor
if (parent == null || !Object.hasOwn(parent, Path.basename(p))) {
this.subscriptions[p].unsubscribe();
this.subscriptions[p]!.unsubscribe();
delete this.subscriptions[p];
}
});
Expand Down
1 change: 1 addition & 0 deletions lib/dag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ function traverseCombine<V extends Value, T>(

assert(ends.length > 0, 'Malformed DAG found, empty Fork node');
const [res] = ends;
assert(res != null); // Typescript is not smart enough to figure out that res cannot be undefined
assert(!res.done, 'Malformed DAG found, disconnected fork branch');

// Combine the results from the branches passing the
Expand Down
6 changes: 2 additions & 4 deletions lib/planner/findPlan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,7 @@ function findConflict(
): [PatchOperation, PatchOperation] | undefined {
const unique = new Map<string, [number, PatchOperation]>();

for (let i = 0; i < ops.length; i++) {
const patches = ops[i];

for (const [i, patches] of ops.entries()) {
for (const o of patches) {
for (const [path, [index, op]] of unique.entries()) {
if (
Expand Down Expand Up @@ -216,7 +214,7 @@ function tryParallel<TState = any>(
// having the fork and empty node, so we need to remove the empty node
// and connect the last action in the branch directly to the existing plan
if (results.length === 1) {
const branch = results[0];
const branch = results[0]!;
// Find the first node for which the next element is the
// empty node created earlier
const last = DAG.find(
Expand Down
2 changes: 1 addition & 1 deletion lib/testing/builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ function fromFork(branches: Branch[]): [Node | null, Node | null] {

// If there is only a branch, call the branch method
if (branches.length === 1) {
return fromBranch(branches[0]);
return fromBranch(branches[0]!);
}

// For multiple branches, create a fork and
Expand Down
10 changes: 7 additions & 3 deletions lib/testing/mermaid.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,11 @@ function fromNode(
if (DAG.isFork(node)) {
const actions = node.next.filter(PlanAction.is);
if (actions.length > 0) {
const [first] = actions;
// In this case we just use id of the first action in the
// fork as we really should not have multiple fork nodes in the
// diagram pointing to the same actions
return DiagramNode.fromId(`j${actions[0].id.substring(0, 7)}`);
return DiagramNode.fromId(`j${first!.id.substring(0, 7)}`);
}

const forks = node.next.filter(DAG.isFork);
Expand Down Expand Up @@ -156,7 +157,7 @@ class DiagramAdjacency {

// The official parent of a node is the last parent
// in the list
return p[p.length - 1];
return p[p.length - 1]!;
}

getAll(node: DiagramNode): DiagramNode[] {
Expand Down Expand Up @@ -260,7 +261,10 @@ class Diagram {
ends.forEach(([_, p]) => this.graph.push(` ${p} --> ${join}`));
this.graph.push(` ${join}:::selected`);

const [first] = ends[0];
const [end] = ends;
assert(end != null);

const [first] = end;

return this.drawPlan(first.next, join);
}
Expand Down
7 changes: 7 additions & 0 deletions lib/utils/deep-equal.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { assert } from '../assert';

function isObject(value: unknown): value is object {
return typeof value === 'object' && value !== null;
}
Expand Down Expand Up @@ -33,6 +35,11 @@ export function deepEqual<T>(value: T, other: T): boolean {
const [vProps, oProps] = [value, other].map(
(a) => Object.getOwnPropertyNames(a) as Array<keyof T>,
);

// This will never fail but it prevents the compiler from
// complaining
assert(vProps != null && oProps != null);

if (vProps.length !== oProps.length) {
// If the property lists are different lengths we don't need
// to check any further
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@
"tar-stream": "3.0.0",
"ts-node": "^10.9.1",
"tsconfig-paths": "^4.1.1",
"typescript": "^5.5.2"
"typescript": "^5.5.4"
},
"dependencies": {
"mahler-wasm": "^0.1.0"
Expand Down
2 changes: 1 addition & 1 deletion tests/composer/tasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ export const installService = App.task({
})();
const s: Service = {
image:
(await docker.getImage(existing.Image).inspect()).RepoTags[0] ||
(await docker.getImage(existing.Image).inspect()).RepoTags[0] ??
existing.Image,
startedAt: new Date(existing.State.StartedAt),
createdAt: new Date(existing.Created),
Expand Down
24 changes: 12 additions & 12 deletions tests/orchestrator/tasks.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ describe('orchestrator/tasks', () => {
s = await doFetch(s);
expect(s.apps['a0']).to.not.be.undefined;
expect(s.images).to.have.property('a0_my-service:r0');
const { dockerId: imageId } = s.images['a0_my-service:r0'];
const { dockerId: imageId } = s.images['a0_my-service:r0']!;
expect(imageId).to.not.be.undefined;
expect(await docker.getImage(imageId!).inspect())
.to.have.property('RepoTags')
Expand Down Expand Up @@ -138,12 +138,12 @@ describe('orchestrator/tasks', () => {

it('should create a container', async () => {
let s = await doInstall(s0);
expect(s.apps['a0'].releases['r0']).to.not.be.undefined;
expect(s.apps['a0'].releases['r0'].services['my-service']).to.not.be
expect(s.apps['a0']!.releases['r0']).to.not.be.undefined;
expect(s.apps['a0']!.releases['r0']!.services['my-service']).to.not.be
.undefined;

const { containerId } =
s.apps['a0'].releases['r0'].services['my-service'];
s.apps['a0']!.releases['r0']!.services['my-service']!;

expect(containerId).to.not.be.undefined;
const container = await docker.getContainer(containerId!).inspect();
Expand All @@ -153,8 +153,8 @@ describe('orchestrator/tasks', () => {
// Installing again with an outated state should
// not create a new container
s = await doInstall(s0);
expect(s.apps['a0'].releases['r0']).to.not.be.undefined;
expect(s.apps['a0'].releases['r0'].services['my-service']).to.not.be
expect(s.apps['a0']!.releases['r0']).to.not.be.undefined;
expect(s.apps['a0']!.releases['r0']!.services['my-service']).to.not.be
.undefined;

expect(await docker.getContainer(containerId!).inspect())
Expand Down Expand Up @@ -223,15 +223,15 @@ describe('orchestrator/tasks', () => {

it('should start a container', async () => {
let s = await doStart(s0);
expect(s.apps['a0'].releases['r0']).to.not.be.undefined;
expect(s.apps['a0'].releases['r0'].services['my-service']).to.not.be
expect(s.apps['a0']!.releases['r0']).to.not.be.undefined;
expect(s.apps['a0']!.releases['r0']!.services['my-service']).to.not.be
.undefined;
expect(
s.apps['a0'].releases['r0'].services['my-service'].status,
s.apps['a0']!.releases['r0']!.services['my-service']!.status,
).to.equal('running');

const { containerId } =
s.apps['a0'].releases['r0'].services['my-service'];
s.apps['a0']!.releases['r0']!.services['my-service']!;

expect(containerId).to.not.be.undefined;
const container = await docker.getContainer(containerId!).inspect();
Expand All @@ -242,8 +242,8 @@ describe('orchestrator/tasks', () => {
// Starting again with an outated state should
// not start the container again
s = await doStart(s0);
expect(s.apps['a0'].releases['r0']).to.not.be.undefined;
expect(s.apps['a0'].releases['r0'].services['my-service']).to.not.be
expect(s.apps['a0']!.releases['r0']).to.not.be.undefined;
expect(s.apps['a0']!.releases['r0']!.services['my-service']).to.not.be
.undefined;

expect(await docker.getContainer(containerId!).inspect())
Expand Down
28 changes: 17 additions & 11 deletions tests/orchestrator/tasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ export const startService = Task.of<Device>().from({
service,
{ releaseUuid, appUuid, serviceName, system: device, target },
) => {
const { releases } = device.apps[appUuid];
const { releases } = device.apps[appUuid]!;

// The task can be applied if the following conditions are met:
return (
Expand All @@ -287,7 +287,7 @@ export const startService = Task.of<Device>().from({
// service cannot be running
Object.keys(releases)
.filter((u) => u !== releaseUuid)
.every((u) => releases[u].services[serviceName]?.status !== 'running')
.every((u) => releases[u]!.services[serviceName]?.status !== 'running')
);
},
effect: (service) => {
Expand Down Expand Up @@ -341,10 +341,13 @@ const renameServiceContainer = Task.of<Device>().from({
_,
{ appUuid, releaseUuid, serviceName, system: device, target },
) => {
const { releases } = device.apps[appUuid];
const { releases } = device.apps[appUuid]!;
const [currentRelease] = Object.keys(releases).filter(
(u) => u !== releaseUuid,
);
if (currentRelease == null) {
return false;
}
const currService = releases[currentRelease]?.services[serviceName];
return (
currService != null &&
Expand All @@ -353,7 +356,7 @@ const renameServiceContainer = Task.of<Device>().from({
);
},
effect: (service, { system: device, appUuid, serviceName, releaseUuid }) => {
const { releases } = device.apps[appUuid];
const { releases } = device.apps[appUuid]!;
const currRelease = Object.keys(releases).find((u) => u !== releaseUuid)!;
const currService = releases[currRelease]?.services[serviceName];

Expand All @@ -364,10 +367,9 @@ const renameServiceContainer = Task.of<Device>().from({
service,
{ system: device, appUuid, serviceName, releaseUuid },
) => {
const { releases } = device.apps[appUuid];
const { releases } = device.apps[appUuid]!;
const currRelease = Object.keys(releases).find((u) => u !== releaseUuid)!;

const currService = releases[currRelease]?.services[serviceName];
const currService = releases[currRelease]!.services[serviceName]!;

// Rename the container
await docker.getContainer(currService.containerId!).rename({
Expand Down Expand Up @@ -399,10 +401,14 @@ export const migrateService = Task.of<Device>().from({
_,
{ appUuid, releaseUuid, serviceName, system: device, target },
) => {
const { releases } = device.apps[appUuid];
const { releases } = device.apps[appUuid]!;
const [currentRelease] = Object.keys(releases).filter(
(u) => u !== releaseUuid,
);
if (!currentRelease) {
return false;
}

const currService = releases[currentRelease]?.services[serviceName];
return (
currService != null &&
Expand All @@ -414,7 +420,7 @@ export const migrateService = Task.of<Device>().from({
_,
{ system: device, appUuid, serviceName, releaseUuid, target },
) => {
const { releases } = device.apps[appUuid];
const { releases } = device.apps[appUuid]!;
const currRelease = Object.keys(releases).find((u) => u !== releaseUuid)!;
return [
renameServiceContainer({ target, appUuid, serviceName, releaseUuid }),
Expand Down Expand Up @@ -443,15 +449,15 @@ export const stopService = Task.of<Device>().from({
service,
{ system: device, appUuid, releaseUuid, serviceName },
) => {
const { releases } = device.apps[appUuid];
const { releases } = device.apps[appUuid]!;
return (
service?.containerId != null &&
service?.status === 'running' &&
Object.keys(releases)
.filter((u) => u !== releaseUuid)
.every(
// If there are equivalent services from other releases they should at least have a container
(u) => releases[u].services[serviceName]?.containerId != null,
(u) => releases[u]!.services[serviceName]?.containerId != null,
)
);
},
Expand Down
1 change: 1 addition & 0 deletions tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
"target": "es2022",
"declaration": true,
"skipLibCheck": true,
"noUncheckedIndexedAccess": true,
"paths": {
"mahler": [
"lib/index.ts"
Expand Down

0 comments on commit 49e25e5

Please sign in to comment.