Skip to content

Commit

Permalink
Resolve/ignore eslint warnings (slackapi#1095)
Browse files Browse the repository at this point in the history
* Resolve/ignore eslint warnings
* Move eslint-disable in tests to .eslintrc.js
  • Loading branch information
seratch authored Aug 29, 2021
1 parent 8326547 commit 992b64a
Show file tree
Hide file tree
Showing 21 changed files with 68 additions and 7 deletions.
7 changes: 7 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,13 @@ module.exports = {
assertionStyle: 'as',
objectLiteralTypeAssertions: 'allow',
}],
// Using any types is so useful for mock objects, we are fine with disabling this rule
'@typescript-eslint/no-explicit-any': 'off',
// Some parts in Bolt (e.g., listener arguments) are unnecessarily optional.
// It's okay to omit this validation in tests.
'@typescript-eslint/no-non-null-assertion': 'off',
// Using ununamed functions (e.g., null logger) in tests is fine
'func-names': 'off',
// In tests, don't force constructing a Symbol with a descriptor, as
// it's probably just for tests
'symbol-description': 'off',
Expand Down
15 changes: 12 additions & 3 deletions src/App.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,13 +127,16 @@ export interface AuthorizeResult {
botUserId?: string; // optional but allows `ignoreSelf` global middleware be more filter more than just message events
teamId?: string;
enterpriseId?: string;
// TODO: for better type safety, we may want to reivit this
// eslint-disable-next-line @typescript-eslint/no-explicit-any
[key: string]: any;
}

export interface ActionConstraints<A extends SlackAction = SlackAction> {
type?: A['type'];
block_id?: A extends BlockAction ? string | RegExp : never;
action_id?: A extends BlockAction ? string | RegExp : never;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
callback_id?: Extract<A, { callback_id?: string }> extends any ? string | RegExp : never;
}

Expand Down Expand Up @@ -365,9 +368,10 @@ export default class App {
}

