Tidy should not check line lengths in UI tests

UI tests regularly have long lines due to the //~ ERROR annotations. So it is not very helpful for tidy to complain about overly long lines... every single time that happened, I ended up adding // ignore-tidy-linelength (the amount of time it took me to figure out the exact way to do this went down from 3-5min the first time to <20s by now). To make test maintenance even more annoying, when an error message changes to become shorter, tidy will complain about the ignore flag being unnecessary! And each time the flag is added or removed, you need to re-bless the tests as line numbers change.

This chore doesn't really seem worth it. We don't rustfmt the test suite either. Can we just disable the line length checker in the test suite please?

We can add a check similar to https://github.com/rust-lang/rust/blob/master/src/tools/tidy/src/style.rs#L171 to disable just the line length limits for UI tests.

Hey, first time contributor here happy to give this a shot! I think I get what's going on here, but do you have any pointers for where I should be looking exactly?

I would add a "in_tests" check similar to the the under_rustfmt one here

rust/src/tools/tidy/src/style.rs

Line 171 in 8ae3b50

let under_rustfmt = filename.ends_with(".rs") &&

, and then add it to the conditional here

rust/src/tools/tidy/src/style.rs

Line 229 in 8ae3b50

if !under_rustfmt

to not run that lint in tests.

Rollup merge of rust-lang#77675 - Anthuang:tidy-line-length, r=Mark-S…

Is the plan suggested in #77675 (comment) still good?

does it make sense to disable the "ignoring length unnecessarily" lint temporarily so this can land? Then once this lands people will stop adding the ignore and it will be easier to turn the lint back on.

@sjakobi

@sjakobi yeah I'd say it is. #77675 actually did most of the work (adjusting tidy); someone just has to do the multi-stage landing process:

in a first PR, land the tidy change with disabled "ignoring length unnecessarily" (and probably adjust no existing tests, to keep conflicts down)
then in a second PR, remove the unnecessary ignore-line-length comments (probably still keeping "ignoring length unnecessarily" disabled as more tests will trickle in that still have the ignore-line-length flag)
after 2 weeks or so, re-enable the "ignoring length unnecessarily" test

Step 3 is assuming that we truly want to keep that check. I personally think it is somewhat silly, but I assume it was added because people want it, so maybe we should keep it.

@rustbot

@rustbot claim

tidy: Add ignore-rules for the line length check

Auto merge of rust-lang#83549 - sjakobi:no-tidy-line-length-1, r=Mark…

Remove redundant `ignore-tidy-linelength` annotations

Rollup merge of rust-lang#83807 - sjakobi:77548-remove-ignore-annotat…

tidy: Re-enable the "ignoring file length unnecessarily" check

Auto merge of rust-lang#84100 - sjakobi:77548-reenable-check, r=jyn514

@sjakobi

@sjakobi thank you so much for pushing this through. :-)

@RalfJung

@RalfJung Thanks to your quick advice and @Anthuang's great preparatory work this was in fact pretty easy. I'm very impressed by the tooling and automation surrounding the contribution process! 💪

Navigation Menu

Repository navigation

Tidy should not check line lengths in UI tests #77548

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Tidy should not check line lengths in UI tests #77548

Description

Activity

Mark-Simulacrum commented on Oct 4, 2020

Anthuang commented on Oct 7, 2020

Mark-Simulacrum commented on Oct 7, 2020

sjakobi commented on Mar 26, 2021

RalfJung commented on Mar 26, 2021

sjakobi commented on Mar 27, 2021

RalfJung commented on Apr 12, 2021

sjakobi commented on Apr 12, 2021

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions