[explicit-module-boundary-types] add option to allow arguments explicitly typed as any

[itmayziii]

It is possible that this is expected behavior, but the docs are very unclear to me. Most of the examples have functions that simply return primitives or higher order functions, but nothing about returning interfaces/types from functions.

Repro
Here is a typescript playground with the minimum reproduction of the issue. Essentially the apolloLoggerAdapter is trying to adapt one Logger interface into another ApolloLogger interface. The function is returning an ApolloLogger interface which is pretty explicit about the expected object. I shouldn't have to retype the methods debug, info, warn, and error.
https://www.typescriptlang.org/play/?ssl=1&ssc=1&pln=40&pc=1#code/C4TwDgpgBAMg9gcwMIEMA2aBGKDGBrKAXigAoIAncucgfgC4oUA7EAGijQgDcI16oAzsHIBLJgnYBbCAIEoEEfkNHipEYCn7MQASiIA+KFzgiAJgChzY4BQBmuaDG68Ip+AgCy6gBZxTUAG9zKBDSaVl5CAZlMQkoHHQsXDwGd1QMbHwdVMQFcmDQknC5BWjhWLUNBm12BIzknOREzLxs2FyKApCimRKowXLVKAA6UekqxhYAbQBdNvc8rtIxWzgAeUwAKwgcYAY4LZ3geY78kIBfSwgADzBqYChrOwd2hDzApZxRYBE61OdOG5EF5gL5TABuJYUKjkf48QHuEFgyGhR5MVZwlxAzw+Pwo0KmCCYACuCExCOBuIh5ku5hud3ID0JODQKHI0FAkCgAEE7hg4AsKEQPiFCSSED0IgotCw2sYzAUVnBJX0ZboGPKLCEAO5spgqyJquUmLVQaHUA3S6qyjUmmlXW73KC2YlMXYiOBMRh8tAC07c0woMA2cgkX1vCiNPJtXlwfmC8giqDs4DE8heoKosWksK9SJlFQIPSZ1HJ9Rpr3hvLDbMS4qRHRLc6sJZK3NS-oxcTFpYhFMVjinYZKy0QRuo5tLXXp9t9AuxHulsupmdVijD9HKgAG06YsQYABIAvWFOct+PQpPUebE6P592k6j+6uhzfRxeLgVLpcgA

I'm using whatever the plugin:@typescript-eslint/recommended settings are.

Expected Result
I should not get warnings about "Missing return type on function" or "Argument 'message' should be typed".

Actual Result
I do get warnings even though the function explicitly returns an object with already typed methods.

Additional Info

Versions

package	version
@typescript-eslint/eslint-plugin	3.0.1
@typescript-eslint/parser	3.0.1
TypeScript	3.9.3
ESLint	7.1.0
node	12.16.1
npm	6.14.5

[bradzacher]

Could you please include a link to your example code?
Dumping it in a code fence in a comment is also fine.

[itmayziii]

I updated typescriptlang.org playground link in my original comment which should have the code, but if not here it is.

type LogCallback = (error?: any, level?: string, message?: string, meta?: any) => void

interface LeveledLogMethod {
    (message: string, callback: LogCallback): Logger
    (message: string, meta: any, callback: LogCallback): Logger
    (message: string, ...meta: any[]): Logger
    (infoObject: object): Logger
  }

export interface Logger {
    critical: LeveledLogMethod;
    error: LeveledLogMethod;
    info: LeveledLogMethod;
    debug: LeveledLogMethod;
}

export declare type ApolloLogger = {
  debug(message?: any): void
  info(message?: any): void
  warn(message?: any): void
  error(message?: any): void
}

export function apolloLoggerAdapter(logger: Logger): ApolloLogger {
  return {
    debug (message: string) {
      return logger.debug(message)
    },
    info (message: string) {
      return logger.info(message)
    },
    warn (message: string) {
      return logger.info(`warning: ${message}`)
    },
    error (message: string) {
      return logger.error(message)
    }
  }
}

[bradzacher]

The issue is the anys.

I could probably be made clearer, but there is one example in the docs:

// All arguments should be typed
export var arrowFn = (arg): string => `test ${arg}`;
export var arrowFn = (arg: any): string => `test ${arg}`;

You can switch from any to unknown, which is explicit and safer.

---

Also looking at your example code - it's super unsafe.

Your interface defines the properties as fn(thing: any) -> void, yet your implementation is typed as fn(thing: string) -> void.

This is a massive safety hole and means your code is liable to break if someone does something like pass a number or an object in.

