swiftlang / swift-format

Formatting technology for Swift source code
Apache License 2.0
2.44k stars 222 forks source link

Whitespace linting rules cannot be disabled #764

Open hamishknight opened 2 months ago

hamishknight commented 2 months ago

With this as my .swift-format:

{
    "version": 1,
    "rules": {}
}

I still get the whitespace linting rules, e.g:

/Users/hamish/src/swift-test-arena/main.swift:4:3: warning: [TrailingWhitespace] remove trailing whitespace
/Users/hamish/src/swift-test-arena/main.swift:11:4: warning: [RemoveLine] remove line break
/Users/hamish/src/swift-test-arena/main.swift:84:17: warning: [RemoveLine] remove line break
/Users/hamish/src/swift-test-arena/main.swift:89:4: warning: [RemoveLine] remove line break

Seems like there ought to be a way to disable them if desired.

ahoppen commented 2 months ago

Synced to Apple’s issue tracker as rdar://131043378

shawnhyam commented 1 month ago

I was thinking about this a bit... if format mode would remove blank lines or trailing whitespace, then doesn't it make sense for lint mode to report it as a warning, since these changes would be made if the formatter was run? Or is this a scenario where it is only being used in lint mode?

The number of line breaks allowed can be controlled with maximumBlankLines, but AFAIK there is no option to prevent trailing whitespace from being removed.

bnbarham commented 1 month ago

if format mode would remove blank lines or trailing whitespace, then doesn't it make sense for lint mode to report it as a warning

That's true... the question is whether we expect any formatting to run if no rules are enabled and whether we should allow turning on/off eg. indentation/spacing/trailing whitespace/line removal/etc. Right now there's no way to turn off the basic whitespace formatting.

dduan commented 1 month ago

Not exactly on-topic: I think a good place to be is have (an) option(s) that make(s) lint mode to not complain about anything format mode can't fix.

shawnhyam commented 1 month ago

Right now there's no way to turn off the basic whitespace formatting.

Whitespace formatting can be turned off with the hidden command line option --debug-disable-pretty-print, which will skip the formatting phase. It will also suppress the whitespace lint warnings when running in lint mode. Maybe we could consider making this a configuration option?

bnbarham commented 1 month ago

--debug-disable-pretty-print

Oh interesting, TIL.

Maybe we could consider making this a configuration option?

Maybe, though it seems like it would be nice to be able to run certain parts - ie. maybe I only want to indent, but not change any other formatting.

shawnhyam commented 1 month ago

Maybe, though it seems like it would be nice to be able to run certain parts - ie. maybe I only want to indent, but not change any other formatting.

I do enjoy the ability to re-indent code without changing much else, but this might be a bit counter to how the formatter is currently working and is likely a big change (especially in testing). I think that this would also boil down to be a very similar request as #470, so that discussion is likely relevant.

allevato commented 1 month ago

The reason --debug-disable-pretty-print isn't a supported user-facing option is because the tree transforming rules in the format pipeline have been written to not agonize over trivia when they make tree changes, because we know that the pretty printer will run later and re-format everything correctly. We'd have to update all of those rules, or the pipeline somehow, to clean that up if we wanted to support that, and we'd have to make sure we aren't introducing any unexpected changes or unpredictability (e.g., would swift-syntax's BasicFormat be enough?). It's probably possible but I don't think it's trivial.

bnbarham commented 1 month ago

Thanks for the details @allevato 🙇‍♂️. Probably depends on the exact cases in the transforms. In general BasicFormat will only insert trivia where necessary to form valid source. Would be interesting to try, though I imagine there's cases where we'd end up with something fairly odd (extra whitespace where there should be any, for example).

Could generalize this a little less and instead have a "indent only" option, which does seem relatively useful?