Deno: Lint: no-inner-declarations will warn std & cli/tests

[magurotuna]

The lint rule no-inner-declarations will come soon https://github.com/denoland/deno_lint/pull/297
When executing lint with this rule, we get lots of errors such as:

(no-inner-declarations) Move function declaration to function root
    function DateOverride(): void {
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    at /home/yusuktan/Repos/github.com/magurotuna/deno/cli/tests/unit/timers_test.ts:382:4

(no-inner-declarations) Move function declaration to module root
      async function asyncFn(): Promise<typeof value> {
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    at /home/yusuktan/Repos/github.com/magurotuna/deno/std/node/_util/_util_callbackify_test.ts:84:6

(no-inner-declarations) Move function declaration to module root
      function promiseFn(): Promise<typeof value> {
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    at /home/yusuktan/Repos/github.com/magurotuna/deno/std/node/_util/_util_callbackify_test.ts:96:6

(no-inner-declarations) Move function declaration to module root
      function thenableFn(): PromiseLike<any> {
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    at /home/yusuktan/Repos/github.com/magurotuna/deno/std/node/_util/_util_callbackify_test.ts:109:6

(no-inner-declarations) Move function declaration to module root
      async function asyncFn(): Promise<never> {
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    at /home/yusuktan/Repos/github.com/magurotuna/deno/std/node/_util/_util_callbackify_test.ts:140:6

(no-inner-declarations) Move function declaration to module root
      function promiseFn(): Promise<never> {
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    at /home/yusuktan/Repos/github.com/magurotuna/deno/std/node/_util/_util_callbackify_test.ts:169:6

(no-inner-declarations) Move function declaration to module root
      function thenableFn(): PromiseLike<never> {
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    at /home/yusuktan/Repos/github.com/magurotuna/deno/std/node/_util/_util_callbackify_test.ts:205:6

(no-inner-declarations) Move function declaration to module root
    async function asyncFn(arg: typeof value): Promise<typeof value> {
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    at /home/yusuktan/Repos/github.com/magurotuna/deno/std/node/_util/_util_callbackify_test.ts:249:4

(no-inner-declarations) Move function declaration to module root
    function promiseFn<T>(arg: typeof value): Promise<typeof value> {
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    at /home/yusuktan/Repos/github.com/magurotuna/deno/std/node/_util/_util_callbackify_test.ts:266:4

(no-inner-declarations) Move variable declaration to module root
              var type = FILETYPE_REGULAR_FILE;
              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    at /home/yusuktan/Repos/github.com/magurotuna/deno/std/wasi/snapshot_preview1.ts:830:14

(no-inner-declarations) Move variable declaration to module root
              var type = FILETYPE_REGULAR_FILE;
              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    at /home/yusuktan/Repos/github.com/magurotuna/deno/std/wasi/snapshot_preview1.ts:834:14

(no-inner-declarations) Move variable declaration to module root
              var type = FILETYPE_SYMBOLIC_LINK;
              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    at /home/yusuktan/Repos/github.com/magurotuna/deno/std/wasi/snapshot_preview1.ts:838:14

(no-inner-declarations) Move variable declaration to module root
              var type = FILETYPE_REGULAR_FILE;
              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    at /home/yusuktan/Repos/github.com/magurotuna/deno/std/wasi/snapshot_preview1.ts:842:14

Found 13 problems

We have to address these errors before the rule comes.

I want to make sure that how I will fix them is correct, so I have two questions:

1. Can the above function declarations that cause errors be replaced with arrow function? like const asyncFn = async (): Promise<typeof value> => { }
2. Can var type = .. be rewritten from

https://github.com/denoland/deno/blob/fa65e49bc689b2b84a3fa2dd123fa616f430f7fe/std/wasi/snapshot_preview1.ts#L828-L846

to

let type: number;
switch (true) {
  case entries[i].isFile:
    type = FILETYPE_REGULAR_FILE;
    break;

  case entries[i].isDirectory:
    type = FILETYPE_REGULAR_FILE;
    break;

  case entries[i].isSymlink:
    type = FILETYPE_SYMBOLIC_LINK;
    break;

  default:
    type = FILETYPE_REGULAR_FILE;
    break;
}

entry_view.setUint8(20, type);

CC @caspervonb @bartlomieju

[bartlomieju]

deno/cli/tests/unit/timers_test.ts:382:4 <--- it seems strange std_lint shouldn't lint any code in cli/tests/

@magurotuna did you check output from eslint with this rule? It'd be good to confirm that they produce same errors right now.

[caspervonb]

Sorry for the lint friction we had a weird blocker initially when merging deno.land/x/wasi into std/wasi (#6436) that led me to ignore es-lint completely 😅

[magurotuna]

@bartlomieju

it seems strange std_lint shouldn't lint any code in cli/tests/

It's kind of weird... I ran cargo test std_lint command, but cli/tests files were included.

did you check output from eslint with this rule?

I checked eslint, but I forgot enabling no-inner-declarations rule. my bad.
Checking again, then I've got output like:

/home/yusuktan/Repos/github.com/magurotuna/deno/cli/tests/unit/timers_test.ts
  382:5  error  Move function declaration to function body root  no-inner-declarations

/home/yusuktan/Repos/github.com/magurotuna/deno/std/node/_util/_util_callbackify_test.ts
   84:7  error  Move function declaration to function body root  no-inner-declarations
   96:7  error  Move function declaration to function body root  no-inner-declarations
  109:7  error  Move function declaration to function body root  no-inner-declarations
  140:7  error  Move function declaration to function body root  no-inner-declarations
  169:7  error  Move function declaration to function body root  no-inner-declarations
  205:7  error  Move function declaration to function body root  no-inner-declarations
  249:5  error  Move function declaration to function body root  no-inner-declarations
  266:5  error  Move function declaration to function body root  no-inner-declarations

[bartlomieju]

@bartlomieju

it seems strange std_lint shouldn't lint any code in cli/tests/

It's kind of weird... I ran cargo test std_lint command, but cli/tests files were included.

I'll investigate it.

did you check output from eslint with this rule?

I checked eslint, but I forgot enabling no-inner-declarations rule. my bad.
Checking again, then I've got output like:

/home/yusuktan/Repos/github.com/magurotuna/deno/cli/tests/unit/timers_test.ts
  382:5  error  Move function declaration to function body root  no-inner-declarations

/home/yusuktan/Repos/github.com/magurotuna/deno/std/node/_util/_util_callbackify_test.ts
   84:7  error  Move function declaration to function body root  no-inner-declarations
   96:7  error  Move function declaration to function body root  no-inner-declarations
  109:7  error  Move function declaration to function body root  no-inner-declarations
  140:7  error  Move function declaration to function body root  no-inner-declarations
  169:7  error  Move function declaration to function body root  no-inner-declarations
  205:7  error  Move function declaration to function body root  no-inner-declarations
  249:5  error  Move function declaration to function body root  no-inner-declarations
  266:5  error  Move function declaration to function body root  no-inner-declarations

Thanks, it's strange that wasm files are not producing diagnostics.

[magurotuna]

I'll investigate it.

Thanks!

Thanks, it's strange that wasm files are not producing diagnostics.

It's because /* eslint-disable */ is placed on the head of the file.

I find the error messages for _util_callbackify_test.ts are different between eslint and deno_lint.
ESLint: Move function declaration to __function body__ root
deno_lint: Move function declaration to __module__ root

[bartlomieju]

It's because /* eslint-disable */ is placed on the head of the file.

Thanks, did not see that.

I find the error messages for _util_callbackify_test.ts are different between eslint and deno_lint.
ESLint: Move function declaration to function body root
deno_lint: Move function declaration to module root

That's a bug in rule implementation then.

[magurotuna]

That's a bug in rule implementation then.

I think so too. Will you look into it?

[bartlomieju]

Sure, could you link to the problematic function?

[magurotuna]

https://github.com/denoland/deno/blob/fa65e49bc689b2b84a3fa2dd123fa616f430f7fe/std/node/_util/_util_callbackify_test.ts#L84-L86

Here!

[magurotuna]

@bartlomieju
Now deno_lint generates the same diagnostics as ESLint!

(no-inner-declarations) Move function declaration to function root
    function DateOverride(): void {
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    at /home/yusuktan/Repos/github.com/magurotuna/deno/cli/tests/unit/timers_test.ts:382:4

(no-inner-declarations) Move function declaration to function root
      async function asyncFn(): Promise<typeof value> {
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    at /home/yusuktan/Repos/github.com/magurotuna/deno/std/node/_util/_util_callbackify_test.ts:84:6

(no-inner-declarations) Move function declaration to function root
      function promiseFn(): Promise<typeof value> {
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    at /home/yusuktan/Repos/github.com/magurotuna/deno/std/node/_util/_util_callbackify_test.ts:96:6

(no-inner-declarations) Move function declaration to function root
      function thenableFn(): PromiseLike<any> {
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    at /home/yusuktan/Repos/github.com/magurotuna/deno/std/node/_util/_util_callbackify_test.ts:109:6

(no-inner-declarations) Move function declaration to function root
      async function asyncFn(): Promise<never> {
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    at /home/yusuktan/Repos/github.com/magurotuna/deno/std/node/_util/_util_callbackify_test.ts:140:6

(no-inner-declarations) Move function declaration to function root
      function promiseFn(): Promise<never> {
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    at /home/yusuktan/Repos/github.com/magurotuna/deno/std/node/_util/_util_callbackify_test.ts:169:6

(no-inner-declarations) Move function declaration to function root
      function thenableFn(): PromiseLike<never> {
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    at /home/yusuktan/Repos/github.com/magurotuna/deno/std/node/_util/_util_callbackify_test.ts:205:6

(no-inner-declarations) Move function declaration to function root
    async function asyncFn(arg: typeof value): Promise<typeof value> {
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    at /home/yusuktan/Repos/github.com/magurotuna/deno/std/node/_util/_util_callbackify_test.ts:249:4

(no-inner-declarations) Move function declaration to function root
    function promiseFn<T>(arg: typeof value): Promise<typeof value> {
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    at /home/yusuktan/Repos/github.com/magurotuna/deno/std/node/_util/_util_callbackify_test.ts:266:4

Found 9 problems

[bartlomieju]

@magurotuna my bad, actually we're linting std/ and cli/tests/unit as well so it's all ok.

[magurotuna]

I see.
Then I'll fix _util_callbackify_test.ts so that deno_lint will generate no errors.