Treat form feed as valid whitespace before a line continuation by soundsonacid · Pull Request #19220 · astral-sh/ruff

soundsonacid · 2025-07-08T21:39:17Z

Fixes #19175

…hars

ntBre · 2025-07-08T21:50:29Z

Thanks for working on this! Could you add a regression test based on the example in the issue?

I'm slightly concerned that is_whitespace is too permissive, but I'm also not quite sure why it was only limited to spaces and tabs before.

github-actions · 2025-07-08T21:51:34Z

`ruff-ecosystem` results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

soundsonacid · 2025-07-08T21:56:48Z

Thanks for working on this! Could you add a regression test based on the example in the issue?

I'm slightly concerned that is_whitespace is too permissive, but I'm also not quite sure why it was only limited to spaces and tabs before.

No problem! I had a similar thought about the is_whitespace check, but I couldn't find (with admittedly not a ton of time searching) a comprehensive list of all valid whitespace Python characters. If you have one handy, I'm happy to make the change to that -- in the meantime I'll keep looking for one myself.

dscorbett · 2025-07-08T22:03:43Z

Section 2.1.9 “Whitespace between tokens” says:

Except at the beginning of a logical line or in string literals, the whitespace characters space, tab and formfeed can be used interchangeably to separate tokens. Whitespace is needed between two tokens only if their concatenation could otherwise be interpreted as a different token (e.g., ab is one token, but a b is two tokens).

Those are the only three characters Python treats as white space within a line.

…per python spec

ntBre

Thanks, this looks great!

And thanks @dscorbett for the report and for the whitespace reference.

I just had a couple of very small suggestions, but this is good to go otherwise.

crates/ruff_linter/src/importer/insertion.rs

crates/ruff_linter/resources/test/fixtures/flake8_type_checking/whitespace.py

MichaReiser · 2025-07-09T07:03:01Z

crates/ruff_linter/src/importer/insertion.rs

+            // space, tab, & formfeed (respectively) are the only three valid whitespace characters
+            // within a line:
+            // https://docs.python.org/3/reference/lexical_analysis.html#whitespace-between-tokens
+            ' ' | '\t' | '\u{000C}' => continue,


We could use

ruff/crates/ruff_python_trivia/src/whitespace.rs

Lines 40 to 46 in 9f3a38d

pub const fn is_python_whitespace(c: char) -> bool {

matches!(

c,

// Space, tab, or form-feed

' ' | '\t' | '\x0C'

)

}

thanks for lmk about this, updated to use this fn

…l chars

ntBre

Thanks!

ntBre · 2025-07-09T15:11:18Z

Looks like we need to update the snapshot for the new comment

soundsonacid · 2025-07-10T04:52:32Z

Weird, i ran cargo insta review and it said no changes, also ran cargo test on my local and it was fine

Happy to do whatever it takes here just let me know what to do, not super familiar with how the tests work yet

MichaReiser · 2025-07-10T06:28:49Z

Can you verify that you have no uncommited changes (git status, git diff)

ntBre · 2025-07-10T12:48:13Z

I checked out the branch locally and see the same test failures, so it might be something with git as Micha said.

I can push a commit if you'd like but wanted to give you the solo credit for the PR if possible :)

soundsonacid · 2025-07-10T13:37:22Z

yep, nothing unstaged, no diffs, no snapshots to review

…re_help * 'main' of https://github.com/astral-sh/ruff: Add simple integration tests for all output formats (astral-sh#19265) [`flake8-return`] Fix false-positive for variables used inside nested functions in `RET504` (astral-sh#18433) [ty] Add a `--quiet` mode (astral-sh#19233) Treat form feed as valid whitespace before a line continuation (astral-sh#19220) [ty] Make `check_file` a salsa query (astral-sh#19255) [ty] Consolidate submodule resolving code between `types.rs` and `ide_support.rs` (astral-sh#19256) [ty] Remove countme from salsa-structs (astral-sh#19257) [ty] Improve and document equivalence for module-literal types (astral-sh#19243) [ty] Optimize protocol subtyping by removing expensive and unnecessary equivalence check from the top of `Type::has_relation_to()` (astral-sh#19230) [ty] Ecosystem analyzer: parallelize, fix race condition (astral-sh#19252) [ty] Add completion kind to playground (astral-sh#19251) [ty] Deploy ecosystem diff to Cloudflare pages (astral-sh#19234) [ty] Add semantic token provider to playground (astral-sh#19232)

(fix): detect continuations with is_whitespace instead of hardcoded c…

8d36e23

…hars

ntBre added bug Something isn't working fixes Related to suggested fixes for violations labels Jul 8, 2025

soundsonacid added 2 commits July 8, 2025 17:22

add regression test, revert to space / tab / formfeed for whitespace …

cb4bb72

…per python spec

(chore): remove artifact

24799f1

soundsonacid changed the title ~~(fix): detect continuations with is_whitespace instead of hardcoded chars~~ (fix): treat unicode formfeed as valid whitespace within match_continuation Jul 8, 2025

ntBre reviewed Jul 8, 2025

View reviewed changes

crates/ruff_linter/src/importer/insertion.rs Outdated Show resolved Hide resolved

crates/ruff_linter/resources/test/fixtures/flake8_type_checking/whitespace.py Show resolved Hide resolved

(chore): pr review

2ccd10e

MichaReiser reviewed Jul 9, 2025

View reviewed changes

(fix): use is_python_whitespace instead of matching against individua…

f3cb204

…l chars

ntBre approved these changes Jul 9, 2025

View reviewed changes

ntBre changed the title ~~(fix): treat unicode formfeed as valid whitespace within match_continuation~~ Treat form feed as valid whitespace before a line continuation Jul 9, 2025

Update snapshots

8f65fc2

MichaReiser force-pushed the main branch from 8e20154 to 8f65fc2 Compare July 10, 2025 14:06

MichaReiser enabled auto-merge (squash) July 10, 2025 14:07

MichaReiser merged commit 83b5bbf into astral-sh:main Jul 10, 2025
34 checks passed

BrewTestBot mentioned this pull request Jul 11, 2025

ruff 0.12.3 Homebrew/homebrew-core#229792

Merged

	pub const fn is_python_whitespace(c: char) -> bool {
	matches!(
	c,
	// Space, tab, or form-feed
	' ' \| '\t' \| '\x0C'
	)
	}

Comments

Conversation

soundsonacid commented Jul 8, 2025

Uh oh!

ntBre commented Jul 8, 2025

Uh oh!

github-actions bot commented Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ruff-ecosystem results

Linter (stable)

Linter (preview)

Uh oh!

soundsonacid commented Jul 8, 2025

Uh oh!

dscorbett commented Jul 8, 2025

Uh oh!

ntBre left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

MichaReiser Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

soundsonacid Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

ntBre left a comment

Choose a reason for hiding this comment

Uh oh!

ntBre commented Jul 9, 2025

Uh oh!

soundsonacid commented Jul 10, 2025

Uh oh!

MichaReiser commented Jul 10, 2025

Uh oh!

ntBre commented Jul 10, 2025

Uh oh!

soundsonacid commented Jul 10, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions bot commented Jul 8, 2025 •

edited

Loading

`ruff-ecosystem` results