Skip to content

Commit

Permalink
refactor(container-runtime): Enable no-explicit-any eslint rule and…
Browse files Browse the repository at this point in the history
… fix violations (#23570)

Enables `any`-related rules from the recommended eslint config and fixes
violations. Specifically,
-  `@typescript-eslint/no-explicit-any` 
- `@typescript-eslint/no-unsafe-assignment`
- `@typescript-eslint/no-unsafe-call`
- `@typescript-eslint/no-unsafe-member-access`
- `@typescript-eslint/no-unsafe-return`

This PR is one piece of a multi-PR process to migrate the
`container-runtime` package to our recommended eslint config.


[AB#3027](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/3027)
  • Loading branch information
Josmithr authored Jan 16, 2025
1 parent cfe94f1 commit 6b35040
Show file tree
Hide file tree
Showing 45 changed files with 515 additions and 288 deletions.
4 changes: 2 additions & 2 deletions common/build/eslint-config-fluid/recommended.js
Original file line number Diff line number Diff line change
Expand Up @@ -150,13 +150,13 @@ module.exports = {

/**
* Disallows calling any variable that is typed as any. The arguments to, and return value of calling an
* any typed variable are not checked at all by TypeScript.
* `any`-typed variable are not checked at all by TypeScript.
*/
"@typescript-eslint/no-unsafe-call": "error",

/**
* Disallows member access on any variable that is typed as any. The arguments to, and return value of
* calling an any typed variable are not checked at all by TypeScript.
* calling an `any`-typed variable are not checked at all by TypeScript.
*/
"@typescript-eslint/no-unsafe-member-access": "error",

Expand Down
15 changes: 15 additions & 0 deletions packages/runtime/container-runtime/.eslintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,21 @@ module.exports = {

// #region TODO:AB#3027: remove overrides and upgrade config to `recommended`

"@typescript-eslint/no-explicit-any": [
"error",
{
/**
* For certain cases, like rest parameters, any is required to allow arbitrary argument types.
* @see https://typescript-eslint.io/rules/no-explicit-any/#ignorerestargs
*/
ignoreRestArgs: true,
},
],
"@typescript-eslint/no-unsafe-assignment": "error",
"@typescript-eslint/no-unsafe-call": "error",
"@typescript-eslint/no-unsafe-member-access": "error",
"@typescript-eslint/no-unsafe-return": "error",

"jsdoc/multiline-blocks": ["error", { noSingleLineBlocks: true }],
"jsdoc/require-description": ["error", { checkConstructors: false }],

Expand Down
1 change: 1 addition & 0 deletions packages/runtime/container-runtime/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@
"@fluidframework/test-runtime-utils": "workspace:~",
"@microsoft/api-extractor": "7.47.8",
"@types/double-ended-queue": "^2.1.0",
"@types/lz4js": "^0.2.0",
"@types/mocha": "^10.0.10",
"@types/node": "^18.19.0",
"@types/sinon": "^17.0.3",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,8 @@ export class BlobManager extends TypedEventEmitter<IBlobManagerEvents> {
(error) => {
this.mc.logger.sendTelemetryEvent({
eventName: "UploadBlobReject",
// TODO: better typing
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
error,
localId,
});
Expand Down
20 changes: 8 additions & 12 deletions packages/runtime/container-runtime/src/channelCollection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ export const AllowTombstoneRequestHeaderKey = "allowTombstone"; // Belongs in th
type PendingAliasResolve = (success: boolean) => void;

interface FluidDataStoreMessage {
content: any;
content: unknown;
type: string;
}

Expand Down Expand Up @@ -217,7 +217,7 @@ function wrapContextForInnerChannel(
): IFluidParentContext {
const context = wrapContext(parentContext);

context.submitMessage = (type: string, content: any, localOpMetadata: unknown) => {
context.submitMessage = (type: string, content: unknown, localOpMetadata: unknown) => {
const fluidDataStoreContent: FluidDataStoreMessage = {
content,
type,
Expand Down Expand Up @@ -651,21 +651,18 @@ export class ChannelCollection implements IFluidDataStoreChannel, IDisposable {
this.createDataStoreId(),
pkg,
LocalDetachedFluidDataStoreContext,
undefined, // props
loadingGroupId,
);
}

public createDataStoreContext(
pkg: Readonly<string[]>,
props?: any,
loadingGroupId?: string,
): IFluidDataStoreContextInternal {
return this.createContext(
this.createDataStoreId(),
pkg,
LocalFluidDataStoreContext,
props,
loadingGroupId,
);
}
Expand All @@ -674,7 +671,6 @@ export class ChannelCollection implements IFluidDataStoreChannel, IDisposable {
id: string,
pkg: Readonly<string[]>,
contextCtor: new (props: ILocalDetachedFluidDataStoreContextProps) => T,
createProps?: any,
loadingGroupId?: string,
) {
assert(loadingGroupId !== "", 0x974 /* loadingGroupId should not be the empty string */);
Expand All @@ -689,7 +685,6 @@ export class ChannelCollection implements IFluidDataStoreChannel, IDisposable {
}),
makeLocallyVisibleFn: () => this.makeDataStoreLocallyVisible(id),
snapshotTree: undefined,
createProps,
loadingGroupId,
channelToDataStoreFn: (channel: IFluidDataStoreChannel) =>
channelToDataStore(
Expand All @@ -709,7 +704,7 @@ export class ChannelCollection implements IFluidDataStoreChannel, IDisposable {
}
public readonly dispose = () => this.disposeOnce.value;

public reSubmit(type: string, content: any, localOpMetadata: unknown) {
public reSubmit(type: string, content: unknown, localOpMetadata: unknown) {
switch (type) {
case ContainerMessageType.Attach:
case ContainerMessageType.Alias:
Expand All @@ -722,7 +717,7 @@ export class ChannelCollection implements IFluidDataStoreChannel, IDisposable {
}
}

protected reSubmitChannelOp(type: string, content: any, localOpMetadata: unknown) {
protected reSubmitChannelOp(type: string, content: unknown, localOpMetadata: unknown) {
const envelope = content as IEnvelope;
const context = this.contexts.get(envelope.address);
// If the data store has been deleted, log an error and throw an error. If there are local changes for a
Expand All @@ -740,7 +735,7 @@ export class ChannelCollection implements IFluidDataStoreChannel, IDisposable {
context.reSubmit(innerContents.type, innerContents.content, localOpMetadata);
}

public rollback(type: string, content: any, localOpMetadata: unknown) {
public rollback(type: string, content: unknown, localOpMetadata: unknown) {
assert(type === ContainerMessageType.FluidDataStoreOp, 0x8e8 /* type */);
const envelope = content as IEnvelope;
const context = this.contexts.get(envelope.address);
Expand Down Expand Up @@ -1068,7 +1063,7 @@ export class ChannelCollection implements IFluidDataStoreChannel, IDisposable {
})
.then(
(pkg) => ({ pkg, error: undefined }),
(error) => ({ pkg: undefined, error }),
(error: Error) => ({ pkg: undefined, error }),
)
.then(({ pkg, error }) => {
this.mc.logger.sendTelemetryEvent(
Expand Down Expand Up @@ -1104,6 +1099,7 @@ export class ChannelCollection implements IFluidDataStoreChannel, IDisposable {
public processSignal(messageArg: IInboundSignalMessage, local: boolean) {
const envelope = messageArg.content as IEnvelope;
const fluidDataStoreId = envelope.address;
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
const message = { ...messageArg, content: envelope.contents };
const context = this.contexts.get(fluidDataStoreId);
// If the data store has been deleted, log an error and ignore this message. This helps prevent document
Expand Down Expand Up @@ -1604,7 +1600,7 @@ export function detectOutboundReferences(
// the address of the DDS is stored in a property called "address". This is not ideal.
// An alternative would be for the op envelope to include the absolute path (built up as it is submitted)
if (key === "address" && ddsAddress === undefined) {
ddsAddress = value;
ddsAddress = value as string | undefined;
}

recursivelyFindHandles(value);
Expand Down
33 changes: 24 additions & 9 deletions packages/runtime/container-runtime/src/containerRuntime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -699,10 +699,14 @@ export function isUnpackedRuntimeMessage(message: ISequencedDocumentMessage): bo
export const agentSchedulerId = "_scheduler";

// safely check navigator and get the hardware spec value
export function getDeviceSpec() {
export function getDeviceSpec(): {
deviceMemory?: number | undefined;
hardwareConcurrency?: number | undefined;
} {
try {
if (typeof navigator === "object" && navigator !== null) {
return {
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-assignment
deviceMemory: (navigator as any).deviceMemory,
hardwareConcurrency: navigator.hardwareConcurrency,
};
Expand All @@ -718,7 +722,12 @@ export function getDeviceSpec() {
*/
export const makeLegacySendBatchFn =
(
submitFn: (type: MessageType, contents: any, batch: boolean, appData?: any) => number,
submitFn: (
type: MessageType,
contents: unknown,
batch: boolean,
appData?: unknown,
) => number,
deltaManager: Pick<IDeltaManager<unknown, unknown>, "flush">,
) =>
(batch: IBatch) => {
Expand Down Expand Up @@ -766,12 +775,15 @@ async function createSummarizer(loader: ILoader, url: string): Promise<ISummariz
if (resolvedContainer.getEntryPoint !== undefined) {
fluidObject = await resolvedContainer.getEntryPoint();
} else {
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-assignment
const response = await (resolvedContainer as any).request({
url: `/${summarizerRequestUrl}`,
});
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
if (response.status !== 200 || response.mimeType !== "fluid/object") {
throw responseToException(response, request);
}
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-assignment
fluidObject = response.value;
}

Expand Down Expand Up @@ -1181,7 +1193,7 @@ export class ContainerRuntime
return runtime;
}

public readonly options: Record<string | number, any>;
public readonly options: Record<string | number, unknown>;
private imminentClosure: boolean = false;

private readonly _getClientId: () => string | undefined;
Expand All @@ -1201,9 +1213,9 @@ export class ContainerRuntime

private readonly submitFn: (
type: MessageType,
contents: any,
contents: unknown,
batch: boolean,
appData?: any,
appData?: unknown,
) => number;
/**
* Although current IContainerContext guarantees submitBatchFn, it is not available on older loaders.
Expand Down Expand Up @@ -1659,6 +1671,7 @@ export class ContainerRuntime
eventName: "GCFeatureMatrix",
metadataValue: JSON.stringify(metadata?.gcFeatureMatrix),
inputs: JSON.stringify({
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
gcOptions_gcGeneration: this.runtimeOptions.gcOptions[gcGenerationOptionName],
}),
});
Expand Down Expand Up @@ -2464,7 +2477,7 @@ export class ContainerRuntime
// last message's sequence number.
// See also lastMessageFromMetadata()
message: explicitSchemaControl
? ({ sequenceNumber: -1 } as any as ISummaryMetadataMessage)
? ({ sequenceNumber: -1 } as unknown as ISummaryMetadataMessage)
: message,
lastMessage: explicitSchemaControl ? message : undefined,
documentSchema,
Expand Down Expand Up @@ -2598,7 +2611,7 @@ export class ContainerRuntime
// TODO: markfields: confirm Local- versus Outbound- ContainerRuntimeMessage typing
private parseLocalOpContent(serializedContents?: string): LocalContainerRuntimeMessage {
assert(serializedContents !== undefined, 0x6d5 /* content must be defined */);
const message: LocalContainerRuntimeMessage = JSON.parse(serializedContents);
const message = JSON.parse(serializedContents) as LocalContainerRuntimeMessage;
assert(message.type !== undefined, 0x6d6 /* incorrect op content format */);
return message;
}
Expand Down Expand Up @@ -3445,7 +3458,6 @@ export class ContainerRuntime
): Promise<IDataStore> {
const context = this.channelCollection.createDataStoreContext(
Array.isArray(pkg) ? pkg : [pkg],
undefined, // props
loadingGroupId,
);
return channelToDataStore(
Expand Down Expand Up @@ -3521,7 +3533,7 @@ export class ContainerRuntime
private createNewSignalEnvelope(
address: string | undefined,
type: string,
content: any,
content: unknown,
): Omit<ISignalEnvelope, "broadcastSignalSequenceNumber"> {
const newEnvelope: Omit<ISignalEnvelope, "broadcastSignalSequenceNumber"> = {
address,
Expand Down Expand Up @@ -4397,9 +4409,12 @@ export class ContainerRuntime
| ContainerMessageType.FluidDataStoreOp
| ContainerMessageType.Alias
| ContainerMessageType.Attach,
// TODO: better typing
// eslint-disable-next-line @typescript-eslint/no-explicit-any
contents: any,
localOpMetadata: unknown = undefined,
): void {
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
this.submit({ type, contents }, localOpMetadata);
}

Expand Down
9 changes: 5 additions & 4 deletions packages/runtime/container-runtime/src/dataStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,12 @@ export interface IDataStoreAliasMessage {
* @returns True if the {@link IDataStoreAliasMessage} is fully implemented, false otherwise
*/
export const isDataStoreAliasMessage = (
maybeDataStoreAliasMessage: any,
maybeDataStoreAliasMessage: unknown,
): maybeDataStoreAliasMessage is IDataStoreAliasMessage => {
return (
typeof maybeDataStoreAliasMessage?.internalId === "string" &&
typeof maybeDataStoreAliasMessage?.alias === "string"
typeof (maybeDataStoreAliasMessage as Partial<IDataStoreAliasMessage>)?.internalId ===
"string" &&
typeof (maybeDataStoreAliasMessage as Partial<IDataStoreAliasMessage>)?.alias === "string"
);
};

Expand Down Expand Up @@ -191,7 +192,7 @@ class DataStore implements IDataStore {
private async ackBasedPromise<T>(
executor: (
resolve: (value: T | PromiseLike<T>) => void,
reject: (reason?: any) => void,
reject: (reason?: unknown) => void,
) => void,
): Promise<T> {
let rejectBecauseDispose: () => void;
Expand Down
20 changes: 6 additions & 14 deletions packages/runtime/container-runtime/src/dataStoreContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,10 +154,6 @@ export interface ILocalFluidDataStoreContextProps extends IFluidDataStoreContext
readonly pkg: Readonly<string[]> | undefined;
readonly snapshotTree: ISnapshotTree | undefined;
readonly makeLocallyVisibleFn: () => void;
/**
* @deprecated 0.16 Issue #1635, #3631
*/
readonly createProps?: any;
}

/**
Expand Down Expand Up @@ -199,7 +195,7 @@ export abstract class FluidDataStoreContext
return this.pkg;
}

public get options(): Record<string | number, any> {
public get options(): Record<string | number, unknown> {
return this.parentContext.options;
}

Expand Down Expand Up @@ -295,6 +291,7 @@ export abstract class FluidDataStoreContext
// We know that if the base snapshot is omitted, then the isRootDataStore flag is not set.
// That means we can skip the expensive call to getInitialSnapshotDetails for virtualized datastores,
// and get the information from the alias map directly.
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access
if (aliasedDataStores !== undefined && (this.baseSnapshot as any)?.omitted === true) {
return aliasedDataStores.has(this.id);
}
Expand Down Expand Up @@ -815,7 +812,7 @@ export abstract class FluidDataStoreContext
return runtime.request(request);
}

public submitMessage(type: string, content: any, localOpMetadata: unknown): void {
public submitMessage(type: string, content: unknown, localOpMetadata: unknown): void {
this.verifyNotClosed("submitMessage");
assert(!!this.channel, 0x146 /* "Channel must exist when submitting message" */);
// Summarizer clients should not submit messages.
Expand Down Expand Up @@ -973,12 +970,12 @@ export abstract class FluidDataStoreContext
return {};
}

public reSubmit(type: string, contents: any, localOpMetadata: unknown) {
public reSubmit(type: string, contents: unknown, localOpMetadata: unknown) {
assert(!!this.channel, 0x14b /* "Channel must exist when resubmitting ops" */);
this.channel.reSubmit(type, contents, localOpMetadata);
}

public rollback(type: string, contents: any, localOpMetadata: unknown) {
public rollback(type: string, contents: unknown, localOpMetadata: unknown) {
if (!this.channel) {
throw new Error("Channel must exist when rolling back ops");
}
Expand All @@ -988,7 +985,7 @@ export abstract class FluidDataStoreContext
this.channel.rollback(type, contents, localOpMetadata);
}

public async applyStashedOp(contents: any): Promise<unknown> {
public async applyStashedOp(contents: unknown): Promise<unknown> {
if (!this.channel) {
await this.realize();
}
Expand Down Expand Up @@ -1251,10 +1248,6 @@ export class RemoteFluidDataStoreContext extends FluidDataStoreContext {
*/
export class LocalFluidDataStoreContextBase extends FluidDataStoreContext {
private readonly snapshotTree: ISnapshotTree | undefined;
/**
* @deprecated 0.16 Issue #1635, #3631
*/
public readonly createProps?: any;

constructor(props: ILocalFluidDataStoreContextProps) {
super(
Expand All @@ -1268,7 +1261,6 @@ export class LocalFluidDataStoreContextBase extends FluidDataStoreContext {
this.identifyLocalChangeInSummarizer("DataStoreCreatedInSummarizer");

this.snapshotTree = props.snapshotTree;
this.createProps = props.createProps;
}

public setAttachState(attachState: AttachState.Attaching | AttachState.Attached): void {
Expand Down
Loading

0 comments on commit 6b35040

Please sign in to comment.