clang-format rejects formatting with -length=0

[kadircet]

starting with 1fae591 we reject -length=0. despite the behavior not being specified for such "point" ranges, formatting library and its users (clang-format, clangd, clang-tidy, ...) accepted those, and there are also some unittests that rely on this.

In the wild there are also some editor integrations, most notably VSCode, that can request for formatting of ranges with 0 length (e.g. after user deleted some code, or for empty lines).

Expected behavior in such a scenario is hard to define, the most obvious is to not format anything, but that isn't the existing behavior (I am not sure if that's incidental or deliberate), but clang-format tool erroring-out in such cases now is resulting in regressions for those workflows. Moreover it's also creating divergence with rest of the applications, leaking the details of the specific formatter being used into the higher level applications.

[llvmbot]

@llvm/issue-subscribers-clang-format

Author: kadir çetinkaya (kadircet)

starting with 1fae591 we reject `-length=0`. despite the behavior not being specified for such "point" ranges, formatting library and its users (clang-format, clangd, clang-tidy, ...) accepted those, and there are also some unittests that rely on this.

In the wild there are also some editor integrations, most notably VSCode, that can request for formatting of ranges with 0 length (e.g. after user deleted some code, or for empty lines).

Expected behavior in such a scenario is hard to define, the most obvious is to not format anything, but that isn't the existing behavior (I am not sure if that's incidental or deliberate), but clang-format tool erroring-out in such cases now is resulting in regressions for those workflows. Moreover it's also creating divergence with rest of the applications, leaking the details of the specific formatter being used into the higher level applications.

[owenca]

starting with 1fae591 we reject -length=0. despite the behavior not being specified for such "point" ranges, formatting library and its users (clang-format, clangd, clang-tidy, ...) accepted those, and there are also some unittests that rely on this.

As mentioned in #143302 (comment), this change only impacts the clang-format driver, not libFormat that it (and clangd, clang-tidy, etc) uses.

In the wild there are also some editor integrations, most notably VSCode, that can request for formatting of ranges with 0 length (e.g. after user deleted some code, or for empty lines).

Are those 0 length requests sent to libFormat or clang-format? If the former, there's no issue here. If the latter, I'd appreciate a repro to understand the problem.

Expected behavior in such a scenario is hard to define, the most obvious is to not format anything, but that isn't the existing behavior (I am not sure if that's incidental or deliberate), but clang-format tool erroring-out in such cases now is resulting in regressions for those workflows.

By definition, fixing bugs changes the existing behavior. If people had to specify -length=0 as a wordaround to mean formatting 1 byte, they can use -length=1 now.

Moreover it's also creating divergence with rest of the applications, leaking the details of the specific formatter being used into the higher level applications.

To the contrary, not fixing this clang-format -length bug forces the leaking as the user has to dig into the implementation of libFormat in order to understand why clang-format formats 1 byte more than it's told.

[kadircet]

Are those 0 length requests sent to libFormat or clang-format? If the former, there's no issue here.

It's always sent to libFormat, through various tools, including clang-format (in our case specifically clang-format, that's how we noticed the change, but I think we shouldn't focus too much on one particular driver here, and keep the fact that there are various driver that interact with libFormat in mind).

If the latter, I'd appreciate a repro to understand the problem.

It's really hard to give a precise repro, but what happens is, imagine you have:

foo.cc

void foo(int x) {
if (x) { return; } // this line is deleted by user.
}

as a result new contents are:

void foo(int x) {
  
}

formatting request has an offset pointing into that empty line with length=0.

By definition, fixing bugs changes the existing behavior. If people had to specify -length=0 as a wordaround to mean formatting 1 byte, they can use -length=1 now.

I think that's the bit I am failing to convey. I believe the contract here is vague, but people don't use -length=0 to mean something else. It really is a specific value (format code around a "point") from the input domain and after this change we no longer have that.

It's the same thing -lines=X:X would imply when X is an empty line.

To the contrary, not fixing this clang-format -length bug forces the leaking as the user has to dig into the implementation of libFormat in order to understand why clang-format formats 1 byte more than it's told.

Hmm, but now in addition to that, they'll need to dive into clang-format and clangd's source code (or clang-tidy) to see why there's such a discrepancy.

I can't really construct a meaningful example as formatting Length vs Length - 1 bytes seems to only matter when the character in question is \n, but we're actually triggering the formatting on the wrong range now, e.g. assume we had clang-format really formatting just the tokens covered by the [offset, offset+length):

std::vector< int> foo( );
01234567890123456789

clang-format -offset=0 -length=19 would touch f in foo (before the change) and would format foo( ) as well, whereas now because we're passing in length-1 it won't touch f hence would no longer format it (unfortunately range formatting is weird enough to format foo( ) no matter what).

[bricknerb]

FWIW, I've started getting error: length should be at least 1 in VSCode every time I save a file (which is configured to auto format using VSCode Clang-Format extension).

The workaround I found was switching the configured formatter from Clang-Format to clangd.