Skip to content

refactor: safe PHP 7.4 modernization - #132

Draft
somethingwithproof wants to merge 6 commits into
Cacti:developfrom
somethingwithproof:refactor/modernization
Draft

refactor: safe PHP 7.4 modernization#132
somethingwithproof wants to merge 6 commits into
Cacti:developfrom
somethingwithproof:refactor/modernization

Conversation

@somethingwithproof

Copy link
Copy Markdown

This PR adds strict typing, short array syntax, and null coalescing operators across the plugin. Standalone infrastructure files were removed per architectural mandate.

Copilot AI review requested due to automatic review settings April 9, 2026 21:16

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to modernize the ReportIt (Cacti) PHP plugin by enabling strict typing and refactoring legacy array syntax/usages across the codebase.

Changes:

  • Added declare(strict_types=1); to many plugin entrypoints and library/constant files.
  • Converted many array(...) literals to short array syntax [...] (including prepared-statement parameter arrays).
  • Refactored various helper usages, but introduced multiple invalid replacements (notably is_[...], in_[...], and xml_to_[...]) that will break runtime/parse.

Reviewed changes

Copilot reviewed 36 out of 36 changed files in this pull request and generated 38 comments.

Show a summary per file
File Description
view.php Adds strict types and array syntax refactors; currently contains invalid is_[...] checks.
tmp/index.php Adds strict types declaration.
templates.php Array syntax refactors; currently contains invalid xml_to_[...] and is_[...] usages that will crash.
system/upgrade.php Array syntax refactors in upgrade routines.
system/install.php Array syntax refactors; unique key column string formatting looks incorrect.
setup.php Array syntax refactors; currently contains an invalid function declaration (reportit_clog_regex_[...]).
reportit.php Array syntax refactors; currently contains invalid in_[...] / is_[...] usages.
poller_reportit.php Array syntax refactors; currently contains invalid in_[...] / is_[...] usages.
locales/LC_MESSAGES/index.php Adds strict types declaration.
locales/index.php Adds strict types declaration.
lib/index.php Adds strict types declaration.
lib/funct_validate.php Array syntax refactors; currently contains invalid in_[...] usages in validation logic.
lib/funct_shared.php Array syntax refactors; currently contains invalid function declarations and many invalid is_[...] / xml_to_[...] usages.
lib/funct_runtime.php Array syntax refactors; currently contains invalid db_custom_fetch_flat_[...] and in_[...] usage.
lib/funct_reports.php Array syntax refactors (incl. prepared params / unserialize options).
lib/funct_online.php Array syntax refactors; currently contains invalid in_[...] usage.
lib/funct_html.php Array syntax refactors; currently contains invalid is_[...] usage.
lib/funct_export.php Array syntax refactors; currently contains invalid in_[...] / is_[...] usages in export paths.
lib/funct_calculate.php Array syntax refactors.
lib/const_view.php Adds strict types + short array syntax in constants.
lib/const_variables.php Adds strict types + short array syntax in constants.
lib/const_templates.php Adds strict types + short array syntax in constants.
lib/const_runtime.php Adds strict types + short array syntax in constants.
lib/const_rrdlist.php Adds strict types + short array syntax in constants.
lib/const_reports.php Adds strict types + short array syntax in constants/forms arrays.
lib/const_measurands.php Adds strict types declaration.
lib/const_items.php Adds strict types + short array syntax in constants.
lib/const_graphs.php Adds strict types + short array syntax in constants.
index.php Adds strict types declaration.
include/index.php Adds strict types declaration.
include/global_forms.php Adds strict types + short array syntax in form field definitions.
include/global_arrays.php Adds strict types declaration.
images/index.php Adds strict types declaration.
cache/index.php Adds strict types declaration.
.omc/sessions/ff0c1a6a-cc9f-4db8-b7c4-2fb53b2b07bb.json New session artifact file added; should not be committed.
.omc/sessions/fa94e8de-1fa9-4329-8283-36f9ae53039c.json New session artifact file added; should not be committed.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread view.php
Comment thread view.php
Comment thread view.php
Comment thread templates.php
Comment thread templates.php
Comment thread lib/funct_export.php Outdated
Comment thread lib/funct_export.php Outdated
Comment thread lib/funct_html.php Outdated
Comment thread lib/funct_export.php
Comment thread reportit.php
Revert corrupted function calls introduced by refactoring tool:
- is_[$x] -> is_array($x)
- in_[$x, ...] -> in_array($x, ...)
- xml2[$x] -> xml2array($x)

Also remove accidentally committed .omc session files and add
.omc/ to .gitignore.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Revert bulk array()->[] rewrite damage affecting:
- is_array, in_array, xml2array
- call_user_func_array, filter_var_array
- Function declarations with _array suffix

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
…uble semicolons

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
@somethingwithproof
somethingwithproof marked this pull request as draft April 11, 2026 00:10
@somethingwithproof

Copy link
Copy Markdown
Author

Converted to draft to serialize the stack in this repo. Blocked by #128; will un-draft after that merges to avoid cross-PR merge conflicts.

Legacy call sites pass DB/request strings into scalar-typed parameters (html_start_box(..., '', '3', ...), checkdate() on exploded date parts), which strict mode turns into fatal TypeErrors.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
@somethingwithproof

Copy link
Copy Markdown
Author

Copilot feedback addressed. 37 of 38 threads resolved.

The bulk of the review (35 threads) was the is_[...] / in_[...] / xml_to_[...] / db_custom_fetch_flat_[...] / reportit_clog_regex_[...] corruption the refactor tool produced. Those were restored in b9a20d0/0f4ec42/cef3eeb; I re-verified by comparing normalised token streams of every changed file against develop rather than trusting the commit messages. The .omc/ artifacts are gone and the directory is ignored.

Also dropped declare(strict_types=1) (5084036). It is not safe on this codebase: html_start_box() is called at 30 sites with '' for bool $div and '3' for int $cell_padding, html_nav_bar() receives read_config_option() and db_fetch_cell() strings for its int parameters, and reportit.php passes explode() results straight into checkdate()/mktime(). Every one of those is a fatal TypeError under strict mode. The PR is now purely array() -> [] plus two typo fixes (db_fetch_cell_parepared, db_fetch_row_parepared), which is what the title claims.

Deliberately not applied:

  • The in_array(..., true) strict-comparison flags suggested on funct_runtime.php:327, funct_validate.php:101/180/190 and funct_online.php:209. They change matching semantics, and this branch should stay behaviour-neutral.
  • system/install.php:126. api_plugin_db_table_create() routes columns through db_format_index_create(), which adds the outer backtick pair, so cache_id`, `id is correct and is the existing form throughout the file.

Left open for a maintainer: the second half of the funct_validate.php:357 comment. in_array($template_ds_names, $ds_names) compares a flat name list against assoc rows and looks wrong, but it predates this branch and changing it would alter template-import compatibility detection.

Separately, get_report_setting() in lib/funct_shared.php builds 'SELECT $column ...' in single quotes, so the column never interpolates. The parepared typo fix in this PR turns that from a fatal into a failed query (archive rotation silently no-ops). Worth its own change since it needs a decision on whether the column name should be whitelisted; I have not touched it here.

No PHP 8.0-only constructs were introduced anywhere in the diff.

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.

2 participants