Consider adding more unimportant php changes to the ignore check level

[hostep]

Hiya

When looking at output from this tool during the upgrade from Magento 2.4.7-p5 to 2.4.8, I'm noticing 2 patterns that occur a lot that are semantically speaking not important:

1. the change of const SOME_CONSTANT = 'some-value'; to public const SOME_CONSTANT = 'some-value';, this is exactly the same
2. the change of a method argument from array $someVariable = null to ?array $someVarialbe = null, this is exactly the same, it's just that the first usage is now considered deprecated in PHP 8.4, but semantically speaking it's exactly the same

Would it make sense to add these 2 cases to the IGNR level of this tool if it's detected that a file only consists of these kind of changes?

Here's a good example that shows all of those things combined in a single file:

diff -ur -N vendor_orig/magento/module-catalog/Helper/Data.php vendor/magento/module-catalog/Helper/Data.php
--- vendor_orig/magento/module-catalog/Helper/Data.php	2025-04-11 11:01:48
+++ vendor/magento/module-catalog/Helper/Data.php	2025-04-02 22:58:40
@@ -1,7 +1,7 @@
 <?php
 /**
- * Copyright © Magento, Inc. All rights reserved.
- * See COPYING.txt for license details.
+ * Copyright 2011 Adobe
+ * All Rights Reserved.
  */
 namespace Magento\Catalog\Helper;
 
