bad function extraction

[stephane-archer]

import 'dart:io';

import 'package:thumblr/thumblr.dart';

/// Returns the duration of the video in seconds as a double.
/// Example: 3.728 means 3 seconds and 728 milliseconds.
Future<double> getVideoDuration(String videoPath) async {
  if (!FileSystemEntity.isFileSync(videoPath)){
    throw ArgumentError("$videoPath is not a file", "videoPath");
  }
  Thumbnail thumbnail = await generateThumbnail(filePath: videoPath); // begin function extraction
  final videoDuration = thumbnail.videoDuration;
  if (videoDuration == null) {
    throw StateError("Video duration not found");
  } // end function extraction
  return videoDuration; // in seconds
}

produced code (which is invalid: A value of type 'double?' can't be returned from the function 'getVideoDuration' because it has a return type of 'Future<double>'.):

import 'dart:io';

import 'package:thumblr/thumblr.dart';

/// Returns the duration of the video in seconds as a double.
/// Example: 3.728 means 3 seconds and 728 milliseconds.
Future<double> getVideoDuration(String videoPath) async {
  if (!FileSystemEntity.isFileSync(videoPath)){
    throw ArgumentError("$videoPath is not a file", "videoPath");
  }
  double? videoDuration = await _getVideoDurationMacAndWindows(videoPath);
  return videoDuration; // in seconds
}

Future<double?> _getVideoDurationMacAndWindows(String videoPath) async {
  Thumbnail thumbnail = await generateThumbnail(filePath: videoPath);
  final videoDuration = thumbnail.videoDuration;
  if (videoDuration == null) {
    throw StateError("Video duration not found");
  }
  return videoDuration;
}

expected code:

import 'dart:io';

import 'package:thumblr/thumblr.dart';

/// Returns the duration of the video in seconds as a double.
/// Example: 3.728 means 3 seconds and 728 milliseconds.
Future<double> getVideoDuration(String videoPath) async {
  if (!FileSystemEntity.isFileSync(videoPath)){
    throw ArgumentError("$videoPath is not a file", "videoPath");
  }
  double videoDuration = await _getVideoDurationMacAndWindows(videoPath);
  return videoDuration; // in seconds
}

Future<double> _getVideoDurationMacAndWindows(String videoPath) async {
  Thumbnail thumbnail = await generateThumbnail(filePath: videoPath);
  final videoDuration = thumbnail.videoDuration;
  if (videoDuration == null) {
    throw StateError("Video duration not found");
  }
  return videoDuration;
}

[FMorschel]

I'm not sure we have any refactors that can take flow analysis into consideration like this, but it is surely interesting.

Maybe @bwilkerson knows.

[stephane-archer]

@FMorschel, I think it is more important that the return value of the extracted function account for the promotion (remove the null type here), as the rest of the code relies on that promotion.

[FMorschel]

Sorry if I wasn't clear, this is what I'm asking here.

This isn't happening today because the returned variable videoDuration has a static type of double?. That type is carried over because the refactoring doesn't consider the promotion (flow analysis) here.

I agree it would be best if this were possible. I'm just not sure we have anything similar today or if this would be the first time (and thus take more time).

[bwilkerson]

We track the type of the variable after inference, but unless there's a reference to the variable after the inferred type has been computed, and before any code that might again change the inferred type, there isn't any record of it that we can access after the fact, which is when the assist is running. Those caveats make it hard to know whether some following reference has the same type that it would have had at the new reference point. (We'd basically have to re-implement or re-run type inference, neither of which is feasible at the moment.)

While it would be nice to be able to get a better result, I think it's unlikely that we can reasonably improve it.