Tslint: Ordered-imports misplaces code in presence of import aliasing

[mihneadb]

Bug Report

• __TSLint version__: 5.16.0
• __TypeScript version__: 3.2.2
• __Running TSLint via__: (pick one) CLI

Reproduction using TSLint Playground

https://palantir.github.io/tslint-playground/?saved=N4Igxg9gJgpiBcICWBbADhATgFwATAGUIUYBZaAVwBsYBfXAM02NwB0QA6Dgei+8hQoIAO25ES5KNRjsA3K2GoMOXADEIEXAF5c4spRod1EeYvRY8e7AAskwgOaNmKNiAACWe9ypIARnIUQABpwEQYkewQQbm5cAE0IClwwAENhXCEoJAYAT1wbGGSwiIpMFOwkEVxrGExCgHckG1wcxMxcCHrhDgUY3FIUgGtCgGdSwptylrbcACsRqqQR3AA3FJ8oINw0qFxs6aTMCnS7bAhe2IgC9qWxmBGt7Ew8sBqwQbtHAqLhBZpGLC4WrMTAjHrCBTABS4GGuGAAD2wMGEUBG7HguAA2uxsCMfMJsPA6gISCiYFB2ABdILQ2HsI40NEIfC0mHsLCwOpQAC0SgsTIx2PSsJhTwoMBpwpFUKlItc9mYFDQ5N55hwAvyRwlrLl7AViTQGqFcrlMpNJvYwhSJHRrgAoojalaqHs1bjcNzcFARAByPAjbApFSNZoeTCOQE9YI682uFDlV629gAPQAFAB+ACEYfsAB9MRxKQBKDgAKnYktjIvZmE5toAjAAGGNy2iVqtmqtskBWm3M9gAeXDruU7s9AaDeBD1lwOYrLYtIHj2ET-ZAyZzZfnstjNbrzIATM2d6327HO13LdaZGuADIQVIuvnqj24CfBpozqNnqvsZerjEUwLSkt2jE9Fw5WpbQAZmPLtaAXXBKRbBDZWQ4VUNQkBaCAA

TypeScript code being linted

import {SomeModule} from "../../common/SomeModule";
import Foo = SomeModule.Foo;
import Something from "@org/lib";

with tslint.json configuration:

"ordered-imports": [
      true,
      {
        "grouped-imports": true,
        "groups": [
          {
            "name": "External imports - don't start with @org or .",
            "match": "^(?!@org|[.]).*",
            "order": 10
          },
          {
            "name": "Org imports - start with @org",
            "match": "^@org.*",
            "order": 20
          },
          {
            "name": "Local imports - start with .",
            "match": "^[.].*",
            "order": 30
          }
        ]
      }
    ]
  }

Actual behavior

Running --fix yields:

import Foo = SomeModule.Foo;
import Something from "@org/lib";

import {SomeModule} from "../../common/SomeModule";

By checking the code and trying to understand, it seems that tslint treats import Foo = SomeModule.Foo as just noise that it has to keep together with the @import. But doing this, the alias is before the actual import of the module being referenced.

Expected behavior

import Something from "@org/lib";

import {SomeModule} from "../../common/SomeModule";
import Foo = SomeModule.Foo;

It might be too hard for the rule to understand which module each alias references. I'm OK with any acceptable implementation that yields a valid file after running the fix.
My initial attempt is in this PR: https://github.com/palantir/tslint/pull/4810.

[mihneadb]

@JoshuaKGoldberg given the context of this bug report, do you think #4591 would be a good enough fix? Any suggestions to make it one?
Thanks!

[JoshuaKGoldberg]

So, the issue with #4591 is that it's a bigger change to how the rule works as a whole, to the point of being a breaking change - which means we can't take it in until v6 is published. That will be soon (#4811) but until then, bringing it in would violate the API agreement of TSLint 5.X. 😥.

Until then, it'd be a smoother change to no longer misplace the code during fixing. That's a bug fix and can be taken in any time.

[mihneadb]

How would that look like? Could you please share some pointers? I could try and shape a solution from them.

[JoshuaKGoldberg]

Good question @mihneadb - my guess is the fixer in orderedImportsRule.ts is forgetting to place any nodes of type ImportEqualsDeclaration. getReplacement would be a good place to start looking.

A few suggestions:

• https://astexplorer.net with the language set to TypeScript is really useful for seeing how the tree of nodes is meant to look
• Use your editor's debugger to place breakpoints in the code, then inspect objects. Much easier than console.logging for these complicated rules! I personally use VS Code.

By the way, just in case you hadn't seen it already, TSLint is being deprecated: #4534. You may want to switch over to https://typescript-eslint.io.

[JBhagat841]

@adidahiya finally got approval from Lori to start working on OSS. Would love to tackle this one.

[adidahiya]

@JBhagat841 sounds good, go for it

[JoshuaKGoldberg]

💀 _It's time!_ 💀

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

👋 It was a pleasure open sourcing with you!

[JoshuaKGoldberg]

🤖 Beep boop! 👉 TSLint is deprecated 👈 and you should switch to typescript-eslint! 🤖

🔒 This issue is being locked to prevent further unnecessary discussions. Thank you! 👋