import-name rule renaming map is not working

[alexeychikk]

Bug Report

• tslint-microsoft-contrib version: 6.2.0
• TSLint version: 5.18.0
• TypeScript version: 3.5.2
• Running TSLint via: VSCode

TypeScript code being linted

import React from 'react';

with tslint.json configuration:

{
  "extends": "tslint-microsoft-contrib",
  "rules": {
    "import-name": [
      true,
      { "react": "React" }
    ]
  }
}

Actual behavior

import React is being replaced with import react on save.

Expected behavior

Nothing should happen because I defined { "react": "React" } in my tslint.json.

[dosentmatter]

Is your config extending anything else? I had the same issue but I found that the tslint-config-airbnb@5.11.1 I was using depended on tslint-microsoft-contrib@5.2.1.

The docs for import-name say

Since version 2.0.9 it is possible to configure this rule with a list of exceptions.

But perhaps they had a bug in version 5.2.1.

Anyway, to test, I installed version tslint-microsoft-contrib@6.2.0 and did rm -rf node_modules/tslint-config-airbnb/node_modules/tslint-microsoft-contrib, to force it to resolve to version 6.2.0.

I got some deprecated rules as expected from jumping up a major version, but the import-name error went away. So, I think it's an issue with some other config you are extending and maybe not tslint-microsoft-contrib. I will create an issue on tslint-config-airbnb.

---

If you have the above issue, and if you want to hack it (in a reproducible way) to force it to resolve to 6.2.0, you can do the following:

Note that I have tried yarn resolutions and npm shrinkwrap. yarn resolutions didn't work for me in a yarn workspaces + lerna monorepo for some reason. I didn't want to use either of those solutions anyway because npm doesn't support resolutions and yarn doesn't support shrinkwrap.

npm install tslint-microsoft-contrib # get 6.2.0

Now you can add tslint-microsoft-contrib to rulesDirectory, but from what I see, the way tslint works is that it requires sub-dependencies before direct dependencies. In other words, require('tslint-config-airbnb/node_modules/tslint-microsoft-contrib') will happen before require('tslint-microsoft-contrib'), because tslint-contrib-airbnb is inside extends, so airbnb's tslint-microsoft-contrib is required first.

You can see it in the output of tslint --print-config:

{
  "rulesDirectory": ["tslint-microsoft-contrib"],
  "extends": ["tslint-config-airbnb"]
}

becomes

  "rulesDirectory": [
    ".../node_modules/tslint-consistent-codestyle/rules",
    ".../node_modules/tslint-eslint-rules/dist/rules",
    ".../node_modules/tslint-config-airbnb/node_modules/tslint-microsoft-contrib",
    ".../node_modules/tslint-microsoft-contrib"
  ]

That doesn't work since tslint-config-airbnb/node_modules/tslint-microsoft-contrib comes before tslint-microsoft-contrib.

---

If you did:

{
  "extends": ["tslint-microsoft-contrib", "tslint-config-airbnb"]
}

Note that the opposite order, "extends": ["tslint-config-airbnb", "tslint-microsoft-contrib"], wouldn't work because "tslint-microsoft-contrib" would get required second.

you get the right thing

  "rulesDirectory": [
    ".../node_modules/tslint-microsoft-contrib",
    ".../node_modules/tslint-consistent-codestyle/rules",
    ".../node_modules/tslint-eslint-rules/dist/rules",
    ".../node_modules/tslint-config-airbnb/node_modules/tslint-microsoft-contrib"
  ]

The problem with this method is that it extends all of the base rules. I don't want any rules, I just want the rulesDirectory.

---

So the final workaround is this:

{
  "extends": ["./tslint-microsoft-contrib.json", "tslint-config-airbnb"]
}

In ./tslint-microsoft-contrib.json, just have the rulesDirectory:

{
  "rulesDirectory": ["tslint-microsoft-contrib"]
}

Now you will get the correct require order to override and you won't extend any rules.

---

You can then turn off deprecated rules and turn on the new ones to stop tslint from complaining:

{
  "rules": {
    "no-function-constructor-with-string-args": false,
    "function-constructor": true,

    "no-increment-decrement": false,
    "increment-decrement": true
  }
}

[alexeychikk]

@dosentmatter Well, thanks for your feedback. But the config I specified in the bug report is the actual config that makes this bug reproducible. I don't extend in my tslint.json any other configs, just tslint-microsoft-contrib. And taking into account referenced issues seems like the bug is there.

[dosentmatter]

@alexeychikk, hmm, so the PR #882 that references this issue was created by me. I used the latest version 6.2.0, as you did. I fixed a few bugs and issues. I'm not exactly sure they apply to your situation because, in your findings, you said it replaces with react. The docs don't say, but the default is to ignoreExternalModule (an option explained below) and only enforce for your own local modules, even though import-name is true.

A way to check is to open node_modules/tslint-microsoft-contrib/importNameRule.js and check for this code: Notice how the index begins and 1 and not 0. Notice that the first option is to extractReplacements() which is the "react": "React" mapping you are trying to create.

Also in the following image, notice that ignoreExternalModule is default in extractConfig:

Here are the ones relevant to you:

1. import-name has an off by one issue. The workaround right now is to pass in null to skip the first option:

   "rules": {
   "import-name": [
     true,
     null,
     { "react": "React" }
   ]
   }

2. There are actually a few more options that aren't explained in the README. I documented them in my fork. It now defaults to ignore import-name for external modules. Try to do the above workaround, and also turn off ignoreExternalModule:

   "rules": {
   "import-name": [
     true,
     null,
     { "react": "React" },
     null,
     { ignoreExternalModule: false }
   ]
   }

   The first null is to skip the off by one error. The second null is just skipping an option field. You can read the updated docs in my fork, but in short, it's a list of modules to ignore.

Note that there is also a bug where camelCase (docs in fork) option doesn't enforce the first character to be lowercase.

[alexeychikk]

@dosentmatter Thanks. After experimenting for a while I figured it out. This works for me well:

 "rules": {
    "import-name": [
      true,
      null,
      {},
      null,
      { "ignoreExternalModule": false, "case": "any-case" }
    ],
  }

[dosentmatter]

@alexeychikk, no problem. Glad I could help. Remember to update it when the bug gets fixed!

[JoshuaKGoldberg]

💀 It's time! 💀

TSLint is deprecated and no longer accepting pull requests other than security fixes. See #876. ☠️ We recommend you instead use typescript-eslint to lint your TypeScript code with ESLint. ✅

👋 It was a pleasure open sourcing with you!