extract method produce invalid code with continue

[stephane-archer]

Future<void> _addDirectoryToZip({
  required ZipEncoder encoder,
  required Directory dir,
  required String zipEntryPath,
  required Set<String> ancestors,
}) async {
  final canonical = await dir.resolveSymbolicLinks();

  if (ancestors.contains(canonical)) {
    return;
  }

  final newAncestors = {...ancestors, canonical};

  await for (final fsEntity in dir.list(recursive: false, followLinks: false)) {
    final name = path_lib.basename(fsEntity.path);
    final entryPath =
        zipEntryPath.isEmpty ? name : path_lib.join(zipEntryPath, name);

    var real = fsEntity;

    if (fsEntity is Link) {
      final resolved = await fsEntity.resolveSymbolicLinks();
      final type = FileSystemEntity.typeSync(resolved);

      if (type == FileSystemEntityType.file) {
        real = File(resolved);
      } else if (type == FileSystemEntityType.directory) {
        if (newAncestors.contains(resolved)) {
          final dirFile = ArchiveFile.directory(entryPath);
          encoder.add(dirFile);
          continue; /// extract any method with this continue inside, and the produced code would be invalid
        }
        real = Directory(resolved);
      } else {
        throw FileSystemException('Broken symlink', fsEntity.path);
      }
    }

    if (real is Directory) {
      final dirFile = ArchiveFile.directory(entryPath);
      encoder.add(dirFile);

      await _addDirectoryToZip(
        encoder: encoder,
        dir: real,
        zipEntryPath: entryPath,
        ancestors: newAncestors,
      );
    }

    if (real is File) {
      final input = InputFileStream(real.path);

      final archiveFile = ArchiveFile.stream(
        entryPath,
        input,
      );

      archiveFile.lastModTime =
          real.lastModifiedSync().millisecondsSinceEpoch ~/ 1000;

      archiveFile.compression = CompressionType.none;

      encoder.add(archiveFile);

      await input.close();
    }
  }
}

[bwilkerson]

Is the request to not provide this assist if there's a break or continue inside the extracted method that would be invalid because the enclosing control structure isn't included in the code to be extracted?

[stephane-archer]

You can fail because of the presence of continue.
Or ideally, you return null and check for null to execute continue in the calling code

[bwilkerson]

Or ideally, you return null and check for null to execute continue in the calling code

Assuming, of course, that null isn't a valid value to return from the extracted method. And assuming that there aren't both break and continue statements, because then it wouldn't be able to tell which behavior to follow.

It's probably better to just not offer the refactoring.

[stephane-archer]

In Go, with the capacity to return multiple values, you could return a bool for each case, but in Dart, I don't know if there is that kind of possibility

[bwilkerson]

Yes, we could return a record, but it isn't the kind of code I'd want our tools to suggest to users. It puts the code that makes a flow control decision too far from the flow control being impacted. That's just not a good programming style, IMO.