Update PHPMailer library

[desrosj]

Version ​6.1.7 of PHPMailer has been released.

A list of all changes since 6.1.6: ​https://github.com/PHPMailer/PHPMailer/compare/v6.1.6...v6.1.7

[ayeshrajans]

This patch overwrites the existing PHPMailer.php and SMTP.php files with a verbatim copy from upstream 6.1.7 tag.

I noticed 3 PHPCS/PHPCompat rules that we added later to the 6.1.6 copy. PHPMailer directory (src/wp-includes/PHPMailer) is already in the main PHPCS configuration file, and this patch adds the directory to phpcompat.xml.dist file as well, so we no longer need to patch PHPMailer to pass PHPComat CI check.

From now on, we will be able to simply copy new PHPMailer releases (PHPMailer.php, Exception.php and SMTP.php) into src/wp-includes/PHPMailer directory.

[ayeshrajans]

Tests: ​https://travis-ci.com/github/Ayesh/wordpress-develop/builds/184375019

[desrosj]

Thanks for 51311.patch​ @ayeshrajans!

I went back to see if I could find a reason for having the inline phpcs:ignore statements in PHPMailer instead of excluding the directory in the PHPCS ruleset. I couldn't find anything specific, but I think that the inline statements were used because of the work done towards achieving a passing compatibility job in [47902], which came before the library was actually updated in [48033] (so the inline statements were copied over).

The only other argument I can think of for using inline statements vs. excluding the directory entirely in the ruleset is that the former would allow WordPress to more easily spot, report, and fix potential compatibility issues in PHPMailer as WordPress' supported versions of PHP are changed.

I don't feel strongly either way, though I lean towards continuing to use the inline statements for the reason above. There are only 3-4 of them, so copying them over is not too time consuming. I'll hold off committing for a bit to allow others to weigh in if they have a preference. @jrf should be able to provide a recommendation here.

[ayeshrajans]

Hi @desrosj - thanks for the feedback.
PHP Compatibility tests are without-doubt helpful, and my thoughts behind ignoring the whole directory was that PHPMailer itself is a well-tested library that promises good backwards compatibility and semver.

Would it be a good middle ground if we were to ignore specific PHP Compat test failures? So we would move the inline ignore clauses to the XML file. That way, CI would still fail if there is a new regression during an update, so we still benefit from PHP Compat without having to manually cherry-pick hunks for each update.

[jrf]

My two pennies:

The PHPCompatibility check is something different than the CS checks for which external packages do not have to comply with WP code style requirement.

All code shipped with WP should be compatible with the PHP versions supported by WP, including external packages.

If we'd exclude those packages via the XML ruleset, it would defeat the purpose, as when a new version of PHPCompatibility comes out which includes new checks for, for instance, PHP 8.0, you wouldn't be notified about incompatibilities in the external packages, so you wouldn't know that:

1. patches would need to be submitted to the external package(s)
2. the next version of WP would need to include a newer version of the external package.

So, with that in mind, I'd advocate using the inline annotations instead of the ruleset excludes.

Copying over/rechecking the inline annotations on an update of an external package is a small price to pay to ensure that the code of those packages are still compatible with the WP supported PHP versions.

If that is too much, I'd suggest trying to convince the authors of the external packages to use PHPCompatibility themselves, would still be better than doing a ruleset exclude. If the external packages would use PHPCompatibility, their package would already include the inline annotations, so when updating the package, we could then just copy & paste the files - including the annotations -.

Does that make sense ?

[desrosj]

Thanks @jrf! That matched my thinking as well. It’s not that the package upstream is not well maintained (it certainly is), it’s just about having an extra check in place just in case something is missed (it happens) by a core committee or upstream, and just an extra safety net to ensure WordPress meets its PHP support policies.

I’ll see about getting this in tomorrow.

[desrosj]

In 49034:

External Libraries: Upgrade PHPMailer to version 6.1.7.

For a full list of changes in this update, see the PHPMailer GitHub: ​https://github.com/PHPMailer/PHPMailer/compare/v6.1.6...v6.1.7.

Props ayeshrajans, jrf.
Fixes #51311.