Skip to content

Align DOM observer registration with PHP 8.5 and add PHP 8.5 CI validation#138

Open
Copilot wants to merge 4 commits into
masterfrom
copilot/fix-deprecated-method-in-htmlmin
Open

Align DOM observer registration with PHP 8.5 and add PHP 8.5 CI validation#138
Copilot wants to merge 4 commits into
masterfrom
copilot/fix-deprecated-method-in-htmlmin

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 18, 2026

  • Inspect the new PR comments and verify whether they require code changes
  • Update the exception messages in HtmlMin to match the current method names
  • Adjust the targeted regression tests to assert the new messages
  • Run targeted validation and security review for the follow-up change

This change is Reviewable

@coveralls
Copy link
Copy Markdown

coveralls commented May 18, 2026

Coverage Status

coverage: 95.68% (-0.05%) from 95.727% — copilot/fix-deprecated-method-in-htmlmin into master

Copilot AI changed the title [WIP] Fix deprecated method SplObjectStorage::attach in HtmlMin.php Align DOM observer registration with PHP 8.5 and add PHP 8.5 CI validation May 18, 2026
Copilot finished work on behalf of voku May 18, 2026 16:29
Copilot AI requested a review from voku May 18, 2026 16:29
Copilot finished work on behalf of voku May 18, 2026 17:39
@voku voku marked this pull request as ready for review May 18, 2026 18:00
@voku
Copy link
Copy Markdown
Owner

voku commented May 18, 2026

/gemini review

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a configuration for Infection mutation testing and strengthens static analysis by removing the exclusion for missing iterable value types in PHPStan. The HtmlMin class has been updated with more precise type annotations, additional null checks for node values, and improved validation for template logic and special script tags. Review feedback suggests aligning exception messages with their respective method names in the newly added validation logic to improve maintainability.

Comment thread src/voku/helper/HtmlMin.php Outdated
$validatedTemplateLogicSyntaxInSpecialScriptTags = [];
foreach ($templateLogicSyntaxInSpecialScriptTags as $tmp) {
if (!\is_string($tmp)) {
throw new \InvalidArgumentException('setTemplateLogicSyntaxInSpecialScriptTags only allows string[]');
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The exception message refers to setTemplateLogicSyntaxInSpecialScriptTags, but the method name is overwriteTemplateLogicSyntaxInSpecialScriptTags. It is recommended to update the message to match the method name for better clarity and maintainability.

                throw new \InvalidArgumentException('overwriteTemplateLogicSyntaxInSpecialScriptTags only allows string[]');

Comment thread src/voku/helper/HtmlMin.php Outdated
$validatedSpecialScriptTags = [];
foreach ($specialScriptTags as $tag) {
if (!\is_string($tag)) {
throw new \InvalidArgumentException('SpecialScriptTags only allows string[]');
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The exception message could be updated to match the method name overwriteSpecialScriptTags for consistency.

                throw new \InvalidArgumentException('overwriteSpecialScriptTags only allows string[]');

@voku
Copy link
Copy Markdown
Owner

voku commented May 18, 2026

@copilot the next llm generated feedback is maybe completely shot or not, you need to check if by yourself : Code Review
This pull request introduces a configuration for Infection mutation testing and strengthens static analysis by removing the exclusion for missing iterable value types in PHPStan. The HtmlMin class has been updated with more precise type annotations, additional null checks for node values, and improved validation for template logic and special script tags. Review feedback suggests aligning exception messages with their respective method names in the newly added validation logic to improve maintainability.

src/voku/helper/HtmlMin.php
$validatedTemplateLogicSyntaxInSpecialScriptTags = [];
foreach ($templateLogicSyntaxInSpecialScriptTags as $tmp) {
if (!\is_string($tmp)) {
throw new \InvalidArgumentException('setTemplateLogicSyntaxInSpecialScriptTags only allows string[]');
@gemini-code-assist
gemini-code-assist Bot
21 minutes ago
medium

The exception message refers to setTemplateLogicSyntaxInSpecialScriptTags, but the method name is overwriteTemplateLogicSyntaxInSpecialScriptTags. It is recommended to update the message to match the method name for better clarity and maintainability.

            throw new \InvalidArgumentException('overwriteTemplateLogicSyntaxInSpecialScriptTags only allows string[]');

@voku Reply...
src/voku/helper/HtmlMin.php
$validatedSpecialScriptTags = [];
foreach ($specialScriptTags as $tag) {
if (!\is_string($tag)) {
throw new \InvalidArgumentException('SpecialScriptTags only allows string[]');
@gemini-code-assist
gemini-code-assist Bot
21 minutes ago
medium

The exception message could be updated to match the method name overwriteSpecialScriptTags for consistency.

            throw new \InvalidArgumentException('overwriteSpecialScriptTags only allows string[]');

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 18, 2026

@copilot the next llm generated feedback is maybe completely shot or not, you need to check if by yourself : Code Review
This pull request introduces a configuration for Infection mutation te...

Updated in f94d668. I aligned both exception messages with the actual method names and adjusted the regression tests accordingly.

Copilot finished work on behalf of voku May 18, 2026 18:27
@sonarqubecloud
Copy link
Copy Markdown

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PHP 8.5 Compatibility: Deprecated Method SplObjectStorage::attach() called in HtmlMin.php

3 participants