diff --git a/.github/workflows/php-quality.yml b/.github/workflows/php-quality.yml new file mode 100644 index 0000000..0d49f2b --- /dev/null +++ b/.github/workflows/php-quality.yml @@ -0,0 +1,38 @@ +name: PHP quality checks + +on: + push: + pull_request: + workflow_dispatch: + +permissions: + contents: read + +jobs: + quality: + name: PHP ${{ matrix.php-version }} + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + php-version: ['8.2', '8.3', '8.4', '8.5'] + + steps: + - name: Check out repository + uses: actions/checkout@v4 + + - name: Set up PHP + uses: shivammathur/setup-php@v2 + with: + php-version: ${{ matrix.php-version }} + coverage: none + tools: parallel-lint, phpcs, phpstan + + - name: Lint PHP sources + run: parallel-lint --exclude .git --exclude vendor src + + - name: Check coding standards + run: phpcs --standard=phpcs.xml.dist + + - name: Run static analysis + run: phpstan analyse --no-progress --configuration=phpstan.neon.dist diff --git a/docs/art-ipf-migration-matrix.md b/docs/art-ipf-migration-matrix.md new file mode 100644 index 0000000..bdf16d5 --- /dev/null +++ b/docs/art-ipf-migration-matrix.md @@ -0,0 +1,19 @@ +# Art → IPF migration matrix + +This module still mixes legacy Art-based persistence patterns with newer ImpressCMS conventions. The matrix below captures the intended migration target without changing runtime behavior in this PR. + +| Legacy area | Current responsibility | IPF-oriented target | Notes for follow-up | +| --- | --- | --- | --- | +| `src/class/art/object.php` and related Art base classes | Shared object lifecycle, validation, and persistence helpers | Replace with IPF object and handler base classes | Start with low-risk handlers that already map cleanly to one table each | +| `src/class/forum.php`, `src/class/topic.php`, `src/class/post.php` | Module domain objects and handler logic | Move table mapping and CRUD concerns into dedicated IPF handlers | Keep public method names stable while adapters are introduced | +| `src/include/functions.ini.php` config loading | Module-wide config bootstrap | Route shared module/config access through the new bootstrap helper first | This reduces repeated dirname lookups before broader refactors | +| Direct `icms_getmodulehandler()` calls throughout `src/` | Per-file handler lookup | Centralize lookup behind helper functions | Lets later refactors swap implementations in one place | +| Install and update hooks in `src/include/module.php` | Schema/bootstrap orchestration | Move schema-aware logic behind dedicated service-style helpers | Leave hook entry points intact for compatibility | +| Template-facing utility functions in `src/include/functions.php` | Mixed rendering and data access helpers | Separate presentation helpers from persistence access | Tackle after handler loading is consolidated | + +## Migration guardrails + +- Preserve the current module entry points and install/update hooks until handler migrations are complete. +- Refactor one handler family at a time so existing forum, topic, and post behavior stays unchanged. +- Prefer adding adapters and compatibility layers before deleting Art-based code. +- Expand CI coverage alongside each phase so new IPF-oriented code is checked on all supported PHP versions. diff --git a/docs/index.md b/docs/index.md index d4ed6ff..3cbd76b 100644 --- a/docs/index.md +++ b/docs/index.md @@ -1,3 +1,6 @@ iForum is a full-featured discussion forum for ImpressCMS. [Templates](templates.md) +[Art → IPF migration matrix](art-ipf-migration-matrix.md) +[PHP 8 compatibility checklist](php8-compatibility-checklist.md) +[Phased refactor roadmap](phased-refactor-roadmap.md) diff --git a/docs/phased-refactor-roadmap.md b/docs/phased-refactor-roadmap.md new file mode 100644 index 0000000..ffdf4a4 --- /dev/null +++ b/docs/phased-refactor-roadmap.md @@ -0,0 +1,27 @@ +# Phased refactor roadmap + +This roadmap describes the intended migration path while keeping the module installable and behaviorally stable. + +## Phase 1: Developer safety nets + +- Add CI coverage for PHP linting, coding standards, and static analysis. +- Document the Art → IPF migration path and PHP 8 review expectations. +- Introduce a shared bootstrap helper for module lookup and handler loading without wiring it into runtime paths yet. + +## Phase 2: Shared access consolidation + +- Replace duplicated module dirname and handler lookup code in low-risk files with the shared helper. +- Keep existing public entry points, hook signatures, and handler names unchanged. +- Add narrowly scoped validation to each migrated slice before widening coverage. + +## Phase 3: Persistence layer migration + +- Migrate one handler family at a time from Art-based patterns to IPF-friendly structures. +- Add adapters where needed so existing call sites keep working during the transition. +- Separate persistence concerns from presentation helpers as handlers move over. + +## Phase 4: Cleanup and expansion + +- Broaden coding standards and static analysis coverage once migrated areas are stable. +- Retire duplicate compatibility wrappers only after all call sites move to the shared abstractions. +- Update developer documentation as each migration phase lands. diff --git a/docs/php8-compatibility-checklist.md b/docs/php8-compatibility-checklist.md new file mode 100644 index 0000000..f5c900f --- /dev/null +++ b/docs/php8-compatibility-checklist.md @@ -0,0 +1,23 @@ +# PHP 8 compatibility checklist + +Use this checklist when touching legacy iForum code while PHP 8.2 through PHP 8.5 remain supported in CI. + +## Baseline checks + +- Keep `parallel-lint` green across the full `src/` tree. +- Keep coding standards and static analysis green for the shared bootstrap helper and any newly added developer-facing files. +- Avoid introducing dynamic properties, removed functions, or deprecated string/array access patterns. + +## Review checklist for changes + +- Confirm `include_once` and `require_once` paths still resolve correctly when dirname helpers are touched. +- Check for implicit `null` to scalar conversions and loose comparisons that become noisier on newer PHP versions. +- Prefer explicit return values over relying on legacy truthy/falsy behavior. +- Avoid adding new references to removed PHP extensions or incompatible third-party tooling. +- Keep legacy global state access (`icms::$module`, config arrays, handler factories) wrapped behind helper functions where possible. + +## Before merging refactors + +- Re-run the PHP quality workflow on PHP 8.2, 8.3, 8.4, and 8.5. +- Verify install/update hooks still load through `src/icms_version.php` and `src/include/module.php`. +- Confirm helper-only refactors do not alter templates, routes, or persisted data. diff --git a/phpcs.xml.dist b/phpcs.xml.dist new file mode 100644 index 0000000..aa2eac1 --- /dev/null +++ b/phpcs.xml.dist @@ -0,0 +1,12 @@ + + + Coding standards coverage for the shared bootstrap helper added in this refactor slice. + + + + + + src/include/bootstrap.php + + + diff --git a/phpstan.neon.dist b/phpstan.neon.dist new file mode 100644 index 0000000..070ba61 --- /dev/null +++ b/phpstan.neon.dist @@ -0,0 +1,7 @@ +parameters: + level: 0 + paths: + - src/include/bootstrap.php + bootstrapFiles: + - tools/phpstan-bootstrap.php + tmpDir: /tmp/phpstan-iforum diff --git a/src/class/art/functions.locale.php b/src/class/art/functions.locale.php index d367c6f..69e5a97 100644 --- a/src/class/art/functions.locale.php +++ b/src/class/art/functions.locale.php @@ -44,7 +44,7 @@ function xoops_local($func) $msg = "The locale version is too old. Please copy
XOOPS/Frameworks/compat/language/english/local.php, local.class.php to XOOPS/language/english/"; if (icms::$config->getConfig("language") != "english") { if (is_dir(ICMS_ROOT_PATH."/Frameworks/compat/language/".icms::$config->getConfig("language")."/")) { - $msg .= "
XOOPS/Frameworks/compat/language/".icms::$config->getConfig("language")."/local.php to XOOPS/language/".icms::$config->getConfig("language"]."/"; + $msg .= "
XOOPS/Frameworks/compat/language/".icms::$config->getConfig("language")."/local.php to XOOPS/language/".icms::$config->getConfig("language")."/"; } else { $msg .= "
And modify XOOPS/language/".icms::$config->getConfig("language")."/local.php according to XOOPS/language/english/local.php"; } diff --git a/src/include/bootstrap.php b/src/include/bootstrap.php new file mode 100644 index 0000000..0adf5bc --- /dev/null +++ b/src/include/bootstrap.php @@ -0,0 +1,68 @@ +getVar('dirname', 'n') === iforum_get_module_dirname() + ) { + $module = icms::$module; + + return $module; + } + + $moduleHandler = icms::handler('icms_module'); + $module = $moduleHandler->getByDirname(iforum_get_module_dirname()); + + return $module; +} + +function &iforum_get_handler($handlerName) +{ + $handler = icms_getmodulehandler($handlerName, iforum_get_module_dirname(), 'iforum'); + + return $handler; +} diff --git a/tools/phpstan-bootstrap.php b/tools/phpstan-bootstrap.php new file mode 100644 index 0000000..41e659f --- /dev/null +++ b/tools/phpstan-bootstrap.php @@ -0,0 +1,25 @@ +