excessively long lines complicates CI based format validation

[scode]

(Disclaimer: I'm not actually implementing CI validation at the moment as I'm mostly learning rust, so it's a hypothetical. But it's one of the first things I'd do if Rust were to be adopted where I work, for example, and I'm intending on doing so with Travis on my personal stuff as well.)

I ran into the impossibility case:

Rustfmt failed at /Users/scode/..../tunytools/src/bin/numstats.rs:94: line exceeded maximum length (maximum: 100, found: 110) (sorry)

This is because I have a comment with a long URL in it:

https://github.com/scode/tunytools/blob/master/src/bin/numstats.rs#L94

If I want to implement a CI check that ensures code is formatted, I would use the diffing mode to get a user friendly diff along with looking for exit code 4:

    4 = Formatted code differs from existing code (write-mode diff only)

However since the case of impossible-to-format has an exit code of 3, and it takes precedence, I now cannot, by exit code, differentiate between "the file is otherwise fine, but there's an impossible-to-format case in it" and "the file would have been changed". The only way I see to do this would be to look at what goes to stdout, rather than pay attention to the exit code.

Because it seems reasonable to have a maximum line length policy that is allowed to be violated in certain cases such as including a link without breaking it up (though reasonable people can disagree), it seems like it would be preferable to allow this through. I can think of a few options:

• Provide a flag that says to not treat impossible-to-format as errors. I'm not sure what unintended consequences this has for other cases that are in this class.
• Provide a mechanism to allow leniency in certain cases (e.g. "line length violation is okay, if it's impossible to format otherwise").
• Provide an additional exit code that represents the combination of 3+4; i.e., there is a diff and it's impossible to format correctly (validation code would then require a 3 for success if it wants to allow impossible to format code).
• Document that it is valid to interpret an empty stdout as an indication that the file did not need to be changed, if the exit code is 3 specifically.

Additionally:

• Document that when exiting with code 3, formatting that is possible has still been done (basic testing indicates to me that this is the case, but I feel uncertain whether this is the contract and generally true).

[Aankhen]

Would disabling error_on_line_overflow in rustfmt.toml help?

[scode]

That would seem to work around this particular case. I'm not sure whether or not it's a general answer though. The documentation of the exit codes suggests that the 'impossible to format' case is general in nature:

    3 = Code is valid, but it is impossible to format it properly

I'm not sure it the intent is for such cases to be a formatting policy violation or not. If that is indeed the case, then yeah - for anything that one would like to let through despite it not being possible to format, turning off the appropriate option seems reasonable.

I took a look at https://github.com/rust-lang-nursery/rustfmt/blob/74f5a515ef025ed629cf3cf1c04ed783c9a371c6/src/lib.rs#L446 and surrounding code, and I'm not yet sure how to, or whether, those cases are differentiated. The comment (https://github.com/rust-lang-nursery/rustfmt/blob/74f5a515ef025ed629cf3cf1c04ed783c9a371c6/src/lib.rs#L627) suggests that error is used only when it cannot be corrected, but yet I see nothing different about the application of TraillingWhitespace (https://github.com/rust-lang-nursery/rustfmt/blob/74f5a515ef025ed629cf3cf1c04ed783c9a371c6/src/lib.rs#L662) which presumably can always be corrected. Basically I was considering whether the error message here should tell the user about the switch being available, but I don't know the rustfmt code base yet.

My overall motivation is for cargo fmt to "just work", including for someone just sticking a line into their travis config without a lot of research and effort. The goal is to incentivize adoption and promote consistency like gofmt has done in the Go world.

I am new to rustfmt and of course defer to others, but my current thoughts are:

• If you are running cargo fmt, it should never fail unless there's a bug in the formatter.
• If you are running cargo fmt -- --write-mode=diff (maybe in the future this will be cargo fmt --test or similar?), it should only fail if (a) there's a bug in the formatter or (b) a diff was produced.

Modes of operation which cause failures for other reasons seem best left as opt-in behaviors in order to promote painless widespread adoption.

Thoughts?

[nrc]

A possible solution is discussed in #1977 (comment) Closing this issue in favour of #1977