Whitespace before classes not inserted when used with "modificationsIfAvailable" in vscode

[maltevesper]

Summary

I am not sure if this is a vscode plugin or ruff issue, that said:

When using the "editor.formatOnSaveMode": "modificationsIfAvailable" setting, Ruff does not add blank lines around newly added classes. This results in the CI checking the entire file finding new issues in the file.

Example:

def oldCode():
    ...
class InsertedClass:    # New
    pass                        # New
def moreOldCode():
    ...

After formating I get the formated class back as (formatting inside the class I added works as expected):

class InsertedClass:    # New
    pass                        # New

I would like the language server to return the new class with surrounding needed whitespace if that is possible, i.e.:

                                  # ruff add whitespace before
                                  # ruff add whitespace before
class InsertedClass:    # New
    pass                        # New
                                  # ruff add whitespace after
                                  # ruff add whitespace after

I have not dug into the language server protocol and I am not sure if this is feasible (i.e. does the language server get enough context (i.e. code outside the edited area), when modificationsIfAvailable is set?

Version

No response

[ntBre]

Ah, that's interesting and does seem like a nice feature. I'm not sure if ruff gets a large enough range to make it feasible either, as you said. Maybe @MichaReiser or @dhruvmanila would know more.

Here are a few related links I found, including support for modificationsIfAvailable in ruff-lsp:

• https://code.visualstudio.com/updates/v1_49#_only-format-modified-text
• Support format modificationsIfAvailable ruff-vscode#319
• Range formatting support ruff-lsp#373

When reproducing this locally, I also see whitespace added after the new class but not before. Maybe that's something with my local setup, though.

Out of curiosity, what's the benefit of using modificationsIfAvailable in this situation, if you're formatting the whole file in CI anyway?

[MichaReiser]

I can reproduce this in the CLI using:

uvx ruff@latest format --range 3:1-5:1 range_formatting.py

The main idea of modificationsIfAvailable is that Ruff leaves unchanged lines alone. Now, Ruff doesn't know if the class was added or removed. It only knows that there's a class and the class is in the range that should be formatted. If it's a new class, it should add the empty lines. If it's an existing class, things are less clear.

Given that Ruff can't tell if the class is new or not, I'm still leaning towards adding the blank lines if the range covers a class (or function, etc.).

The logic for this is in format_range. It's fairly involved because we first:

• Try to find the smallest possible range the formatter can format and that covers the range provided by the user
• After formatting, identify the range again and extract the relevant formatted code. This uses source maps to map the original offsets to the new offsets.

I'm not entirely sure where I would implement the fix. The narrowing seems to be working as expected, it only selects the class. But we then don't include the empty lines when extracting the formatted code. This may require changing Printed.slice_range or emitting more source map markers in FormatSuite (etc)

This test case seems related

ruff/crates/ruff_python_formatter/resources/test/fixtures/ruff/range_formatting/regressions.py

Lines 77 to 85 in 4f7fb56

	# This demonstrates the use case where a user inserts a new function or class after an existing function.
	# In this case, we should avoid formatting the node that directly precedes the new function/class.
	# However, the problem is that the preceding node **must** be formatted to determine the whitespace between the two statements.
	def test_start ():
	print("Ideally this gets not reformatted" )
	<RANGE_START>
	def new_function_inserted_after_test_start ():
	print("This should get formatted" )<RANGE_END>

[maltevesper]

Out of curiosity, what's the benefit of using modificationsIfAvailable in this situation, if you're formatting the whole file in CI anyway?

@ntBre : I think I still owe you an answer: I set my editor to format only the code I changed, to cause less issues with projects not using a formatter, and noticed when a project which uses a pipeline to check formatting complained.
In hindsight I think this issue is minor. It might help with easing in teams which are reluctant on enforced formatting (introducing it for new code first), but as long as CI and formatting are consistent it should be fine. Obviously one would have to write a CI pipeline to only format the changed code for the last usecase to work. I assume this usecase would work with the current implementation as the reformatting should be consistent between duing it in bulk in the ci or running this from the editor.