Skip to content

Conversation

@sreichel
Copy link
Contributor

@sreichel sreichel commented Nov 6, 2025

... added some Docs, renamed some short vars.

Copilot AI review requested due to automatic review settings November 6, 2025 00:01
@sreichel sreichel added the chore label Nov 6, 2025
@github-actions github-actions bot added Component: Catalog Relates to Mage_Catalog Component: Cms Relates to Mage_Cms Component: lib/Varien Relates to lib/Varien Component: Usa Relates to Mage_Usa Component: Api PageRelates to Mage_Api Component: lib/* Relates to lib/* phpstan labels Nov 6, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request improves code quality by addressing PHPStan static analysis issues and enhancing variable naming conventions. The changes focus on making the codebase more maintainable and type-safe.

  • Renamed ambiguous single-letter and unclear variable names to descriptive alternatives (e.g., $a/$b$nodeA/$nodeB, $x/$y$xAxis/$yAxis, $i$index)
  • Fixed type casting issues for strlen() function calls that were passing non-string values
  • Added missing @throws annotations to method docblocks for better API documentation
  • Removed unused variable declarations ($font, $pageY)
  • Fixed incorrect condition logic (strlen(!$serviceAreaCode)!strlen($serviceAreaCode))

Reviewed Changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
lib/Varien/Simplexml/Element.php Improved variable naming in loops and XML parsing logic; updated return type documentation
app/code/core/Mage/Usa/Model/Shipping/Carrier/Dhl/Label/Pdf/PageBuilder.php Renamed coordinate variables for clarity; removed unused variables; fixed logic bug; added exception documentation
app/code/core/Mage/Cms/Model/Wysiwyg/Images/Storage.php Fixed docblock formatting
app/code/core/Mage/Catalog/Model/Layer/Filter/Price.php Improved loop variable naming; fixed type casting for strlen; added exception documentation; repositioned deprecated tags
app/code/core/Mage/Catalog/Model/Layer/Filter/Decimal.php Fixed type casting for strlen; added exception documentation
app/code/core/Mage/Api/Model/Server/Wsi/Handler.php Added missing exception documentation
.phpstan.dist.baseline.neon Removed resolved PHPStan errors from baseline
Comments suppressed due to low confidence (1)

app/code/core/Mage/Usa/Model/Shipping/Carrier/Dhl/Label/Pdf/PageBuilder.php:310

  • [nitpick] The variable $pageY is used in the _drawSenderAddress method but wasn't renamed to follow the naming convention established by the PR. For consistency with other changes in this file where $y was renamed to $yAxis, this variable should be renamed to $yPageAxis or similar.
        if (strlen($lines[0]) > 28) {
            $firstLine = array_shift($lines);
            $pageY = $this->_page->drawLines([$firstLine], $this->_x(25), $this->_y(42), 28);
            $this->_page->drawText($phoneNumber, $this->_x(103), $this->_y(42));
        } else {
            $pageY = $this->_y(42);

@sreichel
Copy link
Contributor Author

sreichel commented Nov 6, 2025

Merge w/o review. Should be safe.

@sreichel sreichel merged commit a657351 into OpenMage:main Nov 6, 2025
20 checks passed
@sreichel sreichel deleted the phpstan/param-strlen branch November 6, 2025 00:18
@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 6, 2025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Component: Api PageRelates to Mage_Api Component: Catalog Relates to Mage_Catalog Component: Cms Relates to Mage_Cms Component: lib/Varien Relates to lib/Varien Component: lib/* Relates to lib/* Component: Usa Relates to Mage_Usa phpstan

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant