feature-rectore-3115670: rule for file_unmanaged_copy

[mohsin-saeed]

file_unmanaged_copy rector

[damontgomery]

Thanks for the work, @mohsin-saeed!

I think you've picked a complicated situation here since the function has an optional $destination argument, but the service method requires it.

We would need to account for this in our rector rule or at least list it in the "Improvement Opportunities".

function file_unmanaged_copy($source, $destination = NULL, $replace = FILE_EXISTS_RENAME) {
  @trigger_error('file_unmanaged_copy() is deprecated in Drupal 8.7.0 and will be removed before Drupal 9.0.0. Use \Drupal\Core\File\FileSystemInterface::copy(). See https://www.drupal.org/node/3006851.', E_USER_DEPRECATED);
  try {
    $file_system = \Drupal::service('file_system');

    // Build a destination URI if necessary.
    if (!isset($destination)) {
      $destination = file_build_uri($file_system->basename($source));
    }
    return $file_system->copy($source, $destination, $replace);
  }
  catch (FileException $e) {
    return FALSE;
  }
}

In the above example, we would probably need to check if $destination is not passed as an argument. You can check $node->args which is an array. If there are at least two arguments, your substitution would work. If not, we should do the more complex replacement as shown above or at least skip over that situation.

Thanks again for the submission. Do you want to make those changes?

[damontgomery]

@mohsin-saeed, it looks like you can actually call file_unmanaged_copy with just the source argument.

Can you create an example with that?

function file_unmanaged_copy($source, $destination = NULL, $replace = FILE_EXISTS_RENAME) {
  @trigger_error('file_unmanaged_copy() is deprecated in Drupal 8.7.0 and will be removed before Drupal 9.0.0. Use \Drupal\Core\File\FileSystemInterface::copy(). See https://www.drupal.org/node/3006851.', E_USER_DEPRECATED);
  try {
    $file_system = \Drupal::service('file_system');

    // Build a destination URI if necessary.
    if (!isset($destination)) {
      $destination = file_build_uri($file_system->basename($source));
    }
    return $file_system->copy($source, $destination, $replace);
  }
  catch (FileException $e) {
    return FALSE;
  }
}

[damontgomery]

Can you provide a link to the change record. This is usually in the function definition comment,

/**
 * Copies a file to a new location without database changes or hook invocation.
 *
 * This is a powerful function that in many ways performs like an advanced
 * version of copy().
 * - Checks if $source and $destination are valid and readable/writable.
 * - If file already exists in $destination either the call will error out,
 *   replace the file or rename the file based on the $replace parameter.
 * - If the $source and $destination are equal, the behavior depends on the
 *   $replace parameter. FILE_EXISTS_REPLACE will error out. FILE_EXISTS_RENAME
 *   will rename the file until the $destination is unique.
 * - Works around a PHP bug where copy() does not properly support streams if
 *   safe_mode or open_basedir are enabled.
 *   @see https://bugs.php.net/bug.php?id=60456
 *
 * @param $source
 *   A string specifying the filepath or URI of the source file.
 * @param $destination
 *   A URI containing the destination that $source should be copied to. The
 *   URI may be a bare filepath (without a scheme). If this value is omitted,
 *   Drupal's default files scheme will be used, usually "public://".
 * @param $replace
 *   Replace behavior when the destination file already exists:
 *   - FILE_EXISTS_REPLACE - Replace the existing file.
 *   - FILE_EXISTS_RENAME - Append _{incrementing number} until the filename is
 *       unique.
 *   - FILE_EXISTS_ERROR - Do nothing and return FALSE.
 *
 * @return
 *   The path to the new file, or FALSE in the event of an error.
 *
 * @deprecated in drupal:8.7.0 and is removed from drupal:9.0.0.
 *   Use \Drupal\Core\File\FileSystemInterface::copy().
 *
 * @see file_copy()
 * @see https://www.drupal.org/node/3006851
 */

[damontgomery]

Unless you are looking at traits for classes, you don't need to add this trait to the rector class.

[damontgomery]

For simple function to service method calls, we have a class you could extend, FunctionToServiceBase.

Unfortunately, I think that this rector rule is more complex. I'll add a comment to the overall review.

[damontgomery]

We've started to use if ($node->getName($node->name) === 'file_unmanaged_copy') { in places like this which is easier to read.

[mohsin-saeed]

Thanks for the work, @mohsin-saeed!

I think you've picked a complicated situation here since the function has an optional $destination argument, but the service method requires it.

We would need to account for this in our rector rule or at least list it in the "Improvement Opportunities".

function file_unmanaged_copy($source, $destination = NULL, $replace = FILE_EXISTS_RENAME) {
  @trigger_error('file_unmanaged_copy() is deprecated in Drupal 8.7.0 and will be removed before Drupal 9.0.0. Use \Drupal\Core\File\FileSystemInterface::copy(). See https://www.drupal.org/node/3006851.', E_USER_DEPRECATED);
  try {
    $file_system = \Drupal::service('file_system');

    // Build a destination URI if necessary.
    if (!isset($destination)) {
      $destination = file_build_uri($file_system->basename($source));
    }
    return $file_system->copy($source, $destination, $replace);
  }
  catch (FileException $e) {
    return FALSE;
  }
}

In the above example, we would probably need to check if $destination is not passed as an argument. You can check $node->args which is an array. If there are at least two arguments, your substitution would work. If not, we should do the more complex replacement as shown above or at least skip over that situation.

Thanks again for the submission. Do you want to make those changes?

Thank you for review @damontgomery. I will make these changes along with your other mentioned changes.

[damontgomery]

@mohsin-saeed, with the update to Rector, all of the Rector\ class names need to be changed to Rector\Core\.

Sorry for moving things from underneath you. :(