Add no-redundant-pipe

[OliverJAsh]

Depends on #224

Closes #221

cc @gabro

[gabro]

Thanks! I'll take a look once we merge #224 and rebase this on main

[gabro]

@OliverJAsh could you please rebase this on main now that #224 is merged?

[OliverJAsh]

Done!

[gabro]

Great job! I have some potentially blocking comment about the AST generation

[gabro]

If I'm reading this correctly, this is due to the fact that we're synthesizing a new AST node. It's hard to preserve trivias (comments, whitespaces, etc) when manipulating the AST because it's a lossy representation of the source code.
This is why I tend to lean towards reading AST but writing tokens (aka characters). It's harder to work with, but it's less invasive for the final user.

Do you think this fix could be implemented without generating a new AST node? Accidentally removing comments is a show-stopper in my opinion, I think a fix that can produce invalid syntax in some edge cases is still preferable to a silent removal of a comment

[OliverJAsh]

Thanks for bringing this up! I agree it's not good if we accidentally remove a comment. To be clear, this is only an issue for comments inside the pipe—comments outside are preserved.

Do you think this fix could be implemented without generating a new AST node?

Yes, I think it would be the same as the fix we used to have for no-redundant-flow before b76044a which was to fix #222. Maybe there is another way to fix #222?

That said, in my opinion it's extremely error prone to manipulate code as text rather than as an AST—although this unfortunately seems to be the status quo for ESLint rules, as far as I can tell. As far as I understand, there is no information about comments in TSESTree's representation of the AST. However, there is some information about comments in TypeScript's AST. I wonder if we could implement the fix using TypeScript's AST instead, e.g. by mapping the TSESTree node to a TS node, constructing a new node and then using TypeScript's AST printer. Do you have any experience doing this inside of ESLint rules?

[gabro]

I don't have direct experience with Typescript AST, no, but if it supports not blowing away the comments maybe it's worth a try.

Otherwise we can think of a more ad-hoc fix for the specific use case

[OliverJAsh]

I think the ad-hoc fix would be to revert b76044a and then find another way to workaround #222, but I'm not sure how else we can workaround this? If the trailing comma is there, we need to adjust the fix to remove that as well.

The TSESTree AST doesn't seem to have any information about whether a Node has a trailing comma, but maybe we can read this from the TS AST instead.