[Experimental Feature] Opt in flag convert disable comments

[KingDarBoja]

PR Checklist

• Addresses an existing issue: fixes Add opt-in flag to convert tslint:disable comments to eslint-disable #136
• That issue was marked as status: accepting prs

Overview

It is a work-in-progress (WIP) so any help would be welcome!

So far, I managed to pull the proper regex pattern to match any tslint comment and split it into capture groups. Each one has enough information to be used on the convertComments logic inside the for-loop (Please check the file).

Updated 4/25 by @JoshuaKGoldberg:

Fleshed out the excellent initial scaffolding from @KingDarBoja and added unit tests. Summarizing the major changes in this PR:

1. Added an optional --comments flag marked as (experimental) to take in globs of source files that should have their comments replaced
   • Followup work: look into allowing tsconfig files and/or also listening to --typescript
2. Each of those files files is parsed into a TypeScript AST, its comments iterated over, and a list of comments that match a TSLint rule flag is produced
3. For each matching comment, the equivalent ESLint inline comment is generated, with any TSLint rule names replaced by a newly called rule converter's output ESLint rules
   • Followup work: re-use the rules generated by the existing tslint.json->.eslintrc.js logic, so that rule arguments are accounted for
4. A summary of those results are printed out to the console

[KingDarBoja]

@JoshuaKGoldberg After digging into TSLint source code and using some of their functions (copy-paste at most) and slighty changing the regex pattern as requested (couldn't use the tslint one, doesn't get the comments as I expected), now I do have better data, specially the rules array:

[JoshuaKGoldberg]

👋 @KingDarBoja this feature is still very exciting but I'm also not clear on what, if anything, I can do to help?

[KingDarBoja]

@JoshuaKGoldberg Hi mate,

Sorry for the delay, I forgot about this one and got busy with work and other stuff. I will see how to solve what's missing, in this case (from the gitter):

Finally managed to do the rule mapping and rule replacement on the file but still have some problems on the start and end positions.

In other words, while trying to replace the old rules text with the new ones, I ended up overriding code instead of placing them in their line without spamming the next ones.

[JoshuaKGoldberg]

Oh gotcha! Could you provide a code snippet / repro case that shows off the issue? I can take a look.

[KingDarBoja]

Oh gotcha! Could you provide a code snippet / repro case that shows off the issue? I can take a look.

Let me rebuild the sample repo as I moved all my stuff to a new pc and forgot to copy that one 🙈

[KingDarBoja]

@JoshuaKGoldberg Use this sample repo to test the branch.

1. Clone this PR branch.
2. npm install
3. npm run tsc
4. Run node path/to/tslint-to-eslint-config/bin/tslint-to-eslint-config -C at the sample repo and see the output index.ts file along with the terminal output.

You will notice how the ruels mapping is able to catch the rules but I am unable to write properly the tslint to eslint comment along with the rules mapping 😢 My knowledge with TS AST is still poor so I would need a hand here.

[JoshuaKGoldberg]

Some thoughts after peeking through the code...

• It'd be nice to be more targeted in the files that get scanned. Perhaps wildcards should be required as option(s) to the CLI option? That way it won't try to convert, say, .eslintrc.js.
• Do we need to create a ts.SourceFile? Feels like the whole thing could be done with text replacements, no?

[KingDarBoja]

Do we need to create a ts.SourceFile? Feels like the whole thing could be done with text replacements, no?

It is better to use source file as you get the whole TypeScript AST Tree. In this case, we are using the AST to find any node kind matching the comment kind.

[JoshuaKGoldberg]

I should be able to get to this within 2-3 days. 😄

[JoshuaKGoldberg]

Status update: it's working locally on sample files as of 35adb28. A couple caveats:

• No unit tests yet to verify edge cases, alas...
• Some TSLint rules output different ESLint rules based on their rule arguments. Therefore, information from the first convertRules conversion is needed to really know which ESLint rule(s) correspond to a TSLint comment.
  • This will require more architecture work, and would be fine as a followup ticket...
  • Let's get the current approach, which works swimmingly for most cases, out the door as an "experimental" release first.

[JoshuaKGoldberg]

Notes on the CLI flag:

• Switched to --comments instead of --convertComments. It felt odd having it be the only flag name that explicitly said "convert". 🤷
• As a followup issue, it'd be cool if it could take in JSON path(s) for tsconfig files...
• Fun fact: chalk.magenta shows up as purple on Cmder! Fine.
  

[JoshuaKGoldberg]

FYI @KingDarBoja I think this is roughly ready. I'll let it sit for a few days in case there's something you or I notice as needing change.

This'll be mentioned in the changelog, of course, but your starting code was very helpful in getting this working. Seriously, it was great; most of the code now still uses its core logic. Thanks so much! 💪

[KingDarBoja]

@JoshuaKGoldberg Glad to see it working properly after all this time, I have to admit, you nailed it as there were several things that I didn't know how to handle at the time being.

I will take a look at it right now and test with the sample repo and of course, review it if I find something missing.

Awesome work! 🚀

[KingDarBoja]

@JoshuaKGoldberg Outstanding job, this work as expected, now what's left is resolving the merge conflicts 👍