"Format Selection" can format the entire file

Repro

Have your local flutter/flutter checkout at commit 4cb6d384926fe831697e1e6cc88a6deb8d602071 (near master at the time of writing)
Open your flutter checkout in VSCode.
Open packages/flutter_tools/lib/src/compile.dart
Move your cursor to any line of code. I happened to choose this one.
Open the command palette, run the Format Selection command.

Expected result. The selected line should be formatted like so

// Before
const List<String> kDartCompilerExperiments = <String>[
];

// After
const List<String> kDartCompilerExperiments = <String>[];

Observed result. The entire file gets formatted.

Diagnostics

Workspace Environment

Dart Code extension: 3.92.0
Flutter extension: 3.93.20240702 (activated)

App: Visual Studio Code
App Host: desktop
Version: mac 1.90.2

Workspace type: Dart, Flutter (LSP)

Dart (3.5.0-321.0.dev): /Users/andrewkolos/Documents/GitHub/flutter/bin/cache/dart-sdk
Flutter (3.23.0-14.0.pre.112): /Users/andrewkolos/Documents/GitHub/flutter (macOS (darwin/macos))

Output from 'dart info'

/Users/andrewkolos/Documents/GitHub/flutter/bin/cache/dart-sdk/bin/dart info

If providing this information as part of reporting a bug, please review the information
below to ensure it only contains things you're comfortable posting publicly.

General info

Dart 3.5.0-321.0.dev (dev) (Mon Jul 1 13:02:44 2024 -0700) on "macos_arm64"
on macos / Version 14.5 (Build 23F79)
locale is en

Process info

Memory	CPU	Elapsed time	Command line
26 MB	0.0%	41:15	dart devtools --machine --allow-embedding --dtd-uri ws:/Ob0NgnqHGxdJ9wu1
25 MB	0.0%	02:38:52	dart devtools --no-launch-browser
72 MB	0.0%	41:18	dart language-server --protocol=lsp --client-id=VS-Code --client-version=3.92.0
605 MB	0.0%	41:15	dart language-server --protocol=lsp --client-id=VS-Code --client-version=3.92.0
26 MB	0.0%	41:18	dart tooling-daemon --machine
27 MB	0.0%	41:16	dart tooling-daemon --machine
60 MB	0.0%	41:16	flutter_tools.snapshot daemon

Output from 'flutter doctor'

/Users/andrewkolos/Documents/GitHub/flutter/bin/flutter doctor -v

[!] Flutter (Channel [user-branch], 3.23.0-14.0.pre.112, on macOS 14.5 23F79 darwin-arm64, locale en)
    ! Flutter version 3.23.0-14.0.pre.112 on channel [user-branch] at /Users/andrewkolos/Documents/GitHub/flutter
      Currently on an unknown channel. Run `flutter channel` to switch to an official channel.
      If that doesn't fix the issue, reinstall Flutter by following instructions at https://flutter.dev/setup.
    ! Upstream repository https://github.com/andrewkolos/flutter.git is not a standard remote.
      Set environment variable "FLUTTER_GIT_URL" to https://github.com/andrewkolos/flutter.git to dismiss this error.
    • Framework revision 7507cb44cb (2 hours ago), 2024-07-02 13:55:04 -0700
    • Engine revision 4427894144
    • Dart version 3.5.0 (build 3.5.0-321.0.dev)
    • DevTools version 2.37.0
    • If those were intentional, you can disregard the above warnings; however it is recommended to use "git" directly to perform update checks and upgrades.

[✓] Android toolchain - develop for Android devices (Android SDK version 34.0.0)
    • Android SDK at /Users/andrewkolos/Library/Android/sdk
    • Platform android-34, build-tools 34.0.0
    • ANDROID_SDK_ROOT = /Users/andrewkolos/Library/Android/sdk
    • Java binary at: /Users/andrewkolos/Applications/Android Studio.app/Contents/jbr/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 17.0.7+0-17.0.7b1000.6-10550314)
    • All Android licenses accepted.

[!] Xcode - develop for iOS and macOS (Xcode 15.0.1)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Build 15A507
    ! CocoaPods 1.12.0 out of date (1.13.0 is recommended).
        CocoaPods is a package manager for iOS or macOS platform code.
        Without CocoaPods, plugins will not work on iOS or macOS.
        For more info, see https://flutter.dev/to/platform-plugins
      To update CocoaPods, see https://guides.cocoapods.org/using/getting-started.html#updating-cocoapods

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 2023.1)
    • Android Studio at /Users/andrewkolos/Applications/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 17.0.7+0-17.0.7b1000.6-10550314)

[✓] VS Code (version 1.90.2)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.93.20240702

[✓] Connected device (3 available)
    • macOS (desktop)                 • macos                 • darwin-arm64   • macOS 14.5 23F79 darwin-arm64
    • Mac Designed for iPad (desktop) • mac-designed-for-ipad • darwin         • macOS 14.5 23F79 darwin-arm64
    • Chrome (web)                    • chrome                • web-javascript • Google Chrome 126.0.6478.127

[✓] Network resources
    • All expected network resources are available.

! Doctor found issues in 2 categories.

