When "git checkout" moves a file, the analysis server doesn't process the removal of the old file

[DanTup]

I noticed this while trying to repro https://github.com/Dart-Code/Dart-Code/issues/2329 (I'm not sure if it's related to that issue yet). I ran all this on macOS - I'm not sure if it's OS-specific.

When changing Git branch, files that are moved are retained in the analysis server memory. This means you can have invalid code (eg. importing a file that's been since moved) but not get any errors for it until you restart the IDE/force reanalyze.

These steps will create a repo that has two files (a.dart and b.dart, where a.dart imports and calls a function from b.dart):

mkdir /tmp/my_repro_folder
cd /tmp/my_repro_folder
git init
echo "import 'b.dart'; main() {foo();}" > a.dart
echo "foo() {}" > b.dart
git add .
git commit -m "Added A and B"

This creates a second branch where b.dart has been moved to a folder (without the import being updated).

git checkout -b branch_two
mkdir b
mv b.dart b/b.dart
git add .
git commit -m "Moved B into B/"

Finally, switch back to the master branch:

git checkout master

Now open this project in VS Code (to avoid any confusion with priority files, no not open either of the Dart files) and run the Dart: Open Analyzer Diagnostics command. Note that the "Added files" section contains both files as expected.

Now change branch in a terminal:

git checkout branch_two

You'll notice two things:

1. The editor is not showing an error in a.dart for the (now-invalid) import.
2. Reloading the diagnostics page still shows b.dart under "Added files", but the file does not exist anymore

Here's the relevant part of the instrumentation file:

1586166259731:Req:{"id"::"4","method"::"diagnostic.getServerPort","clientRequestTime"::1586166259730}
1586166259742:Res:{"id"::"4","result"::{"port"::51182}}
1586166269724:Watch:/private/tmp/my_repro_folder:/private/tmp/my_repro_folder/.git/index:modify
1586166269724:Watch:/private/tmp/my_repro_folder:/private/tmp/my_repro_folder/.git/logs/HEAD:modify
1586166269724:Watch:/private/tmp/my_repro_folder:/private/tmp/my_repro_folder/.git/HEAD:modify
1586166269725:Watch:/private/tmp/my_repro_folder:/private/tmp/my_repro_folder/b/b.dart:add
1586166269730:Noti:{"event"::"server.status","params"::{"analysis"::{"isAnalyzing"::true}}}
1586166269743:Noti:{"event"::"analysis.errors","params"::{"file"::"/private/tmp/my_repro_folder/b/b.dart","errors"::[]}}
1586166269743:Noti:{"event"::"server.status","params"::{"analysis"::{"isAnalyzing"::false}}}

What's interesting is that there doesn't seem to be any event for the file being deleted (there's an add for b/b.dart - I don't know if that's a move/rename being interpreted wrong, or if an event is being missed here).

(@bwilkerson fyi - I don't have powers to label issues)

[devoncarew]

We received reports from flutter framework engineers in the past which map to something like this issue - a series of changing git branches will confuse the analysis server.

We may need to investigate changes in how we use file watching? Improvements to the file watching package?

[DanTup]

This might be an issue in the watcher package. I created a script that just prints watch events and switches between my branches:

import 'dart:io';

import 'package:watcher/watcher.dart';

final repoFolder = '/tmp/my_repro_folder';

void main() async {
  await checkout('master');

  print('Setting up watcher...');
  DirectoryWatcher(repoFolder).events.listen((event) {
    if (event.path.contains('.git')) return;
    print('    ${event.type}: ${event.path}');
  });

  await Future.delayed(Duration(seconds: 5));
  await checkout('branch_two');
  await Future.delayed(Duration(seconds: 5));
  await checkout('master');
  await Future.delayed(Duration(seconds: 5));
  await checkout('branch_two');
  await Future.delayed(Duration(seconds: 5));
}

Future<void> checkout(String branch) async {
  print('');
  print('Checking out branch $branch...');
  await Process.run('git', ['checkout', branch], workingDirectory: repoFolder);
  await Future.delayed(Duration(seconds: 2));
  final files = await Directory(repoFolder)
      .list(recursive: true)
      .where((f) => !f.path.contains('.git'))
      .toList();
  print('  Current files:\n    ${files.join('\n    ')}');
}

This is the output when I run it:

Checking out branch master...
  Current files:
    File: '/tmp/my_repro_folder/two.txt'
    File: '/tmp/my_repro_folder/b.dart'        // Note this file exists and disappears without any event
    Directory: '/tmp/my_repro_folder/folder'
    File: '/tmp/my_repro_folder/a.dart'
Setting up watcher...

Checking out branch branch_two...
    add: /tmp/my_repro_folder/b/b.dart
  Current files:
    File: '/tmp/my_repro_folder/two.txt'
    Directory: '/tmp/my_repro_folder/folder'
    File: '/tmp/my_repro_folder/a.dart'
    Directory: '/tmp/my_repro_folder/b'
    File: '/tmp/my_repro_folder/b/b.dart'

Checking out branch master...
    remove: /tmp/my_repro_folder/b/b.dart
  Current files:
    File: '/tmp/my_repro_folder/two.txt'
    File: '/tmp/my_repro_folder/b.dart'                // And it's back, without any event
    Directory: '/tmp/my_repro_folder/folder'
    File: '/tmp/my_repro_folder/a.dart'

Checking out branch branch_two...
    add: /tmp/my_repro_folder/b/b.dart
  Current files:
    File: '/tmp/my_repro_folder/two.txt'
    Directory: '/tmp/my_repro_folder/folder'
    File: '/tmp/my_repro_folder/a.dart'
    Directory: '/tmp/my_repro_folder/b'
    File: '/tmp/my_repro_folder/b/b.dart'

[natebosch]

@DanTup - I cannot reproduce this locally.

Does this only happen on windows? Just noticed it was on mac - I'll try there.

[natebosch]

I found the bug causing a problem for this specific reproduction case - it happens because of a shared prefix between /tmp/my_repro_folder/b and tmp/my_rpro_folder/b.dart. I'll send out a PR shortly.

[DanTup]

@natebosch thanks!

I'm not familiar with how these packages end up in the SDK - is this the right place for me to check when it's rolled into the SDK?

[natebosch]

@DanTup - yes that's exactly it.

[DanTup]

@natebosch cool, thanks! If it's as simple as updating that file, I could open a CL? Though I guess it pulls from Pub so this will need publishing there first?

Thanks!

[natebosch]

We don't technically need to publish to pub, we could pull it in by SHA. I'll get started with that.

[DanTup]

Awesome, thanks!