Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feature-rectore-3115670: rule for file_unmanaged_copy #43

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feature-rectore-3115670: rule for file_unmanaged_copy #43

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
49f1233
feature-rectore-3115670: rule for file_unmanaged_copy
mohsin-saeed-epam Apr 17, 2020
afc22f5
release version yaml confilict merge
mohsin-saeed-epam Apr 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions config/drupal-8/drupal-8.7-deprecations.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@
services:
DrupalRector\Rector\Deprecation\FilePrepareDirectoryRector: ~
DrupalRector\Rector\Deprecation\FileCreateDirectoryRector: ~
DrupalRector\Rector\Deprecation\FileUnmanagedCopyRector: ~
DrupalRector\Rector\Deprecation\FileExistsReplaceRector: ~
5 changes: 5 additions & 0 deletions deprecation-index.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,3 +137,8 @@
- DrupalRenderRootStaticUpdated.php
- drupal_render_root.php
- drupal_render_root_updated.php
'file_unmanaged_copy()':
Rector: FileUnmanagedCopyRector.php
PHPStan: 'Call to deprecated function file_unmanaged_copy(). Deprecated in Drupal 8.7.0, will be removed before Drupal 9.0.0. Use \Drupal\Core\File\FileSystemInterface::copy().'
Examples:
- file_unmanaged_copy.php
10 changes: 10 additions & 0 deletions rector_examples/file_unmanaged_copy.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?php

/**
* A simple example using the minimum required (all) number of arguments.
*/
function simple_example() {
$source = '/test/directory';
$destination = '/test/directory_2';
file_unmanaged_copy($source, $destination,FILE_CREATE_DIRECTORY);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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;
  }
}

}
71 changes: 71 additions & 0 deletions src/Rector/Deprecation/FileUnmanagedCopyRector.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
<?php

namespace DrupalRector\Rector\Deprecation;

use DrupalRector\Utility\TraitsByClassHelperTrait;
use PhpParser\Node;
use Rector\Rector\AbstractRector;
use Rector\RectorDefinition\CodeSample;
use Rector\RectorDefinition\RectorDefinition;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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. :(


/**
* Replaces deprecated file_unmanaged_copy() calls.
*
* See https://api.drupal.org/api/drupal/core%21includes%21file.inc/function/file_unmanaged_copy/8.7.x.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
 */

*
* What is covered:
* - File System Service used
*
* Improvement opportunities
* - Dependency Injection
*/
final class FileUnmanagedCopyRector extends AbstractRector {
Copy link
Contributor

@damontgomery damontgomery Apr 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

use TraitsByClassHelperTrait;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


/**
* @inheritdoc
*/
public function getDefinition(): RectorDefinition {
return new RectorDefinition('Fixes deprecated file_unmanaged_copy() calls', [
new CodeSample(
<<<'CODE_BEFORE'
file_unmanaged_copy($source, $destination, $replace);
CODE_BEFORE
,
<<<'CODE_AFTER'
\Drupal::service('file_system')->copy($source, $destination, $replace);
CODE_AFTER
),
]);
}

/**
* @inheritdoc
*/
public function getNodeTypes(): array {
return [
Node\Expr\FuncCall::class,
];
}

/**
* @inheritdoc
*/
public function refactor(Node $node): ?Node {
if ($node->name instanceof Node\Name && 'file_unmanaged_copy' === (string) $node->name) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

$file_system_service = new Node\Expr\StaticCall(new Node\Name\FullyQualified('Drupal'), 'service', [new Node\Arg(new Node\Scalar\String_('file_system'))]);

$method_name = 'copy';

$method = new Node\Identifier($method_name);

$node = new Node\Expr\MethodCall($file_system_service, $method, $node->args);

return $node;

}

return NULL;
}

}