Skip to content

RemoveExtraParametersRector incorrectly removes required parameters from function defined in same file #9578

@kojiromike

Description

@kojiromike

Bug Report

Subject Details
Rector version 2.3.0

Description

RemoveExtraParametersRector incorrectly removed required parameters from calls to a function that is defined in the same file. This caused a cascade of incorrect transformations:

  1. RemoveExtraParametersRector removed the $message argument from formatErrorMessage() calls
  2. After removal, all catch blocks appeared to have identical bodies
  3. MultiExceptionCatchRector merged them into a single multi-catch
  4. RemoveUnusedVariableInCatchRector removed the $e variable

The resulting code is broken and will throw TypeError at runtime.

Source Code

This occurred in the OpenEMR project:

Diff Produced by Rector

-                            } catch (InvalidEmailAddressException) {
-                                $strMsg .= formatErrorMessage(xlt("Invalid email address"));
-                                echo(nl2br($strMsg));
-                                continue;
-                            } catch (SmtpNotConfiguredException) {
-                                $strMsg .= formatErrorMessage(xlt("SMTP not configured"));
-                                echo(nl2br($strMsg));
-                                continue;
-                            } catch (EmailSendFailedException $e) {
-                                $strMsg .= formatErrorMessage(xlt("Failed to send email") . ": " . text($e->getMessage()));
-                                echo(nl2br($strMsg));
-                                continue;
-                            } catch (\PHPMailer\PHPMailer\Exception $e) {
-                                $strMsg .= formatErrorMessage(xlt("Email error") . ": " . text($e->getMessage()));
+                            } catch (InvalidEmailAddressException|SmtpNotConfiguredException|EmailSendFailedException|\PHPMailer\PHPMailer\Exception) {
+                                $strMsg .= formatErrorMessage();
                                 echo(nl2br($strMsg));
                                 continue;
                             }

Applied Rules

Applied rules:
 * RemoveExtraParametersRector
 * MultiExceptionCatchRector
 * RemoveUnusedVariableInCatchRector

Why This Is Wrong

  1. formatErrorMessage(string $message) is defined in the same file and clearly requires a parameter - see line 362
  2. Each catch block had a different error message - they should not have been merged
  3. The resulting formatErrorMessage() call with no arguments will throw TypeError

Reproducibility

This bug appears to be intermittent. After clearing the cache (rm -rf /tmp/rector) and re-running Rector, the issue did not reproduce. However:

  • I have observed this bug multiple times on this codebase
  • The git diff shown above is proof it occurred

This matches the behavior reported in #9088.

Related Issues

Environment

  • Rector 2.3.0
  • PHP 8.2
  • macOS (Darwin)
  • Using ->withDeadCodeLevel(5) and ->withPhpSets() in rector.php
  • Parallel processing enabled (default settings)

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions