[ban-types] Doesn't work with namespaced type

[SimenB]

Repro

{
  "rules": {
    "@typescript-eslint/ban-types": [
      "error",
      {
        "types": {
          "React.FunctionComponent": {
            "message": "Use React.FC instead",
            "fixWith": "React.FC"
          }
        }
      }
    ]
  }
}

import * as React from 'react';

const Thing: React.FunctionComponent = () => 'foobar';

Expected Result
Should be reported, and fixed to React.FC

Actual Result
Not reported

Additional Info
Removing React. from the rule and the code makes it work correctly

Versions

package	version
@typescript-eslint/eslint-plugin	1.1.0
@typescript-eslint/parser	1.1.0
TypeScript	3.2.4
ESLint	5.12.1
node	10.15.0
yarn	1.13.0

[bradzacher]

The rule is intentionally simple, but it looks like we made it too simple.

The last part of your issue

Removing React. from the rule and the code makes it work correctly
Highlights a bug in the code, if the identifier is namespaced, we shouldn't be reporting on it.

Or else with the default config this would fail

/*
{
    types: {
      String: {
        message: 'Use string instead',
        fixWith: 'string'
      },
}
*/

namespace Foo {
  export type String = 'some string';
}

// error: Use string instead
const x : Foo.String = 'some string';

(i've added the bug tag for this half of the issue)

[bradzacher]

in regards to your actual reported issue.
We should definitely look into adding explicit support for TSQualifiedName.

I.e. if you supply a key with a . in it, we convert it to a TSQualifiedName.

[SimenB]

Highlights a bug in the code, if the identifier is namespaced, we shouldn't be reporting on it.

No, I meant removing React. from the eslint config and from the code - that is just having FunctionComponent and just fixing to FC. I think that's expected and not a bug? We might be talking past each other, though

[armano2]

@bradzacher its not going to error, we have check for namespaces / TSQualifiedName

let e: foo.String;

https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/tests/lib/rules/ban-types.js#L52

[bradleyayers]

Is the goal of this rule to replace the TSLint rule? I arrived at this issue because I found that the ESLint ban-types rule doesn't support regex rules, which I was using in TSLint, and was hoping to start migrating over to ESLint. Is the simplicity of the rule driven by performance requirements, or something else?

[bradzacher]

@bradleyayers - with time a lot of the tslint rules build up features that nobody ends up using.

When contributors PR rules, we don't look for exact 1:1 parity with tslint, because asking contributors to do a bunch of extra work that nobody has asked for yet (and may never ask for) is a good way to annoy people and turn away contributors.

With that being said, we encourage and accept PRs - so if there's something that you think is missing and you'll get value out of, please feel free to raise an issue and/or a PR.