move to file doesn't move associated private function

[stephane-archer]

Future<void> revealIn(String filePath) async {
  if (Platform.isMacOS) {
    await _revealInFinder(filePath);
    return;
  }
  if (Platform.isWindows) {
    await _revealInExplorer(filePath);
    return;
  }
  throw UnsupportedError("Unsupported Platform");
}

Future<void> _revealInExplorer(String filePath) async {
  if (FileSystemEntity.isDirectorySync(filePath)) {
    final uri = Uri.file(filePath);
    await launchUrl(uri);
    return;
  }
  final uri = Uri.file(path_lib.dirname(filePath));
  await launchUrl(uri);
}

Future<void> _revealInFinder(String filePath) async {
  final openFileMacosPlugin = OpenFileMacos();
  await openFileMacosPlugin.open(
    filePath,
    viewInFinder: true,
  );
}

move to file option in vscode to revealIn doesn't move the private helper functions associated, so it doesn't work

[bwilkerson]

That is definitely a bug. The tool should not be producing invalid code. The only question is how to fix the bug. I can think of several options.

1. The refactoring could be unavailable in situations where the code to be moved contains references to private declarations that are not included in the code to be moved.

2. Same as 1, with the exception that if the code to be moved is going to a part in the same library the restriction is ignored.

3. The refactoring could be allowed if the private declarations can be implicitly included in the move (that is, if there are no references to any of the private declarations in code that would remain in the original library), and disallowed otherwise.

4. The private declarations that are referenced in code that would remain in the original library could be duplicated rather than moved.

5. The private declarations that are referenced in code that would remain in the original library could be made public.

The first option is the most restrictive, but is also probably the easiest to implement, so we might want to start with that just to fix the bug as quickly as possible. Though if it proves to not be too difficult to implement the second option that would be even better.

I would be in favor of the third option if we could give the user the ability to choose whether to continue with the refactoring after being told about the references. If the user isn't aware of the references, or doesn't want the private declarations moved for some other reason, then it could be surprising to have them moved without any notification. Of course, it could also be confusing if we can't explain why the refactoring isn't available in some cases, so we have to balance those two factors.

I would not want the fourth option without the ability for the user to choose between duplicating (some of) the private members and expanding the move to include the declarations that would have been left that also reference those private declarations. In other words, if a private declaration is referenced by both code to be moved and code to be left, duplicating the code is not at all an obvious choice of action, as illustrated by the fifth option, which is also not an obvious choice.

[FMorschel]

I'd opened #56791 to have this discussion previously. If you prefer we can defere here or there either way. I'm on my phone now so I will only be able to move the comments easily later.

---

Edit: Just as a quick vote, I'd prefer 2, 3 and 5 combined if possible.

[FMorschel]

Just reviewed that issue and there seems to be no new suggestion or info that wasn't already discussed here. Closed that one since this already has more conversation.