Skip to content

Adding rector updates#6893

Closed
TheWitness wants to merge 1 commit intoCacti:developfrom
TheWitness:rector-changes
Closed

Adding rector updates#6893
TheWitness wants to merge 1 commit intoCacti:developfrom
TheWitness:rector-changes

Conversation

@TheWitness
Copy link
Copy Markdown
Member

No description provided.

Copy link
Copy Markdown
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 PR applies Rector-driven modernization updates across Cacti’s PHP scripts and core libraries, primarily to improve type-safety and adopt newer PHP callable/match syntaxes while aligning formatting with the project’s tab indentation.

Changes:

  • Replace legacy callable strings/switch statements with first-class callables and match expressions.
  • Add explicit (string) casts around string-processing functions to avoid warnings from mixed values.
  • Update Rector configuration to enforce tab indentation.

Reviewed changes

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

Show a summary per file
File Description
scripts/ss_webseer.php Switch to first-class callable for CLI dispatch.
scripts/ss_sql.php First-class callable dispatch; add string casts in preg_replace/trim.
scripts/ss_poller.php Add string casts around db_fetch_cell() results before explode().
scripts/ss_nimble_alletra_volumes.php Refactor loops/casts; adjust bit-string handling logic (see review comments).
scripts/ss_nimble_alletra_total.php Switch to first-class callable for CLI dispatch.
scripts/ss_netsnmp_lmsensors.php First-class callables; modern string helpers and numeric ops; add casts.
scripts/ss_net_snmp_disk_io.php Add casts for SNMP values and cached DB values; safer string ops.
scripts/ss_net_snmp_disk_bytes.php Add casts for SNMP values and cached DB values; safer string ops.
scripts/ss_multicpu_avg.php First-class callable dispatch; loop simplification; compound assignment.
scripts/ss_mikrotik_wrcount.php Switch to first-class callable for CLI dispatch.
scripts/ss_mikrotik_wireless_reg.php Switch to first-class callable for CLI dispatch.
scripts/ss_mikrotik_wapcount.php Switch to first-class callable for CLI dispatch.
scripts/ss_mikrotik_users.php Switch to first-class callable for CLI dispatch.
scripts/ss_mikrotik_uptime.php Switch to first-class callable for CLI dispatch.
scripts/ss_mikrotik_trees.php Switch to first-class callable for CLI dispatch.
scripts/ss_mikrotik_snmpget.php Switch to first-class callable for CLI dispatch.
scripts/ss_mikrotik_qusers.php Switch to first-class callable for CLI dispatch.
scripts/ss_mikrotik_queues.php Switch to first-class callable; replace switch with match.
scripts/ss_mikrotik_qtrees.php Switch to first-class callable; replace switch with match.
scripts/ss_mikrotik_qcount.php Switch to first-class callable for CLI dispatch.
scripts/ss_mikrotik_procs.php Switch to first-class callable for CLI dispatch.
scripts/ss_mikrotik_mem.php Switch to first-class callable for CLI dispatch.
scripts/ss_mikrotik_interfaces.php Switch to first-class callable; replace switch with match.
scripts/ss_mikrotik_health.php Switch to first-class callable for CLI dispatch.
scripts/ss_mikrotik_disk.php Switch to first-class callable for CLI dispatch.
scripts/ss_mikrotik_cpu.php Switch to first-class callable for CLI dispatch.
scripts/ss_hstats.php Switch to first-class callable; replace switch with match.
scripts/ss_host_disk.php Switch to first-class callable; add casts around SNMP auth/DB string ops.
scripts/ss_host_cpu.php Switch to first-class callable; add casts around SNMP auth and explode().
scripts/ss_gexport.php Switch to first-class callable for CLI dispatch.
scripts/ss_fping.php Switch to first-class callable; replace switch with match.
scripts/ss_fortigate_virus.php Switch to first-class callable; minor loop refactor.
scripts/ss_fortigate_ipsec.php Switch to first-class callable for CLI dispatch.
scripts/ss_fortigate_ips.php Switch to first-class callable for CLI dispatch.
scripts/ss_esxi_vhosts.php Switch to first-class callable; add casts for SNMP value string ops.
scripts/ss_cpoller.php Switch to first-class callable; add casts around explode() source.
scripts/ss_count_oids.php Switch to first-class callable for CLI dispatch.
scripts/ss_aruba_oscx_6x00.php Switch to first-class callable; simplify loops and cast OIDs.
scripts/ss_aruba_instant_client.php Switch to first-class callable for CLI dispatch.
scripts/ss_aruba_instant_ap.php Switch to first-class callable for CLI dispatch.
scripts/ss_apache_stats.php Switch to first-class callable for CLI dispatch.
scripts/query_host_partitions.php Add cast around SNMP auth parsing.
scripts/query_host_cpu.php Add cast around SNMP auth parsing and regex checks.
scripts/query_baytech_circuits.php Replace sizeof() with count() in loops.
scripts/query_akcp_temperature.php Replace sizeof() with count() in loops.
scripts/query_akcp_humidity.php Replace sizeof() with count() in loops.
scripts/query_akcp_fourtwenty.php Replace sizeof() with count() in loops.
scripts/query_akcp_airflow.php Replace sizeof() with count() in loops.
scripts/aruba_instant_wlan_client.php Replace sizeof() with count() in loop.
rector.php Configure Rector indentation to tabs.
lib/vdef.php Replace switch with match for vdef item naming.
lib/variables.php Add casts; adjust substring logic (see review comments).
lib/utility.php Add casts around string ops; first-class callable for uksort/array_map; safer trims.
lib/timespan_settings.php Cast user input before strtotime()/preg_match().
lib/template.php Cast before preg_match/str_contains/str_starts_with; safer parsing.
lib/spikekill.php Cast mixed values before string ops and XML parsing helpers.
lib/snmpagent.php Cast before explode() on enterprise OID.
lib/snmp.php Adopt first-class callable; add casts before trims/regex/string searches; remove unused catch var.
lib/rrdcheck.php Cast before explode() and logging; replace switch with match.
lib/rrd.php Cast around encryption output; first-class callable for array_map; safer string ops.
lib/reports.php Add casts for base64/json/string ops; first-class callable for usort; safer URL checks.
lib/poller.php First-class callable for array_map; cast before strlen/str_contains/preg_split and path ops.
lib/plugins.php Add casts around security/path checks; first-class callable for array_map; base64 decode casts.
lib/ping.php First-class callable for set_error_handler; cast before strlen().
lib/package.php Cast before path ops; base64 encode casts; safer basename/dirname.
lib/mib_cache.php Use constructor property promotion; cast before trimming stored values.
lib/ldap.php First-class callable error handlers; instance error recording; cast before entity decode.
lib/installer.php Add readonly properties; convert control-flow switches to match; minor static call adjustments.
lib/import.php Cast base64 inputs; cast before preg_match/explode; safer string checks.
lib/html_utility.php Cast read_user_setting before json_decode; cast sort column strings before checks.
lib/html_tree.php Cast request vars before string ops; first-class callable for usort; safer parsing.
lib/html_reports.php Cast before explode/preg_match/strtotime usage.
lib/html_graph.php Cast request vars/headers before string ops and explode.
lib/html_form_template.php Cast type codes before preg_match.
lib/html_form.php Cast before trim/is_file/is_dir/parse_url/explode.
lib/html_filter.php Constructor property promotion; cast before str_contains/trim.
lib/html.php Cast page/url inputs before string ops and basename/parse_url/preg_match.
lib/graph_variables.php Cast regex match group before preg_match.
lib/export.php Cast before base64 encode; cast before explode().
lib/dsstats.php Cast before explode/strlen/trim; first-class callable error handler; match refactor.
lib/dsdebug.php First-class callable error handler.
lib/dbparallel.php Cast before stripos/explode.
lib/database.php Cast DB-returned strings before str_contains/explode/trim/preg_match; first-class callable helpers.
lib/clog_webapi.php Cast before basename/dirname; safer JS/link building with casts.
lib/cdef.php Replace switch with match for cdef item naming.
lib/boost.php First-class callable usage; cast before strlen/trim/str_contains/dirname/strcmp.
lib/auth.php Cast cookie values/password/referer strings before string ops and explode/trim.
lib/api_tree.php Cast before strnatcasecmp.
lib/api_scheduler.php Cast before strtotime/explode; refactor week/day mapping to match.
lib/api_graph.php Cast before explode() for IDs.
lib/api_device.php Cast before strlen/substr_count/preg_match/explode; safer validation.
lib/api_data_source.php First-class callable for array_map; cast before string checks/trims.
lib/api_automation.php Cast before trim/explode/substr; cast field names before explode.
lib/api_aggregate.php First-class callable error handler; cast before preg_match/explode/str_contains.
lib/aggregate.php Cast before preg_replace on graph title.
include/themes/midwinter/update_hash.php Cast filesystem iterator values before explode/dirname.
include/session.php Use first-class callables for session handlers and shutdown function.
include/global_session.php Cast $_SERVER values before strstr/str_contains.
include/global_languages.php Cast/first-class callables for translation wrappers; cast before parsing locale strings.
include/global.php First-class callables for error/shutdown handlers; cast header before strcasecmp.
include/auth.php Cast config JSON before json_decode.