@@ -21,36 +21,37 @@
  *
  * @SuppressWarnings(PHPMD.TooManyFields)
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
+ * @SuppressWarnings(PHPMD.CookieAndSessionMisuse)
  * @since 100.0.2
  */
 class Data extends \Magento\Framework\App\Helper\AbstractHelper
 {
-    const PRICE_SCOPE_GLOBAL = 0;
+    public const PRICE_SCOPE_GLOBAL = 0;
 
-    const PRICE_SCOPE_WEBSITE = 1;
+    public const PRICE_SCOPE_WEBSITE = 1;
 
-    const XML_PATH_PRICE_SCOPE = 'catalog/price/scope';
+    public const XML_PATH_PRICE_SCOPE = 'catalog/price/scope';
 
-    const CONFIG_USE_STATIC_URLS = 'cms/wysiwyg/use_static_urls_in_catalog';
+    public const CONFIG_USE_STATIC_URLS = 'cms/wysiwyg/use_static_urls_in_catalog';
 
     /**
      * @deprecated
      * @see \Magento\Catalog\Helper\Output::isDirectivesExists
      */
-    const CONFIG_PARSE_URL_DIRECTIVES = 'catalog/frontend/parse_url_directives';
+    public const CONFIG_PARSE_URL_DIRECTIVES = 'catalog/frontend/parse_url_directives';
 
-    const XML_PATH_DISPLAY_PRODUCT_COUNT = 'catalog/layered_navigation/display_product_count';
+    public const XML_PATH_DISPLAY_PRODUCT_COUNT = 'catalog/layered_navigation/display_product_count';
 
     /**
      * Cache context
      */
-    const CONTEXT_CATALOG_SORT_DIRECTION = 'catalog_sort_direction';
+    public const CONTEXT_CATALOG_SORT_DIRECTION = 'catalog_sort_direction';
 
-    const CONTEXT_CATALOG_SORT_ORDER = 'catalog_sort_order';
+    public const CONTEXT_CATALOG_SORT_ORDER = 'catalog_sort_order';
 
-    const CONTEXT_CATALOG_DISPLAY_MODE = 'catalog_mode';
+    public const CONTEXT_CATALOG_DISPLAY_MODE = 'catalog_mode';
 
-    const CONTEXT_CATALOG_LIMIT = 'catalog_limit';
+    public const CONTEXT_CATALOG_LIMIT = 'catalog_limit';
 
     /**
      * Breadcrumb Path cache
@@ -67,22 +68,18 @@
     protected $_storeId;
 
     /**
-     * Core registry
+     * Framework Core registry
      *
      * @var \Magento\Framework\Registry
      */
     protected $_coreRegistry;
 
     /**
-     * Catalog product
-     *
      * @var Product
      */
     protected $_catalogProduct;
 
     /**
-     * Catalog category
-     *
      * @var Category
      */
     protected $_catalogCategory;
@@ -98,50 +95,36 @@
     protected $_templateFilterModel;
 
     /**
-     * Catalog session
-     *
      * @var \Magento\Catalog\Model\Session
      */
     protected $_catalogSession;
 
     /**
-     * Store manager
-     *
      * @var \Magento\Store\Model\StoreManagerInterface
      */
     protected $_storeManager;
 
     /**
-     * Template filter factory
-     *
      * @var \Magento\Catalog\Model\Template\Filter\Factory
      */
     protected $_templateFilterFactory;
 
     /**
-     * Tax class key factory
-     *
      * @var \Magento\Tax\Api\Data\TaxClassKeyInterfaceFactory
      */
     protected $_taxClassKeyFactory;
 
     /**
-     * Tax helper
-     *
      * @var \Magento\Tax\Model\Config
      */
     protected $_taxConfig;
 
     /**
-     * Quote details factory
-     *
      * @var \Magento\Tax\Api\Data\QuoteDetailsInterfaceFactory
      */
     protected $_quoteDetailsFactory;
 
     /**
-     * Quote details item factory
-     *
      * @var \Magento\Tax\Api\Data\QuoteDetailsItemInterfaceFactory
      */
     protected $_quoteDetailsItemFactory;
@@ -159,8 +142,6 @@
     protected $_taxCalculationService;
 
     /**
-     * Price currency
-     *
      * @var PriceCurrencyInterface
      */
     protected $priceCurrency;
@@ -495,7 +476,7 @@
      * @param array $taxAddress
      * @return \Magento\Customer\Api\Data\AddressInterface|null
      */
-    private function convertDefaultTaxAddress(array $taxAddress = null)
+    private function convertDefaultTaxAddress(?array $taxAddress = null)
     {
         if (empty($taxAddress)) {
             return null;

I only see changes in comments, the public constants and the nullable method argument. So in theory this file its changes are not important to be reported.

Opinions?

[convenient]

Makes sense.

Currently only whitespace and comments are ignored.

Further PHP parts can be ignored by adding more here

ampersand-magento2-upgrade-patch-helper/src/Ampersand/PatchHelper/Patchfile/Entry.php

Line 596 in 19c4fbb

$contents = Sanitiser::stripCommentsFromPhp($contents);

[hostep]

Just FYI that the second one may be difficult to implement in all cases, as sometimes the types names can change signicantly if they are being aliased, like in this example the switch from \Magento\Quote\Api\Data\AddressInterface to ?QuoteAddressInterface which is in the end the same type:

--- vendor_orig/magento/module-checkout/Model/PaymentInformationManagement.php	2025-04-11 11:01:47
+++ vendor/magento/module-checkout/Model/PaymentInformationManagement.php	2025-04-02 22:58:40
@@ -10,11 +10,15 @@
 use Magento\Checkout\Api\PaymentProcessingRateLimiterInterface;
 use Magento\Checkout\Api\PaymentSavingRateLimiterInterface;
 use Magento\Customer\Api\AddressRepositoryInterface;
+use Magento\Customer\Api\Data\AddressInterface;
 use Magento\Framework\App\ObjectManager;
 use Magento\Framework\Exception\CouldNotSaveException;
 use Magento\Framework\Exception\LocalizedException;
 use Magento\Quote\Api\CartRepositoryInterface;
+use Magento\Quote\Api\Data\AddressInterface as QuoteAddressInterface;
+use Magento\Quote\Api\Data\PaymentInterface;
 use Magento\Quote\Model\Quote;
+use Magento\Quote\Model\Quote\Address;
 use Psr\Log\LoggerInterface;
 
 /**
@@ -137,8 +141,8 @@
      */
     public function savePaymentInformationAndPlaceOrder(
         $cartId,
-        \Magento\Quote\Api\Data\PaymentInterface $paymentMethod,
-        \Magento\Quote\Api\Data\AddressInterface $billingAddress = null
+        PaymentInterface $paymentMethod,
+        ?QuoteAddressInterface $billingAddress = null
     ) {
         $this->paymentRateLimiter->limit();
         try {
@@ -180,8 +184,8 @@
      */
     public function savePaymentInformation(
         $cartId,
-        \Magento\Quote\Api\Data\PaymentInterface $paymentMethod,
-        \Magento\Quote\Api\Data\AddressInterface $billingAddress = null
+        PaymentInterface $paymentMethod,
+        ?QuoteAddressInterface $billingAddress = null
     ) {
         if (!$this->saveRateLimiterDisabled) {
             try {

Trying to implement this check using regexes may be pretty difficult to detect this one.
But this case doesn't happen very often, so I'm fine if these sort of cases aren't being handled as this is probably technical very difficult to do without switching to an AST parser.

[convenient]

I wonder if we could pull in rector or something here. There's bound to be rules that would extract out the Full class name, and pace in the import. And then also apply the nully rule.

If both files are the same after applying these and the current rules that could work 🤔