let usingOauth = false;
const httpReceiver = (this.receiver as HTTPReceiver);
if (
(this.receiver as HTTPReceiver).installer !== undefined &&
(this.receiver as HTTPReceiver).installer!.authorize !== undefined
httpReceiver.installer !== undefined &&
httpReceiver.installer.authorize !== undefined
) {
// This supports using the built in HTTPReceiver, declaring your own HTTPReceiver
// and theoretically, doing a fully custom (non express) receiver that implements OAuth
Expand Down Expand Up @@ -396,7 +400,8 @@ export default class App {
} else if (authorize !== undefined && usingOauth) {
throw new AppInitializationError(`Both authorize options and oauth installer options provided. ${tokenUsage}`);
} else if (authorize === undefined && usingOauth) {
this.authorize = (this.receiver as HTTPReceiver).installer!.authorize;
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
this.authorize = httpReceiver.installer!.authorize;
} else if (authorize !== undefined && !usingOauth) {
this.authorize = authorize;
} else {
Expand Down Expand Up @@ -455,6 +460,7 @@ export default class App {
return this.receiver.start(...args) as ReturnType<HTTPReceiver['start']>;
}

// eslint-disable-next-line @typescript-eslint/no-explicit-any
public stop(...args: any[]): Promise<unknown> {
return this.receiver.stop(...args);
}
Expand Down Expand Up @@ -693,6 +699,7 @@ export default class App {
authorizeResult = await this.authorize(source as AuthorizeSourceData<false>, bodyArg);
}
} catch (error) {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const e = error as any;
this.logger.warn('Authorization of incoming event did not succeed. No listeners will be called.');
e.code = ErrorCode.AuthorizationError;
Expand Down Expand Up @@ -766,6 +773,7 @@ export default class App {
/** Respond function might be set below */
respond?: RespondFn;
/** Ack function might be set below */
// eslint-disable-next-line @typescript-eslint/no-explicit-any
ack?: AckFn<any>;
} = {
body: bodyArg,
Expand Down Expand Up @@ -894,6 +902,7 @@ export default class App {
},
);
} catch (error) {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const e = error as any;
// disabling due to https://github.com/typescript-eslint/typescript-eslint/issues/1277
// eslint-disable-next-line consistent-return
Expand Down
1 change: 1 addition & 0 deletions src/WorkflowStep.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,7 @@ describe('WorkflowStep', () => {
const { processStepMiddleware } = await importWorkflowStep();

const fn1 = sinon.spy((async ({ next: continuation }) => {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await continuation!();
}) as Middleware<WorkflowStepEdit>);
const fn2 = sinon.spy(async () => {});
Expand Down
3 changes: 3 additions & 0 deletions src/WorkflowStep.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
import {
KnownBlock,
Block,
Expand Down Expand Up @@ -153,6 +154,8 @@ export class WorkflowStep {
if (isStepEvent(args) && this.matchesConstraints(args)) {
return this.processEvent(args);
}
// `next` here exists for sure bu the typing on the MiddlwareArgs is not great enough
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
return args.next!();
};
}
Expand Down
2 changes: 2 additions & 0 deletions src/conversation-store.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
import { Middleware, AnyMiddlewareArgs } from './types';
import { getTypeAndConversation } from './helpers';

Expand Down Expand Up @@ -74,6 +75,7 @@ export function conversationContext<ConversationState = any>(
logger.debug('No conversation ID for incoming event');
}
// TODO: remove the non-null assertion operator
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await next!();
};
}
2 changes: 2 additions & 0 deletions src/helpers.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
import {
SlackEventMiddlewareArgs,
SlackCommandMiddlewareArgs,
Expand Down Expand Up @@ -51,6 +52,7 @@ export function getTypeAndConversation(body: any): { type?: IncomingEventType; c
// Using non-null assertion (!) because the alternative is to use `foundConversation: (string | undefined)`, which
// impedes the very useful type checker help above that ensures the value is only defined to strings, not
// undefined. This is safe when used in combination with the || operator with a default value.
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
return foundConversationId! || undefined;
})();

Expand Down
13 changes: 13 additions & 0 deletions src/middleware/builtin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export const onlyActions: Middleware<AnyMiddlewareArgs & { action?: SlackAction

// It matches so we should continue down this middleware listener chain
// TODO: remove the non-null assertion operator
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await next!();
};

Expand All @@ -57,6 +58,7 @@ export const onlyShortcuts: Middleware<AnyMiddlewareArgs & { shortcut?: SlackSho

// It matches so we should continue down this middleware listener chain
// TODO: remove the non-null assertion operator
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await next!();
};

Expand All @@ -71,6 +73,7 @@ export const onlyCommands: Middleware<AnyMiddlewareArgs & { command?: SlashComma

// It matches so we should continue down this middleware listener chain
// TODO: remove the non-null assertion operator
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await next!();
};

Expand All @@ -85,6 +88,7 @@ export const onlyOptions: Middleware<AnyMiddlewareArgs & { options?: SlackOption

// It matches so we should continue down this middleware listener chain
// TODO: remove the non-null assertion operator
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await next!();
};

Expand All @@ -99,6 +103,7 @@ export const onlyEvents: Middleware<AnyMiddlewareArgs & { event?: SlackEvent }>

// It matches so we should continue down this middleware listener chain
// TODO: remove the non-null assertion operator
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await next!();
};

Expand All @@ -113,6 +118,7 @@ export const onlyViewActions: Middleware<AnyMiddlewareArgs & { view?: ViewOutput

// It matches so we should continue down this middleware listener chain
// TODO: remove the non-null assertion operator
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await next!();
};

Expand Down Expand Up @@ -201,6 +207,7 @@ export function matchConstraints(
}

// TODO: remove the non-null assertion operator
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await next!();
};
}
Expand Down Expand Up @@ -234,6 +241,7 @@ export function matchMessage(
}

// TODO: remove the non-null assertion operator
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await next!();
};
}
Expand All @@ -249,6 +257,7 @@ export function matchCommandName(pattern: string | RegExp): Middleware<SlackComm
}

