nicklockwood / SwiftFormat

A command-line tool and Xcode Extension for formatting Swift code
MIT License
7.62k stars 623 forks source link

Spaces are incorrectly added before indented parameters while formatting a fragment #1738

Open vinocher-bc opened 1 week ago

vinocher-bc commented 1 week ago

Spaces are incorrectly added before indented parameters while formatting the parameters of a function call in a fragment. This was reproduced on Windows.

Repro: (UseCtrl+Z, Enter to signal EOF in stdin): Note the spaces added before foo1 & foo2

C:\>swiftformat --fragment true stdin
   foo: bar,
   foo1: bar2,
   foo2: bar3
^Z
Running SwiftFormat...
   foo: bar,
       foo1: bar2,
       foo2: bar3
SwiftFormat completed successfully.

The formatting of the indentation is correct if the entire function call is provided:

C:\>swiftformat --fragment true stdin
myFunc(
   foo: bar,
   foo1: bar2,
   foo2: bar3
)
^Z
Running SwiftFormat...
myFunc(
    foo: bar,
    foo1: bar2,
    foo2: bar3
)
SwiftFormat completed successfully.
calda commented 1 week ago

I believe SwiftFormat expects all input to be syntactically-valid Swift code, and the behavior of rules is more of less undefined when passing in invalid Swift code.

Since this input code isn't valid Swift code, to me it isn't unexpected that the output would be unusual:

   foo: bar,
   foo1: bar2,
   foo2: bar3

Do you have a repro case for this where the indentation is incorrect when passing in text that is valid Swift code?

vinocher-bc commented 1 week ago

@calda This fragment is part of valid Swift Code, and the expectation is that since --fragment true is passed, SwiftFormat will be conservative in its formatting and keep the original indentation. Is that expectation correct? Thanks.

This scenario can occur in editors like VSCode which have a "editor.formatOnSaveMode": "modifications" setting to format only code changes -- see Only Format Modified Text. Such a setting prevents unnecessary diffs when a document is saved, by formatting only the new changes.

In this case, the following parameters are part of valid Swift Code. For example, assume that a user made changes to the parameters of an existing function call in a document. In that case, a fragment containing the parameters will need to be formatted.

Fragment:

   foo: bar,
   foo1: bar2,
   foo2: bar3

Context:

  myFunc(
    foo: bar,
    foo1: bar2,
    foo2: bar3
  )
nicklockwood commented 1 week ago

@calda yeah @vinocher-bc is correct. I think the simplest fix here is just to disable the wrap rule when running in --fragment mode (there are probably many other rules that also should be disabled but currently aren't)

nicklockwood commented 1 week ago

@vinocher-bc that said, I think the vs-code extension should not be using --fragment. It was originally introduced for this purpose as part of the SwiftFormat Xcode extension, but I replaced it with --linerange instead, which is much more reliable because it takes the entire file into account for parsing but limits any changes to just the selected region.

vinocher-bc commented 1 week ago

@vinocher-bc that said, I think the vs-code extension should not be using --fragment. It was originally introduced for this purpose as part of the SwiftFormat Xcode extension, but I replaced it with --linerange instead, which is much more reliable because it takes the entire file into account for parsing but limits any changes to just the selected region.

@nicklockwood I'm seeing some weirdness with --linerange formatting ,where the output indentation is incorrect:

C:\>swiftformat --linerange 3, 4 stdin
myFunc(
    foo: bar,
    foo1: bar1,
        foo2: bar2,
        foo3: bar3
)
^Z
Running SwiftFormat...
myFunc(
    foo: bar,
    foo1: bar1,
        foo2: bar2,
    foo3: bar3
)
SwiftFormat completed successfully.
vinocher-bc commented 1 week ago

@calda a similar issue occurs with method chaining. The indentation is removed. Expected: the indentation should not be removed. The behavior seems worse with--linerange. If needed, I can open another issue to track this. Thanks.

 C:\>swiftformat --fragment true stdin
    .foo ( )
^Z
Running SwiftFormat...
.foo()
SwiftFormat completed successfully.
C:\>swiftformat --linerange 0,0 stdin
   .foo ( )
^Z
Running SwiftFormat...
.foo ( )
SwiftFormat completed successfully.

It is formatted correctly when the context is provided:

C:\>swiftformat --fragment true stdin
    baz
      .foo  ( )
      .bar (    )
^Z
Running SwiftFormat...
    baz
        .foo()
        .bar()
SwiftFormat completed successfully.
nicklockwood commented 3 days ago

@vinocher-bc --linerange assumes the entire file is passed as input, but only applies changes to the specified range. If you are passing only a fragment of the file then --fragment is the correct argument to use