no-shadow if a type alias and function argument share the same name

[sebiniemann]

What version of TypeScript are you using?
2.7.1

What version of typescript-eslint-parser are you using?
14.0.0

What code were you trying to parse?

type foo = any;
function bar(foo: any) {}

What did you expect to happen?
Not a no-shadow error.

What happened?

2:14  error  'foo' is already declared in the upper scope  no-shadow

In contrast, this works as expected (no such error):

type foo = any;
let foo: any;

[JamesHenry]

I think a custom rule may need to be created in the plugin to handle this. It would be fantastic if one of the people affected by this issue could contribute to it.

[sebiniemann]

While I would like to help you out on this, I am currently not familiar enough with this project's code base as well as too busy, to find enough spare time to provide you with a working pull request soon.

I think it would take me at least four weeks, before I could start to look into it.

However, maybe you could clear something up for me. If I got it correctly, the current behaviour is invalid and therefore caused by a bug at either this side or https://github.com/nzakas/eslint-plugin-typescript. Following this up, I did not understood how a custom rule will help. I would assume that some of the existing code needs to be fixed, correct? Do you have a hunch about which of these two projects could be the cause for this? Or in in other words, where I should contribute to?

[JamesHenry]

The standard no-shadow from ESLint will never have any understanding of TypeScript-specific constructs, such as type.

Therefore, we will need to create our own custom rule within the plugin which will be a superset of the standard no-shadow rule, which includes an understanding of TypeScript-specific things.

This is the same idea as https://github.com/nzakas/eslint-plugin-typescript/blob/master/docs/rules/no-unused-vars.md extending the standard no-unused-vars rule.

The contributions will be in https://github.com/nzakas/eslint-plugin-typescript

Hope that makes sense!

[sebiniemann]

Thanks 👍 Coincidentally, I just looked into https://github.com/nzakas/eslint-plugin-typescript/blob/master/lib/rules/no-unused-vars.js and assumed it works this way. So this makes perfectly sense to me.

Could you also give me some quick information about how the plugin and core rules interact with each other? Are the plugin's rules replacing the core's ones, or are both called? And if the later one is true, in which ordering?

From what I saw under https://github.com/nzakas/eslint-plugin-typescript/blob/master/lib/rules/no-unused-vars.js, it seems to be no replacement for https://github.com/eslint/eslint/blob/master/lib/rules/no-unused-vars.js, and only fixes the TS specific cases. I would also expect that it is called before the core rules, so that this line within the plugin:

variables[i].eslintUsed = true;

is called before collectUnusedVariables is executed in the core rule, correct?

Following this train of thought, it seems I need to write a rule, that changes something used by checkForShadows, to get around the error message.

If I reduce the function to its important parts, it boils down to me as

function checkForShadows(scope) {
  const variables = scope.variables;

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

    /* .. */

    // Gets shadowed variable.
    const shadowed = astUtils.getVariableByName(scope.upper, variable.name);

    if (shadowed /* ... */) {
      context.report({/* ... */});
    }
  }
}

So it seems that astUtils.getVariableByName should return no result for type foo = any; (from my example at the start of this issue) and if I look into this within https://github.com/eslint/eslint/blob/master/lib/ast-utils.js, it seems that if (variable) { and more specific scope.set.get(name) needs to evaluate to false/null/....

However, now I am lost how on to do this, without removing the type from the scope or rewriting its representation, avoiding to fail other rules which rely on this information.

Are these assumptions even correct? Maybe it needs to extend checkForShadows to allow for other ways to ignore type definitions?