// TODO: remove the non-null assertion operator
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await next!();
};
}
Expand Down Expand Up @@ -286,6 +295,7 @@ export function matchEventType(pattern: EventTypePattern): Middleware<SlackEvent
}

// TODO: remove the non-null assertion operator
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await next!();
};
}
Expand Down Expand Up @@ -320,6 +330,7 @@ export function ignoreSelf(): Middleware<AnyMiddlewareArgs> {

// If all the previous checks didn't skip this message, then its okay to resume to next
// TODO: remove the non-null assertion operator
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await args.next!();
};
}
Expand All @@ -328,6 +339,7 @@ export function subtype(subtype1: string): Middleware<SlackEventMiddlewareArgs<'
return async ({ message, next }) => {
if (message.subtype === subtype1) {
// TODO: remove the non-null assertion operator
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await next!();
}
};
Expand Down Expand Up @@ -365,6 +377,7 @@ export function directMention(): Middleware<SlackEventMiddlewareArgs<'message'>>
}

// TODO: remove the non-null assertion operator
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await next!();
};
}
Expand Down
1 change: 1 addition & 0 deletions src/receivers/AwsLambdaReceiver.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
import { Logger, ConsoleLogger, LogLevel } from '@slack/logger';
import querystring from 'querystring';
import crypto from 'crypto';
Expand Down
7 changes: 5 additions & 2 deletions src/receivers/ExpressReceiver.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
import { createServer, Server, ServerOptions } from 'http';
import { createServer as createHttpsServer, Server as HTTPSServer, ServerOptions as HTTPSServerOptions } from 'https';
import { ListenOptions } from 'net';
Expand Down Expand Up @@ -187,7 +188,7 @@ export default class ExpressReceiver implements Receiver {
logLevel,
logger, // pass logger that was passed in constructor, not one created locally
stateStore: installerOptions.stateStore,
authVersion: installerOptions.authVersion!,
authVersion: installerOptions.authVersion ?? 'v2',
clientOptions: installerOptions.clientOptions,
authorizationUrl: installerOptions.authorizationUrl,
});
Expand All @@ -199,15 +200,17 @@ export default class ExpressReceiver implements Receiver {
'/slack/oauth_redirect' :
installerOptions.redirectUriPath;
this.router.use(redirectUriPath, async (req, res) => {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await this.installer!.handleCallback(req, res, installerOptions.callbackOptions);
});

const installPath = installerOptions.installPath === undefined ? '/slack/install' : installerOptions.installPath;
this.router.get(installPath, async (_req, res, next) => {
try {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const url = await this.installer!.generateInstallUrl({
metadata: installerOptions.metadata,
scopes: scopes!,
scopes: scopes ?? [],
userScopes: installerOptions.userScopes,
});
if (installerOptions.directInstall) {
Expand Down
5 changes: 5 additions & 0 deletions src/receivers/HTTPReceiver.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
import { createServer, Server, ServerOptions, RequestListener, IncomingMessage, ServerResponse } from 'http';
import { createServer as createHttpsServer, Server as HTTPSServer, ServerOptions as HTTPSServerOptions } from 'https';
import { ListenOptions } from 'net';
Expand Down Expand Up @@ -288,6 +289,7 @@ export default class HTTPReceiver implements Receiver {
// NOTE: the domain and scheme of the following URL object are not necessarily accurate. The URL object is only
// meant to be used to parse the path and query
const { pathname: path } = new URL(req.url as string, `http://${req.headers.host}`);
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const method = req.method!.toUpperCase();

if (this.endpoints.includes(path) && method === 'POST') {
Expand All @@ -297,6 +299,7 @@ export default class HTTPReceiver implements Receiver {

if (this.installer !== undefined && method === 'GET') {
// When installer is defined then installPath and installRedirectUriPath are always defined
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const [installPath, installRedirectUriPath] = [this.installPath!, this.installRedirectUriPath!];

if (path === installPath) {
Expand Down Expand Up @@ -454,6 +457,7 @@ export default class HTTPReceiver implements Receiver {
try {
// This function is only called from within unboundRequestListener after checking that installer is defined, and
// when installer is defined then installUrlOptions is always defined too.
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const [installer, installUrlOptions] = [this.installer!, this.installUrlOptions!];

// Generate the URL for the "Add to Slack" button.
Expand Down Expand Up @@ -488,6 +492,7 @@ export default class HTTPReceiver implements Receiver {
private handleInstallRedirectRequest(req: IncomingMessage, res: ServerResponse) {
// This function is only called from within unboundRequestListener after checking that installer is defined, and
// when installer is defined then installCallbackOptions is always defined too.
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const [installer, installCallbackOptions] = [this.installer!, this.installCallbackOptions!];

installer.handleCallback(req, res, installCallbackOptions).catch((err) => {
Expand Down
7 changes: 5 additions & 2 deletions src/receivers/SocketModeReceiver.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
import { SocketModeClient } from '@slack/socket-mode';
import { createServer } from 'http';
import { Logger, ConsoleLogger, LogLevel } from '@slack/logger';
Expand Down Expand Up @@ -88,7 +89,7 @@ export default class SocketModeReceiver implements Receiver {
logLevel,
logger, // pass logger that was passed in constructor, not one created locally
stateStore: installerOptions.stateStore,
authVersion: installerOptions.authVersion!,
authVersion: installerOptions.authVersion ?? 'v2',
clientOptions: installerOptions.clientOptions,
authorizationUrl: installerOptions.authorizationUrl,
});
Expand All @@ -106,12 +107,14 @@ export default class SocketModeReceiver implements Receiver {
const server = createServer(async (req, res) => {
if (req.url !== undefined && req.url.startsWith(redirectUriPath)) {
// call installer.handleCallback to wrap up the install flow
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await this.installer!.handleCallback(req, res, installerOptions.callbackOptions);
} else if (req.url !== undefined && req.url.startsWith(installPath)) {
try {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const url = await this.installer!.generateInstallUrl({
metadata: installerOptions.metadata,
scopes: scopes!,
scopes: scopes ?? [],
userScopes: installerOptions.userScopes,
});
if (directInstallEnabled) {
Expand Down
1 change: 1 addition & 0 deletions src/test-helpers.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
// eslint-disable-next-line import/no-extraneous-dependencies
import sinon, { SinonSpy } from 'sinon';
import { Logger } from '@slack/logger';
Expand Down
2 changes: 2 additions & 0 deletions src/types/actions/block-action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ export interface BlockAction<ElementAction extends BasicElementAction = BlockEle
user?: string; // undocumented that this is optional, it won't be there for bot messages
ts: string;
text?: string; // undocumented that this is optional, but how could it exist on block kit based messages?
// eslint-disable-next-line @typescript-eslint/no-explicit-any
[key: string]: any;
};
view?: ViewOutput;
Expand All @@ -242,6 +243,7 @@ export interface BlockAction<ElementAction extends BasicElementAction = BlockEle
container: StringIndexed;

// this appears in the block_suggestions schema, but we're not sure when its present or what its type would be
// eslint-disable-next-line @typescript-eslint/no-explicit-any
app_unfurl?: any;

// exists for enterprise installs
Expand Down
1 change: 1 addition & 0 deletions src/types/actions/workflow-step-edit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export interface WorkflowStepEdit {
step_id: string;
inputs: {
[key: string]: {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
value: any;
};
};
Expand Down
1 change: 1 addition & 0 deletions src/types/events/base-events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -882,6 +882,7 @@ export interface WorkflowStepExecuteEvent {
step_id: string;
inputs: {
[key: string]: {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
value: any;
};
};
Expand Down
2 changes: 2 additions & 0 deletions src/types/events/message-events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -347,13 +347,15 @@ interface File {
has_rich_preview?: boolean;

shares?: {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
[key: string]: any;
};
channels: string[] | null;
groups: string[] | null;
users?: string[];
pinned_to?: string[];
reactions?: {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
[key: string]: any;
}[];
is_starred?: boolean;
Expand Down
Loading

0 comments on commit 992b64a

Please sign in to comment.