[itmayziii]

@bradzacher
That docs do not make sense to me with that example.

The issue is the anys.

any is still a type. Whether it's a good or bad type to use (in this case the anys are coming from a library. The comment says // All arguments should be typed but any is very much a type in Typescript, it is even listed as a basic type in the official docs.

Fair point on the unsafe interfaces there, I actually have a similar example though where this still happens without the anys / unsafe interface.

Playground link

export declare type CacheValue<T> = T | (() => T) | (() => Promise<T>);
export interface CacheOptions {
    group?: string;
}
export interface ExpireCacheOptions extends CacheOptions {
    duration?: number | null;
}

export interface Cache {
    get<T>(key: string, options?: CacheOptions): Promise<T | undefined>;
    set<T>(key: string, value: CacheValue<T>, options?: ExpireCacheOptions): Promise<T>;
    delete(key: string | string[], options?: CacheOptions): Promise<boolean>;
    deleteGroup(group: string | string[]): Promise<boolean>;
    remember<T>(key: string, value: CacheValue<T>, options?: ExpireCacheOptions): Promise<T>;
    clear(): Promise<boolean>;
}

export interface KeyValueCacheSetOptions {
  ttl?: number | null
};
export interface KeyValueCache<V = string> {
  get(key: string): Promise<V | undefined>;
  set(key: string, value: V, options?: KeyValueCacheSetOptions): Promise<void>;
  delete(key: string): Promise<boolean | void>;
}

export function apolloKeyValueCacheAdapter(cache: Cache): KeyValueCache {
  return {
    get (key) {
      return cache.get(key)
    },
    set (key, value, options = {}) {
      return cache.set(key, value, { duration: options.ttl || 3600 })
        .then(() => undefined)
    },
    delete (key) {
      return cache.delete(key)
    }
  }
}

One way to get the error to stop in this case and the last example I gave is to create a local variable like

export function apolloKeyValueCacheAdapter (cache: Cache): KeyValueCache {
  const keyValueCache: KeyValueCache = {
    get (key) {
      return cache.get(key)
    },
    set (key, value, options = {}) {
      return cache.set(key, value, { duration: options.ttl || 3600 })
        .then(() => undefined)
    },
    delete (key) {
      return cache.delete(key)
    }
  }
  return keyValueCache
}

Obviously this local variable is redundant and should not be needed but it removes the warnings I get from explicit-module-boundary-types which does not make sense to me.

Sorry for the walls of code, I'm just copying the interfaces from node_modules until I have them all accounted for to make the example function work.

[AlCalzone]

I'm also running into the latter case. ESLint complains that target and property should be typed

return new Proxy(apiInstance, {
	get: (target, property) => {
		// implementation
	},
});

although they clearly are:

because they are inferred from the Proxy type definitions:

interface ProxyHandler<T extends object> {
    get? (target: T, p: PropertyKey, receiver: any): any;
    // ... other definitions...
}

interface ProxyConstructor {
    revocable<T extends object>(target: T, handler: ProxyHandler<T>): { proxy: T; revoke: () => void; };
    new <T extends object>(target: T, handler: ProxyHandler<T>): T;
}

[bradzacher]

For the case of a returned function requiring explicit types - that is a bug (#1845, PR #2084).

[bradzacher]

The intention of the rule is that you provide clear and explicit module boundaries.
any is a type for sure, but it's an unsafe type and can cause many issues. The intention is to prevent people from satisfying the rule up by just annotating everything with any (which is essentially what you did in your first example).

Explicitly typing any is usually no better than just turning off the noImplicitAny compiler option or using a // @ts-ignore.

This is working as expected. If you'd like to contribute a clarification to the docs, I'm happy to accept a PR.

As for this rule and alternatives:

• If you want to enforce clear and explicit module boundaries, there's this rule.
• If you're looking for just enforcing a return type, there's the explicit-function-return-type rule.
• If you just want parameter type definitions, there's the typedef rule.

[bradzacher]

I have added an option - allowArgumentsExplicitlyTypedAsAny.
The docs have also been clarified.

@AlCalzone - the PR I mentioned didn't fix that issue, so I raised #2134, and fixed it as well.

[itmayziii]

@bradzacher

Thanks for the update. I feel like this eslint rule though would be better served not caring about anys at all because as you pointed out there is already something built into the compiler to catch explicit anys. If somebody was using an any then they would have already opted into this behavior by turning off the noExplicitAny compiler option, so for this rule to also care about anys feels redundant.