Comment on lines 540 to 543
if ($item['value'] != '') {
if ($max_chars > 0) {
$item['value'] = substr($item['field_value'], 0, $max_chars);
$item['value'] = substr((string) $item['field_value'], 0, $max_chars);
}
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

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

In substitute_data_input_data(), the query only selects dif.data_name and did.value, but the code truncates using $item['field_value']. This key is not present and will raise an undefined index notice when $max_chars > 0, and truncation won't work. Use $item['value'] (or include the correct column in the SELECT) for the substring operation.

Copilot uses AI. Check for mistakes.
Comment on lines 162 to 164
foreach ($low as $key => $value) {
$long = isset($high[$key]) ? $high[$key] : '' . $value;
$long = $high[$key] ?? '' . $value;

Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

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

When composing the 64-bit counter from $high and $low, $long = $high[$key] ?? '' . $value; returns only the high bits when present (it does not append the low bits). This will produce incorrect bindec() values for large counters. Concatenate the high and low parts, e.g. ($high[$key] ?? '') . $value.

Copilot uses AI. Check for mistakes.
@TheWitness
Copy link
Copy Markdown
Member Author

Guys, these changes will not work as is. In fact Rector is full of it. It's casting in several places when the variable is already declared to be a string. So, I just wanted to see what this looked like. I do like the use of match, but the changes as is will not work. It'll actually break Cacti as converted.

@somethingwithproof, I recommend we pull rector out completely and use Lint, php-cs-fixer, and PHPStan exclusively.

@TheWitness TheWitness closed this Mar 27, 2026
@somethingwithproof
Copy link
Copy Markdown
Contributor

@TheWitness Agree drop Rector for now. Here's why it can't work on Cacti as-is:

Rector's static analysis is only as good as the type information available to it. Without declare(strict_types=1) and with heavy mixed returns from DB functions throughout, it makes unsafe assumptions. The two bugs Copilot caught in the PR prove it:

lib/variables.php — truncated against $item['field_value'], a key that isn't even in the SELECT. Silent undefined index.
ss_nimble_alletra_volumes.php — converted isset() ? : '' to ?? without accounting for PHP operator precedence. . binds tighter than ??, so the 64-bit counter assembly via bindec() silently returns only the high bits. Data corruption.

Rector is a power tool designed for greenfield codebases or projects with strict types declared throughout — modern Laravel/Symfony apps, or codebases already at a high PHPStan level with a comprehensive test suite as a safety net. On a 20+ year old codebase with intentional loose comparisons it needs to run one rule at a time with manual review of every output, which defeats the purpose entirely. The one exception: purely mechanical, syntactically unambiguous transforms like sizeof()count() or string callable literals → first-class callables are safe regardless of type information. Those specific rules can stay if we want them.

For everything else, PHPStan is the right path. The (string) casts Rector scattered across $_SERVER values and mixed DB returns are real issues, but the fix is improving the return type signatures on read_config_option(), db_fetch_cell(), and the SNMP wrappers to return string instead of mixed, not scattering casts at every call site. PHPStan's baseline feature lets us capture the current violation count and only fail on new ones, so we can work down to level 6 incrementally without a big-bang fix. Rector's cast additions were treating the symptom; PHPStan at baseline points at the source.

Correct sequencing: PHPStan baseline → fix mixed returns on core DB/SNMP functions → re-evaluate Rector for the mechanical-only rules once the type information is actually reliable.

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.

4 participants