Warn on // tidy-ignore-linelength

[jieyouxu]

I've ran into this many times, where I tried to write

// tidy-ignore-linelength

or other tidy-ignore-* tidy directives. But this is actually wrong and does nothing, since the actual syntax is

// ignore-tidy-linelength

It might be helpful if tidy warns on seeing // tidy-ignore-linelength. It's not a big deal, just a minor papercut.

[cod10129]

I implemented this warning here, and it warns on any appearance of the text tidy-ignore in the code. The error message I made looks like this:¹

<snip>
tidy check
tidy error: rust-checkout-path\library\core\src\lib.rs:107: use ignore-tidy-[check], not tidy-ignore-[check]
some tidy checks failed
<snip>

where I inserted the line // tidy-ignore-linelength into library/core/src/lib.rs at line 107.

However, I still have a couple questions.
First, I don't really know where this code should go in tidy. I put it in style.rs inside check(), but I'm not familiar with tidy's code so I don't know what a good place for it would be.
Second, is there a place for tidy tests? I would think that I should add a test for this new functionality, but it doesn't look like tidy has tests anywhere. Am I missing something obvious?

Footnotes

1. I would be fine with changing it ↩

[jieyouxu]

First, I don't really know where this code should go in tidy. I put it in style.rs inside check(), but I'm not familiar with tidy's code so I don't know what a good place for it would be.

That seems reasonable.

Second, is there a place for tidy tests? I would think that I should add a test for this new functionality, but it doesn't look like tidy has tests anywhere. Am I missing something obvious?

I think tidy is in general under-tested. It may be possible to slightly rework how the tidy ignore directives are handled to permit testing it against a provided string line without actually touching fs, but yeah.

[onur-ozkan]

How about checking if the word "tidy" has - as a prefix or suffix? If it does, tidy should expect it to be a valid directive. If it's unrecognized we could fail with an error message saying "unrecognized tidy directive". Because this could potentially happen with any other tidy-related directives as well.

[jieyouxu]

How about checking if the word "tidy" has - as a prefix or suffix? If it does, tidy should expect it to be a valid directive. If it's unrecognized we could fail with an error message saying "unrecognized tidy directive". Because this could potentially happen with any other tidy-related directives as well.

I think that would make sense yes. I don't quite recall what tidy directives exist.

I just mentioned the ignore-tidy vs tidy-ignore form because this form trips me up every time lol.

[cod10129]

How about checking if the word "tidy" has - as a prefix or suffix? If it does, tidy should expect it to be a valid directive. If it's unrecognized we could fail with an error message saying "unrecognized tidy directive". Because this could potentially happen with any other tidy-related directives as well.

This was more complicated than I expected (tidy doesn't really have anywhere central to collect all the valid directives), so I made an implementation that only knows about the "ignore-tidy-FOO" directives used in style.rs and tidy-alphabetical-{start, end}. I ran ./x test tidy with my code and nothing violates the check in the repository, so it should be fine.
(I probably missed some checks that tidy accepts, but aren't used anywhere in the rust repo. Since there's no centralization in tidy, I couldn't really find them. Please tell me if they do exist.)

I've updated the PR #135421 with this change.