I think there are two issues here. Because of how the Dart formatter works, we actually have to format the entire document and then we try to minimize the results to a small set of diffs. This isn't a complete diff algorithm, it works by parsing the before/after code and walking through the tokens looking for differences in whitespace (and more recently, commas).

The first issue here is that this process is failing. When we walk through the token streams, we expect every token to be the same (expect for commas) because the formatter should only have modified whitespace. However, in this case, the unwrapped of [\n] seems to be changing the tokens in a way we don't allow:

unformatted token "[" does not match formatted token "[]"
#0      generateMinimalEdits.<anonymous closure> (package::analysis_server/src/lsp/source_edits.dart::327::9)
#1      ErrorOrRecord2Extension.mapResultsSync (package::analysis_server/src/lsp/error_or.dart::136::13)
#2      generateMinimalEdits (package::analysis_server/src/lsp/source_edits.dart::141::33)
#3      generateEditsForFormatting (package::analysis_server/src/lsp/source_edits.dart::107::10)
#4      FormatRangeHandler.formatRange (package::analysis_server/src/lsp/handlers/handler_format_range.dart::38::12)

Before we had range formatting, the reasonable thing to do when we hit an error like this was to just send back a single edit that replaces the entire file (this gives the user their formatted file as requested). However with range formatting, this can result in a single range that intersects with the requested range so the whole file is replaced.

So I think there are two things to fix:

When we fail to produce minimized edits and the request was to format a range, we should return no edits and/or an error instead of the entire file
We need to fix the case that this code doesn't handle where the empty list is producing different tokens (I presume this is intended functionality from the parser and we need to handle it here, but we should review this - one of the expectations when we decided to do it this way was that the tokens wouldn't changed based on the results of formatting).

btw, thanks for the precise repro - it's so much faster to understand bugs when the steps are so explicit :-)

The issue is not only cause dby a newline within brackets, but affects every selection after any white space between brackets.

In the code sample below, select the first line to format and it will format only that line (i.e. remove the second space before the =). Select the third line, and it will format the entire file. Remove the space between brackets in the second line, and formatting a selection will work everywhere as expected.

final before  = 1;
final list = [ ];
final after  = 3;

I observed the issue at least 3 months ago. I hope this helps somebody to fix it.

Correct - what happens here is that when there is any empty collection that is modified by the formatter, the algorithm to produce minimal edits fails. In that case, we can't produce a set of range edits, and fall back to formatting the entire document. This was fine before we supported Format Selection, but no good here.

The first fix is to ensure we only fall back to a full-document-edit when we're formatting an entire document (a Format Selection should fall back to formatting nothing), which I'm using this issue for. The other issue (which I'm splitting into #5198) is to fix the diff algorithm to handle the differences between empty collections with/without whitespace (so instead of falling back to nothing, we produce the expected edit).

I observed the issue at least 3 months ago

btw, if you find problems that don't have existing issues, please do file new ones. The reason this bug wasn't fixed sooner is that I wasn't aware of it. I would much rather get duplicate issues to close than not know about bugs :-)

Thanks for reacting. The reason I did not spot this bug report three months ago is that the issue I then observed did not involve white space within empty lists, but white space at the end of a line with a line comment. Consider the following lines:

const a = 0; // define as zero
const b = 0; // define as zero commented with trailing space

When I select the first line or anything before, Format Selection works fine. When I select the second line or anything after, Format Selection will format the entire file. When I remove the space at the end of the second line (that you might not see in this markdown text), Format Selection works as expected. The issue is not limited to lists. If the diff algorithm is the culprit, a fix should also look at line comments.

@p0mmy

@p0mmy

The reason I did not spot this bug report three months ago is that the issue I then observed did not involve white space within empty lists, but white space at the end of a line with a line comment.

Gotcha - though it would've been worth filing an issue with that information too :-)

I just tested this, and it seems like what you've hit is another case we don't handle. I can reproduce your issue and while the fix I'm working on at https://dart-review.googlesource.com/c/sdk/+/377422 will prevent the full document format, it also will prevent the selection being formatted here as it seems there's something else wrong. I've opened #5200 for that issue.

[analysis_server] Don't use full-document-format as a fallback when f…

The main issue here (that Format Selection might ever format the entire file) is fixed by dart-lang/sdk@822b1f2. This is a fix in the language server that ships as part of the SDK, so the fix will show up in a future SDK release (not a Dart-Code release).

The other issues are tracked here:

Formatting does not produce minimal edits when empty collections are unwrapped onto a single line #5198
Formatting a selection with trailing whitespace in a comment doesn't work #5200

Navigation Menu

Repository navigation

"Format Selection" can format the entire file #5169

Repro

Diagnostics

General info

Process info

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

"Format Selection" can format the entire file #5169

Description

Repro

Diagnostics

General info

Process info

Activity

DanTup commented on Jul 3, 2024

DanTup commented on Jul 3, 2024

p0mmy commented on Jul 12, 2024

DanTup commented on Jul 24, 2024

DanTup commented on Jul 24, 2024

p0mmy commented on Jul 24, 2024

DanTup commented on Jul 24, 2024

DanTup commented on Jul 24, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions