Skip to content

Feature/package for packagist#62

Open
LC43 wants to merge 23 commits into
mainfrom
feature/package-for-packagist
Open

Feature/package for packagist#62
LC43 wants to merge 23 commits into
mainfrom
feature/package-for-packagist

Conversation

@LC43

@LC43 LC43 commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Add more checks and type safety

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

Copy link
Copy Markdown
Contributor

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 focuses on improving code quality, type safety, and static analysis across the codebase. It introduces strict parameter and return type declarations, adds comprehensive PHPStan annotations, and implements defensive checks (such as is_array, is_scalar, and is_wp_error) to prevent runtime errors. Additionally, quality check commands were documented in the README, and several features were refactored for better readability. Feedback on these changes highlights a potential PHP warning in SaltusAdminCols.php if get_post_status_object() returns null, as well as a minor indentation inconsistency in Core.php.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/Features/AdminCols/SaltusAdminCols.php Outdated
Comment thread src/Core.php Outdated
@LC43

LC43 commented Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

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

Copy link
Copy Markdown
Contributor

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 comprehensive type-hinting, return type declarations, and defensive checks across the codebase to achieve a clean PHPStan level 7 run. Key changes include tightening property and method signatures in core services, models, and features, as well as handling potential WordPress errors and type mismatches. The review feedback identifies two important issues: a potential PHP 8.x TypeError in UpdateMenuDragAndDrop.php when parsing the order parameter, and a potential undefined index notice in WalkerTaxonomyDropdown.php if the taxonomy argument is missing.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/Features/DragAndDrop/UpdateMenuDragAndDrop.php Outdated
Comment thread src/Features/AdminFilters/WalkerTaxonomyDropdown.php
@LC43

LC43 commented Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review :(

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

Copy link
Copy Markdown
Contributor

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 focuses on tightening type declarations, adding return types, and improving static analysis compliance across the codebase to achieve a clean PHPStan level 7 run. Key feedback points out several critical issues: an undefined variable $hook_name in Core.php that will cause a crash on initialization, a missing authorization check in the AJAX callback within UpdateMenuDragAndDrop.php, and inconsistent usage of raw model names instead of the newly introduced sanitized registration name in PostType.php, Taxonomy.php, and ModelFactory.php. Additionally, get_registration_name() should be made public, and GenericContainer.php needs to be updated to properly pass and handle registered dependencies.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/Core.php Outdated
Comment thread src/Features/DragAndDrop/UpdateMenuDragAndDrop.php
Comment thread src/Models/PostType.php
Comment thread src/Models/Taxonomy.php
Comment thread src/Models/BaseModel.php Outdated
Comment thread src/Models/ModelFactory.php Outdated
Comment thread src/Infrastructure/Container/GenericContainer.php Outdated
Comment thread src/Infrastructure/Container/GenericContainer.php Outdated
@LC43

LC43 commented Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

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

Copy link
Copy Markdown
Contributor

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 focuses on tightening types, adding PHPStan type annotations, and improving static analysis compliance across the codebase to reach PHPStan level 7. Key feedback includes addressing a bug in GenericContainer.php where constructor dependencies are ignored during instantiation, fixing a security vulnerability in UpdateMenuDragAndDrop.php due to a missing authorization check on the AJAX callback, and resolving an issue in Taxonomy.php where string-based associations are not properly registered.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/Infrastructure/Container/GenericContainer.php Outdated
Comment thread src/Features/DragAndDrop/UpdateMenuDragAndDrop.php
Comment thread src/Models/Taxonomy.php
@LC43

LC43 commented Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

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

Copy link
Copy Markdown
Contributor

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 focuses on tightening types, adding property and return type declarations, and updating PHPDoc annotations across the codebase to achieve a clean PHPStan level 7 run. It also updates development dependencies and documents quality checks in the README. The review feedback highlights opportunities to improve usability and code quality, specifically by falling back to plain text instead of rendering empty links when term links are invalid, and by removing unnecessary HTML-escaping and localization from developer-facing exception messages.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/Features/AdminCols/SaltusAdminCols.php Outdated
Comment thread src/Core.php
Comment thread src/Infrastructure/Container/ContainerAssembler.php
Comment thread src/Infrastructure/Service/ServiceFactory.php
Comment thread src/Infrastructure/Container/ServiceContainer.php
When get_edit_term_link returns null or a WP_Error, the previous code
rendered an empty href attribute (<a href="">), which is confusing for
screen readers and provides no usable navigation. Now falls back to
rendering just the escaped term name without a link, matching the
behavior used when the user lacks edit_terms capability.
@LC43

LC43 commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

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

Copy link
Copy Markdown
Contributor

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 focuses on tightening types, adding PHPStan type annotations, and improving overall code quality across the repository to achieve a clean PHPStan level 7 run. Key changes include adding strict type declarations, return types, and parameter types across core files, features, and infrastructure classes, as well as upgrading dev dependencies. The review feedback identifies a critical security vulnerability where the AJAX handler in UpdateMenuDragAndDrop.php lacks a user capability check, allowing unauthorized users to modify the menu order. Additionally, it is recommended to cast $post->menu_order to an integer in SaltusDragAndDrop.php to prevent potential SQL injection or malformed queries.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/Features/DragAndDrop/UpdateMenuDragAndDrop.php
Comment thread src/Features/DragAndDrop/SaltusDragAndDrop.php Outdated
Comment thread src/Features/DragAndDrop/SaltusDragAndDrop.php Outdated
Cast $post->menu_order to (int) in previous_post_where() and
next_post_where() to prevent SQL injection or malformed queries
from unexpected property data.
@LC43

LC43 commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review 🤞

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

Copy link
Copy Markdown
Contributor

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 refactors the codebase to achieve a clean PHPStan level 7 run by tightening types, adding return types, and updating annotations across various services, models, and features. It also introduces a get_registration_name() method in BaseModel to sanitize model names for WordPress registration. The reviewer recommends validating WordPress length limits (20 characters for post types and 32 for taxonomies) within this new method to prevent silent registration failures.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/Models/BaseModel.php
…name()

WordPress imposes strict length limits on registered keys: 20 characters
for post types and 32 characters for taxonomies. The get_registration_name()
method now validates these limits based on the model type to prevent silent
registration failures or database errors.
@LC43

LC43 commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review 🤞

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

Copy link
Copy Markdown
Contributor

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 focuses on tightening type safety, adding type hints, and improving static analysis compliance across the codebase, alongside updating development dependencies. Key changes include adding parameter and return types, introducing defensive type checks, and adding a sanitized model name resolver for WordPress APIs. The review feedback highlights a critical signature compatibility issue in WalkerTaxonomyDropdown::start_el that would cause a PHP Fatal Error, as well as opportunities to clean up an exception message in Core.php and correct an instance call of a static method in ModelFactory.php.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/Features/AdminFilters/WalkerTaxonomyDropdown.php Outdated
Comment thread src/Core.php Outdated
*/
$hook_name = static::HOOK_PREFIX . static::SERVICES_FILTER;
if ( empty( $hook_name ) ) {
throw new \InvalidArgumentException( esc_html__( 'Hook name cannot be empty', 'saltus' ) );

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Using esc_html__ inside an InvalidArgumentException is an anti-pattern. Exception messages are developer-facing and logged in system logs or shown in CLI environments, where HTML escaping is unnecessary and can make the messages harder to read (e.g., converting quotes to HTML entities). Since this is a developer-facing error, a plain English string is preferred.

				throw new \\InvalidArgumentException( 'Hook name cannot be empty' );

Comment thread src/Models/ModelFactory.php Outdated
// make sure $args is an array
$args = is_array( $args ) ? $args : [];
$service = $this->app->get( $normalized_feature_name );
$service_imp = $service->make( $cpt->get_registration_name(), $this->project, $args );

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The make method is defined as a static method in the Assembly interface. Calling a static method as an instance method (using ->) is a code smell and can be flagged by static analysis tools. It is cleaner and more standard to call it statically using the :: operator.

			$service_imp = $service::make( $cpt->get_registration_name(), $this->project, $args );

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ohh i missed this one!

LC43 added 3 commits June 22, 2026 23:38
The parent class Walker::start_el does not declare a return type.
Adding : void in the child causes a PHP fatal error at runtime
due to incompatible signature.
The Assembly interface declares make() as a static method.
Calling it via -> works but is a code smell flagged by
static analysis. Use :: for clarity.
Exception messages are developer-facing and logged in system
logs or CLI environments. HTML escaping with esc_html__ or
esc_html makes them harder to read and is unnecessary.
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.

1 participant