Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Whitespace linting rules cannot be disabled #764

Open
hamishknight opened this issue Jul 3, 2024 · 9 comments
Open

Whitespace linting rules cannot be disabled #764

hamishknight opened this issue Jul 3, 2024 · 9 comments

Comments

@hamishknight
Copy link
Contributor

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
Copy link
Member

ahoppen commented Jul 3, 2024

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

@shawnhyam
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

dduan commented Jul 26, 2024

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
Copy link
Contributor

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
Copy link
Contributor

--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
Copy link
Contributor

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
Copy link
Member

allevato commented Jul 30, 2024

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
Copy link
Contributor

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants