diff --git a/CHANGELOG.md b/CHANGELOG.md index 74d0098..d3135cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,45 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project uses [Semantic Versioning](https://semver.org/) with pre-release tags (`1.0.0-alpha.N`). +## Unreleased + +_Nothing yet._ + +## [1.0.0-alpha.3] — 1.0.0 Alpha 3 — 2026-09-21 + +### Fixed (Goffy / wgSimpleAcc field report) + +- Inspections no longer report every finding twice (file-level `visitFile` now runs only on the view-provider's primary PSI file). +- `isResultSet` quick-fix inserts before the current PSI statement (Inspect Code batch apply no longer no-ops after the first click in a file). +- An early-exit `isResultSet` / `mysqli_result` guard covers later `fetch*` of the same variable, including `while (list(...) = $db->fetchRow($result))`. +- ROOT_PATH guard: `die` and `exit` after `namespace` / `use` are recognized; the quick-fix inserts *after* the namespace (never before — invalid PHP). +- ROOT_PATH guard is not required on 404/403 directory stubs, `admin/` CP scripts, `xoops_version.php`, or files whose first work is including `mainfile.php` / `header.php` / `admin_header.php`. + +### Fixed (release review, PR #4) + +- Replaced deprecated `StartupActivity` with `ProjectActivity`. +- `FilenameIndex.getVirtualFilesByName` now passes `Project` (the two-arg overload is deprecated). +- Marketplace Plugin Verifier **Critical** on IntelliJ IDEA is the missing `com.jetbrains.php` plugin in IU, not a PhpStorm incompatibility — verify against PhpStorm. +- Persist Alpha 2 `coreProfile` into `coreVersion` (legacy-only XML keeps 2.5/2.7/4.0; explicit new `Auto` is not overwritten). +- Batch isResultSet quick-fix reuses inspection analysis at the fetch offset; reassignment inside a positive `if` is unguarded. +- ROOT_PATH open-tag handling is shared (` '…'`; commented-out entries are ignored by the scanner too. A positive `isResultSet(...) xor …` guard is not a guard. The comment mask steps over quoted strings, so `//` or `#` inside a string (a URL in a description) no longer hides the rest of the line from the manifest, guard and template readers. A `module.json`-only module is not scanned for unregistered templates. The missing-registered-template inspection reads the manifest through the comment-only mask (it had masked the string literals it needed and never reported). `define()` must be a real call (`mydefine('_MI_…')` is not indexed). The register-template quick-fix ignores commented-out entries. A positive `isResultSet` guard does not cover a fetch inside a closure. The ROOT_PATH quick-fix is attached only where it can insert. An unreadable or oversized `xoops_version.php` yields one `SCAN_ERROR` instead of every template being reported unregistered. Commented-out `define()` lines are not indexed as language constants. Heredoc/nowdoc bodies are skipped by the comment-only mask, so `//` or `/*` inside them cannot hide later code. A VFS event on the `language` directory itself invalidates the constant cache. A `define()` inside a string literal is not indexed. Unbraced `if`/`while`/`for`/`foreach` bodies are reported without an isResultSet quick-fix; add braces before applying the guard fix. Bootstrap detection matches exact filenames (`custom-header.php`, `mainfile.php.bak` do not count). Template registrations are read only from `$modversion['templates']` / `$modversion['blocks']` statements (shared `XoopsManifestTemplates` reader). An `elseif` early exit is not a dominating guard. Core version detection ignores `12.5` / `12.7` / `14.0`. The isResultSet quick-fix declines when the statement assigns the result before fetching. Ctrl+B prefers a `define()` in the same module when several modules define the same name. + +### Added + +- Language-constant completion scans every `language/**/*.php` (not a five-name allowlist). +- Ctrl+B / Find Usages on `_MI_` / `_AM_` / `_MD_` / `_CO_` / `_MB_` constants (resolves to `define()` in `language/english/` when present). +- Inspection: `.tpl` on disk under `templates/` or `blocks/` not listed in `xoops_version.php` (inverse of missing registered template), with a register-in-manifest quick-fix. +- Inspection: `XOBJ_DTYPE_UNICODE_*` deprecated since 2.7.3, rename quick-fix to the non-UNICODE successor (silent when Core Version is 2.5). +- JUnit 4 analyzer / policy / scanner / plugin.xml tests. + +### Changed + +- Plugin version is taken only from `gradle.properties` (`plugin.xml` no longer hard-codes ``). + ## [1.0.0-alpha.2] — 1.0.0 Alpha 2 — 2026-08-12 ### Fixed @@ -23,7 +62,7 @@ and this project uses [Semantic Versioning](https://semver.org/) with pre-releas ## [1.0.0-alpha.1] — 1.0.0 Alpha 1 — 2026-08-11 -First public alpha of **XOOPS Support** — a PhpStorm / IntelliJ helper for XOOPS 2.5 / 2.7 / 4.0 module and core work. +First public alpha of **XOOPS Support** — a PhpStorm / IntelliJ helper for XOOPS 2.5 / 2.7 / 4.0 Core and module development. Early preview: APIs, inspections, and quick fixes may change before a stable 1.0. @@ -44,7 +83,7 @@ Early preview: APIs, inspections, and quick fixes may change before a stable 1.0 - Wrong Smarty delimiters (XOOPS `<{ … }>` vs bare `{ … }`) - **Live templates** — `xoguard`, `xofetch`, `xofetchdb`, `xohead`, `xolang`, `xocriteria`, `xorequest`, `xoexec` - **Language-constant completion** — `_MI_` / `_AM_` / `_MD_` / … from `language/**/*.php`, with project cache and VFS invalidation -- **Settings** — enable/disable, suppress startup notification, core profile, table prefix +- **Settings** — enable/disable, suppress startup notification, Core Version, table prefix - **Dynamic plugin** — no `require-restart`; install / disable / enable without IDE restart when unload succeeds - **CI / release** — GitHub Actions (`check`, `verifyPlugin`, `buildPlugin`); tag `v*` must match `pluginVersion` - **Compatibility** — PhpStorm **2024.3+** (`since-build=243`, open-ended `until-build` for 2025.x / 2026.2.x) @@ -56,4 +95,6 @@ Early preview: APIs, inspections, and quick fixes may change before a stable 1.0 - Overview scans are sequenced so a slower older scan cannot overwrite a newer refresh - License: GPL-2.0 (SPDX **GPL-2.0-or-later** in packaging docs) +[1.0.0-alpha.3]: https://github.com/XOOPS/phpstorm-plugin/releases/tag/v1.0.0-alpha.3 +[1.0.0-alpha.2]: https://github.com/XOOPS/phpstorm-plugin/releases/tag/v1.0.0-alpha.2 [1.0.0-alpha.1]: https://github.com/XOOPS/phpstorm-plugin/releases/tag/v1.0.0-alpha.1 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index cd68014..7a489ad 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -24,6 +24,9 @@ Thanks for helping improve the PhpStorm plugin for XOOPS developers. - Java 21, package root `org.xoops.support`. - Keep inspections **fast and heuristic** unless a full PSI analysis is clearly worth it. +- File-level `visitFile` inspections must call `PhpTextUtil.isPrimaryPsiFile` (PHP+HTML dual PSI). +- Quick-fixes compute ranges from the PSI element at apply-time, not frozen offsets. +- Tests are **JUnit 4** (`org.junit.Test`, public classes/methods) via `testFramework(TestFrameworkType.Platform)`. Jupiter will not start the IPG executor. - Prefer **quick fixes** that are safe and local (single file / small edit). - Do not hard-code AI vendor names or branding in UI strings. - Index / PSI access from background threads must use `ReadAction` (see `XoopsProjectService`). @@ -38,11 +41,11 @@ CI runs the same command set. ## Version bumps -1. Update `pluginVersion` in `gradle.properties`. -2. Update `` and change-notes in `src/main/resources/META-INF/plugin.xml`. -3. Update `CHANGELOG.md` and `whats-new.html`. +1. Update `pluginVersion` in `gradle.properties` (this is the single source; do not add `` to `plugin.xml`). +2. Update change-notes in `src/main/resources/META-INF/plugin.xml`. +3. Update `CHANGELOG.md` and `whats-new.html` (and the version table in `README.md`). 4. Tag release with the **same** version as `pluginVersion` in `gradle.properties`, e.g. - `git tag v1.0.0-alpha.1 && git push origin v1.0.0-alpha.1` (triggers release workflow). + `git tag v1.0.0-alpha.3 && git push origin v1.0.0-alpha.3` (triggers release workflow). ## Reporting issues diff --git a/README.md b/README.md index 26fcfcf..4ef295c 100644 --- a/README.md +++ b/README.md @@ -8,15 +8,15 @@ It brings XOOPS conventions into the IDE: inspections with Alt+Enter quick fixes | --- | --- | | Repository | [github.com/XOOPS/phpstorm-plugin](https://github.com/XOOPS/phpstorm-plugin) | | Plugin id | `org.xoops.plugin.support` | -| Version | **1.0.0 Alpha 1** (`1.0.0-alpha.1`) | +| Version | **1.0.0 Alpha 3** (`1.0.0-alpha.3`) | | Compatibility | PhpStorm **2024.3+** (since-build `243`, no upper cap — includes **2026.2.x**) | | License | [GPL-2.0-or-later](LICENSE) | ## Features -- **Inspections + quick fixes** — root-path guards, `isResultSet` before `fetch*`, `query()` vs `exec()`, deprecated `queryF`/`quoteString`, missing registered templates, wrong Smarty delimiters, `include` → `include_once` for headers. For input: **keyed** `$_GET`/`$_POST`/`$_COOKIE['key']` offer `\Xmf\Request::getString` fixes; **bare** `$_GET`/`$_POST`/`$_REQUEST`/`$_COOKIE` and **keyed `$_REQUEST`** are warnings only (no auto-fix when the source is ambiguous) +- **Inspections + quick fixes** — root-path guards (namespaced files; 404/entry-point skips), `isResultSet` before `fetch*` (including `while (list = fetchRow)`), `query()` vs `exec()`, deprecated `queryF`/`quoteString` and `XOBJ_DTYPE_UNICODE_*`, missing *and unregistered* templates, wrong Smarty delimiters, `include` → `include_once` for headers. For input: **keyed** `$_GET`/`$_POST`/`$_COOKIE['key']` offer `\Xmf\Request::getString` fixes; **bare** `$_GET`/`$_POST`/`$_REQUEST`/`$_COOKIE` and **keyed `$_REQUEST`** are warnings only (no auto-fix when the source is ambiguous) - **Live templates** — `xoguard`, `xofetch`, `xofetchdb`, `xohead`, `xolang`, `xocriteria`, `xorequest`, `xoexec` -- **Language constants** — completion for `_MI_` / `_AM_` / `_MD_` / … from `language/**/*.php` +- **Language constants** — completion and Ctrl+B for `_MI_` / `_AM_` / `_MD_` / `_CO_` / `_MB_` from every `language/**/*.php` - **Project tools** — detection balloon, scanner tool window, **Tools → XOOPS Support** - **Module scaffold** — legacy or hybrid (PSR-4 / composer) via **New → XOOPS Module…** @@ -81,7 +81,7 @@ xoops-support/ Edit `gradle.properties` for version and platform target: ```properties -pluginVersion=1.0.0-alpha.1 +pluginVersion=1.0.0-alpha.3 platformVersion=2024.3.5 pluginSinceBuild=243 pluginUntilBuild= # empty = open-ended (2026.2+) diff --git a/TUTORIAL.md b/TUTORIAL.md index b7ff7a8..5d279d5 100644 --- a/TUTORIAL.md +++ b/TUTORIAL.md @@ -22,7 +22,7 @@ Then **Install Plugin from Disk…** → `build/distributions/xoops-support-*.zi 1. Open a **real XOOPS install** (with `mainfile.php` or `htdocs/mainfile.php`). This plugin repository alone is not a XOOPS site root. 2. Optional balloon: “XOOPS Support active”. 3. **Tools → XOOPS Support → Show XOOPS Project Info** (runs in the background). -4. **Settings** (search “XOOPS Support”): enable/disable, profile, suppress notification. +4. **Settings** (search “XOOPS Support”): enable/disable, Core Version, suppress notification. ## 3. Tool window / scanner @@ -30,12 +30,250 @@ Then **Install Plugin from Disk…** → `build/distributions/xoops-support-*.zi 2. **Refresh** (or **Tools → XOOPS Support → Refresh XOOPS Overview**) 3. Click findings to open files. +![XOOPS Support Overview tool window: modules table and findings](https://plugins.jetbrains.com/files/33478/screenshot_200ef8b4-f28f-4bbd-95f3-3f310b27fed7) + +**Reading the Overview.** The header line gives the totals (here 22 modules, 316 findings) and the detected core line (**XOOPS 2.7.x**, from the Core Version setting or auto-detection) with the web root the scan used. + +The **Modules** table lists every directory under `modules/` that has a manifest, one row per module: + +| Column | Meaning | +| --- | --- | +| Manifest | `xoops_version.php` (legacy), `module.json` (4.0), or `hybrid` when both exist | +| TPL | `.tpl` files under `templates/` | +| Lang | `.php` files under `language/` (all locales) | +| Pre | `.php` files under `preloads/` | +| Cls | `.php` files under `class/` and `src/` together | + +The counts are a quick health read: **TPL 0** means the module renders nothing of its own (fine for a library module such as `protector` or `xwhoops`), **Lang** grows with every shipped locale (each adds its own set of files), and an unexpectedly large **Cls** usually means a vendored library sits under `class/` or `src/`. + +**Findings** are listed per module in the same order as the table. Each line is a kind, a message and a `file:line` link that opens the editor at that spot. The kinds map onto the section 4 inspections: + +| Finding | Inspection | +| --- | --- | +| `MUTATING_QUERY` | 4.6 `XoopsQueryExec` | +| `DEPRECATED_QUERY_F`, `DEPRECATED_QUOTE_STRING` | 4.1 `XoopsDeprecatedDbApi` | +| `RAW_REQUEST` | 4.7 `XoopsSuperglobal`, `$_REQUEST` only; keyed `$_GET` / `$_POST` are left to the editor | +| `MISSING_REGISTERED_TEMPLATE` | 4.8 `XoopsMissingRegisteredTemplate` | +| `TEMPLATE_CASE_MISMATCH` | 4.8 `XoopsMissingRegisteredTemplate`: filename/directory spelling differs from disk | +| `UNREGISTERED_TEMPLATE` | 4.9 `XoopsUnregisteredTemplate` | +| `WRONG_SMARTY_DELIMITER` | 4.10 `XoopsWrongSmartyDelimiter` | +| `SCAN_ERROR` | a file or directory the scanner could not read | + +The scanner reports the **first** occurrence per file for the code-pattern kinds, so one `RAW_REQUEST` line can stand for several uses in that file; the editor inspection marks each one. The guard and `isResultSet` rules run only in the editor, where they have the PSI they need. + +Use the Overview as the whole-site view: triage a large install or a module you have just imported, then open a file and let Alt+Enter do the individual fixes. The scan runs as a background task and honours **Cancel**. + ## 4. Inspections + Alt+Enter 1. Copy `test-fixtures/bad_module_sample.php` under `htdocs/modules/_xoops_demo/`. 2. Open it — highlights for guards, query/exec, Request, etc. 3. **Alt+Enter** on each highlight and apply the fix. 4. For Smarty: a `.tpl` with bare `{if …}` should offer delimiter conversion. +5. Goffy / wgSimpleAcc shapes: `test-fixtures/wgsimpleacc_shapes.php` (namespaced `die` guard + `while (list = fetchRow)`) and `test-fixtures/index_404_stub.php` (must **not** warn). + +All inspections live under **Settings → Editor → Inspections → XOOPS**. Each can be switched off or downgraded there, and the Inspection ID below is what you put in a `@noinspection` comment or an `.idea/inspectionProfiles` file. Every inspection is silent in `vendor/`, `templates_c/`, `cache/` and `node_modules/`. + +### 4.1 Deprecated database API — `XoopsDeprecatedDbApi` + +Flags `queryF()` and `quoteString()`. Use `query()` / `exec()` and `quote()` instead. + +**Why.** `queryF()` was the "force" variant that skipped the write block on `query()`. Since XOOPS 2.5.12 the split is explicit: `query()` refuses mutating SQL and `exec()` is the only sanctioned write path, so `queryF()` no longer has a job. `quoteString()` is a plain alias of `quote()` kept for backward compatibility and scheduled for removal. + +```php +// before +$db->queryF('UPDATE ' . $db->prefix('news') . ' SET hits = hits + 1'); +$name = $db->quoteString($name); + +// after +$db->exec('UPDATE ' . $db->prefix('news') . ' SET hits = hits + 1'); +$name = $db->quote($name); +``` + +**Quick-fixes.** `quoteString()` is renamed to `quote()`. For `queryF()` the inspection reads the first string argument to decide what to offer: + +- SQL starting with `SELECT` / `SHOW` / `DESCRIBE` / `EXPLAIN`: rename to `query()` only. +- SQL starting with `INSERT` / `UPDATE` / `DELETE` / `REPLACE` / `TRUNCATE` / `ALTER` / `DROP` / `CREATE`: `exec()` is offered first, `query()` second. +- SQL that cannot be read (a variable, a concatenation): both renames are offered; pick by intent. + +`exec()` takes one argument, so the `exec()` rename is withheld while the call still passes `$limit` / `$start`. Drop those first, then re-run the fix. + +### 4.2 Deprecated `XOBJ_DTYPE_UNICODE_*` — `XoopsDeprecatedUnicodeDtype` + +`XOBJ_DTYPE_UNICODE_*` constants were deprecated in XOOPS 2.7.3 (core issue #164) and are scheduled for removal in 4.0. The quick-fix renames the constant to its non-UNICODE successor (`XOBJ_DTYPE_TXTBOX`, `TXTAREA`, `URL`, `EMAIL`, `ARRAY`, `OTHER`). Value migration stays a core concern and is not performed here. Silent when the project Core Version is set to 2.5. + +**Why.** The `UNICODE_*` types date from the pre-UTF-8 era when a field had to declare that it might hold multibyte text. Every supported core runs on `utf8mb4`, so the distinction is dead weight: the sanitizer treats `XOBJ_DTYPE_UNICODE_TXTBOX` and `XOBJ_DTYPE_TXTBOX` identically. Renaming is safe at the `initVar()` site because it changes only which constant name is looked up, not how the stored value is read or written. + +```php +// before +$this->initVar('title', XOBJ_DTYPE_UNICODE_TXTBOX, null, true, 255); + +// after +$this->initVar('title', XOBJ_DTYPE_TXTBOX, null, true, 255); +``` + +**Core Version gate.** The 2.5 LTS line never deprecated these constants, so a module that must keep running on 2.5 gets no warning. Set Core Version to **2.5** in Settings for that project; **Auto**, **2.7** and **4.0** all report. + +### 4.3 `isResultSet` guard — `XoopsResultSetGuard` + +Warns when `fetchArray` / `fetchRow` / `fetchBoth` appears without a dominating `isResultSet` check. Prefer the two-part guard: + +```php +$result = $db->query($sql); +if (!$db->isResultSet($result) || !$result instanceof \mysqli_result) { + throw new \RuntimeException('Database query failed'); +} +while (list($id, $title) = $db->fetchRow($result)) { + // ... +} +``` + +An early-exit guard covers later fetches of the same variable, including `while (list(...) = $db->fetchRow($result))`, until that variable is reassigned. The quick-fix inserts before the enclosing statement using the current PSI (safe to apply several times after Inspect Code). + +**Why two parts.** `query()` returns `false` on failure, and `fetchArray(false)` is a fatal `TypeError` on PHP 8. `isResultSet()` is the XOOPS check; the `instanceof \mysqli_result` half is for static analysers such as Scrutinizer and PHPStan, which do not know that `isResultSet()` narrows the type. + +**What counts as guarded.** The analysis is textual, so it follows a few explicit rules rather than full control flow: + +- A negative check that throws, returns or exits (`if (!$db->isResultSet($result)) { return; }`) covers every later fetch of `$result` in the same block. It stops covering at a reassignment of `$result`, at a nested `function`, or when the enclosing `}` closes. +- A positive check (`if ($db->isResultSet($result)) { ... }`) covers fetches inside its body, brace-less body included. `||` and `xor` in a positive condition disqualify it: `if ($db->isResultSet($result) || $fallback)` can enter the body with `$result === false`, so the fetch inside is still reported. `||` is fine only in the terminating negative form above. +- `$result = $db->fetchArray($result)` inside a guarded region stays guarded: the fetch reads the old value. `$result = false or $db->fetchArray($result)` does not, because `or` binds looser than `=` and the assignment completes first. +- `&&`, `and` and `xor` in the guard condition disqualify it. `if (!isResultSet($r) && $x) return;` can fall through while `$r` is still `false`. +- Comments and strings are masked first, so a commented-out guard never counts. + +**Guard quick-fix scope.** The guard is inserted within the existing statement list. Unbraced `if`, `while`, `for` and `foreach` bodies receive a warning without a quick-fix; add braces first so the guard can stay inside the control flow. + +### 4.4 `include_once` for headers — `XoopsIncludeOnceHeader` + +XOOPS module entry points should load `header.php` / `footer.php` with `include_once` (not bare `include`). + +**Why.** `header.php` starts the theme, opens the output buffer and pulls in `$xoopsTpl`; `footer.php` renders and flushes. Including either twice, which happens easily once a page delegates to a shared `include/` file, produces duplicate headers or a blank page. `include_once` makes the second include a no-op. The rule applies to the core files and to a module's own `header.php` / `footer.php`, with or without the `XOOPS_ROOT_PATH .` prefix. + +```php +// before +include XOOPS_ROOT_PATH . '/header.php'; +include __DIR__ . '/footer.php'; + +// after +include_once XOOPS_ROOT_PATH . '/header.php'; +include_once __DIR__ . '/footer.php'; +``` + +**Quick-fix** rewrites the keyword in place. `require` / `require_once` are not touched; only a bare `include` of a `header.php` / `footer.php` path is reported. + +### 4.5 Direct-access guard — `XoopsRootPathGuard` + +Reports include-only PHP files (classes, preloads, includes, blocks) that lack a terminating `defined('XOOPS_ROOT_PATH') || exit/die(...)` guard. The guard must be the first executable statement after `query()`. Use `exec()` for mutations (XOOPS 2.5.12+ convention). + +**Why.** Since 2.5.12 `query()` inspects the statement and blocks writes on a GET request, logging `query() called with a mutating statement; use exec()`. Code that worked on 2.5.11 silently stops writing on upgrade, and the only symptom is a log line. Splitting reads and writes at the call site also makes a future prepared-statement layer possible, because `exec()` never needs `$limit` / `$start`. + +```php +// before +$db->query('DELETE FROM ' . $db->prefix('news') . ' WHERE storyid = ' . $id); + +// after +$db->exec('DELETE FROM ' . $db->prefix('news') . ' WHERE storyid = ' . $id); +``` + +**Quick-fix** renames `query` to `exec` when the call has a single argument. With `$limit` / `$start` present the problem is still reported, but the fix is withheld: `exec()` takes one argument, so the extra ones must be removed by hand first. SQL held in a variable is not inspected; only a string literal as the first argument is read. + +### 4.7 Raw superglobals — `XoopsSuperglobal` + +Flags raw `$_GET`, `$_POST`, `$_REQUEST`, and `$_COOKIE` in module and XMF code. Prefer `\Xmf\Request`. + +**Why.** `\Xmf\Request` does the type coercion, default handling and filtering that hand-written `isset($_POST['x']) ? (int) $_POST['x'] : 0` tends to get subtly wrong, and it makes the input source explicit. `$_REQUEST` merges GET, POST and COOKIE in `php.ini`-dependent order, so a cookie can override a form field; that is why the fix always names a source and never offers `$_REQUEST` as one. + +```php +// before +$op = $_GET['op']; +$name = $_POST['name']; + +// after +$op = \Xmf\Request::getString('op', '', 'GET'); +$name = \Xmf\Request::getString('name', '', 'POST'); +``` + +**Quick-fix scope.** Only a **keyed** read with a string-literal key (`$_GET['op']`, `$_POST['name']`, `$_COOKIE['sid']`) gets an Alt+Enter replacement, and it always uses `getString`. Change it to `getInt`, `getBool`, `getArray` or `getCmd` afterwards when the value is not free text. A bare `$_POST` (passed to a function, iterated, or used with a variable key) and any use of `$_REQUEST` are warnings only, because the right method and source cannot be inferred. + +### 4.8 Missing registered template — `XoopsMissingRegisteredTemplate` + +A template listed in `xoops_version.php` (`file` / `template` key) was not found under the module `templates/` (or `blocks/`) directory. A case-only difference is reported as `TEMPLATE_CASE_MISMATCH`, with the actual disk spelling and no create-file quick-fix. + +**Why.** On install and update XOOPS reads `$modversion['templates']` and copies each listed file into the `tplfile` table. A missing file is skipped without an error, so the page or block later fails with a Smarty "unable to read resource" message that names a template you registered but never created, or that was renamed on disk without the manifest following. + +```php +$modversion['templates'][] = [ + 'file' => 'demo_index.tpl', // must exist as templates/demo_index.tpl + 'description' => 'Index page', +]; +``` + +Both template inspections read `xoops_version.php` only, so they cover legacy and hybrid modules; a `module.json`-only module is not checked. + +**Quick-fix** creates the file under `templates/`, keeping any sub-path in the name (`blocks/demo_block.tpl` becomes `templates/blocks/demo_block.tpl`), with a Smarty comment `<{* name *}>` as placeholder so the manifest and disk agree; fill in the markup afterwards. The inspection runs on `xoops_version.php` and is the inverse of 4.9. + +### 4.9 Unregistered template — `XoopsUnregisteredTemplate` + +A `.tpl` file under the module `templates/` or `blocks/` directory is not listed in `xoops_version.php`, so Smarty cannot `display()` it as a registered module template. Theme overrides under `themes/` are ignored. The quick-fix appends a `$modversion['templates'][]` entry to the manifest. + +**Why.** The `db:` template resource resolves through the `tplfile` table, and only registered templates get a row. An unregistered `.tpl` renders fine from the file system on a stock template set during development, then breaks on a site whose template set was imported, or on the first module update that resyncs templates. Registering it at creation time removes the surprise. + +**Matching.** A manifest entry may be spelled relative to `templates/` (`demo_index.tpl`, the XOOPS convention) or from the module root (`templates/demo_index.tpl`); both are accepted. Case-insensitive matching identifies the registration, but filename and directory spelling must match the disk exactly. The missing-template inspection and scanner report casing differences as a case mismatch, not a missing or unregistered template. + +**Quick-fix** appends this at the end of the manifest (before a trailing `?>` if there is one), unless the name is already quoted somewhere in the file: + +```php +$modversion['templates'][] = [ + 'file' => 'demo_orphan.tpl', + 'description' => '', +]; +``` + +Files under `themes/` and `templates_c/` are never reported: a theme override is not a module template, and compiled templates are generated. + +### 4.10 Wrong Smarty delimiters — `XoopsWrongSmartyDelimiter` + +XOOPS Smarty uses `<{` and `}>` delimiters. Bare `{if}` / `{$var}` tags are usually a mistake and will not render. + +**Why.** XOOPS configures Smarty with `<{` `}>` so that plain braces in CSS, JavaScript and JSON inside a template are left alone. A tag written in stock Smarty syntax is not an error: it is passed to the browser as literal text, so the symptom is `{$title}` appearing on the page, or an `{if}` block that always shows both branches. + +```smarty + +{if $items}
    {foreach $items as $item}
  • {$item.title}
    • {/foreach}
{/if} + + +<{if $items}>
    <{foreach $items as $item}>
  • <{$item.title}>
    • <{/foreach}>
<{/if}> +``` + +**What is matched.** A brace immediately followed by `$` or by one of the common tag keywords (`if`, `/if`, `foreach`, `/foreach`, `include`, `assign`, `block`, `literal`) up to the closing brace on the same line. Tags already written as `<{ … }>` are skipped, and so are braces that do not look like a tag, which keeps inline CSS and JavaScript quiet. + +**Quick-fix** wraps the tag in `<{ }>`. Apply it per tag, or run **Code → Inspect Code** on the `templates/` folder and use **Apply fix** on the group to convert a whole template. ## 5. Live templates @@ -54,7 +292,7 @@ In a PHP file, type the abbreviation and press **Tab**: ## 6. Language constants -Type `_MI_` (or `_AM_`, `_MD_`, …) and **Ctrl+Space** — completions from `language/**/*.php` defines. +Type `_MI_` (or `_AM_`, `_MD_`, `_CO_`, `_MB_`) and **Ctrl+Space** — completions from every `language/**/*.php` `define()`. **Ctrl+B** (Go to Declaration) on a constant jumps to the `define()` (prefers `language/english/`). ## 7. New module @@ -67,7 +305,7 @@ Type `_MI_` (or `_AM_`, `_MD_`, …) and **Ctrl+Space** — completions from `la | Action | Purpose | | --- | --- | -| Show XOOPS Project Info | Dialog: profile, web root, modules | +| Show XOOPS Project Info | Dialog: Core Version, web root, modules | | Refresh XOOPS Overview | Rescan + tool window | | New XOOPS Module Stub… | Scaffold module | diff --git a/build.gradle.kts b/build.gradle.kts index f2c18c5..953d40f 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -1,4 +1,5 @@ import org.jetbrains.intellij.platform.gradle.IntelliJPlatformType +import org.jetbrains.intellij.platform.gradle.TestFrameworkType fun prop(key: String): String = providers.gradleProperty(key).orNull @@ -24,7 +25,9 @@ dependencies { phpstorm(prop("platformVersion")) bundledPlugin("com.jetbrains.php") pluginVerifier() + testFramework(TestFrameworkType.Platform) } + testImplementation("junit:junit:4.13.2") } java { @@ -57,8 +60,8 @@ intellijPlatform { pluginVerification { ides { - // Same PhpStorm we compile against — keeps CI time/bandwidth predictable. - // Expand with recommended() / select { ... } when you want a wider matrix. + // PhpStorm only. Verifying IntelliJ IDEA (IU) without the PHP plugin + // yields a false Critical: "missing mandatory dependency com.jetbrains.php". create(IntelliJPlatformType.PhpStorm, prop("platformVersion")) } } @@ -74,6 +77,10 @@ tasks { gradleVersion = prop("gradleVersion") } + test { + useJUnit() + } + runIde { autoReload = true jvmArgumentProviders += CommandLineArgumentProvider { diff --git a/gradle.properties b/gradle.properties index 1f139f7..c7b54bb 100644 --- a/gradle.properties +++ b/gradle.properties @@ -5,8 +5,8 @@ pluginGroup=org.xoops pluginName=xoops-support pluginId=org.xoops.plugin.support -# Technical version (ZIP / Marketplace). Display: "1.0.0 Alpha 2" -pluginVersion=1.0.0-alpha.2 +# Technical version (ZIP / Marketplace). Display: "1.0.0 Alpha 3" +pluginVersion=1.0.0-alpha.3 # Platform / compatibility (PhpStorm build numbers) # 243 = 2024.3; leave until open so 2025.x / 2026.x (e.g. 2026.2.1 = 262.*) install cleanly. diff --git a/src/main/java/org/xoops/support/XoopsProjectService.java b/src/main/java/org/xoops/support/XoopsProjectService.java index 5f3f9e7..39df2e7 100644 --- a/src/main/java/org/xoops/support/XoopsProjectService.java +++ b/src/main/java/org/xoops/support/XoopsProjectService.java @@ -53,6 +53,7 @@ public boolean isXoopsProject() { @RequiresReadLock private @Nullable VirtualFile findMainfileUnderReadLock() { Collection files = FilenameIndex.getVirtualFilesByName( + project, "mainfile.php", GlobalSearchScope.projectScope(project) ); @@ -78,6 +79,7 @@ public boolean isXoopsProject() { @RequiresReadLock private @NotNull List findXoopsVersionFilesUnderReadLock() { Collection files = FilenameIndex.getVirtualFilesByName( + project, "xoops_version.php", GlobalSearchScope.projectScope(project) ); @@ -121,6 +123,7 @@ public boolean isXoopsProject() { @RequiresReadLock private @Nullable VirtualFile findXoopsLibUnderReadLock() { Collection dirs = FilenameIndex.getVirtualFilesByName( + project, "xoops_lib", GlobalSearchScope.projectScope(project) ); diff --git a/src/main/java/org/xoops/support/XoopsStartupActivity.java b/src/main/java/org/xoops/support/XoopsStartupActivity.java index 4e0e201..7c6f432 100644 --- a/src/main/java/org/xoops/support/XoopsStartupActivity.java +++ b/src/main/java/org/xoops/support/XoopsStartupActivity.java @@ -6,8 +6,11 @@ import com.intellij.openapi.application.ModalityState; import com.intellij.openapi.project.DumbService; import com.intellij.openapi.project.Project; -import com.intellij.openapi.startup.StartupActivity; +import com.intellij.openapi.startup.ProjectActivity; +import kotlin.Unit; +import kotlin.coroutines.Continuation; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.xoops.support.settings.XoopsSettingsState; /** @@ -19,55 +22,60 @@ *

All deferred work is expired with {@link Project#getDisposed()} so pending callbacks * do not pin the plugin classloader across unload. */ -public final class XoopsStartupActivity implements StartupActivity { +public final class XoopsStartupActivity implements ProjectActivity { @Override - public void runActivity(@NotNull Project project) { - if (project.isDisposed()) { - return; - } - XoopsSettingsState settings = XoopsSettingsState.getInstance(project); - if (!settings.enabled || settings.suppressStartupNotification) { + public @Nullable Object execute(@NotNull Project project, @NotNull Continuation continuation) { + scheduleNotification(project); + return Unit.INSTANCE; + } + + private void scheduleNotification(@NotNull Project project) { + if (project.isDisposed() || !notificationsEnabled(project)) { return; } - DumbService.getInstance(project).runWhenSmart(() -> { if (project.isDisposed()) { return; } - ApplicationManager.getApplication().executeOnPooledThread(() -> { - if (project.isDisposed()) { - return; - } - XoopsSettingsState current = XoopsSettingsState.getInstance(project); - if (!current.enabled || current.suppressStartupNotification) { - return; - } - XoopsProjectService service = XoopsProjectService.getInstance(project); - if (!service.isXoopsProject()) { - return; - } - int modules = service.findModuleDirnames().size(); - ApplicationManager.getApplication().invokeLater( - () -> { - if (project.isDisposed()) { - return; - } - NotificationGroupManager.getInstance() - .getNotificationGroup("XOOPS Support") - .createNotification( - "XOOPS Support active", - "Detected XOOPS markers (" + modules + " module(s) with xoops_version.php). " - + "See Settings → Editor → Inspections → XOOPS, " - + "and Tools → XOOPS Support.", - NotificationType.INFORMATION - ) - .notify(project); - }, - ModalityState.nonModal(), - project.getDisposed() - ); - }); + ApplicationManager.getApplication().executeOnPooledThread(() -> notifyIfXoops(project)); }); } + + private static boolean notificationsEnabled(@NotNull Project project) { + XoopsSettingsState settings = XoopsSettingsState.getInstance(project); + return settings.enabled && !settings.suppressStartupNotification; + } + + private static void notifyIfXoops(@NotNull Project project) { + if (project.isDisposed() || !notificationsEnabled(project)) { + return; + } + XoopsProjectService service = XoopsProjectService.getInstance(project); + if (!service.isXoopsProject()) { + return; + } + int modules = service.findModuleDirnames().size(); + ApplicationManager.getApplication().invokeLater( + () -> showBalloon(project, modules), + ModalityState.nonModal(), + project.getDisposed() + ); + } + + private static void showBalloon(@NotNull Project project, int modules) { + if (project.isDisposed()) { + return; + } + NotificationGroupManager.getInstance() + .getNotificationGroup("XOOPS Support") + .createNotification( + "XOOPS Support active", + "Detected XOOPS markers (" + modules + " module(s) with xoops_version.php). " + + "See Settings → Editor → Inspections → XOOPS, " + + "and Tools → XOOPS Support.", + NotificationType.INFORMATION + ) + .notify(project); + } } diff --git a/src/main/java/org/xoops/support/actions/ShowXoopsProjectInfoAction.java b/src/main/java/org/xoops/support/actions/ShowXoopsProjectInfoAction.java index 37c15a0..a1a8837 100644 --- a/src/main/java/org/xoops/support/actions/ShowXoopsProjectInfoAction.java +++ b/src/main/java/org/xoops/support/actions/ShowXoopsProjectInfoAction.java @@ -16,6 +16,7 @@ import org.xoops.support.scanner.XoopsModuleInfo; import org.xoops.support.scanner.XoopsProjectReport; import org.xoops.support.scanner.XoopsProjectScanner; +import org.xoops.support.settings.XoopsSettingsState; import java.nio.file.Path; @@ -34,7 +35,10 @@ public void run(@NotNull ProgressIndicator indicator) { XoopsProjectReport report; try { indicator.setText("Scanning XOOPS modules (cancellable)…"); - report = new XoopsProjectScanner().scan(Path.of(basePath)); + report = new XoopsProjectScanner().scan( + Path.of(basePath), + XoopsSettingsState.getInstance(project).resolvedCoreVersion() + ); } catch (ProcessCanceledException pce) { // Let the progress framework treat this as a user cancel, not a failure dialog. throw pce; @@ -62,7 +66,7 @@ public void run(@NotNull ProgressIndicator indicator) { if (!report.xoopsProject()) { sb.append("No XOOPS markers found (mainfile.php / xoops_version.php)."); } else { - sb.append(report.profile().displayName()).append('\n'); + sb.append(report.coreVersion().displayName()).append('\n'); sb.append("Web root: ").append(report.webRoot()).append('\n'); sb.append("Modules: ").append(report.modules().size()).append('\n'); int i = 0; diff --git a/src/main/java/org/xoops/support/completion/XoopsLanguageConstantParser.java b/src/main/java/org/xoops/support/completion/XoopsLanguageConstantParser.java new file mode 100644 index 0000000..257da64 --- /dev/null +++ b/src/main/java/org/xoops/support/completion/XoopsLanguageConstantParser.java @@ -0,0 +1,87 @@ +package org.xoops.support.completion; + +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.xoops.support.inspections.PhpTextUtil; + +import java.util.ArrayList; +import java.util.List; +import java.util.Locale; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * Parses {@code define('_MI_…', …)} (and AM/MD/CO/MB) from a language PHP file. + * Pure regex so it can be unit-tested without PSI. + */ +public final class XoopsLanguageConstantParser { + + public static final Pattern DEFINE = Pattern.compile( + "(?:])\\\\?define\\b\\s*\\(\\s*['\"](_(?:MI|AM|MD|CO|MB)_[A-Z0-9_]+)['\"]", + Pattern.CASE_INSENSITIVE + ); + + public static final Pattern CONSTANT_NAME = Pattern.compile( + "_(?:MI|AM|MD|CO|MB)_[A-Z0-9_]+" + ); + + private XoopsLanguageConstantParser() { + } + + /** + * @return occurrences of {@code define('NAME'} as (name, offset-of-name) + */ + public static @NotNull List parse(@NotNull String phpSource) { + List out = new ArrayList<>(); + // Comments masked, strings kept: a commented-out define() is not a definition, + // and the mask preserves offsets so Ctrl+B still lands on the real name. + // The fully masked copy tells whether the define keyword itself sits in code: + // inside a string literal it is blanked there, so the occurrence is skipped. + String code = PhpTextUtil.maskCommentsAndStrings(phpSource); + Matcher m = DEFINE.matcher(PhpTextUtil.maskCommentsOnly(phpSource)); + while (m.find()) { + if (code.charAt(m.start()) == ' ') { + continue; + } + out.add(new Occurrence(m.group(1), m.start(1))); + } + return out; + } + + /** + * Extract a XOOPS language-constant name from a PSI element's text + * (quoted string, bare identifier, {@code $smarty.const._MI_FOO}, or Smarty token). + * Recognition is case-insensitive; the returned spelling is the original. + */ + public static @Nullable String extractName(@NotNull String elementText) { + String trimmed = elementText.strip(); + if (trimmed.length() >= 2) { + char a = trimmed.charAt(0); + char b = trimmed.charAt(trimmed.length() - 1); + if ((a == '\'' && b == '\'') || (a == '"' && b == '"')) { + trimmed = trimmed.substring(1, trimmed.length() - 1); + } + } + if (trimmed.startsWith("<{") && trimmed.endsWith("}>") && trimmed.length() > 4) { + trimmed = trimmed.substring(2, trimmed.length() - 2).strip(); + } + String upper = trimmed.toUpperCase(Locale.ROOT); + if (upper.startsWith("$SMARTY.CONST.")) { + trimmed = trimmed.substring("$SMARTY.CONST.".length()); + upper = trimmed.toUpperCase(Locale.ROOT); + } + return CONSTANT_NAME.matcher(upper).matches() ? trimmed : null; + } + + public static boolean isLanguagePath(@Nullable String path) { + if (path == null) { + return false; + } + String p = path.replace('\\', '/').toLowerCase(Locale.ROOT); + return (p.contains("/language/") || p.endsWith("/language")) + && (p.endsWith(".php") || p.endsWith("/language") || !p.contains(".")); + } + + public record Occurrence(@NotNull String name, int offset) { + } +} diff --git a/src/main/java/org/xoops/support/completion/XoopsLanguageConstantReference.java b/src/main/java/org/xoops/support/completion/XoopsLanguageConstantReference.java new file mode 100644 index 0000000..eba6abb --- /dev/null +++ b/src/main/java/org/xoops/support/completion/XoopsLanguageConstantReference.java @@ -0,0 +1,43 @@ +package org.xoops.support.completion; + +import com.intellij.openapi.util.TextRange; +import com.intellij.psi.PsiElement; +import com.intellij.psi.PsiFile; +import com.intellij.psi.PsiReferenceBase; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +/** + * Resolves a {@code _MI_}/{@code _AM_}/{@code _MD_}/{@code _CO_}/{@code _MB_} name + * to its {@code define()} in a language PHP file. + */ +public final class XoopsLanguageConstantReference extends PsiReferenceBase { + + private final String name; + + public XoopsLanguageConstantReference(@NotNull PsiElement element, @NotNull String name) { + super(element, TextRange.from(0, element.getTextLength()), true); + this.name = name; + } + + public XoopsLanguageConstantReference( + @NotNull PsiElement element, + @NotNull TextRange rangeInElement, + @NotNull String name + ) { + super(element, rangeInElement, true); + this.name = name; + } + + @Override + public @Nullable PsiElement resolve() { + PsiFile file = getElement().getContainingFile(); + return XoopsLanguageConstantsCache.getInstance(getElement().getProject()) + .resolve(name, file == null ? null : file.getVirtualFile()); + } + + @Override + public @NotNull String getCanonicalText() { + return name; + } +} diff --git a/src/main/java/org/xoops/support/completion/XoopsLanguageConstantReferenceContributor.java b/src/main/java/org/xoops/support/completion/XoopsLanguageConstantReferenceContributor.java new file mode 100644 index 0000000..51eccd8 --- /dev/null +++ b/src/main/java/org/xoops/support/completion/XoopsLanguageConstantReferenceContributor.java @@ -0,0 +1,65 @@ +package org.xoops.support.completion; + +import com.intellij.lang.Language; +import com.intellij.patterns.PlatformPatterns; +import com.intellij.psi.PsiElement; +import com.intellij.psi.PsiReference; +import com.intellij.psi.PsiReferenceContributor; +import com.intellij.psi.PsiReferenceProvider; +import com.intellij.psi.PsiReferenceRegistrar; +import com.intellij.util.ProcessingContext; +import com.jetbrains.php.lang.psi.elements.ConstantReference; +import com.jetbrains.php.lang.psi.elements.StringLiteralExpression; +import org.jetbrains.annotations.NotNull; +import org.xoops.support.XoopsSupportPlugin; + +/** + * Ctrl+B / Find Usages for XOOPS language constants ({@code _MD_FOO}, {@code '_MI_BAR'}). + */ +public final class XoopsLanguageConstantReferenceContributor extends PsiReferenceContributor { + + @Override + public void registerReferenceProviders(@NotNull PsiReferenceRegistrar registrar) { + PsiReferenceProvider provider = new PsiReferenceProvider() { + @Override + public PsiReference @NotNull [] getReferencesByElement( + @NotNull PsiElement element, + @NotNull ProcessingContext context + ) { + if (!XoopsSupportPlugin.isEnabled(element.getProject())) { + return PsiReference.EMPTY_ARRAY; + } + String name = nameOf(element); + if (name == null) { + return PsiReference.EMPTY_ARRAY; + } + return new PsiReference[]{new XoopsLanguageConstantReference(element, name)}; + } + }; + registrar.registerReferenceProvider(PlatformPatterns.psiElement(StringLiteralExpression.class), provider); + registrar.registerReferenceProvider(PlatformPatterns.psiElement(ConstantReference.class), provider); + Language smarty = Language.findLanguageByID("Smarty"); + if (smarty != null) { + registrar.registerReferenceProvider( + PlatformPatterns.psiElement().withLanguage(smarty), + provider + ); + } + } + + private static String nameOf(@NotNull PsiElement element) { + if (element instanceof StringLiteralExpression stringLiteral) { + return XoopsLanguageConstantParser.extractName(stringLiteral.getContents()); + } + if (element instanceof ConstantReference constantReference) { + String n = constantReference.getName(); + return n == null ? null : XoopsLanguageConstantParser.extractName(n); + } + // Smarty (and any other host): only leaves. A parent whose text is + // <{$smarty.const._MI_FOO}> would otherwise double-count with the leaf. + if (element.getChildren().length == 0) { + return XoopsLanguageConstantParser.extractName(element.getText()); + } + return null; + } +} diff --git a/src/main/java/org/xoops/support/completion/XoopsLanguageConstantsCache.java b/src/main/java/org/xoops/support/completion/XoopsLanguageConstantsCache.java index f3e2d0a..23d4e2a 100644 --- a/src/main/java/org/xoops/support/completion/XoopsLanguageConstantsCache.java +++ b/src/main/java/org/xoops/support/completion/XoopsLanguageConstantsCache.java @@ -6,10 +6,14 @@ import com.intellij.openapi.project.IndexNotReadyException; import com.intellij.openapi.project.Project; import com.intellij.openapi.util.Key; +import com.intellij.openapi.util.SimpleModificationTracker; import com.intellij.openapi.vfs.AsyncFileListener; import com.intellij.openapi.vfs.VirtualFile; import com.intellij.openapi.vfs.VirtualFileManager; import com.intellij.openapi.vfs.newvfs.events.VFileEvent; +import com.intellij.openapi.vfs.newvfs.events.VFileMoveEvent; +import com.intellij.openapi.vfs.newvfs.events.VFilePropertyChangeEvent; +import com.intellij.psi.PsiElement; import com.intellij.psi.PsiFile; import com.intellij.psi.PsiManager; import com.intellij.psi.search.FilenameIndex; @@ -17,49 +21,39 @@ import com.intellij.psi.util.CachedValue; import com.intellij.psi.util.CachedValueProvider; import com.intellij.psi.util.CachedValuesManager; -import com.intellij.psi.util.PsiModificationTracker; import com.intellij.util.concurrency.annotations.RequiresReadLock; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.LinkedHashMap; import java.util.LinkedHashSet; +import java.util.List; import java.util.Locale; +import java.util.Map; import java.util.Set; -import java.util.regex.Matcher; -import java.util.regex.Pattern; /** - * Project-level cache of XOOPS language constants from language/*.php define() lines. - * Tied to {@link PsiModificationTracker#MODIFICATION_COUNT} so unsaved PSI edits invalidate - * the cache; VFS listener also clears the soft project key on disk changes. - * - *

Implements {@link Disposable}: the VFS listener parent must be the service - * (not the project). Parenting to the project keeps the listener after plugin unload and - * pins the plugin classloader — "didn't unload fully". + * Project-level cache of XOOPS language constants from every PHP file under + * {@code language/}. Depends on language PSI files and relevant VFS changes; + * VFS listener parent is this service (not the project) so plugin unload disposes it. */ @Service(Service.Level.PROJECT) public final class XoopsLanguageConstantsCache implements Disposable { - private static final Key>> CACHE_KEY = + private static final Key> CACHE_KEY = Key.create("xoops.support.languageConstants"); - private static final Pattern DEFINE = Pattern.compile( - "define\\s*\\(\\s*['\"](_(?:MI|AM|MD|CO|MB)_[A-Z0-9_]+)['\"]", - Pattern.CASE_INSENSITIVE - ); - - private static final String[] LANGUAGE_FILES = { - "modinfo.php", "main.php", "admin.php", "blocks.php", "common.php" - }; + private static final int MAX_CONSTANTS = 5000; private final Project project; + private final SimpleModificationTracker languageChanges = new SimpleModificationTracker(); private volatile boolean disposed; public XoopsLanguageConstantsCache(@NotNull Project project) { this.project = project; - // Parent Disposable = this service (disposed on plugin unload), not the Project. VirtualFileManager.getInstance().addAsyncFileListener( events -> { if (disposed || project.isDisposed()) { @@ -68,12 +62,25 @@ public XoopsLanguageConstantsCache(@NotNull Project project) { boolean hit = false; for (VFileEvent event : events) { VirtualFile file = event.getFile(); - if (file != null && isLanguagePath(file.getPath())) { + // ponytail: directory events conservatively invalidate; subtree tracking only if needed. + if (file != null && (file.isDirectory() + || XoopsLanguageConstantParser.isLanguagePath(file.getPath()))) { + hit = true; + break; + } + if (event instanceof VFileMoveEvent move + && XoopsLanguageConstantParser.isLanguagePath( + move.getNewParent().getPath() + "/" + move.getFile().getName())) { + hit = true; + break; + } + if (event instanceof VFilePropertyChangeEvent rename + && XoopsLanguageConstantParser.isLanguagePath(rename.getNewPath())) { hit = true; break; } String path = event.getPath(); - if (path != null && isLanguagePath(path)) { + if (path != null && XoopsLanguageConstantParser.isLanguagePath(path)) { hit = true; break; } @@ -100,81 +107,144 @@ public void afterVfsChange() { public void invalidate() { if (!project.isDisposed()) { - project.putUserData(CACHE_KEY, null); + languageChanges.incModificationCount(); } } public @NotNull Set getConstants() { + Index index = getIndex(); + return index == null ? Collections.emptySet() : index.names; + } + + /** + * PSI element of the {@code define('_FOO_'} name in a language file (prefers {@code /english/}). + * Exact spelling only: {@code _MI_FOO} does not resolve to {@code define('_mi_foo')}. + */ + public @Nullable PsiElement resolve(@NotNull String name) { + return resolve(name, null); + } + + /** + * Same as {@link #resolve(String)}, preferring a definition in the same module as + * {@code from} (the {@code /modules//} segment) when several modules define the name. + */ + public @Nullable PsiElement resolve(@NotNull String name, @Nullable VirtualFile from) { + Index index = getIndex(); + if (index == null) { + return null; + } + List defs = index.defs.get(name); + if (defs == null || defs.isEmpty()) { + return null; + } + Def chosen = pickDefinition(defs, from == null ? null : moduleSegment(from.getPath())); + if (project.isDisposed()) { + return null; + } + PsiFile psi = PsiManager.getInstance(project).findFile(chosen.file); + if (psi == null) { + return null; + } + return psi.findElementAt(chosen.offset); + } + + @Override + public void dispose() { + disposed = true; + project.putUserData(CACHE_KEY, null); + } + + private @Nullable Index getIndex() { if (disposed || project.isDisposed()) { - return Collections.emptySet(); + return null; } try { return ReadAction.compute(() -> { if (disposed || project.isDisposed()) { - return Collections.emptySet(); + return null; } - CachedValue> cached = project.getUserData(CACHE_KEY); + CachedValue cached = project.getUserData(CACHE_KEY); if (cached == null) { cached = CachedValuesManager.getManager(project).createCachedValue( - () -> { - Set value = collectUnderReadLock(); - return CachedValueProvider.Result.create( - value, - PsiModificationTracker.MODIFICATION_COUNT - ); - }, + this::collectUnderReadLock, false ); project.putUserData(CACHE_KEY, cached); } - Set result = cached.getValue(); - return result != null ? result : Collections.emptySet(); + return cached.getValue(); }); } catch (IndexNotReadyException e) { - return Collections.emptySet(); + return null; } } - @Override - public void dispose() { - disposed = true; - invalidate(); - } - @RequiresReadLock - private @NotNull Set collectUnderReadLock() { + private @NotNull CachedValueProvider.Result collectUnderReadLock() { + List dependencies = new ArrayList<>(); + dependencies.add(languageChanges); Set names = new LinkedHashSet<>(); + Map> defs = new LinkedHashMap<>(); PsiManager psiManager = PsiManager.getInstance(project); GlobalSearchScope scope = GlobalSearchScope.projectScope(project); - for (String fileName : LANGUAGE_FILES) { - Collection files = FilenameIndex.getVirtualFilesByName(fileName, scope); - for (VirtualFile vf : files) { - String path = vf.getPath().replace('\\', '/').toLowerCase(Locale.ROOT); - if (!path.contains("/language/") || path.contains("/vendor/")) { - continue; - } - PsiFile psi = psiManager.findFile(vf); - if (psi == null) { - continue; - } - Matcher m = DEFINE.matcher(psi.getText()); - while (m.find()) { - names.add(m.group(1)); - if (names.size() > 5000) { - return Collections.unmodifiableSet(names); - } + Collection files = FilenameIndex.getAllFilesByExt(project, "php", scope); + for (VirtualFile vf : files) { + if (!XoopsLanguageConstantParser.isLanguagePath(vf.getPath()) + || vf.getPath().replace('\\', '/').toLowerCase(Locale.ROOT).contains("/vendor/")) { + continue; + } + PsiFile psi = psiManager.findFile(vf); + if (psi == null) { + continue; + } + // PSI dependencies include committed editor changes, even before saving to disk. + dependencies.add(psi); + for (XoopsLanguageConstantParser.Occurrence occ : XoopsLanguageConstantParser.parse(psi.getText())) { + if (!names.contains(occ.name()) && names.size() >= MAX_CONSTANTS) { + continue; // cap new names; definitions of known names are still recorded } + names.add(occ.name()); + defs.computeIfAbsent(occ.name(), k -> new ArrayList<>()).add(new Def(vf, occ.offset())); } } - return Collections.unmodifiableSet(names); + return CachedValueProvider.Result.create( + new Index(Collections.unmodifiableSet(names), Map.copyOf(defs)), dependencies.toArray()); } - private static boolean isLanguagePath(@Nullable String path) { - if (path == null) { - return false; + /** Same module and /english/ first, then same module, then any /english/, then the first. */ + static @NotNull Def pickDefinition(@NotNull List defs, @Nullable String moduleSegment) { + Def sameModule = null; + Def english = null; + for (Def d : defs) { + String path = d.file.getPath().replace('\\', '/').toLowerCase(Locale.ROOT); + boolean inModule = moduleSegment != null && moduleSegment.equals(moduleSegment(path)); + boolean isEnglish = path.contains("/english/"); + if (inModule && isEnglish) { + return d; + } + if (inModule && sameModule == null) { + sameModule = d; + } + if (isEnglish && english == null) { + english = d; + } } + return sameModule != null ? sameModule : (english != null ? english : defs.get(0)); + } + + /** {@code /modules//} of a path, lower-case, or null when not under modules/. */ + static @Nullable String moduleSegment(@NotNull String path) { String p = path.replace('\\', '/').toLowerCase(Locale.ROOT); - return p.contains("/language/") - && (p.endsWith(".php") || p.endsWith("/language") || !p.contains(".")); + int i = p.lastIndexOf("/modules/"); + if (i < 0) { + return null; + } + int end = p.indexOf('/', i + "/modules/".length()); + return end < 0 ? null : p.substring(i, end + 1); + } + + private record Def(@NotNull VirtualFile file, int offset) { + } + + private record Index(@NotNull Set names, @NotNull Map> defs) { } } diff --git a/src/main/java/org/xoops/support/inspections/CreateMissingTemplateQuickFix.java b/src/main/java/org/xoops/support/inspections/CreateMissingTemplateQuickFix.java index 4a96454..7405471 100644 --- a/src/main/java/org/xoops/support/inspections/CreateMissingTemplateQuickFix.java +++ b/src/main/java/org/xoops/support/inspections/CreateMissingTemplateQuickFix.java @@ -12,14 +12,18 @@ import java.nio.charset.StandardCharsets; /** - * Creates templates/{name} under the module that owns xoops_version.php. + * Creates the on-disk template for a missing {@code xoops_version.php} registration. + * Block registrations land under {@code templates/blocks/} unless the name already + * includes a {@code templates/} or {@code blocks/} prefix. */ public final class CreateMissingTemplateQuickFix implements LocalQuickFix { private final String templateName; + private final boolean block; - public CreateMissingTemplateQuickFix(@NotNull String templateName) { + public CreateMissingTemplateQuickFix(@NotNull String templateName, boolean block) { this.templateName = templateName.replace('\\', '/'); + this.block = block; } @Override @@ -34,22 +38,24 @@ public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descri return; } VirtualFile moduleRoot = file.getVirtualFile().getParent(); + String relative = XoopsManifestTemplates.diskPath(templateName, block); try { WriteAction.runAndWait(() -> { - VirtualFile templates = moduleRoot.findChild("templates"); - if (templates == null) { - templates = moduleRoot.createChildDirectory(this, "templates"); - } - // Support nested names like admin/list.tpl - String[] parts = templateName.split("/"); - VirtualFile dir = templates; + String[] parts = relative.split("/"); + VirtualFile dir = moduleRoot; for (int i = 0; i < parts.length - 1; i++) { + if (dir == null) { + return; + } VirtualFile next = dir.findChild(parts[i]); if (next == null) { next = dir.createChildDirectory(this, parts[i]); } dir = next; } + if (dir == null) { + return; + } String leaf = parts[parts.length - 1]; if (dir.findChild(leaf) != null) { return; diff --git a/src/main/java/org/xoops/support/inspections/InsertBeforeStatementQuickFix.java b/src/main/java/org/xoops/support/inspections/InsertBeforeStatementQuickFix.java new file mode 100644 index 0000000..68e6de6 --- /dev/null +++ b/src/main/java/org/xoops/support/inspections/InsertBeforeStatementQuickFix.java @@ -0,0 +1,105 @@ +package org.xoops.support.inspections; + +import com.intellij.codeInspection.LocalQuickFix; +import com.intellij.codeInspection.ProblemDescriptor; +import com.intellij.openapi.editor.Document; +import com.intellij.openapi.project.Project; +import com.intellij.psi.PsiDocumentManager; +import com.intellij.psi.PsiElement; +import com.intellij.psi.util.PsiTreeUtil; +import com.jetbrains.php.lang.psi.elements.GroupStatement; +import com.jetbrains.php.lang.psi.elements.Statement; + +import java.util.regex.Pattern; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +/** + * Inserts an isResultSet throw-guard immediately before the enclosing statement of + * the problem element. Offset and indent are computed at apply-time so a batch + * Inspect Code run still works after an earlier fix in the same file shifted lines. + */ +public final class InsertBeforeStatementQuickFix implements LocalQuickFix { + + private static final String FAIL_ACTION = "throw new \\RuntimeException('Database query failed');"; + + private final String dbExpr; + private final String resultVar; + + public InsertBeforeStatementQuickFix(@NotNull String dbExpr, @NotNull String resultVar) { + this.dbExpr = dbExpr; + this.resultVar = resultVar; + } + + @Override + public @NotNull String getFamilyName() { + return "Insert isResultSet guard before fetch"; + } + + @Override + public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) { + PsiElement leaf = descriptor.getPsiElement(); + if (leaf == null) { + return; + } + Statement stmt = insertionStatement(leaf); + if (stmt == null) { + return; + } + Document document = DocumentEditHelper.documentOf(project, leaf); + if (document == null) { + return; + } + int insertAt = stmt.getTextRange().getStartOffset(); + if (insertAt < 0 || insertAt > document.getTextLength()) { + return; + } + String text = document.getText(); + int fetchOffset = leaf.getTextRange().getStartOffset(); + if (XoopsResultSetGuardInspection.isFetchGuardedAt(text, fetchOffset, resultVar)) { + return; + } + if (assignsResultBeforeFetch(text, insertAt, Math.max(insertAt, fetchOffset), resultVar)) { + // e.g. while (($result = $db->query($sql)) && $db->fetchRow($result)): + // a guard above the statement would test a stale value. Leave it to the user. + return; + } + String indent = guessIndent(text, insertAt); + String block = indent + "if (!" + dbExpr + "->isResultSet(" + resultVar + + ") || !" + resultVar + " instanceof \\mysqli_result) {\n" + + indent + " " + FAIL_ACTION + "\n" + + indent + "}\n"; + document.insertString(insertAt, block); + PsiDocumentManager.getInstance(project).commitDocument(document); + } + + /** Only offer insertion where the fetch statement is already in a statement list. */ + static @Nullable Statement insertionStatement(@NotNull PsiElement leaf) { + Statement stmt = PsiTreeUtil.getParentOfType(leaf, Statement.class, false); + if (stmt == null) { + return null; + } + Statement up = PsiTreeUtil.getParentOfType(stmt, Statement.class, true); + GroupStatement block = PsiTreeUtil.getParentOfType(stmt, GroupStatement.class, true); + return up == null || up instanceof GroupStatement + || block != null && PsiTreeUtil.isAncestor(up, block, false) ? stmt : null; + } + + static boolean assignsResultBeforeFetch( + @NotNull String text, int from, int to, @NotNull String resultVar + ) { + String masked = PhpTextUtil.maskCommentsAndStrings(text).substring(from, to); + return Pattern.compile(Pattern.quote(resultVar) + "(?![\\w])\\s*=(?![=>])") + .matcher(masked).find(); + } + + private static String guessIndent(@NotNull String text, int offset) { + int start = text.lastIndexOf('\n', Math.max(0, offset - 1)); + start = start < 0 ? 0 : start + 1; + int i = start; + while (i < text.length() && (text.charAt(i) == ' ' || text.charAt(i) == '\t')) { + i++; + } + return text.substring(start, i); + } +} diff --git a/src/main/java/org/xoops/support/inspections/InsertRootPathGuardQuickFix.java b/src/main/java/org/xoops/support/inspections/InsertRootPathGuardQuickFix.java index 1fe4d2d..200c084 100644 --- a/src/main/java/org/xoops/support/inspections/InsertRootPathGuardQuickFix.java +++ b/src/main/java/org/xoops/support/inspections/InsertRootPathGuardQuickFix.java @@ -13,19 +13,13 @@ import java.util.regex.Pattern; /** - * Inserts a file-leading XOOPS_ROOT_PATH guard after a leading PHP open tag. - * Handles {@code = 0) { + insertGuardAt(document, project, policyOffset); return; } - // Leading short \n"); @@ -73,12 +58,10 @@ public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descri return; } - // No recognized leading open tag — only prepend if file has no PHP open at all if (!text.contains(" 0) { + inPhp = true; + i += tagLen; + continue; + } + if (chars[i] != '\n' && chars[i] != '\r') { + chars[i] = ' '; + } + i++; + continue; + } + if (i + 1 < n && chars[i] == '?' && chars[i + 1] == '>') { + inPhp = false; + i += 2; + continue; + } // Heredoc / nowdoc: <<')) { chars[i++] = ' '; } continue; } // # line comment if (chars[i] == '#') { - while (i < n && chars[i] != '\n') { + while (i < n && chars[i] != '\n' + && !(chars[i] == '?' && i + 1 < n && chars[i + 1] == '>')) { chars[i++] = ' '; } continue; @@ -166,6 +210,17 @@ static boolean looksLikeVendorOrCache(@NotNull PsiFile file) { continue; } if (!maskStrings) { + // Keep the string, but step over it so // and # inside it are not comments. + if (chars[i] == '\'' || chars[i] == '"') { + char quote = chars[i++]; + while (i < n && chars[i] != quote) { + i += (chars[i] == '\\' && i + 1 < n) ? 2 : 1; + } + if (i < n) { + i++; + } + continue; + } i++; continue; } @@ -208,6 +263,48 @@ static boolean looksLikeVendorOrCache(@NotNull PsiFile file) { return new String(chars); } + private static boolean containsPhpOpenTag(@NotNull String text) { + for (int i = 0; i < text.length(); i++) { + if (text.charAt(i) == '<' && phpOpenTagLength(text, i) > 0) { + return true; + } + } + return false; + } + + /** + * Length of a PHP open tag at {@code i}, or 0. Recognizes {@code = n || text.charAt(i) != '<' || text.charAt(i + 1) != '?') { + return 0; + } + if (i + 2 < n && text.charAt(i + 2) == '=') { + return 3; + } + if (startsIgnoreCase(text, i, " text.length()) { + return false; + } + return text.regionMatches(true, i, prefix, 0, n); + } + /** * First string-literal argument after {@code openParen} ('...' or "..."). * Returns content without quotes, or null if the first arg is not a string. diff --git a/src/main/java/org/xoops/support/inspections/RegisterTemplateQuickFix.java b/src/main/java/org/xoops/support/inspections/RegisterTemplateQuickFix.java new file mode 100644 index 0000000..f23741f --- /dev/null +++ b/src/main/java/org/xoops/support/inspections/RegisterTemplateQuickFix.java @@ -0,0 +1,86 @@ +package org.xoops.support.inspections; + +import com.intellij.codeInspection.LocalQuickFix; +import com.intellij.codeInspection.ProblemDescriptor; +import com.intellij.openapi.editor.Document; +import com.intellij.openapi.project.Project; +import com.intellij.openapi.vfs.VirtualFile; +import com.intellij.psi.PsiDocumentManager; +import com.intellij.psi.PsiElement; +import com.intellij.psi.PsiFile; +import com.intellij.psi.PsiManager; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.Locale; + +/** + * Appends a {@code $modversion['templates'][]} entry for the unregistered .tpl. + */ +public final class RegisterTemplateQuickFix implements LocalQuickFix { + + private final String templateName; + + public RegisterTemplateQuickFix(@NotNull String templateName) { + this.templateName = XoopsManifestTemplates.registrationName(templateName); + } + + @Override + public @NotNull String getFamilyName() { + return "Register template in xoops_version.php"; + } + + @Override + public @Nullable PsiElement getElementToMakeWritable(@NotNull PsiFile currentFile) { + PsiFile manifest = findManifestPsi(currentFile.getProject(), currentFile); + return manifest != null ? manifest : currentFile; + } + + @Override + public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) { + PsiFile tplFile = descriptor.getPsiElement() == null + ? null + : descriptor.getPsiElement().getContainingFile(); + PsiFile manifestPsi = findManifestPsi(project, tplFile); + if (manifestPsi == null) { + return; + } + Document document = PsiDocumentManager.getInstance(project).getDocument(manifestPsi); + if (document == null) { + return; + } + String text = document.getText(); + String lowerName = XoopsManifestTemplates.diskPath(templateName, false).toLowerCase(Locale.ROOT); + // Same parser as the inspection: a commented-out entry is not a registration. + if (XoopsUnregisteredTemplateInspection.registeredTemplates(text).contains(lowerName)) { + return; + } + String entry = "\n$modversion['templates'][] = [\n" + + " 'file' => '" + templateName.replace("'", "\\'") + "',\n" + + " 'description' => '',\n" + + "];\n"; + int insertAt = text.length(); + String code = PhpTextUtil.maskCommentsAndStrings(text); + int closePhp = code.lastIndexOf("?>"); + if (closePhp >= 0 && code.lastIndexOf(" document.getTextLength()) { + return; + } + DocumentEditHelper.replace( + project, + document, + range.getStartOffset(), + range.getEndOffset(), + replacement, + expected + ); + } +} diff --git a/src/main/java/org/xoops/support/inspections/XoopsDeprecatedDbApiInspection.java b/src/main/java/org/xoops/support/inspections/XoopsDeprecatedDbApiInspection.java index 16e6173..22e1dae 100644 --- a/src/main/java/org/xoops/support/inspections/XoopsDeprecatedDbApiInspection.java +++ b/src/main/java/org/xoops/support/inspections/XoopsDeprecatedDbApiInspection.java @@ -29,7 +29,7 @@ public final class XoopsDeprecatedDbApiInspection extends LocalInspectionTool { return new PsiElementVisitor() { @Override public void visitFile(@NotNull PsiFile file) { - if (!XoopsSupportPlugin.isEnabled(file)) { + if (!XoopsSupportPlugin.isEnabled(file) || !PhpTextUtil.isPrimaryPsiFile(file)) { return; } if (!PhpTextUtil.isPhpFile(file) || PhpTextUtil.looksLikeVendorOrCache(file)) { diff --git a/src/main/java/org/xoops/support/inspections/XoopsDeprecatedUnicodeDtypeInspection.java b/src/main/java/org/xoops/support/inspections/XoopsDeprecatedUnicodeDtypeInspection.java new file mode 100644 index 0000000..116a2c1 --- /dev/null +++ b/src/main/java/org/xoops/support/inspections/XoopsDeprecatedUnicodeDtypeInspection.java @@ -0,0 +1,68 @@ +package org.xoops.support.inspections; + +import com.intellij.codeInspection.LocalInspectionTool; +import com.intellij.codeInspection.ProblemsHolder; +import com.intellij.psi.PsiElement; +import com.intellij.psi.PsiElementVisitor; +import com.intellij.psi.PsiFile; +import org.jetbrains.annotations.NotNull; +import org.xoops.support.XoopsSupportPlugin; +import org.xoops.support.settings.XoopsSettingsState; + +import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * {@code XOBJ_DTYPE_UNICODE_*} was deprecated in XOOPS 2.7.3 (#164) with removal + * scheduled for 4.0. Rename-only quick-fix; value migration stays a core concern. + * Silent when the project Core Version is explicitly 2.5. + */ +public final class XoopsDeprecatedUnicodeDtypeInspection extends LocalInspectionTool { + + static final Map SUCCESSOR = Map.of( + "XOBJ_DTYPE_UNICODE_TXTBOX", "XOBJ_DTYPE_TXTBOX", + "XOBJ_DTYPE_UNICODE_TXTAREA", "XOBJ_DTYPE_TXTAREA", + "XOBJ_DTYPE_UNICODE_URL", "XOBJ_DTYPE_URL", + "XOBJ_DTYPE_UNICODE_EMAIL", "XOBJ_DTYPE_EMAIL", + "XOBJ_DTYPE_UNICODE_ARRAY", "XOBJ_DTYPE_ARRAY", + "XOBJ_DTYPE_UNICODE_OTHER", "XOBJ_DTYPE_OTHER" + ); + + private static final Pattern UNICODE_DTYPE = Pattern.compile( + "\\b(XOBJ_DTYPE_UNICODE_(?:TXTBOX|TXTAREA|URL|EMAIL|ARRAY|OTHER))\\b" + ); + + @Override + public @NotNull PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly) { + return new PsiElementVisitor() { + @Override + public void visitFile(@NotNull PsiFile file) { + if (!XoopsSupportPlugin.isEnabled(file) || !PhpTextUtil.isPrimaryPsiFile(file)) { + return; + } + if (!PhpTextUtil.isPhpFile(file) || PhpTextUtil.looksLikeVendorOrCache(file)) { + return; + } + if ("2.5".equals(XoopsSettingsState.getInstance(file.getProject()).resolvedCoreVersion())) { + return; + } + String text = file.getText(); + String code = PhpTextUtil.maskCommentsAndStrings(text); + Matcher m = UNICODE_DTYPE.matcher(code); + while (m.find()) { + String old = m.group(1); + String next = SUCCESSOR.get(old); + PsiElement leaf = next == null ? null : PhpTextUtil.leafAt(file, m.start(1)); + if (next != null && leaf != null) { + holder.registerProblem( + leaf, + "XOOPS: " + old + " is deprecated since 2.7.3; use " + next, + new ReplacePsiTextQuickFix("Replace with " + next, next, old) + ); + } + } + } + }; + } +} diff --git a/src/main/java/org/xoops/support/inspections/XoopsIncludeOnceHeaderInspection.java b/src/main/java/org/xoops/support/inspections/XoopsIncludeOnceHeaderInspection.java index 5dadffe..8ab2b09 100644 --- a/src/main/java/org/xoops/support/inspections/XoopsIncludeOnceHeaderInspection.java +++ b/src/main/java/org/xoops/support/inspections/XoopsIncludeOnceHeaderInspection.java @@ -27,7 +27,7 @@ public final class XoopsIncludeOnceHeaderInspection extends LocalInspectionTool return new PsiElementVisitor() { @Override public void visitFile(@NotNull PsiFile file) { - if (!XoopsSupportPlugin.isEnabled(file)) { + if (!XoopsSupportPlugin.isEnabled(file) || !PhpTextUtil.isPrimaryPsiFile(file)) { return; } if (!PhpTextUtil.isPhpFile(file) || PhpTextUtil.looksLikeVendorOrCache(file)) { diff --git a/src/main/java/org/xoops/support/inspections/XoopsManifestTemplates.java b/src/main/java/org/xoops/support/inspections/XoopsManifestTemplates.java new file mode 100644 index 0000000..2142d74 --- /dev/null +++ b/src/main/java/org/xoops/support/inspections/XoopsManifestTemplates.java @@ -0,0 +1,114 @@ +package org.xoops.support.inspections; + +import org.jetbrains.annotations.NotNull; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Locale; +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * Template registrations in {@code xoops_version.php}, shared by the two template + * inspections and the project scanner so they agree on what counts as registered. + * + *

A registration is a {@code 'file' => 'x.tpl'} / {@code 'template' => 'x.tpl'} pair, or the + * {@code ['file'] = 'x.tpl'} / {@code ['template'] = 'x.tpl'} assignment form, that sits inside a + * statement starting with {@code $modversion['templates']} or {@code $modversion['blocks']}. + * Registration-like text in an unrelated string ({@code $help = "'file' => 'x.tpl'";}) is ignored. + */ +public final class XoopsManifestTemplates { + + private static final String TEMPLATES_PREFIX = "templates/"; + + public enum Section { + TEMPLATES, + BLOCKS + } + + /** A registered template name and the offset of that name in the manifest text. */ + public record Registration(@NotNull String name, int nameOffset, @NotNull Section section) { + public boolean block() { + return section == Section.BLOCKS; + } + } + + private static final Pattern MODVERSION_TEMPLATES = Pattern.compile( + "(?i)\\$modversion\\s*\\[\\s*['\"](templates|blocks)['\"]\\s*\\]" + ); + private static final Pattern FILE_OR_TEMPLATE = Pattern.compile( + "(?is)['\"](?:file|template)['\"]\\s*\\]?\\s*=>?\\s*['\"]([^'\"]+\\.tpl)['\"]" + ); + + private XoopsManifestTemplates() { + } + + /** + * @param commentMasked manifest source with comments masked and strings kept + * ({@link PhpTextUtil#maskCommentsOnly(String)}); offsets are preserved + */ + public static @NotNull List find(@NotNull String commentMasked) { + String code = PhpTextUtil.maskCommentsAndStrings(commentMasked); + List out = new ArrayList<>(); + Set seen = new LinkedHashSet<>(); + Matcher stmt = MODVERSION_TEMPLATES.matcher(commentMasked); + int searchFrom = 0; + while (stmt.find(searchFrom)) { + if (code.charAt(stmt.start()) != '$') { + searchFrom = stmt.end(); + continue; + } + int end = code.indexOf(';', stmt.end()); + end = end < 0 ? code.length() : end + 1; + Section section = "blocks".equalsIgnoreCase(stmt.group(1)) ? Section.BLOCKS : Section.TEMPLATES; + Matcher m = FILE_OR_TEMPLATE.matcher(commentMasked); + m.region(stmt.end(), end); + while (m.find()) { + int assignment = commentMasked.indexOf('=', m.start()); + if (code.charAt(assignment) == '=' && seen.add(m.start(1))) { + out.add(new Registration(m.group(1).replace('\\', '/'), m.start(1), section)); + } + } + searchFrom = Math.max(end, stmt.end()); + if (searchFrom >= commentMasked.length()) { + break; + } + } + return out; + } + + /** Module-root-relative, lower-case disk paths, preserving block/page identity. */ + public static @NotNull Set keys(@NotNull String manifestText) { + Set out = new LinkedHashSet<>(); + for (Registration r : find(PhpTextUtil.maskCommentsOnly(manifestText))) { + out.add(diskPath(r.name(), r.block()).toLowerCase(Locale.ROOT)); + } + return out; + } + + /** + * Module-root-relative path where a missing registered file should be created. + * Bare block names go under {@code templates/blocks/}; already-prefixed names stay as spelled. + */ + public static @NotNull String diskPath(@NotNull String name, boolean block) { + String n = String.join("/", Arrays.stream(name.replace('\\', '/').split("/")) + .filter(part -> !part.isEmpty() && !part.equals(".")).toList()); + String lower = n.toLowerCase(Locale.ROOT); + if (lower.startsWith(TEMPLATES_PREFIX) || lower.startsWith("blocks/")) { + return n; + } + return block ? TEMPLATES_PREFIX + "blocks/" + n : TEMPLATES_PREFIX + n; + } + + /** Shortest registration spelling that still resolves to the same module-relative path. */ + public static @NotNull String registrationName(@NotNull String relativePath) { + String normalized = relativePath.replace('\\', '/'); + String candidate = normalized.startsWith(TEMPLATES_PREFIX) + ? normalized.substring(TEMPLATES_PREFIX.length()) : normalized; + return diskPath(candidate, false).equals(normalized) ? candidate : normalized; + } + +} diff --git a/src/main/java/org/xoops/support/inspections/XoopsMissingRegisteredTemplateInspection.java b/src/main/java/org/xoops/support/inspections/XoopsMissingRegisteredTemplateInspection.java index b7042e9..c6aa9ce 100644 --- a/src/main/java/org/xoops/support/inspections/XoopsMissingRegisteredTemplateInspection.java +++ b/src/main/java/org/xoops/support/inspections/XoopsMissingRegisteredTemplateInspection.java @@ -9,27 +9,22 @@ import org.jetbrains.annotations.NotNull; import org.xoops.support.XoopsSupportPlugin; -import java.util.regex.Matcher; -import java.util.regex.Pattern; /** * Flags templates listed in xoops_version.php that are missing on disk. */ public final class XoopsMissingRegisteredTemplateInspection extends LocalInspectionTool { - private static final Pattern REGISTERED_TEMPLATE = Pattern.compile( - "(?is)['\"](?:file|template)['\"]\\s*=>\\s*['\"]([^'\"]+\\.tpl)['\"]" - ); @Override public @NotNull PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly) { return new PsiElementVisitor() { @Override public void visitFile(@NotNull PsiFile file) { - if (!XoopsSupportPlugin.isEnabled(file)) { + if (!XoopsSupportPlugin.isEnabled(file) || !PhpTextUtil.isPrimaryPsiFile(file)) { return; } - if (!"xoops_version.php".equalsIgnoreCase(file.getName())) { + if (!"xoops_version.php".equalsIgnoreCase(file.getName()) || PhpTextUtil.looksLikeVendorOrCache(file)) { return; } VirtualFile vf = file.getVirtualFile(); @@ -38,20 +33,25 @@ public void visitFile(@NotNull PsiFile file) { } VirtualFile moduleRoot = vf.getParent(); String text = file.getText(); - String code = PhpTextUtil.maskCommentsAndStrings(text); - Matcher m = REGISTERED_TEMPLATE.matcher(code); - while (m.find()) { - String template = m.group(1).replace('\\', '/'); - boolean exists = childExists(moduleRoot, "templates/" + template) - || childExists(moduleRoot, "blocks/" + template) - || childExists(moduleRoot, template); - if (!exists) { - PsiElement leaf = PhpTextUtil.leafAt(file, m.start(1)); + // Comments only: the registration key and file name are string literals. + String code = PhpTextUtil.maskCommentsOnly(text); + for (XoopsManifestTemplates.Registration reg : XoopsManifestTemplates.find(code)) { + String template = reg.name(); + String expected = XoopsManifestTemplates.diskPath(template, reg.block()); + String actual = XoopsTemplatePaths.existingPath(moduleRoot, expected); + if (actual != null && !expected.equals(actual)) { + PsiElement leaf = PhpTextUtil.leafAt(file, reg.nameOffset()); + if (leaf != null) { + holder.registerProblem(leaf, + "XOOPS: template filename case mismatch: " + expected + " (on disk: " + actual + ")"); + } + } else if (actual == null) { + PsiElement leaf = PhpTextUtil.leafAt(file, reg.nameOffset()); if (leaf != null) { holder.registerProblem( leaf, "XOOPS: registered template missing on disk: " + template, - new CreateMissingTemplateQuickFix(template) + new CreateMissingTemplateQuickFix(template, reg.block()) ); } } @@ -60,15 +60,4 @@ public void visitFile(@NotNull PsiFile file) { }; } - private static boolean childExists(VirtualFile root, String relative) { - String[] parts = relative.split("/"); - VirtualFile cur = root; - for (String part : parts) { - if (cur == null) { - return false; - } - cur = cur.findChild(part); - } - return cur != null && !cur.isDirectory(); - } } diff --git a/src/main/java/org/xoops/support/inspections/XoopsQueryExecInspection.java b/src/main/java/org/xoops/support/inspections/XoopsQueryExecInspection.java index e400898..2861c27 100644 --- a/src/main/java/org/xoops/support/inspections/XoopsQueryExecInspection.java +++ b/src/main/java/org/xoops/support/inspections/XoopsQueryExecInspection.java @@ -29,7 +29,7 @@ public final class XoopsQueryExecInspection extends LocalInspectionTool { return new PsiElementVisitor() { @Override public void visitFile(@NotNull PsiFile file) { - if (!XoopsSupportPlugin.isEnabled(file)) { + if (!XoopsSupportPlugin.isEnabled(file) || !PhpTextUtil.isPrimaryPsiFile(file)) { return; } if (!PhpTextUtil.isPhpFile(file) || PhpTextUtil.looksLikeVendorOrCache(file)) { diff --git a/src/main/java/org/xoops/support/inspections/XoopsResultSetGuardInspection.java b/src/main/java/org/xoops/support/inspections/XoopsResultSetGuardInspection.java index 9e9764a..2fea082 100644 --- a/src/main/java/org/xoops/support/inspections/XoopsResultSetGuardInspection.java +++ b/src/main/java/org/xoops/support/inspections/XoopsResultSetGuardInspection.java @@ -5,7 +5,6 @@ import com.intellij.psi.PsiElement; import com.intellij.psi.PsiElementVisitor; import com.intellij.psi.PsiFile; -import com.intellij.psi.util.PsiTreeUtil; import com.jetbrains.php.lang.psi.elements.Statement; import org.jetbrains.annotations.NotNull; import org.xoops.support.XoopsSupportPlugin; @@ -60,8 +59,8 @@ public final class XoopsResultSetGuardInspection extends LocalInspectionTool { + "\\s*\\)?" ); - private static final Pattern NEG_INSTANCEOF = Pattern.compile( - "(?is)!\\s*\\(?\\s*(\\$[A-Za-z_][\\w]*)\\s*instanceof" + private static final Pattern INSTANCEOF_RESULT = Pattern.compile( + "(?is)\\$[A-Za-z_][\\w]*\\s+instanceof\\s+\\\\?mysqli_result" ); /** Single exit statement only (no nested control structure). */ @@ -69,14 +68,12 @@ public final class XoopsResultSetGuardInspection extends LocalInspectionTool { "(?is)^\\s*(return|throw|exit|die|break|continue)\\b[^;{]*;?\\s*$" ); - private static final String FAIL_ACTION = "throw new \\RuntimeException('Database query failed');"; - @Override public @NotNull PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly) { return new PsiElementVisitor() { @Override public void visitFile(@NotNull PsiFile file) { - if (!XoopsSupportPlugin.isEnabled(file)) { + if (!XoopsSupportPlugin.isEnabled(file) || !PhpTextUtil.isPrimaryPsiFile(file)) { return; } if (!PhpTextUtil.isPhpFile(file) || PhpTextUtil.looksLikeVendorOrCache(file)) { @@ -99,64 +96,206 @@ public void visitFile(@NotNull PsiFile file) { } String message = "XOOPS: call isResultSet($result) (and prefer mysqli_result check) before fetch*"; - Statement stmt = PsiTreeUtil.getParentOfType(leaf, Statement.class, false); + Statement stmt = InsertBeforeStatementQuickFix.insertionStatement(leaf); if (stmt == null) { - // No safe statement boundary — report without auto-fix. holder.registerProblem(leaf, message); continue; } - int insertAt = stmt.getTextRange().getStartOffset(); - String indentGuess = guessIndent(text, insertAt); - String block = indentGuess + "if (!" + dbExpr + "->isResultSet(" + resultVar - + ") || !" + resultVar + " instanceof \\mysqli_result) {\n" - + indentGuess + " " + FAIL_ACTION + "\n" - + indentGuess + "}\n"; - String expectedAt = text.substring( - insertAt, - Math.min(text.length(), insertAt + Math.min(32, stmt.getTextLength())) - ); holder.registerProblem( leaf, message, - new InsertBeforeOffsetQuickFix( - "Insert isResultSet guard before fetch", - insertAt, - block, - expectedAt - ) + new InsertBeforeStatementQuickFix(dbExpr, resultVar) ); } } }; } + /** + * True when {@code fetchOffset} in {@code text} is already proven-safe for {@code resultVar}. + * Used by the batch quick-fix on the current document (not a raw substring window). + */ + static boolean isFetchGuardedAt(@NotNull String text, int fetchOffset, @NotNull String resultVar) { + String code = PhpTextUtil.maskCommentsAndStrings(text); + if (fetchOffset < 0 || fetchOffset >= code.length()) { + return false; + } + return isFetchAlreadyGuarded(code, fetchOffset, resultVar, findIfConditions(code)); + } + private static boolean isFetchAlreadyGuarded( @NotNull String code, int fetchOffset, @NotNull String resultVar, @NotNull List allIfs ) { + List before = ifsBefore(allIfs, fetchOffset); + if (before.isEmpty()) { + return false; + } + if (isInsidePositiveIsResultSetGuard(code, fetchOffset, resultVar, before)) { + return true; + } + return hasDominatingEarlyExit(code, fetchOffset, resultVar, before); + } + + private static @NotNull List ifsBefore(@NotNull List allIfs, int fetchOffset) { List before = new ArrayList<>(); for (IfCond ic : allIfs) { if (ic.ifStart < fetchOffset) { before.add(ic); } } - if (before.isEmpty()) { + return before; + } + + /** + * Dominating early-exit: a proven {@code !isResultSet($var)} throw/return covers later + * fetch* of {@code $var} until reassignment, a nested function, or a closing {@code }}. + */ + private static boolean hasDominatingEarlyExit( + @NotNull String code, + int fetchOffset, + @NotNull String resultVar, + @NotNull List before + ) { + for (int i = before.size() - 1; i >= 0; i--) { + IfCond ic = before.get(i); + if (ic.ifEnd > fetchOffset || ic.chained || !startsAtStatementBoundary(code, ic.ifStart)) { + // An elseif / else-if branch is skipped whenever an earlier branch matched, + // so its early exit proves nothing about the fall-through path. + continue; + } + if (isSafeEarlyExitCondition(ic.condition, resultVar) + && bodyIsSimpleEarlyExit(code, ic) + && !assignsResultVar(code, ic.ifEnd, fetchOffset, resultVar) + && !closesOutOfScope(code, ic.ifEnd, fetchOffset) + && !hasFunctionKeyword(code, ic.ifEnd, fetchOffset)) { + return true; + } + } + return false; + } + + /** + * Visible for tests: unguarded {@code fetch*} start offsets in {@code text}. + */ + static @NotNull List unguardedFetchOffsets(@NotNull String text) { + String code = PhpTextUtil.maskCommentsAndStrings(text); + List allIfs = findIfConditions(code); + List out = new ArrayList<>(); + Matcher m = FETCH.matcher(code); + while (m.find()) { + String resultVar = m.group(3); + if (!isFetchAlreadyGuarded(code, m.start(), resultVar, allIfs)) { + out.add(m.start()); + } + } + return out; + } + + private static boolean startsAtStatementBoundary(@NotNull String code, int start) { + int previous = start - 1; + while (previous >= 0 && Character.isWhitespace(code.charAt(previous))) { + previous--; + } + // ponytail: only standalone statements prove dominance; labels/alternative syntax need PSI. + return previous < 0 || ";{}".indexOf(code.charAt(previous)) >= 0 + || previous >= 4 && code.regionMatches(true, previous - 4, "= 1 && code.regionMatches(previous - 1, "])"); + Matcher m = assign.matcher(code); + while (m.find()) { + if (m.start() >= from && m.start() < to) { + if (assignmentRhsEnd(code, m.start()) <= to) { + return true; + } + } + } + return false; + } + + /** + * End offset of the right-hand side of the assignment starting at {@code assignStart} + * (the {@code $} of {@code $var = …}): the first {@code ;} or {@code ,} at depth 0, + * an unmatched closer, or a depth-0 {@code or} / {@code and} / {@code xor}, which bind + * looser than {@code =}. {@code ?:}, {@code ??}, {@code ||}, {@code &&} bind tighter, + * so they stay inside the right-hand side. A fetch inside that range reads the old value; + * a fetch after it ({@code ($r = false) || fetch($r)}, {@code $r = false or fetch($r)}) + * sees the new value. + */ + private static int assignmentRhsEnd(@NotNull String code, int assignStart) { + int depth = 0; + for (int i = assignStart; i < code.length(); i++) { + char c = code.charAt(i); + if (c == '(' || c == '[' || c == '{') { + depth++; + } else if (c == ')' || c == ']' || c == '}') { + if (depth == 0) { + return i; + } + depth--; + } else if (depth == 0 && (c == ';' || c == ',')) { + return i; + } else if (depth == 0 && startsWordOperator(code, i)) { + return i; + } + } + return code.length(); + } + + private static final Pattern WORD_OPERATOR = Pattern.compile("(?i)(?:or|and|xor)\\b"); + + /** True when a PHP {@code or} / {@code and} / {@code xor} keyword starts at {@code i}. */ + private static boolean startsWordOperator(@NotNull String code, int i) { + char c = Character.toLowerCase(code.charAt(i)); + if (c != 'o' && c != 'a' && c != 'x') { return false; } + if (i > 0) { + char prev = code.charAt(i - 1); + if (Character.isLetterOrDigit(prev) || prev == '_' || prev == '$') { + return false; + } + } + Matcher m = WORD_OPERATOR.matcher(code); + m.region(i, Math.min(code.length(), i + 4)); + return m.lookingAt(); + } - IfCond last = before.get(before.size() - 1); - // Early-exit: only when every fall-through path implies isResultSet($var). - // Requires pure-enough negation (no top-level &&) + single exit body. - if (last.ifEnd <= fetchOffset - && isOnlyWhitespace(code.substring(last.ifEnd, fetchOffset)) - && isSafeEarlyExitCondition(last.condition, resultVar) - && bodyIsSimpleEarlyExit(code, last)) { - return true; + private static boolean closesOutOfScope(@NotNull String code, int from, int to) { + int depth = 0; + int end = Math.min(to, code.length()); + for (int i = from; i < end; i++) { + char c = code.charAt(i); + if (c == '{') { + depth++; + } else if (c == '}') { + depth--; + if (depth < 0) { + return true; + } + } } + return false; + } - return isInsidePositiveIsResultSetGuard(code, fetchOffset, resultVar, before); + private static boolean hasFunctionKeyword(@NotNull String code, int from, int to) { + Matcher m = Pattern.compile("\\bfunction\\b", Pattern.CASE_INSENSITIVE).matcher(code); + while (m.find()) { + if (m.start() >= from && m.start() < to) { + return true; + } + } + return false; } private static @NotNull List findIfConditions(@NotNull String text) { @@ -185,7 +324,8 @@ && bodyIsSimpleEarlyExit(code, last)) { int semi = text.indexOf(';', after); ifEnd = semi < 0 ? text.length() : semi + 1; } - out.add(new IfCond(m.start(), openParen, closeParen, openBrace, bodyStart, ifEnd, cond)); + boolean chained = m.group().regionMatches(true, 0, "else", 0, 4); + out.add(new IfCond(m.start(), openParen, closeParen, openBrace, bodyStart, ifEnd, cond, chained)); } return out; } @@ -228,15 +368,6 @@ private static int matchingCloseBrace(@NotNull String text, int openBrace) { return -1; } - private static boolean isOnlyWhitespace(@NotNull String s) { - for (int i = 0; i < s.length(); i++) { - if (!Character.isWhitespace(s.charAt(i))) { - return false; - } - } - return true; - } - /** * Only a single top-level exit statement counts (no nested if/blocks). * Avoids treating {@code if (!isResultSet($r)) { if ($x) return; }} as safe. @@ -281,12 +412,6 @@ private static boolean conditionNegatesIsResultSet(@NotNull String cond, @NotNul return true; } } - Matcher mi = NEG_INSTANCEOF.matcher(cond); - while (mi.find()) { - if (resultVar.equals(mi.group(1))) { - return true; - } - } return false; } @@ -319,6 +444,15 @@ private static boolean hasBoolOpAnywhere(@NotNull String cond, boolean orOp) { return false; } + private static boolean hasXorAnywhere(@NotNull String cond) { + for (int i = 0; i < cond.length(); i++) { + if (isWordAt(cond, i, "xor")) { + return true; + } + } + return false; + } + private static boolean isWordAt(@NotNull String s, int i, @NotNull String word) { int n = word.length(); if (i + n > s.length()) { @@ -339,14 +473,18 @@ private static boolean isWordAt(@NotNull String s, int i, @NotNull String word) * parenthesized forms). */ private static boolean isSafePositiveGuardCondition(@NotNull String cond, @NotNull String resultVar) { + if (hasUnprovenPolarity(cond)) { + return false; + } if (!conditionMentionsIsResultSet(cond, resultVar)) { return false; } if (conditionNegatesIsResultSet(cond, resultVar)) { return false; } - // Reject OR-paths at any depth: isResultSet($r) || $fallback / (isResultSet($r) || $x) - return !hasBoolOpAnywhere(cond, true); + // Reject OR-paths at any depth: isResultSet($r) || $fallback / (isResultSet($r) || $x), + // and xor: isResultSet($r) xor $x is true when $r is not a result set and $x is. + return !hasBoolOpAnywhere(cond, true) && !hasXorAnywhere(cond); } /** @@ -356,11 +494,49 @@ private static boolean isSafePositiveGuardCondition(@NotNull String cond, @NotNu * parenthesized forms) where exit is conditional. */ private static boolean isSafeEarlyExitCondition(@NotNull String cond, @NotNull String resultVar) { + if (hasUnprovenPolarity(cond)) { + return false; + } if (!conditionNegatesIsResultSet(cond, resultVar)) { return false; } - // Reject AND-paths at any depth that make the exit conditional on other predicates. - return !hasBoolOpAnywhere(cond, false); + // Reject AND-paths at any depth that make the exit conditional on other predicates, + // and xor, which can be false while !isResultSet($var) is true. + return !hasBoolOpAnywhere(cond, false) && !hasXorAnywhere(cond); + } + + private static boolean hasUnprovenPolarity(@NotNull String condition) { + // ponytail: only simple negated atoms are proven; other expression forms need PSI. + for (int i = 0; i < condition.length(); i++) { + if (condition.charAt(i) == '>' && i > 0 && condition.charAt(i - 1) == '-') { + continue; + } + if ("=<>?:".indexOf(condition.charAt(i)) >= 0 + || condition.charAt(i) == '!' && hasUnprovenNegation(condition, i)) { + return true; + } + } + return false; + } + + private static boolean hasUnprovenNegation(@NotNull String condition, int bangIndex) { + int next = bangIndex + 1; + while (next < condition.length() && Character.isWhitespace(condition.charAt(next))) { + next++; + } + if (next < condition.length() && condition.charAt(next) == '!') { + return true; + } + if (next >= condition.length() || condition.charAt(next) != '(') { + return false; + } + int close = matchingCloseParen(condition, next); + if (close < 0) { + return true; + } + String group = condition.substring(next + 1, close); + return !NEG_IS_RESULT_SET.matcher(condition.substring(bangIndex, close + 1)).matches() + && !INSTANCEOF_RESULT.matcher(group.strip()).matches(); } private static boolean isInsidePositiveIsResultSetGuard( @@ -392,30 +568,17 @@ private static boolean isInsidePositiveIsResultSetGuard( } } if (depth > 0) { - return true; + // A closure can run after $var is reassigned, so the guard does not dominate it. + return !assignsResultVar(code, ic.openBrace + 1, fetchOffset, resultVar) + && !hasFunctionKeyword(code, ic.openBrace + 1, fetchOffset); } } else if (fetchOffset > ic.condEnd && fetchOffset < ic.ifEnd) { - // Brace-less: if (isResultSet($r)) $row = $db->fetch...; - return true; + return !assignsResultVar(code, ic.condEnd + 1, fetchOffset, resultVar); } } return false; } - private static String guessIndent(String text, int offset) { - int start = lineStart(text, offset); - int i = start; - while (i < text.length() && (text.charAt(i) == ' ' || text.charAt(i) == '\t')) { - i++; - } - return text.substring(start, i); - } - - private static int lineStart(String text, int offset) { - int i = text.lastIndexOf('\n', Math.max(0, offset - 1)); - return i < 0 ? 0 : i + 1; - } - private record IfCond( int ifStart, int openParen, @@ -423,7 +586,8 @@ private record IfCond( int openBrace, int bodyStart, int ifEnd, - @NotNull String condition + @NotNull String condition, + boolean chained ) { } } diff --git a/src/main/java/org/xoops/support/inspections/XoopsRootPathGuardInspection.java b/src/main/java/org/xoops/support/inspections/XoopsRootPathGuardInspection.java index eddb288..7bd7b74 100644 --- a/src/main/java/org/xoops/support/inspections/XoopsRootPathGuardInspection.java +++ b/src/main/java/org/xoops/support/inspections/XoopsRootPathGuardInspection.java @@ -8,63 +8,40 @@ import org.jetbrains.annotations.NotNull; import org.xoops.support.XoopsSupportPlugin; -import java.util.Locale; import java.util.regex.Matcher; import java.util.regex.Pattern; /** - * Flags module/class PHP files that lack a terminating direct-access guard - * as the first executable statement after the opening PHP tag. + * Flags include-only PHP files that lack a terminating direct-access guard + * as the first executable statement after {@code Entry points (admin pages, mainfile includes) and directory-protection 404 stubs + * are skipped — see {@link XoopsRootPathGuardPolicy}. */ public final class XoopsRootPathGuardInspection extends LocalInspectionTool { private static final Pattern OPEN_PHP = Pattern.compile("<\\?php\\b", Pattern.CASE_INSENSITIVE); private static final Pattern OPEN_ANY = Pattern.compile("<\\?(?:php|=)?", Pattern.CASE_INSENSITIVE); - /** defined('XOOPS_ROOT_PATH') || exit/die(...); */ - private static final Pattern GUARD_OR = Pattern.compile( - "(?is)^\\s*defined\\s*\\(\\s*['\"]XOOPS_ROOT_PATH['\"]\\s*\\)\\s*\\|\\|\\s*(?:exit|die)\\s*\\s*(?:\\([^;]*\\))?\\s*;" - ); - - /** if (!defined('XOOPS_ROOT_PATH')) { exit/die(...); } */ - private static final Pattern GUARD_IF = Pattern.compile( - "(?is)^\\s*if\\s*\\(\\s*!\\s*defined\\s*\\(\\s*['\"]XOOPS_ROOT_PATH['\"]\\s*\\)\\s*\\)\\s*\\{" - + "\\s*(?:exit|die)\\s*(?:\\([^;]*\\))?\\s*;\\s*\\}" - ); - @Override public @NotNull PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly) { return new PsiElementVisitor() { @Override public void visitFile(@NotNull PsiFile file) { - if (!XoopsSupportPlugin.isEnabled(file)) { + if (!XoopsSupportPlugin.isEnabled(file) || !PhpTextUtil.isPrimaryPsiFile(file)) { return; } if (!PhpTextUtil.isPhpFile(file) || PhpTextUtil.looksLikeVendorOrCache(file)) { return; } - if (PhpTextUtil.looksLikeLanguageFile(file)) { - return; - } String path = file.getVirtualFile() != null - ? file.getVirtualFile().getPath().replace('\\', '/').toLowerCase(Locale.ROOT) - : ""; - boolean inModuleOrClass = path.contains("/modules/") - || path.contains("/class/") - || path.contains("/preloads/") - || path.contains("/kernel/"); - if (!inModuleOrClass) { - return; - } + ? file.getVirtualFile().getPath() + : file.getName(); String text = file.getText(); if (text == null || text.isBlank()) { return; } - if (!text.contains(" splitStatements(@NotNull String code) { + List out = new ArrayList<>(); + char quote = 0; + int start = 0; + for (int i = 0; i < code.length(); i++) { + char c = code.charAt(i); + if (quote != 0) { + if (c == '\\') { + i++; + } else if (c == quote) { + quote = 0; + } + } else if (c == '\'' || c == '"') { + quote = c; + } else if (c == ';') { + out.add(code.substring(start, i)); + start = i + 1; + } + } + out.add(code.substring(start)); + return out; + } + + static boolean isBootstrapEntryPoint(@NotNull String firstExecutable) { + return BOOTSTRAP_INCLUDE.matcher(firstExecutable).lookingAt(); + } + + /** + * Comment-masked source from the first executable statement after open tag, + * {@code declare}, {@code namespace}, and {@code use} statements. + */ + static @NotNull String firstExecutable(@NotNull String text) { + int openEnd = firstOpenTagEnd(text); + if (openEnd < 0) { + return ""; + } + String masked = PhpTextUtil.maskCommentsOnly(text); + int pos = skipDeclaresAndNamespace(masked, skipWs(masked, openEnd)); + pos = skipUseStatements(masked, pos); + if (pos >= masked.length()) { + return ""; + } + String rest = masked.substring(pos).stripLeading(); + return rest.replaceFirst("(?s)\\?>\\s*$", "").strip(); + } + + /** + * True when {@link InsertRootPathGuardQuickFix} has a safe place to insert: a + * file-leading {@code = 0) { + return true; + } + Matcher echo = OPEN_ECHO.matcher(text); + if (echo.find() && isFileLeading(text, echo.start())) { + return true; + } + return !text.contains("= 0; + } + + /** End offset of the first {@code = s.length()) { + return pos; + } + Matcher m = pattern.matcher(s.substring(pos)); + if (m.find() && m.start() == 0) { + return pos + m.end(); + } + return pos; + } +} diff --git a/src/main/java/org/xoops/support/inspections/XoopsSuperglobalInspection.java b/src/main/java/org/xoops/support/inspections/XoopsSuperglobalInspection.java index 380e663..5f81067 100644 --- a/src/main/java/org/xoops/support/inspections/XoopsSuperglobalInspection.java +++ b/src/main/java/org/xoops/support/inspections/XoopsSuperglobalInspection.java @@ -29,7 +29,7 @@ public final class XoopsSuperglobalInspection extends LocalInspectionTool { return new PsiElementVisitor() { @Override public void visitFile(@NotNull PsiFile file) { - if (!XoopsSupportPlugin.isEnabled(file)) { + if (!XoopsSupportPlugin.isEnabled(file) || !PhpTextUtil.isPrimaryPsiFile(file)) { return; } if (!PhpTextUtil.isPhpFile(file) || PhpTextUtil.looksLikeVendorOrCache(file)) { diff --git a/src/main/java/org/xoops/support/inspections/XoopsTemplatePaths.java b/src/main/java/org/xoops/support/inspections/XoopsTemplatePaths.java new file mode 100644 index 0000000..d48827a --- /dev/null +++ b/src/main/java/org/xoops/support/inspections/XoopsTemplatePaths.java @@ -0,0 +1,77 @@ +package org.xoops.support.inspections; + +import com.intellij.openapi.vfs.VirtualFile; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.stream.Stream; + +/** Resolves registered template paths while retaining their actual disk spelling. */ +public final class XoopsTemplatePaths { + private XoopsTemplatePaths() { + } + + /** Actual module-relative path, preferring exact spelling; null when no file matches. */ + public static @Nullable String existingPath(@NotNull VirtualFile root, @NotNull String relative) { + VirtualFile current = root; + List actual = new ArrayList<>(); + for (String part : relative.split("/")) { + if (!current.isDirectory()) { + return null; + } + String name = matchingName(Arrays.stream(current.getChildren()).map(VirtualFile::getName).toList(), part); + if (name == null) { + return null; + } + actual.add(name); + current = current.findChild(name); + if (current == null) { + return null; + } + } + return current.isDirectory() ? null : String.join("/", actual); + } + + /** Scanner equivalent of the VFS lookup; I/O errors remain distinguishable from missing files. */ + public static @Nullable String existingPath(@NotNull Path root, @NotNull String relative) throws IOException { + Path current = root; + List actual = new ArrayList<>(); + for (String part : relative.split("/")) { + if (!Files.isDirectory(current)) { + return null; + } + String name; + try (Stream children = Files.list(current)) { + name = matchingName(children.map(p -> p.getFileName().toString()).toList(), part); + } + if (name == null) { + return null; + } + actual.add(name); + current = current.resolve(name); + } + return Files.isRegularFile(current) ? String.join("/", actual) : null; + } + + private static @Nullable String matchingName(List names, String requested) { + if (requested.isEmpty() || requested.equals(".") || requested.equals("..")) { + return null; + } + String fallback = null; + for (String name : names) { + if (name.equals(requested)) { + return name; + } + if (name.equalsIgnoreCase(requested)) { + fallback = name; + } + } + return fallback; + } +} diff --git a/src/main/java/org/xoops/support/inspections/XoopsUnregisteredTemplateInspection.java b/src/main/java/org/xoops/support/inspections/XoopsUnregisteredTemplateInspection.java new file mode 100644 index 0000000..47eb9d1 --- /dev/null +++ b/src/main/java/org/xoops/support/inspections/XoopsUnregisteredTemplateInspection.java @@ -0,0 +1,93 @@ +package org.xoops.support.inspections; + +import com.intellij.codeInspection.LocalInspectionTool; +import com.intellij.codeInspection.ProblemsHolder; +import com.intellij.openapi.vfs.VirtualFile; +import com.intellij.openapi.vfs.VfsUtilCore; +import com.intellij.psi.PsiElement; +import com.intellij.psi.PsiElementVisitor; +import com.intellij.psi.PsiFile; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.xoops.support.XoopsSupportPlugin; + +import java.util.Locale; +import java.util.Set; + +/** + * Flags a module {@code .tpl} that sits under {@code templates/} (or {@code blocks/}) + * but is never listed in {@code xoops_version.php} — the inverse of + * {@link XoopsMissingRegisteredTemplateInspection}. + */ +public final class XoopsUnregisteredTemplateInspection extends LocalInspectionTool { + + + @Override + public @NotNull PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly) { + return new PsiElementVisitor() { + @Override + public void visitFile(@NotNull PsiFile file) { + VirtualFile vf = candidateTpl(file); + if (vf == null) { + return; + } + VirtualFile moduleRoot = moduleRootOf(vf); + String relative = moduleRoot == null ? null : VfsUtilCore.getRelativePath(vf, moduleRoot, '/'); + if (relative == null || !(relative.startsWith("templates/") || relative.startsWith("blocks/"))) { + return; + } + VirtualFile manifest = moduleRoot == null ? null : moduleRoot.findChild("xoops_version.php"); + PsiFile manifestPsi = manifest == null ? null : file.getManager().findFile(manifest); + if (relative == null || manifestPsi == null) { + return; + } + Set registered = registeredTemplates(manifestPsi.getText()); + String key = relative.toLowerCase(Locale.ROOT); + if (registered.contains(key)) { + return; + } + PsiElement anchor = file.getFirstChild() != null ? file.getFirstChild() : file; + holder.registerProblem( + anchor, + "XOOPS: template is not registered in xoops_version.php: " + relative, + new RegisterTemplateQuickFix(relative) + ); + } + }; + } + + private static @Nullable VirtualFile candidateTpl(@NotNull PsiFile file) { + if (!XoopsSupportPlugin.isEnabled(file) || !PhpTextUtil.isPrimaryPsiFile(file)) { + return null; + } + String name = file.getName().toLowerCase(Locale.ROOT); + if (!name.endsWith(".tpl") || PhpTextUtil.looksLikeVendorOrCache(file)) { + return null; + } + VirtualFile vf = file.getVirtualFile(); + if (vf == null) { + return null; + } + String path = vf.getPath().replace('\\', '/').toLowerCase(Locale.ROOT); + if (path.contains("/themes/") || path.contains("/templates_c/")) { + return null; + } + return vf; + } + + /** Registered names as lookup keys; see {@link XoopsManifestTemplates#keys(String)}. */ + static @NotNull Set registeredTemplates(@NotNull String manifestText) { + return XoopsManifestTemplates.keys(manifestText); + } + + private static @Nullable VirtualFile moduleRootOf(@NotNull VirtualFile tpl) { + VirtualFile dir = tpl.getParent(); + while (dir != null) { + if (dir.findChild("xoops_version.php") != null) { + return dir; + } + dir = dir.getParent(); + } + return null; + } +} diff --git a/src/main/java/org/xoops/support/inspections/XoopsWrongSmartyDelimiterInspection.java b/src/main/java/org/xoops/support/inspections/XoopsWrongSmartyDelimiterInspection.java index 89764e2..bd1dfb4 100644 --- a/src/main/java/org/xoops/support/inspections/XoopsWrongSmartyDelimiterInspection.java +++ b/src/main/java/org/xoops/support/inspections/XoopsWrongSmartyDelimiterInspection.java @@ -25,7 +25,7 @@ public final class XoopsWrongSmartyDelimiterInspection extends LocalInspectionTo return new PsiElementVisitor() { @Override public void visitFile(@NotNull PsiFile file) { - if (!XoopsSupportPlugin.isEnabled(file)) { + if (!XoopsSupportPlugin.isEnabled(file) || !PhpTextUtil.isPrimaryPsiFile(file)) { return; } String name = file.getName().toLowerCase(java.util.Locale.ROOT); diff --git a/src/main/java/org/xoops/support/scanner/CoreProfile.java b/src/main/java/org/xoops/support/scanner/CoreVersion.java similarity index 81% rename from src/main/java/org/xoops/support/scanner/CoreProfile.java rename to src/main/java/org/xoops/support/scanner/CoreVersion.java index fb0ddc0..087f373 100644 --- a/src/main/java/org/xoops/support/scanner/CoreProfile.java +++ b/src/main/java/org/xoops/support/scanner/CoreVersion.java @@ -1,9 +1,9 @@ package org.xoops.support.scanner; /** - * Detected XOOPS core line . + * Detected XOOPS core line. */ -public enum CoreProfile { +public enum CoreVersion { XOOPS_25("XOOPS 2.5.x"), XOOPS_27("XOOPS 2.7.x"), XOOPS_40("XOOPS 4.0"), @@ -13,7 +13,7 @@ public enum CoreProfile { private final String displayName; - CoreProfile(String displayName) { + CoreVersion(String displayName) { this.displayName = displayName; } diff --git a/src/main/java/org/xoops/support/scanner/XoopsProjectReport.java b/src/main/java/org/xoops/support/scanner/XoopsProjectReport.java index 2b668af..aed04f6 100644 --- a/src/main/java/org/xoops/support/scanner/XoopsProjectReport.java +++ b/src/main/java/org/xoops/support/scanner/XoopsProjectReport.java @@ -7,7 +7,7 @@ public record XoopsProjectReport( boolean xoopsProject, Path projectRoot, Path webRoot, - CoreProfile profile, + CoreVersion coreVersion, List modules, List findings ) { diff --git a/src/main/java/org/xoops/support/scanner/XoopsProjectScanner.java b/src/main/java/org/xoops/support/scanner/XoopsProjectScanner.java index 73e8da4..82af773 100644 --- a/src/main/java/org/xoops/support/scanner/XoopsProjectScanner.java +++ b/src/main/java/org/xoops/support/scanner/XoopsProjectScanner.java @@ -2,12 +2,18 @@ import com.intellij.openapi.progress.ProgressManager; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.xoops.support.inspections.PhpTextUtil; +import org.xoops.support.inspections.XoopsManifestTemplates; +import org.xoops.support.inspections.XoopsTemplatePaths; import java.io.IOException; +import java.io.UncheckedIOException; import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; import java.util.Comparator; +import java.util.LinkedHashSet; import java.util.List; import java.util.Locale; import java.util.Optional; @@ -34,27 +40,28 @@ public final class XoopsProjectScanner { "smarty_compile", "templates_c", "uploads", "vendor", "xoops_data" ); - private static final Pattern VERSION_25 = Pattern.compile("(?i)(?:XOOPS[ _-]?)?2\\.5(?:\\.|[^0-9]|$)"); - private static final Pattern VERSION_27 = Pattern.compile("(?i)(?:XOOPS[ _-]?)?2\\.7(?:\\.|[^0-9]|$)"); - private static final Pattern VERSION_40 = Pattern.compile("(?i)(?:XOOPS[ _-]?)?4\\.0(?:\\.|[^0-9]|$)"); - private static final Pattern REGISTERED_TEMPLATE = Pattern.compile( - "(?is)['\"](?:file|template)['\"]\\s*=>\\s*['\"]([^'\"]+\\.tpl)['\"]" - ); + static final Pattern VERSION_25 = Pattern.compile("(?i)(?:XOOPS[ _-]?)?(?)\\s*['\"]([a-z0-9_-]+)['\"]" ); - private static final Pattern RAW_REQUEST = Pattern.compile("\\$_REQUEST\\b"); - private static final Pattern QUERY_F = Pattern.compile("->\\s*queryF\\s*\\("); - private static final Pattern QUOTE_STRING = Pattern.compile("->\\s*quoteString\\s*\\("); - private static final Pattern MUTATING_QUERY = Pattern.compile( + static final Pattern RAW_REQUEST = Pattern.compile("\\$_REQUEST\\b"); + static final Pattern QUERY_F = Pattern.compile("->\\s*queryF\\s*\\("); + static final Pattern QUOTE_STRING = Pattern.compile("->\\s*quoteString\\s*\\("); + static final Pattern MUTATING_QUERY = Pattern.compile( "(?is)->\\s*query\\s*\\(\\s*['\"]\\s*(?:INSERT|UPDATE|DELETE|REPLACE|ALTER|CREATE|DROP|TRUNCATE)\\b" ); - private static final Pattern WRONG_SMARTY = Pattern.compile( + static final Pattern WRONG_SMARTY = Pattern.compile( "(?i)(? moduleRoots = findModuleRoots(projectRoot, webRoot, standaloneModule); + List findings = new ArrayList<>(); + List moduleRoots = findModuleRoots(projectRoot, webRoot, standaloneModule, findings); // Inspect + scan each module in one cancel-aware loop so Cancel is observed // during metadata walks (inspectModule / countFiles), not only during source scan. // One check per module (not per file) at this level; file walks throttle below. List modules = new ArrayList<>(); - List findings = new ArrayList<>(); for (Path moduleRoot : moduleRoots) { ProgressManager.checkCanceled(); - modules.add(inspectModule(moduleRoot)); + modules.add(inspectModule(moduleRoot, findings)); scanModule(moduleRoot, findings); } modules.sort(Comparator.comparing(XoopsModuleInfo::dirname, String.CASE_INSENSITIVE_ORDER)); @@ -84,11 +91,40 @@ public XoopsProjectReport scan(Path requestedRoot) { .comparing((XoopsFinding f) -> f.path().toString(), String.CASE_INSENSITIVE_ORDER) .thenComparingInt(XoopsFinding::line)); - CoreProfile profile = standaloneModule && !isCoreRoot(webRoot) - ? CoreProfile.MODULE_ONLY - : detectProfile(projectRoot, webRoot); + CoreVersion coreVersion = resolveScanCoreVersion( + coreVersionSetting, standaloneModule && !isCoreRoot(webRoot), projectRoot, webRoot + ); - return new XoopsProjectReport(true, projectRoot, webRoot, profile, modules, findings); + return new XoopsProjectReport(true, projectRoot, webRoot, coreVersion, modules, findings); + } + + /** + * Explicit Settings → Core Version wins. {@code Auto} (and unknown values) fall back + * to layout detection ({@code MODULE_ONLY}) or {@link #detectCoreVersion}. + */ + static @NotNull CoreVersion resolveScanCoreVersion( + @NotNull String setting, + boolean standaloneModule, + @NotNull Path projectRoot, + @NotNull Path webRoot + ) { + CoreVersion fromSetting = coreVersionFromSetting(setting); + if (fromSetting != null) { + return fromSetting; + } + if (standaloneModule) { + return CoreVersion.MODULE_ONLY; + } + return detectCoreVersion(projectRoot, webRoot); + } + + static @Nullable CoreVersion coreVersionFromSetting(@NotNull String setting) { + return switch (setting.trim()) { + case "2.5" -> CoreVersion.XOOPS_25; + case "2.7" -> CoreVersion.XOOPS_27; + case "4.0" -> CoreVersion.XOOPS_40; + default -> null; + }; } private static Path detectWebRoot(Path projectRoot) { @@ -106,7 +142,7 @@ private static boolean isCoreRoot(Path candidate) { return Files.isRegularFile(candidate.resolve("mainfile.php")); } - private static List findModuleRoots(Path projectRoot, Path webRoot, boolean standaloneModule) { + private static List findModuleRoots(Path projectRoot, Path webRoot, boolean standaloneModule, List findings) { if (standaloneModule && !isCoreRoot(webRoot)) { return List.of(projectRoot); } @@ -120,21 +156,22 @@ private static List findModuleRoots(Path projectRoot, Path webRoot, boolea .filter(path -> Files.isRegularFile(path.resolve("xoops_version.php")) || Files.isRegularFile(path.resolve("module.json"))) .toList(); - } catch (IOException ignored) { + } catch (IOException | UncheckedIOException exception) { + findings.add(scanError(modulesDirectory, "Could not list modules: " + exception.getMessage())); return List.of(); } } - private XoopsModuleInfo inspectModule(Path moduleRoot) { + private XoopsModuleInfo inspectModule(Path moduleRoot, List findings) { return new XoopsModuleInfo( readModuleDirname(moduleRoot), moduleRoot, Files.isRegularFile(moduleRoot.resolve("xoops_version.php")), Files.isRegularFile(moduleRoot.resolve("module.json")), - countFiles(moduleRoot.resolve("templates"), ".tpl"), - countFiles(moduleRoot.resolve("language"), ".php"), - countFiles(moduleRoot.resolve("preloads"), ".php"), - countFiles(moduleRoot.resolve("class"), ".php") + countFiles(moduleRoot.resolve("src"), ".php") + countFiles(moduleRoot.resolve("templates"), ".tpl", findings), + countFiles(moduleRoot.resolve("language"), ".php", findings), + countFiles(moduleRoot.resolve("preloads"), ".php", findings), + countFiles(moduleRoot.resolve("class"), ".php", findings) + countFiles(moduleRoot.resolve("src"), ".php", findings) ); } @@ -144,7 +181,7 @@ private static String readModuleDirname(Path moduleRoot) { return matcher.find() ? matcher.group(1) : moduleRoot.getFileName().toString(); } - private static long countFiles(Path directory, String suffix) { + private static long countFiles(Path directory, String suffix, List findings) { if (!Files.isDirectory(directory)) { return 0; } @@ -158,7 +195,8 @@ private static long countFiles(Path directory, String suffix) { }) .filter(path -> path.getFileName().toString().toLowerCase(Locale.ROOT).endsWith(suffix)) .count(); - } catch (IOException ignored) { + } catch (IOException | UncheckedIOException exception) { + findings.add(scanError(directory, "Could not count files: " + exception.getMessage())); return 0; } } @@ -173,7 +211,7 @@ private static void checkCanceledEvery(int[] pathCounter) { } } - private static CoreProfile detectProfile(Path projectRoot, Path webRoot) { + private static CoreVersion detectCoreVersion(Path projectRoot, Path webRoot) { // Prefer core include/version.php — composer.json dependency ranges often mislead (e.g. "2.5"). for (Path candidate : List.of( webRoot.resolve("include/version.php"), @@ -185,13 +223,13 @@ private static CoreProfile detectProfile(Path projectRoot, Path webRoot) { } String text = body.get(); if (VERSION_40.matcher(text).find()) { - return CoreProfile.XOOPS_40; + return CoreVersion.XOOPS_40; } if (VERSION_27.matcher(text).find()) { - return CoreProfile.XOOPS_27; + return CoreVersion.XOOPS_27; } if (VERSION_25.matcher(text).find()) { - return CoreProfile.XOOPS_25; + return CoreVersion.XOOPS_25; } } // Fallback: bind package name to its version constraint (not independent whole-file matches). @@ -200,19 +238,19 @@ private static CoreProfile detectProfile(Path projectRoot, Path webRoot) { if (body.isEmpty()) { continue; } - CoreProfile fromComposer = profileFromComposerJson(body.get()); - if (fromComposer != CoreProfile.UNKNOWN) { + CoreVersion fromComposer = coreVersionFromComposerJson(body.get()); + if (fromComposer != CoreVersion.UNKNOWN) { return fromComposer; } } - return CoreProfile.UNKNOWN; + return CoreVersion.UNKNOWN; } /** * Match a single Composer require entry whose package name contains "xoops" * and apply version patterns only to that entry's constraint. */ - private static CoreProfile profileFromComposerJson(@NotNull String json) { + private static CoreVersion coreVersionFromComposerJson(@NotNull String json) { // "xoops/something": "2.5.11" or "xoopsmodules/foo": "^2.7" Pattern entry = Pattern.compile( "(?is)\"([^\"]*xoops[^\"]*)\"\\s*:\\s*\"([^\"]+)\"" @@ -226,16 +264,16 @@ private static CoreProfile profileFromComposerJson(@NotNull String json) { continue; } if (VERSION_40.matcher(constraint).find()) { - return CoreProfile.XOOPS_40; + return CoreVersion.XOOPS_40; } if (VERSION_27.matcher(constraint).find()) { - return CoreProfile.XOOPS_27; + return CoreVersion.XOOPS_27; } if (VERSION_25.matcher(constraint).find()) { - return CoreProfile.XOOPS_25; + return CoreVersion.XOOPS_25; } } - return CoreProfile.UNKNOWN; + return CoreVersion.UNKNOWN; } private void scanModule(Path moduleRoot, List findings) { @@ -252,13 +290,10 @@ private void scanModule(Path moduleRoot, List findings) { checkCanceledEvery(seen); scanSourceFile(path, findings); }); + } catch (UncheckedIOException exception) { + findings.add(scanError(moduleRoot, "Could not scan module: " + walkMessage(exception))); } catch (IOException exception) { - findings.add(new XoopsFinding( - "SCAN_ERROR", - moduleRoot, - 1, - "Could not scan module: " + exception.getMessage() - )); + findings.add(scanError(moduleRoot, "Could not scan module: " + exception.getMessage())); } } @@ -308,27 +343,101 @@ private static void addFirst( } } - private static void checkRegisteredTemplates(Path moduleRoot, List findings) { + static void checkRegisteredTemplates(Path moduleRoot, List findings) { Path manifest = moduleRoot.resolve("xoops_version.php"); - String content = readSmallFile(manifest).orElse(null); - if (content == null) { + if (!Files.isRegularFile(manifest)) { + return; // module.json-only module: no legacy manifest to compare templates against + } + Optional read = readSmallFile(manifest); + if (read.isEmpty()) { + // Unreadable or oversized manifest: no data to compare against, so report + // that instead of flagging every template as unregistered. + findings.add(new XoopsFinding( + "SCAN_ERROR", + manifest, + 1, + "xoops_version.php could not be read (unreadable or larger than " + MAX_SOURCE_BYTES + " bytes)" + )); return; } - Matcher matcher = REGISTERED_TEMPLATE.matcher(content); - while (matcher.find()) { - String template = matcher.group(1).replace('\\', '/'); - boolean exists = Files.isRegularFile(moduleRoot.resolve("templates").resolve(template)) - || Files.isRegularFile(moduleRoot.resolve("blocks").resolve(template)) - || Files.isRegularFile(moduleRoot.resolve(template)); - if (!exists) { - findings.add(new XoopsFinding( - "MISSING_REGISTERED_TEMPLATE", - manifest, - lineAt(content, matcher.start(1)), - "Registered template is missing: " + template - )); + String content = read.get(); + Set registered = new LinkedHashSet<>(); + if (!content.isEmpty()) { + // Masked copy keeps offsets, so lineAt() on the original content stays right. + for (XoopsManifestTemplates.Registration reg + : XoopsManifestTemplates.find(PhpTextUtil.maskCommentsOnly(content))) { + String template = reg.name(); + String relative = XoopsManifestTemplates.diskPath(template, reg.block()); + registered.add(relative.toLowerCase(Locale.ROOT)); + String actual; + try { + actual = XoopsTemplatePaths.existingPath(moduleRoot, relative); + } catch (IOException | UncheckedIOException exception) { + findings.add(scanError(manifest, "Could not locate template: " + exception.getMessage())); + continue; + } + if (actual != null && !relative.equals(actual)) { + findings.add(new XoopsFinding( + "TEMPLATE_CASE_MISMATCH", manifest, lineAt(content, reg.nameOffset()), + "Template filename case mismatch: " + relative + " (on disk: " + actual + ")")); + } else if (actual == null) { + findings.add(new XoopsFinding( + "MISSING_REGISTERED_TEMPLATE", + manifest, + lineAt(content, reg.nameOffset()), + "Registered template is missing: " + template + )); + } } } + addUnregisteredTemplates(moduleRoot, moduleRoot.resolve("templates"), registered, findings); + addUnregisteredTemplates(moduleRoot, moduleRoot.resolve("blocks"), registered, findings); + } + + private static void addUnregisteredTemplates( + Path moduleRoot, + Path directory, + Set registered, + List findings + ) { + if (!Files.isDirectory(directory)) { + return; + } + int[] seen = {0}; + try (Stream paths = Files.walk(directory, 6)) { + paths.peek(p -> checkCanceledEvery(seen)) + .filter(Files::isRegularFile) + .filter(p -> p.getFileName().toString().toLowerCase(Locale.ROOT).endsWith(".tpl")) + .forEach(path -> { + String relative = moduleRoot.relativize(path).toString().replace('\\', '/'); + String key = relative.toLowerCase(Locale.ROOT); + boolean listed = registered.contains(key); + if (!listed) { + findings.add(new XoopsFinding( + "UNREGISTERED_TEMPLATE", + path, + 1, + "Template is not registered in xoops_version.php: " + relative + )); + } + }); + } catch (UncheckedIOException exception) { + findings.add(scanError(directory, "Could not scan templates: " + walkMessage(exception))); + } catch (IOException exception) { + findings.add(scanError(directory, "Could not scan templates: " + exception.getMessage())); + } + } + + private static @NotNull XoopsFinding scanError(@NotNull Path path, @NotNull String message) { + return new XoopsFinding("SCAN_ERROR", path, 1, message); + } + + private static @NotNull String walkMessage(@NotNull UncheckedIOException exception) { + Throwable cause = exception.getCause(); + String detail = cause != null && cause.getMessage() != null + ? cause.getMessage() + : exception.getMessage(); + return detail == null ? exception.getClass().getSimpleName() : detail; } private static int lineAt(String content, int offset) { diff --git a/src/main/java/org/xoops/support/settings/XoopsConfigurable.java b/src/main/java/org/xoops/support/settings/XoopsConfigurable.java index c478a97..b1d5e6b 100644 --- a/src/main/java/org/xoops/support/settings/XoopsConfigurable.java +++ b/src/main/java/org/xoops/support/settings/XoopsConfigurable.java @@ -24,7 +24,7 @@ public final class XoopsConfigurable implements Configurable { private JCheckBox enabledBox; private JCheckBox suppressNotifyBox; private JCheckBox autoScanBox; - private JComboBox profileBox; + private JComboBox coreVersionBox; private JTextField prefixField; private JPanel panel; @@ -61,10 +61,10 @@ public XoopsConfigurable(Project project) { form.add(autoScanBox, c); c.gridy++; - form.add(new JLabel("Core profile:"), c); + form.add(new JLabel("Core Version:"), c); c.gridx = 1; - profileBox = new JComboBox<>(new String[]{"Auto", "2.5", "2.7", "4.0"}); - form.add(profileBox, c); + coreVersionBox = new JComboBox<>(new String[]{"Auto", "2.5", "2.7", "4.0"}); + form.add(coreVersionBox, c); c.gridx = 0; c.gridy++; @@ -82,13 +82,13 @@ public XoopsConfigurable(Project project) { @Override public boolean isModified() { XoopsSettingsState s = XoopsSettingsState.getInstance(project); - String selectedProfile = String.valueOf(profileBox.getSelectedItem()); - String storedProfile = s.coreProfile == null ? "Auto" : s.coreProfile; + String selectedCoreVersion = String.valueOf(coreVersionBox.getSelectedItem()); + String storedCoreVersion = s.resolvedCoreVersion(); String storedPrefix = s.tablePrefix == null ? "" : s.tablePrefix; return enabledBox.isSelected() != s.enabled || suppressNotifyBox.isSelected() != s.suppressStartupNotification || autoScanBox.isSelected() != s.autoScanOnToolWindowOpen - || !Objects.equals(selectedProfile, storedProfile) + || !Objects.equals(selectedCoreVersion, storedCoreVersion) || !Objects.equals(prefixField.getText().trim(), storedPrefix); } @@ -98,7 +98,7 @@ public void apply() { s.enabled = enabledBox.isSelected(); s.suppressStartupNotification = suppressNotifyBox.isSelected(); s.autoScanOnToolWindowOpen = autoScanBox.isSelected(); - s.coreProfile = String.valueOf(profileBox.getSelectedItem()); + s.coreVersion = String.valueOf(coreVersionBox.getSelectedItem()); s.tablePrefix = prefixField.getText().trim(); } @@ -108,7 +108,7 @@ public void reset() { enabledBox.setSelected(s.enabled); suppressNotifyBox.setSelected(s.suppressStartupNotification); autoScanBox.setSelected(s.autoScanOnToolWindowOpen); - profileBox.setSelectedItem(s.coreProfile == null ? "Auto" : s.coreProfile); + coreVersionBox.setSelectedItem(s.resolvedCoreVersion()); prefixField.setText(s.tablePrefix == null ? "" : s.tablePrefix); } } diff --git a/src/main/java/org/xoops/support/settings/XoopsSettingsState.java b/src/main/java/org/xoops/support/settings/XoopsSettingsState.java index 83e4d83..85a89ce 100644 --- a/src/main/java/org/xoops/support/settings/XoopsSettingsState.java +++ b/src/main/java/org/xoops/support/settings/XoopsSettingsState.java @@ -8,6 +8,7 @@ import com.intellij.util.xmlb.XmlSerializerUtil; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.TestOnly; @Service(Service.Level.PROJECT) @State(name = "XoopsSupportSettings", storages = @Storage("xoopsSupport.xml")) @@ -21,21 +22,68 @@ public final class XoopsSettingsState implements PersistentStateComponent").append(escape(report.profile().displayName())).append("") + .append("

").append(escape(report.coreVersion().displayName())).append("

") .append("

Web root: ") .append(escape(report.webRoot().toString())) .append("

") diff --git a/src/main/java/org/xoops/support/ui/XoopsToolWindowPanel.java b/src/main/java/org/xoops/support/ui/XoopsToolWindowPanel.java index 9ab9df9..120e31a 100644 --- a/src/main/java/org/xoops/support/ui/XoopsToolWindowPanel.java +++ b/src/main/java/org/xoops/support/ui/XoopsToolWindowPanel.java @@ -119,7 +119,10 @@ public void run(@NotNull ProgressIndicator indicator) { try { indicator.setText("Scanning XOOPS modules (cancellable)…"); indicator.checkCanceled(); - XoopsProjectReport report = new XoopsProjectScanner().scan(Path.of(basePath)); + XoopsProjectReport report = new XoopsProjectScanner().scan( + Path.of(basePath), + XoopsSettingsState.getInstance(project).resolvedCoreVersion() + ); indicator.checkCanceled(); String html = new XoopsReportHtmlRenderer().render(report); ApplicationManager.getApplication().invokeLater( diff --git a/src/main/resources/META-INF/plugin.xml b/src/main/resources/META-INF/plugin.xml index 652d71d..a6b0b24 100644 --- a/src/main/resources/META-INF/plugin.xml +++ b/src/main/resources/META-INF/plugin.xml @@ -3,23 +3,32 @@ org.xoops.plugin.support XOOPS Support - 1.0.0-alpha.2 + XOOPS Project XOOPS Support — PhpStorm / IntelliJ helper for XOOPS 2.5 / 2.7 / 4.0 module and core work.

-

1.0.0 Alpha 2 — on-demand overview scan; inspections under top-level XOOPS group.

+

XOOPS Support — PhpStorm / IntelliJ helper for XOOPS 2.5 / 2.7 / 4.0 Core and module development.

+

1.0.0 Alpha 3 — Goffy field-report fixes; language-constant navigation; unregistered templates; UNICODE dtype.

  • Project detection, scanner, and HTML overview tool window
  • Inspections with Alt+Enter quick fixes (guards, query/exec, Request, Smarty, templates)
    • -
  • Live templates, language-constant completion, hybrid module scaffold
    • +
  • Live templates, language-constant completion and Ctrl+B, hybrid module scaffold
  • Dynamic install/update (no IDE restart when unload succeeds)
]]> 1.0.0 Alpha 3 (1.0.0-alpha.3) +
    +
  • Goffy / wgSimpleAcc: inspections no longer report every finding twice (PHP+HTML PSI).
    • +
  • isResultSet quick-fix uses the current PSI statement (Inspect Code batch apply works); early-exit guards cover while (list = fetchRow).
    • +
  • ROOT_PATH guard: recognize die after namespace/use; insert after namespace; skip 404 stubs, admin entry points, and mainfile/header bootstraps.
    • +
  • Language constants: scan every language/**/*.php; Ctrl+B / Find Usages on _MD_* (and MI/AM/CO/MB).
    • +
  • Unregistered .tpl inspection (inverse of missing registered template).
    • +
  • XOBJ_DTYPE_UNICODE_* deprecation with rename quick-fix (off when Core Version is 2.5).
    • +

1.0.0 Alpha 2 (1.0.0-alpha.2)

  • Performance: Overview no longer auto-scans the whole module tree when the tool window opens (monorepo boot freeze). Click Refresh to scan. Optional setting re-enables auto-scan.
    • @@ -37,6 +46,9 @@
]]> + com.intellij.modules.platform com.jetbrains.php @@ -131,6 +143,23 @@ displayName="Wrong Smarty delimiter (use XOOPS <{ }>)" shortName="XoopsWrongSmartyDelimiter"/> + + + + @@ -139,6 +168,13 @@ + + + diff --git a/src/main/resources/META-INF/pluginIcon.svg b/src/main/resources/META-INF/pluginIcon.svg index f2f5445..5c890b1 100644 --- a/src/main/resources/META-INF/pluginIcon.svg +++ b/src/main/resources/META-INF/pluginIcon.svg @@ -1,4 +1,34 @@ - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/main/resources/icons/toolWindowXoops.svg b/src/main/resources/icons/toolWindowXoops.svg index 20a2eff..ee55126 100644 --- a/src/main/resources/icons/toolWindowXoops.svg +++ b/src/main/resources/icons/toolWindowXoops.svg @@ -1,4 +1,34 @@ - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/main/resources/inspectionDescriptions/XoopsDeprecatedUnicodeDtype.html b/src/main/resources/inspectionDescriptions/XoopsDeprecatedUnicodeDtype.html new file mode 100644 index 0000000..4e496ba --- /dev/null +++ b/src/main/resources/inspectionDescriptions/XoopsDeprecatedUnicodeDtype.html @@ -0,0 +1,21 @@ + + + +Deprecated XOBJ_DTYPE_UNICODE_* + + +XOBJ_DTYPE_UNICODE_* constants were deprecated in XOOPS 2.7.3 +(core issue #164) and are scheduled for removal in 4.0. +

+Quick-fix renames the constant to its non-UNICODE successor: +XOBJ_DTYPE_UNICODE_TXTBOX → XOBJ_DTYPE_TXTBOX, +XOBJ_DTYPE_UNICODE_TXTAREA → XOBJ_DTYPE_TXTAREA, +XOBJ_DTYPE_UNICODE_URL → XOBJ_DTYPE_URL, +XOBJ_DTYPE_UNICODE_EMAIL → XOBJ_DTYPE_EMAIL, +XOBJ_DTYPE_UNICODE_ARRAY → XOBJ_DTYPE_ARRAY, +XOBJ_DTYPE_UNICODE_OTHER → XOBJ_DTYPE_OTHER. +Value migration stays a core concern and is not performed here. +

+

Silent when the project Core Version is set to 2.5.

+ + diff --git a/src/main/resources/inspectionDescriptions/XoopsResultSetGuard.html b/src/main/resources/inspectionDescriptions/XoopsResultSetGuard.html index 5ec96ce..dc50fac 100644 --- a/src/main/resources/inspectionDescriptions/XoopsResultSetGuard.html +++ b/src/main/resources/inspectionDescriptions/XoopsResultSetGuard.html @@ -1,8 +1,16 @@ -Warns when fetchArray / fetchRow / fetchBoth appears without a nearby -isResultSet check. Prefer the two-part guard: -
if (!$db->isResultSet($result) || !$result instanceof \mysqli_result) { … }
+Warns when fetchArray / fetchRow / fetchBoth appears without a +dominating isResultSet check. Prefer the two-part guard: +
if (!$db->isResultSet($result) || !$result instanceof \mysqli_result) {
+    throw new \RuntimeException('Database query failed');
+}
+

+An early-exit guard covers later fetches of the same variable, including +while (list(...) = $db->fetchRow($result)), until that variable is reassigned. +The quick-fix inserts before the enclosing statement using the current PSI +(safe to apply several times after Inspect Code). +

diff --git a/src/main/resources/inspectionDescriptions/XoopsRootPathGuard.html b/src/main/resources/inspectionDescriptions/XoopsRootPathGuard.html index 5be037f..f129935 100644 --- a/src/main/resources/inspectionDescriptions/XoopsRootPathGuard.html +++ b/src/main/resources/inspectionDescriptions/XoopsRootPathGuard.html @@ -1,8 +1,20 @@ -Reports PHP files under modules/, class/, preloads/, or kernel/ -that do not contain a defined('XOOPS_ROOT_PATH') direct-access guard. -

Quick-fix inserts the standard guard after <?php.

+Reports include-only PHP files (classes, preloads, includes, blocks) that lack a +terminating defined('XOOPS_ROOT_PATH') || exit/die(...) guard. +

+The guard must be the first executable statement after <?php, +declare and namespace; a use block before or +after it is fine. die and exit are both accepted. +In namespaced files the quick-fix inserts the guard after the namespace and +before the use block (never before namespace, that is invalid PHP). +

+

+Not reported: language files, vendor/cache, tests, xoops_version.php, +admin/ control-panel scripts, files whose first work is including +mainfile.php / header.php / admin_header.php, +and directory-protection stubs that only send HTTP 404/403. +

diff --git a/src/main/resources/inspectionDescriptions/XoopsUnregisteredTemplate.html b/src/main/resources/inspectionDescriptions/XoopsUnregisteredTemplate.html new file mode 100644 index 0000000..975e47b --- /dev/null +++ b/src/main/resources/inspectionDescriptions/XoopsUnregisteredTemplate.html @@ -0,0 +1,13 @@ + + + +Unregistered XOOPS template + + +A .tpl file under the module templates/ or blocks/ +directory is not listed in xoops_version.php, so Smarty cannot +display() it as a registered module template. +

Theme overrides under themes/ are ignored.

+

Quick-fix appends a $modversion['templates'][] entry to the manifest.

+ + diff --git a/src/test/java/org/xoops/support/PluginDescriptorTest.java b/src/test/java/org/xoops/support/PluginDescriptorTest.java new file mode 100644 index 0000000..3d562bc --- /dev/null +++ b/src/test/java/org/xoops/support/PluginDescriptorTest.java @@ -0,0 +1,33 @@ +package org.xoops.support; + +import org.junit.Test; +import org.w3c.dom.Element; + +import javax.xml.parsers.DocumentBuilderFactory; +import java.io.InputStream; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +public final class PluginDescriptorTest { + + @Test + public void everyLocalInspectionHasADescriptionFile() throws Exception { + try (InputStream descriptor = getClass().getResourceAsStream("/META-INF/plugin.xml")) { + assertNotNull("plugin.xml must be on the test classpath", descriptor); + var document = DocumentBuilderFactory.newInstance().newDocumentBuilder().parse(descriptor); + var inspections = document.getElementsByTagName("localInspection"); + assertTrue("expected the XOOPS inspection pack", inspections.getLength() >= 8); + for (int i = 0; i < inspections.getLength(); i++) { + Element inspection = (Element) inspections.item(i); + String shortName = inspection.getAttribute("shortName"); + assertFalse("shortName", shortName.isBlank()); + assertNotNull( + "missing inspectionDescriptions/" + shortName + ".html", + getClass().getResource("/inspectionDescriptions/" + shortName + ".html") + ); + } + } + } +} diff --git a/src/test/java/org/xoops/support/completion/LanguageCacheRegressionTest.java b/src/test/java/org/xoops/support/completion/LanguageCacheRegressionTest.java new file mode 100644 index 0000000..412ec83 --- /dev/null +++ b/src/test/java/org/xoops/support/completion/LanguageCacheRegressionTest.java @@ -0,0 +1,92 @@ +package org.xoops.support.completion; + +import com.intellij.openapi.command.WriteCommandAction; +import com.intellij.psi.PsiDocumentManager; +import com.intellij.testFramework.fixtures.BasePlatformTestCase; + +public final class LanguageCacheRegressionTest extends BasePlatformTestCase { + public void testAncestorModulesDirectoryDoesNotChooseModule() { + assertEquals("/modules/news/", XoopsLanguageConstantsCache.moduleSegment( + "/srv/modules/work/site/modules/news/file.php")); + } + + public void testCacheIgnoresUnrelatedEditsButTracksUnsavedLanguageChanges() { + var language = myFixture.addFileToProject("modules/news/language/english/main.php", + " { + manager.getDocument(unrelated).setText(" { + manager.getDocument(language).setText(" { + try { + moved.getVirtualFile().move(this, language.getVirtualFile().getParent()); + } catch (java.io.IOException e) { + throw new RuntimeException(e); + } + }); + assertTrue(cache.getConstants().contains("_MI_MOVED")); + WriteCommandAction.runWriteCommandAction(getProject(), () -> { + try { + language.getVirtualFile().getParent().getParent().rename(this, "translations"); + } catch (java.io.IOException e) { + throw new RuntimeException(e); + } + }); + assertFalse(cache.getConstants().contains("_MI_MOVED")); + } + public void testMoveIntoEmptyLanguageDirectoryInvalidatesEmptyCache() throws Exception { + var directory = myFixture.getTempDirFixture().findOrCreateDir("modules/news/language/english"); + var moved = myFixture.addFileToProject("modules/news/moved.php", " { + try { + moved.getVirtualFile().move(this, directory); + } catch (java.io.IOException e) { + throw new RuntimeException(e); + } + }); + assertTrue(cache.getConstants().contains("_MI_FIRST")); + } + public void testRenameIntoPhpInvalidatesCache() { + var file = myFixture.addFileToProject("modules/news/language/english/main.txt", " { + try { file.getVirtualFile().rename(this, "main.php"); } + catch (java.io.IOException e) { throw new RuntimeException(e); } + }); + assertTrue(cache.getConstants().contains("_MI_RENAMED")); + } +} diff --git a/src/test/java/org/xoops/support/completion/XoopsLanguageConstantParserTest.java b/src/test/java/org/xoops/support/completion/XoopsLanguageConstantParserTest.java new file mode 100644 index 0000000..a60ad62 --- /dev/null +++ b/src/test/java/org/xoops/support/completion/XoopsLanguageConstantParserTest.java @@ -0,0 +1,118 @@ +package org.xoops.support.completion; + +import org.junit.Test; + +import java.util.List; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +public final class XoopsLanguageConstantParserTest { + + @Test + public void parsesDefineNamesIncludingSearchPhpStyleFiles() { + String php = """ + occ = XoopsLanguageConstantParser.parse(php); + assertEquals(5, occ.size()); + assertEquals("_MD_WGSIMPLEACC_FOO", occ.get(0).name()); + } + + @Test + public void extractNameAcceptsQuotedAndBare() { + assertEquals("_MD_FOO", XoopsLanguageConstantParser.extractName("'_MD_FOO'")); + assertEquals("_MD_FOO", XoopsLanguageConstantParser.extractName("\"_MD_FOO\"")); + assertEquals("_MD_FOO", XoopsLanguageConstantParser.extractName("_MD_FOO")); + assertNull(XoopsLanguageConstantParser.extractName("not_a_const")); + assertNull(XoopsLanguageConstantParser.extractName("'_XX_FOO'")); + } + + @Test + public void parseIgnoresDefineInsideStringLiterals() { + String php = """ + occ = XoopsLanguageConstantParser.parse(php); + assertEquals(2, occ.size()); + assertEquals("_MI_HELP", occ.get(0).name()); + assertEquals("_MI_REAL", occ.get(1).name()); + } + + @Test + public void parseIgnoresCommentedOutDefines() { + String php = """ + occ = XoopsLanguageConstantParser.parse(php); + assertEquals(1, occ.size()); + assertEquals("_MI_LIVE", occ.get(0).name()); + assertEquals(php.indexOf("_MI_LIVE"), occ.get(0).offset()); + } + + @Test + public void parseIgnoresEmbeddedFunctionNames() { + String php = """ + define('_MI_NOT_EITHER', 'x'); + \\define('_MI_NAMESPACED', 'y'); + define('_MI_PLAIN', 'z'); + """; + List occ = XoopsLanguageConstantParser.parse(php); + assertEquals(2, occ.size()); + assertEquals("_MI_NAMESPACED", occ.get(0).name()); + assertEquals("_MI_PLAIN", occ.get(1).name()); + } + + @Test + public void parsePreservesDistinctSpellings() { + String php = """ + occ = XoopsLanguageConstantParser.parse(php); + assertEquals(2, occ.size()); + assertEquals("_mi_foo", occ.get(0).name()); + assertEquals("_MI_FOO", occ.get(1).name()); + } + + @Test + public void extractNamePreservesSpellingAndSmartyConst() { + assertEquals("_mi_foo", XoopsLanguageConstantParser.extractName("_mi_foo")); + assertEquals("_MI_FOO", XoopsLanguageConstantParser.extractName("$smarty.const._MI_FOO")); + assertEquals("_MI_FOO", XoopsLanguageConstantParser.extractName("<{$smarty.const._MI_FOO}>")); + assertNull(XoopsLanguageConstantParser.extractName("'customer.const._MI_FOO'")); + } + + @Test + public void moduleSegmentIsExtracted() { + assertEquals("/modules/news/", + org.xoops.support.completion.XoopsLanguageConstantsCache.moduleSegment("C:/site/htdocs/modules/News/language/english/main.php")); + assertNull(org.xoops.support.completion.XoopsLanguageConstantsCache.moduleSegment("C:/site/htdocs/class/Foo.php")); + } + + @Test + public void languagePathFilter() { + assertTrue(XoopsLanguageConstantParser.isLanguagePath("C:/m/language/english/search.php")); + assertTrue(XoopsLanguageConstantParser.isLanguagePath("/modules/news/language/german/mail.php")); + assertFalse(XoopsLanguageConstantParser.isLanguagePath("/modules/news/class/Item.php")); + // The directory itself (VFS move/rename event) must invalidate the cache too. + assertTrue(XoopsLanguageConstantParser.isLanguagePath("/modules/news/language")); + assertTrue(XoopsLanguageConstantParser.isLanguagePath("C:\\m\\language")); + } +} diff --git a/src/test/java/org/xoops/support/inspections/PhpTextUtilTest.java b/src/test/java/org/xoops/support/inspections/PhpTextUtilTest.java new file mode 100644 index 0000000..b92fe50 --- /dev/null +++ b/src/test/java/org/xoops/support/inspections/PhpTextUtilTest.java @@ -0,0 +1,79 @@ +package org.xoops.support.inspections; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +public final class PhpTextUtilTest { + + + @Test + public void htmlIsOpaqueToBothInspectionMasks() { + String source = "

Don't use XOBJ_DTYPE_UNICODE_TXTBOX

" + + "$modversion['templates'][] = ['file' => 'html.tpl'];"; + for (String masked : new String[]{PhpTextUtil.maskCommentsOnly(source), + PhpTextUtil.maskCommentsAndStrings(source)}) { + assertEquals(source.length(), masked.length()); + assertFalse(masked.contains("XOBJ_DTYPE_UNICODE_TXTBOX")); + assertFalse(masked.contains("html.tpl")); + assertTrue(masked.contains("$live = 1;")); + } + } + + @Test + public void closingTagEndsLineCommentsBeforeNextPhpRegion() { + for (String comment : new String[]{"//", "#"}) { + String source = "Don't + 'real.tpl']; + """; + String commentsOnly = PhpTextUtil.maskCommentsOnly(php); + assertEquals(php.length(), commentsOnly.length()); + assertFalse(commentsOnly.contains("defined('XOOPS_ROOT_PATH')")); + assertTrue(commentsOnly.contains("$modversion['templates'][] = ['file' => 'real.tpl']")); + String full = PhpTextUtil.maskCommentsAndStrings(php); + assertFalse(full.contains("real.tpl")); + assertTrue(full.contains("$modversion")); + } + + @Test + public void fullMaskWipesHeredocButKeepsOffsets() { + String php = "isResultSet($result) === false", + "$db->isResultSet($result) ? false : true"}) { + assertEquals(condition, 1, XoopsResultSetGuardInspection.unguardedFetchOffsets( + "fetchArray($result); }").size()); + } + assertEquals(1, XoopsResultSetGuardInspection.unguardedFetchOffsets( + "isResultSet($result) === false) return; $db->fetchArray($result);").size()); + } + + public void testRegistrationExamplesInStringsAreIgnored() { + String php = """ + 'real.tpl', + 'description' => "Example: 'file' => 'ghost.tpl'"]; + $help = '$modversion["templates"][] = ["file" => "fake.tpl"];'; + """; + assertEquals(java.util.Set.of("templates/real.tpl"), XoopsManifestTemplates.keys(php)); + } + + public void testRegistrationStaysInsidePhp() { + int module = 0; + for (String suffix : new String[]{"?>

HTML ?> suffix

", + "$note = '?>';", "?>HTML';"}) { + var manifest = myFixture.addFileToProject("module" + module + "/xoops_version.php", " + new RegisterTemplateQuickFix("new.tpl").applyFix(getProject(), descriptor)); + PsiDocumentManager.getInstance(getProject()).commitAllDocuments(); + assertEquals(suffix, java.util.Set.of("templates/new.tpl"), + XoopsManifestTemplates.keys(manifest.getText())); + } + } + public void testTemplateCaseMismatchIsReportedWithoutCreateFix() { + var manifest = myFixture.addFileToProject("case-module/xoops_version.php", + " 'Admin/Foo.tpl'];"); + var template = myFixture.addFileToProject("case-module/templates/admin/foo.tpl", "template"); + var holder = new com.intellij.codeInspection.ProblemsHolder( + InspectionManager.getInstance(getProject()), manifest, false); + new XoopsMissingRegisteredTemplateInspection().buildVisitor(holder, false).visitFile(manifest); + assertEquals(1, holder.getResults().size()); + assertTrue(holder.getResults().get(0).getDescriptionTemplate().contains("case mismatch")); + var fixes = holder.getResults().get(0).getFixes(); + assertTrue(fixes == null || fixes.length == 0); + var inverse = new com.intellij.codeInspection.ProblemsHolder( + InspectionManager.getInstance(getProject()), template, false); + new XoopsUnregisteredTemplateInspection().buildVisitor(inverse, false).visitFile(template); + assertTrue(inverse.getResults().isEmpty()); + } + public void testPrefixedPathsPreserveCase() { + assertEquals("Templates/Admin.tpl", XoopsManifestTemplates.diskPath("Templates/Admin.tpl", false)); + assertEquals("Blocks/Header.tpl", XoopsManifestTemplates.diskPath("Blocks/Header.tpl", true)); + } + public void testHeredocDescriptionDoesNotHideRegistration() { + String source = """ + << 'real.tpl' + ]; + """; + assertEquals(java.util.Set.of("templates/real.tpl"), XoopsManifestTemplates.keys(source)); + } + public void testArbitraryInstanceofDoesNotGuardFetch() { + assertEquals(1, XoopsResultSetGuardInspection.unguardedFetchOffsets( + "fetchArray($result);").size()); + } + public void testVendorManifestIsIgnored() { + for (String prefix : new String[]{"vendor", "cache", "templates_c", "node_modules"}) { + var file = myFixture.addFileToProject(prefix + "/demo/xoops_version.php", + " 'missing.tpl'];"); + var holder = new com.intellij.codeInspection.ProblemsHolder(InspectionManager.getInstance(getProject()), file, false); + new XoopsMissingRegisteredTemplateInspection().buildVisitor(holder, false).visitFile(file); + assertTrue(prefix, holder.getResults().isEmpty()); + } + } + public void testQuickFixDoesNotHoistGuardOutOfConditional() { + int index = 0; + for (String control : new String[]{"if ($enabled)", "while ($enabled)", + "for ($i = 0; $i < 2; $i++)", "foreach ($items as $item)"}) { + String source = "fetchArray($result);"; + var file = myFixture.addFileToProject("modules/demo/include/example" + index++ + ".php", source); + var leaf = file.findElementAt(source.indexOf("$db")); + var descriptor = InspectionManager.getInstance(getProject()).createProblemDescriptor( + leaf, "Unguarded", false, new com.intellij.codeInspection.LocalQuickFix[0], ProblemHighlightType.GENERIC_ERROR_OR_WARNING); + WriteCommandAction.runWriteCommandAction(getProject(), () -> + new InsertBeforeStatementQuickFix("$db", "$result").applyFix(getProject(), descriptor)); + assertEquals(source, file.getText()); + var holder = new com.intellij.codeInspection.ProblemsHolder(InspectionManager.getInstance(getProject()), file, false); + new XoopsResultSetGuardInspection().buildVisitor(holder, false).visitFile(file); + assertEquals(1, holder.getResults().size()); + var fixes = holder.getResults().get(0).getFixes(); + assertTrue(fixes == null || fixes.length == 0); + } + } + public void testGuardFixStillWorksInBracedBody() { + String source = "fetchArray($result); }"; + var file = myFixture.addFileToProject("modules/demo/include/braced.php", source); + var holder = new com.intellij.codeInspection.ProblemsHolder(InspectionManager.getInstance(getProject()), file, false); + new XoopsResultSetGuardInspection().buildVisitor(holder, false).visitFile(file); + assertEquals(1, holder.getResults().size()); + var fix = (com.intellij.codeInspection.LocalQuickFix) holder.getResults().get(0).getFixes()[0]; + WriteCommandAction.runWriteCommandAction(getProject(), () -> fix.applyFix(getProject(), holder.getResults().get(0))); + assertTrue(file.getText().startsWith(" <<<" + opener + + "\nText; text\n DESC, 'file' => 'real.tpl'];\n" + + "$modversion['blocks'][1]['template'] = 'block.tpl';"; + assertEquals(java.util.Set.of("templates/real.tpl", "templates/blocks/block.tpl"), + XoopsManifestTemplates.keys(source)); + } + } +} diff --git a/src/test/java/org/xoops/support/inspections/XoopsDeprecatedUnicodeDtypeTest.java b/src/test/java/org/xoops/support/inspections/XoopsDeprecatedUnicodeDtypeTest.java new file mode 100644 index 0000000..4cf1832 --- /dev/null +++ b/src/test/java/org/xoops/support/inspections/XoopsDeprecatedUnicodeDtypeTest.java @@ -0,0 +1,19 @@ +package org.xoops.support.inspections; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; + +public final class XoopsDeprecatedUnicodeDtypeTest { + + @Test + public void successorMapCoversTheSixCoreAliases() { + assertEquals(6, XoopsDeprecatedUnicodeDtypeInspection.SUCCESSOR.size()); + assertEquals("XOBJ_DTYPE_TXTBOX", XoopsDeprecatedUnicodeDtypeInspection.SUCCESSOR.get("XOBJ_DTYPE_UNICODE_TXTBOX")); + assertEquals("XOBJ_DTYPE_TXTAREA", XoopsDeprecatedUnicodeDtypeInspection.SUCCESSOR.get("XOBJ_DTYPE_UNICODE_TXTAREA")); + assertEquals("XOBJ_DTYPE_URL", XoopsDeprecatedUnicodeDtypeInspection.SUCCESSOR.get("XOBJ_DTYPE_UNICODE_URL")); + assertEquals("XOBJ_DTYPE_EMAIL", XoopsDeprecatedUnicodeDtypeInspection.SUCCESSOR.get("XOBJ_DTYPE_UNICODE_EMAIL")); + assertEquals("XOBJ_DTYPE_ARRAY", XoopsDeprecatedUnicodeDtypeInspection.SUCCESSOR.get("XOBJ_DTYPE_UNICODE_ARRAY")); + assertEquals("XOBJ_DTYPE_OTHER", XoopsDeprecatedUnicodeDtypeInspection.SUCCESSOR.get("XOBJ_DTYPE_UNICODE_OTHER")); + } +} diff --git a/src/test/java/org/xoops/support/inspections/XoopsManifestTemplatesTest.java b/src/test/java/org/xoops/support/inspections/XoopsManifestTemplatesTest.java new file mode 100644 index 0000000..f277625 --- /dev/null +++ b/src/test/java/org/xoops/support/inspections/XoopsManifestTemplatesTest.java @@ -0,0 +1,76 @@ +package org.xoops.support.inspections; + +import org.junit.Test; + +import java.util.List; +import java.util.Set; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +public final class XoopsManifestTemplatesTest { + + @Test + public void registrationLikeTextOutsideModversionIsIgnored() { + String manifest = """ + 'ghost.tpl'"; + $other['templates'][] = ['file' => 'not_modversion.tpl']; + $modversion['templates'][] = ['file' => 'real.tpl', 'description' => 'x']; + """; + Set keys = XoopsManifestTemplates.keys(manifest); + assertTrue(keys.contains("templates/real.tpl")); + assertFalse(keys.contains("templates/ghost.tpl")); + assertFalse(keys.contains("templates/not_modversion.tpl")); + } + + @Test + public void singleStatementArrayYieldsEveryFile() { + String manifest = """ + 'a.tpl', 'description' => 'see https://xoops.org; ok'], + ['file' => 'b.tpl'], + ]; + $modversion['blocks'][1]['template'] = 'c_block.tpl'; + // $modversion['templates'][] = ['file' => 'commented.tpl']; + """; + List regs = + XoopsManifestTemplates.find(PhpTextUtil.maskCommentsOnly(manifest)); + assertEquals(3, regs.size()); + assertEquals("a.tpl", regs.get(0).name()); + assertEquals(XoopsManifestTemplates.Section.TEMPLATES, regs.get(0).section()); + assertEquals(manifest.indexOf("a.tpl"), regs.get(0).nameOffset()); + assertEquals("b.tpl", regs.get(1).name()); + assertEquals("c_block.tpl", regs.get(2).name()); + assertEquals(XoopsManifestTemplates.Section.BLOCKS, regs.get(2).section()); + assertTrue(regs.get(2).block()); + } + + @Test + public void diskPathPutsBareBlockNamesUnderTemplatesBlocks() { + assertEquals("templates/demo.tpl", XoopsManifestTemplates.diskPath("demo.tpl", false)); + assertEquals("templates/blocks/demo_block.tpl", XoopsManifestTemplates.diskPath("demo_block.tpl", true)); + assertEquals("templates/rooted.tpl", XoopsManifestTemplates.diskPath("templates/rooted.tpl", false)); + assertEquals("blocks/legacy.tpl", XoopsManifestTemplates.diskPath("blocks/legacy.tpl", true)); + } + + @Test + public void registrationSpellingPreservesNestedDirectories() { + assertEquals("foo.tpl", XoopsManifestTemplates.registrationName("templates/foo.tpl")); + for (String path : new String[]{"templates/templates/foo.tpl", "templates/blocks/foo.tpl", "blocks/foo.tpl"}) { + assertEquals(path, XoopsManifestTemplates.registrationName(path)); + assertEquals(path, XoopsManifestTemplates.diskPath(XoopsManifestTemplates.registrationName(path), false)); + } + } + + @Test + public void keysCoverBareAndPrefixedSpellings() { + Set keys = XoopsManifestTemplates.keys( + " 'templates/blocks/deep_block.tpl'];"); + assertTrue(keys.contains("templates/blocks/deep_block.tpl")); + assertFalse(keys.contains("blocks/deep_block.tpl")); + assertFalse(keys.contains("deep_block.tpl")); + } +} diff --git a/src/test/java/org/xoops/support/inspections/XoopsResultSetGuardInspectionTest.java b/src/test/java/org/xoops/support/inspections/XoopsResultSetGuardInspectionTest.java new file mode 100644 index 0000000..7a08b70 --- /dev/null +++ b/src/test/java/org/xoops/support/inspections/XoopsResultSetGuardInspectionTest.java @@ -0,0 +1,277 @@ +package org.xoops.support.inspections; + +import org.junit.Test; + +import java.util.List; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +public final class XoopsResultSetGuardInspectionTest { + + @Test + public void unguardedFetchIsReported() { + String php = """ + query($sql); + while (list($sumIn, $sumOut) = $xoopsDB->fetchRow($result)) { + $amount += $sumIn; + } + """; + List hits = XoopsResultSetGuardInspection.unguardedFetchOffsets(php); + assertEquals(1, hits.size()); + } + + @Test + public void whileConditionFetchIsCoveredByPrecedingEarlyExit() { + String php = """ + query($sql); + if (!$xoopsDB->isResultSet($result) || !$result instanceof \\mysqli_result) { + throw new \\RuntimeException('Database query failed'); + } + while (list($sumIn, $sumOut) = $xoopsDB->fetchRow($result)) { + $amount += $sumIn; + } + """; + assertTrue(XoopsResultSetGuardInspection.unguardedFetchOffsets(php).isEmpty()); + } + + @Test + public void reassignmentClearsTheGuard() { + String php = """ + query($sql); + if (!$xoopsDB->isResultSet($result) || !$result instanceof \\mysqli_result) { + throw new \\RuntimeException('Database query failed'); + } + $result = $xoopsDB->query($sql2); + $row = $xoopsDB->fetchArray($result); + """; + assertEquals(1, XoopsResultSetGuardInspection.unguardedFetchOffsets(php).size()); + } + + @Test + public void positiveIfBodyIsGuarded() { + String php = """ + query($sql); + if ($db->isResultSet($result)) { + $row = $db->fetchArray($result); + } + """; + assertTrue(XoopsResultSetGuardInspection.unguardedFetchOffsets(php).isEmpty()); + } + + @Test + public void orFallbackIsNotASafePositiveGuard() { + String php = """ + query($sql); + if ($db->isResultSet($result) || $fallback) { + $row = $db->fetchArray($result); + } + """; + assertEquals(1, XoopsResultSetGuardInspection.unguardedFetchOffsets(php).size()); + } + + @Test + public void reassignmentInsidePositiveGuardIsUnguarded() { + String php = """ + query($sql); + if ($db->isResultSet($result)) { + $result = $db->query($sql2); + $row = $db->fetchArray($result); + } + """; + assertEquals(1, XoopsResultSetGuardInspection.unguardedFetchOffsets(php).size()); + } + + @Test + public void fetchOnRightHandSideOfAssignmentIsStillGuarded() { + String php = """ + query($sql); + if ($db->isResultSet($result)) { + $result = $db->fetchArray($result); + } + """; + assertTrue(XoopsResultSetGuardInspection.unguardedFetchOffsets(php).isEmpty()); + } + + @Test + public void completedAssignmentBeforeFetchInSameStatementIsUnguarded() { + String php = """ + query($sql); + if (!$db->isResultSet($result)) { + return; + } + ($result = false) || $db->fetchArray($result); + """; + assertEquals(1, XoopsResultSetGuardInspection.unguardedFetchOffsets(php).size()); + int fetchAt = php.indexOf("fetchArray"); + assertFalse(XoopsResultSetGuardInspection.isFetchGuardedAt(php, fetchAt, "$result")); + } + + @Test + public void wordOrOperatorCompletesAssignmentBeforeFetch() { + String php = """ + query($sql); + if (!$db->isResultSet($result)) { + return; + } + $result = false or $db->fetchArray($result); + """; + assertEquals(1, XoopsResultSetGuardInspection.unguardedFetchOffsets(php).size()); + } + + @Test + public void tightBindingOperatorsKeepFetchInsideAssignment() { + for (String op : new String[] {"?:", "??", "||", "&&"}) { + String php = """ + query($sql); + if ($db->isResultSet($result)) { + $result = $a %s $db->fetchArray($result); + } + """.formatted(op); + assertTrue(op, XoopsResultSetGuardInspection.unguardedFetchOffsets(php).isEmpty()); + } + } + + @Test + public void xorEarlyExitIsNotADominatingGuard() { + String php = """ + query($sql); + if (!$db->isResultSet($result) xor $fallback) { + return; + } + $row = $db->fetchArray($result); + """; + assertEquals(1, XoopsResultSetGuardInspection.unguardedFetchOffsets(php).size()); + } + + @Test + public void xorPositiveGuardIsNotAGuard() { + String php = """ + query($sql); + if ($db->isResultSet($result) xor $fallback) { + $row = $db->fetchArray($result); + } + """; + assertEquals(1, XoopsResultSetGuardInspection.unguardedFetchOffsets(php).size()); + } + + @Test + public void closureInsidePositiveGuardIsNotGuarded() { + String php = """ + query($sql); + if ($db->isResultSet($result)) { + $later = function () use ($db, &$result) { + return $db->fetchArray($result); + }; + } + """; + assertEquals(1, XoopsResultSetGuardInspection.unguardedFetchOffsets(php).size()); + } + + @Test + public void elseifEarlyExitDoesNotDominate() { + String php = """ + query($sql); + if ($skip) { + $log->info('skipped'); + } elseif (!$db->isResultSet($result)) { + return; + } + $row = $db->fetchArray($result); + """; + assertEquals(1, XoopsResultSetGuardInspection.unguardedFetchOffsets(php).size()); + } + + @Test + public void commentContainingIsResultSetDoesNotGuard() { + String php = """ + query($sql); + // if (!$db->isResultSet($result)) { return; } + $row = $db->fetchArray($result); + """; + assertEquals(1, XoopsResultSetGuardInspection.unguardedFetchOffsets(php).size()); + int fetchAt = php.indexOf("fetchArray"); + assertFalse(XoopsResultSetGuardInspection.isFetchGuardedAt(php, fetchAt, "$result")); + } + + @Test + public void earlyExitStillGuardsLaterFetch() { + String php = """ + query($sql); + if (!$db->isResultSet($result)) { + throw new \\RuntimeException('fail'); + } + $row = $db->fetchArray($result); + """; + int fetchAt = php.indexOf("fetchArray"); + assertTrue(XoopsResultSetGuardInspection.isFetchGuardedAt(php, fetchAt, "$result")); + } + + @Test + public void commentAssignmentDoesNotCountAsAssignBeforeFetch() { + for (String prefix : new String[]{"// $result = ignored\n$row = ", + "$log = '$result = ignored';\n$row = "}) { + assertFalse(InsertBeforeStatementQuickFix.assignsResultBeforeFetch( + prefix, 0, prefix.length(), "$result")); + } + String prefix = "($result = $db->query($sql)) && "; + assertTrue(InsertBeforeStatementQuickFix.assignsResultBeforeFetch( + prefix, 0, prefix.length(), "$result")); + } + + @Test + public void assignmentCheckMasksWholePhpDocumentBeforeSlicing() { + String source = "fetchArray($result));"; + assertFalse(InsertBeforeStatementQuickFix.assignsResultBeforeFetch( + source, source.indexOf("log("), source.indexOf("$db->fetchArray"), "$result")); + } + @Test + public void nestedNegationsDoNotProveAnEarlyExit() { + for (String condition : new String[]{"!!$db->isResultSet($result)", + "!(!$db->isResultSet($result))", "!($other || !$db->isResultSet($result))", + "!!($result instanceof \\mysqli_result)"}) { + assertEquals(condition, 1, XoopsResultSetGuardInspection.unguardedFetchOffsets( + "fetchArray($result);").size()); + } + } + + @Test + public void unbracedControlBodiesDoNotDominateLaterFetches() { + for (String prefix : new String[]{"if ($enabled)", "while ($enabled)", + "foreach ($items as $item)", "for ($i = 0; $i < 2; $i++)"}) { + String source = "isResultSet($result)) return; $db->fetchArray($result);"; + assertEquals(prefix, 1, XoopsResultSetGuardInspection.unguardedFetchOffsets(source).size()); + assertFalse(prefix, XoopsResultSetGuardInspection.isFetchGuardedAt(source, + source.indexOf("$db->fetchArray"), "$result")); + } + assertTrue(XoopsResultSetGuardInspection.unguardedFetchOffsets( + "isResultSet($result)) return; $db->fetchArray($result); }").isEmpty()); + } + @Test + public void unrecognizedNegatedGroupsDoNotProvePositiveGuards() { + for (String condition : new String[]{"!(($db->isResultSet($result)))", + "!($other || $db->isResultSet($result))"}) { + assertEquals(condition, 1, XoopsResultSetGuardInspection.unguardedFetchOffsets( + "fetchArray($result); }").size()); + } + assertTrue(XoopsResultSetGuardInspection.unguardedFetchOffsets( + "isResultSet($result))) return; $db->fetchArray($result);").isEmpty()); + } +} diff --git a/src/test/java/org/xoops/support/inspections/XoopsRootPathGuardPolicyTest.java b/src/test/java/org/xoops/support/inspections/XoopsRootPathGuardPolicyTest.java new file mode 100644 index 0000000..d5c1df2 --- /dev/null +++ b/src/test/java/org/xoops/support/inspections/XoopsRootPathGuardPolicyTest.java @@ -0,0 +1,238 @@ +package org.xoops.support.inspections; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +public final class XoopsRootPathGuardPolicyTest { + + @Test + public void recognizesDieGuardAfterNamespaceAndUse() { + String source = """ + 0); + String before = source.substring(0, offset); + String after = source.substring(offset); + assertTrue(before.contains("namespace XoopsModules\\Wgsimpleacc;")); + assertFalse(before.contains("use Foo")); + assertTrue(after.stripLeading().startsWith("use Foo")); + } + + @Test + public void flagsUnguardedClassFile() { + assertTrue(XoopsRootPathGuardPolicy.requiresGuard( + "C:/site/htdocs/modules/wgsimpleacc/class/Accounts.php", + "\n"; + assertTrue(XoopsRootPathGuardPolicy.requiresGuard( + "C:/site/htdocs/modules/demo/include/view.php", + source + )); + assertEquals(-1, XoopsRootPathGuardPolicy.insertOffset(source)); + assertFalse(XoopsRootPathGuardPolicy.canInsertGuard(source)); + assertTrue(XoopsRootPathGuardPolicy.canInsertGuard("\n")); + assertTrue(XoopsRootPathGuardPolicy.canInsertGuard("plain text, no php\n")); + } + + @Test + public void echoOpenTagAloneStillRequiresGuard() { + String source = "\n"; + assertTrue(XoopsRootPathGuardPolicy.requiresGuard( + "C:/site/htdocs/modules/demo/include/view.php", + source + )); + assertEquals(-1, XoopsRootPathGuardPolicy.insertOffset(source)); + assertTrue(XoopsRootPathGuardPolicy.canInsertGuard(source)); + } + + @Test + public void unmatchedHtmlApostropheDoesNotHidePhpBody() { + String source = """ +
+ 0); + String before = source.substring(0, offset); + assertTrue(before.contains("namespace Example;")); + assertFalse(before.contains("namespace Example;\nuse")); + assertTrue(source.substring(offset).stripLeading().startsWith("use Foo")); + } + + @Test + public void multipleDeclareStatementsAreSkipped() { + String source = """ + 'demo_index.tpl', + 'description' => 'Index', + ]; + $modversion['templates'][] = ['template' => 'demo_block.tpl']; + // 'file' => 'commented.tpl' + """; + Set names = XoopsUnregisteredTemplateInspection.registeredTemplates(manifest); + assertTrue(names.contains("templates/demo_index.tpl")); + assertTrue(names.contains("templates/demo_block.tpl")); + assertFalse(names.contains("templates/commented.tpl")); + } + + @Test + public void commentMarkersInsideStringsAreNotComments() { + String manifest = """ + 'see https://xoops.org #1', 'file' => 'listed.tpl']; + // 'file' => 'commented.tpl' + """; + Set names = XoopsUnregisteredTemplateInspection.registeredTemplates(manifest); + assertTrue(names.contains("templates/listed.tpl")); + assertFalse(names.contains("templates/commented.tpl")); + } + + @Test + public void assignmentSyntaxRegistersTemplatesToo() { + String manifest = """ + names = XoopsUnregisteredTemplateInspection.registeredTemplates(manifest); + assertTrue(names.contains("templates/blocks/demo_block.tpl")); + assertTrue(names.contains("templates/demo_list.tpl")); + } + + @Test + public void moduleRootPrefixedRegistrationsMatchRelativeNames() { + String manifest = """ + 'templates/rooted.tpl']; + $modversion['templates'][] = ['template' => 'blocks/rooted_block.tpl']; + """; + Set names = XoopsUnregisteredTemplateInspection.registeredTemplates(manifest); + assertTrue(names.contains("templates/rooted.tpl")); + assertTrue(names.contains("blocks/rooted_block.tpl")); + assertTrue(names.contains("templates/rooted.tpl")); + Set full = XoopsUnregisteredTemplateInspection.registeredTemplates( + " 'templates/blocks/deep_block.tpl'];"); + assertTrue(full.contains("templates/blocks/deep_block.tpl")); + assertFalse(full.contains("deep_block.tpl")); + } +} diff --git a/src/test/java/org/xoops/support/scanner/XoopsProjectScannerTest.java b/src/test/java/org/xoops/support/scanner/XoopsProjectScannerTest.java new file mode 100644 index 0000000..d3442c3 --- /dev/null +++ b/src/test/java/org/xoops/support/scanner/XoopsProjectScannerTest.java @@ -0,0 +1,315 @@ +package org.xoops.support.scanner; + +import org.junit.Test; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +public final class XoopsProjectScannerTest { + + + @Test + public void pageTemplateDoesNotSatisfyBlockRegistration() throws Exception { + Path moduleRoot = Files.createTempDirectory("xoops-template-kind"); + try { + Files.writeString(moduleRoot.resolve("xoops_version.php"), + " findings = new ArrayList<>(); + XoopsProjectScanner.checkRegisteredTemplates(moduleRoot, findings); + assertTrue(findings.stream().anyMatch(f -> "MISSING_REGISTERED_TEMPLATE".equals(f.kind()))); + assertTrue(findings.stream().anyMatch(f -> "UNREGISTERED_TEMPLATE".equals(f.kind()))); + Files.createDirectories(moduleRoot.resolve("templates/blocks")); + Files.writeString(moduleRoot.resolve("templates/blocks/shared.tpl"), "block"); + findings.clear(); + XoopsProjectScanner.checkRegisteredTemplates(moduleRoot, findings); + assertFalse(findings.stream().anyMatch(f -> "MISSING_REGISTERED_TEMPLATE".equals(f.kind()))); + assertEquals(1, findings.stream().filter(f -> "UNREGISTERED_TEMPLATE".equals(f.kind())).count()); + } finally { + deleteRecursively(moduleRoot); + } + } + + @Test + public void versionPatternsPinTheDocumentedCoreLines() { + assertTrue(XoopsProjectScanner.VERSION_25.matcher("XOOPS 2.5.11").find()); + assertTrue(XoopsProjectScanner.VERSION_27.matcher("XOOPS 2.7.3").find()); + assertTrue(XoopsProjectScanner.VERSION_40.matcher("XOOPS 4.0.0").find()); + assertFalse(XoopsProjectScanner.VERSION_25.matcher("XOOPS 2.7.3").find()); + assertFalse(XoopsProjectScanner.VERSION_27.matcher("version 12.5-extra").find()); + assertFalse(XoopsProjectScanner.VERSION_25.matcher("version 12.5").find()); + assertFalse(XoopsProjectScanner.VERSION_27.matcher("php: 12.7").find()); + assertFalse(XoopsProjectScanner.VERSION_40.matcher("build 14.0").find()); + } + + @Test + public void coreVersionSettingOverridesAutoDetection() { + assertEquals(CoreVersion.XOOPS_25, XoopsProjectScanner.coreVersionFromSetting("2.5")); + assertEquals(CoreVersion.XOOPS_27, XoopsProjectScanner.coreVersionFromSetting("2.7")); + assertEquals(CoreVersion.XOOPS_40, XoopsProjectScanner.coreVersionFromSetting("4.0")); + assertEquals(null, XoopsProjectScanner.coreVersionFromSetting("Auto")); + Path ignored = Path.of("."); + assertEquals( + CoreVersion.XOOPS_27, + XoopsProjectScanner.resolveScanCoreVersion("2.7", true, ignored, ignored) + ); + assertEquals( + CoreVersion.MODULE_ONLY, + XoopsProjectScanner.resolveScanCoreVersion("Auto", true, ignored, ignored) + ); + } + + @Test + public void manifestDirnameAndRegisteredTemplateRegexes() { + String manifest = """ + $modversion['dirname'] = 'wgsimpleacc'; + $modversion['templates'][] = ['file' => 'wgsimpleacc_index.tpl', 'description' => '']; + """; + var regs = org.xoops.support.inspections.XoopsManifestTemplates.find(manifest); + assertEquals(1, regs.size()); + assertEquals("wgsimpleacc_index.tpl", regs.get(0).name()); + } + + @Test + public void mutatingQueryAndSmartyDelimiterRegexes() { + assertTrue(XoopsProjectScanner.MUTATING_QUERY.matcher("$db->query('INSERT INTO t')").find()); + assertFalse(XoopsProjectScanner.MUTATING_QUERY.matcher("$db->query('SELECT * FROM t')").find()); + assertTrue(XoopsProjectScanner.WRONG_SMARTY.matcher("{if $x}").find()); + assertFalse(XoopsProjectScanner.WRONG_SMARTY.matcher("<{if $x}>").find()); + assertTrue(XoopsProjectScanner.RAW_REQUEST.matcher("$_REQUEST['id']").find()); + assertTrue(XoopsProjectScanner.QUERY_F.matcher("$db->queryF($sql)").find()); + assertTrue(XoopsProjectScanner.QUOTE_STRING.matcher("$db->quoteString($s)").find()); + } + + @Test + public void inverseTemplateScan() throws Exception { + Path moduleRoot = Files.createTempDirectory("xoops-mod"); + try { + Files.writeString(moduleRoot.resolve("xoops_version.php"), """ + 'listed.tpl', 'description' => '']; + """); + Path templates = moduleRoot.resolve("templates"); + Files.createDirectories(templates); + Files.writeString(templates.resolve("listed.tpl"), "<{if $x}><{/if}>\n"); + Files.writeString(templates.resolve("orphan.tpl"), "<{if $y}><{/if}>\n"); + + List findings = new ArrayList<>(); + XoopsProjectScanner.checkRegisteredTemplates(moduleRoot, findings); + assertTrue(findings.stream().anyMatch(f -> "UNREGISTERED_TEMPLATE".equals(f.kind()) + && f.message().contains("orphan.tpl"))); + assertFalse(findings.stream().anyMatch(f -> f.message().contains("listed.tpl") + && "UNREGISTERED_TEMPLATE".equals(f.kind()))); + } finally { + deleteRecursively(moduleRoot); + } + } + + @Test + public void moduleRootRelativeRegistrationMatchesWalkKey() throws Exception { + Path moduleRoot = Files.createTempDirectory("xoops-mod"); + try { + Files.writeString(moduleRoot.resolve("xoops_version.php"), """ + 'templates/rooted.tpl', 'description' => '']; + """); + Path templates = moduleRoot.resolve("templates"); + Files.createDirectories(templates); + Files.writeString(templates.resolve("rooted.tpl"), "<{$x}>"); + + List findings = new ArrayList<>(); + XoopsProjectScanner.checkRegisteredTemplates(moduleRoot, findings); + assertTrue(findings.isEmpty()); + } finally { + deleteRecursively(moduleRoot); + } + } + + @Test + public void blockTemplateUnderTemplatesBlocksIsRegisteredByBareName() throws Exception { + Path moduleRoot = Files.createTempDirectory("xoops-mod"); + try { + Files.writeString(moduleRoot.resolve("xoops_version.php"), """ + "); + + List findings = new ArrayList<>(); + XoopsProjectScanner.checkRegisteredTemplates(moduleRoot, findings); + assertTrue(findings.toString(), findings.isEmpty()); + } finally { + deleteRecursively(moduleRoot); + } + } + + @Test + public void commentedManifestEntryIsNotARegistration() throws Exception { + Path moduleRoot = Files.createTempDirectory("xoops-mod"); + try { + Files.writeString(moduleRoot.resolve("xoops_version.php"), """ + 'ghost2.tpl']; */ + """); + Files.createDirectories(moduleRoot.resolve("templates")); + List findings = new ArrayList<>(); + XoopsProjectScanner.checkRegisteredTemplates(moduleRoot, findings); + assertTrue(findings.toString(), findings.isEmpty()); + } finally { + deleteRecursively(moduleRoot); + } + } + + @Test + public void urlInDescriptionDoesNotHideRegistrationOnSameLine() throws Exception { + Path moduleRoot = Files.createTempDirectory("xoops-mod"); + try { + Files.writeString(moduleRoot.resolve("xoops_version.php"), """ + 'see https://xoops.org #1', 'file' => 'listed.tpl']; + """); + Path templates = moduleRoot.resolve("templates"); + Files.createDirectories(templates); + Files.writeString(templates.resolve("listed.tpl"), "<{$x}>"); + List findings = new ArrayList<>(); + XoopsProjectScanner.checkRegisteredTemplates(moduleRoot, findings); + assertTrue(findings.toString(), findings.isEmpty()); + } finally { + deleteRecursively(moduleRoot); + } + } + + @Test + public void moduleJsonOnlyModuleIsNotScannedForTemplates() throws Exception { + Path moduleRoot = Files.createTempDirectory("xoops-mod"); + try { + Files.writeString(moduleRoot.resolve("module.json"), "{\"name\": \"demo\"}"); + Path templates = moduleRoot.resolve("templates"); + Files.createDirectories(templates); + Files.writeString(templates.resolve("demo_index.tpl"), "<{$x}>"); + List findings = new ArrayList<>(); + XoopsProjectScanner.checkRegisteredTemplates(moduleRoot, findings); + assertTrue(findings.toString(), findings.isEmpty()); + } finally { + deleteRecursively(moduleRoot); + } + } + + @Test + public void oversizedManifestReportsScanErrorNotUnregisteredTemplates() throws Exception { + Path moduleRoot = Files.createTempDirectory("xoops-mod"); + try { + byte[] big = new byte[1_500_001]; + java.util.Arrays.fill(big, (byte) ' '); + Files.write(moduleRoot.resolve("xoops_version.php"), big); + Path templates = moduleRoot.resolve("templates"); + Files.createDirectories(templates); + Files.writeString(templates.resolve("demo_index.tpl"), "<{$x}>"); + + List findings = new ArrayList<>(); + XoopsProjectScanner.checkRegisteredTemplates(moduleRoot, findings); + assertEquals(findings.toString(), 1, findings.size()); + assertEquals("SCAN_ERROR", findings.get(0).kind()); + } finally { + deleteRecursively(moduleRoot); + } + } + + @Test + public void registrationLikeStringIsNotAMissingTemplate() throws Exception { + Path moduleRoot = Files.createTempDirectory("xoops-mod"); + try { + Files.writeString(moduleRoot.resolve("xoops_version.php"), """ + 'ghost.tpl'"; + """); + Files.createDirectories(moduleRoot.resolve("templates")); + List findings = new ArrayList<>(); + XoopsProjectScanner.checkRegisteredTemplates(moduleRoot, findings); + assertTrue(findings.toString(), findings.isEmpty()); + } finally { + deleteRecursively(moduleRoot); + } + } + + @Test + public void missingRegisteredTemplateScan() throws Exception { + Path moduleRoot = Files.createTempDirectory("xoops-mod"); + try { + Files.writeString(moduleRoot.resolve("xoops_version.php"), """ + 'ghost.tpl', 'description' => '']; + """); + Files.createDirectories(moduleRoot.resolve("templates")); + List findings = new ArrayList<>(); + XoopsProjectScanner.checkRegisteredTemplates(moduleRoot, findings); + assertTrue(findings.stream().anyMatch(f -> "MISSING_REGISTERED_TEMPLATE".equals(f.kind()) + && f.message().contains("ghost.tpl"))); + } finally { + deleteRecursively(moduleRoot); + } + } + + private static void deleteRecursively(Path root) throws Exception { + if (!Files.exists(root)) { + return; + } + try (var walk = Files.walk(root)) { + walk.sorted((a, b) -> Integer.compare(b.getNameCount(), a.getNameCount())) + .forEach(p -> { + try { + Files.deleteIfExists(p); + } catch (Exception ignored) { + // temp cleanup + } + }); + } + } + @Test + public void templateCaseMismatchIsNeitherMissingNorUnregistered() throws Exception { + Path root = Files.createTempDirectory("xoops-template-case"); + try { + Files.createDirectories(root.resolve("templates/admin")); + Files.writeString(root.resolve("templates/admin/foo.tpl"), "template"); + Files.writeString(root.resolve("xoops_version.php"), + " 'Admin/Foo.tpl'];"); + List findings = new ArrayList<>(); + XoopsProjectScanner.checkRegisteredTemplates(root, findings); + assertEquals(1, findings.size()); + assertEquals("TEMPLATE_CASE_MISMATCH", findings.get(0).kind()); + Files.writeString(root.resolve("xoops_version.php"), + " 'admin/foo.tpl'];"); + findings.clear(); + XoopsProjectScanner.checkRegisteredTemplates(root, findings); + assertTrue(findings.isEmpty()); + } finally { + deleteRecursively(root); + } + } + @Test + public void harmlessPathComponentsDoNotCreateTemplateFindings() throws Exception { + Path root = Files.createTempDirectory("xoops-template-dots"); + try { + Files.createDirectories(root.resolve("templates/admin")); + Files.writeString(root.resolve("templates/admin/foo.tpl"), "template"); + Files.writeString(root.resolve("xoops_version.php"), + " './admin//./foo.tpl'];"); + List findings = new ArrayList<>(); + XoopsProjectScanner.checkRegisteredTemplates(root, findings); + assertTrue(findings.toString(), findings.isEmpty()); + } finally { + deleteRecursively(root); + } + } +} diff --git a/src/test/java/org/xoops/support/settings/XoopsSettingsStateTest.java b/src/test/java/org/xoops/support/settings/XoopsSettingsStateTest.java new file mode 100644 index 0000000..f0f26f8 --- /dev/null +++ b/src/test/java/org/xoops/support/settings/XoopsSettingsStateTest.java @@ -0,0 +1,49 @@ +package org.xoops.support.settings; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; + +public final class XoopsSettingsStateTest { + + @Test + public void legacyOnlyUsesCoreProfile() { + XoopsSettingsState s = XoopsSettingsState.migrateForTest(null, "2.5"); + assertEquals("2.5", s.coreVersion); + assertNull(s.coreProfile); + } + + @Test + public void newOnlyKeepsCoreVersion() { + XoopsSettingsState s = XoopsSettingsState.migrateForTest("2.7", null); + assertEquals("2.7", s.coreVersion); + assertNull(s.coreProfile); + } + + @Test + public void bothKeysPreferCoreVersionIncludingExplicitAuto() { + XoopsSettingsState s = XoopsSettingsState.migrateForTest("Auto", "2.5"); + assertEquals("Auto", s.coreVersion); + assertNull(s.coreProfile); + } + + @Test + public void bothKeysPreferExplicitNewOverLegacy() { + XoopsSettingsState s = XoopsSettingsState.migrateForTest("4.0", "2.5"); + assertEquals("4.0", s.coreVersion); + } + + @Test + public void neitherKeyDefaultsToAuto() { + XoopsSettingsState s = XoopsSettingsState.migrateForTest(null, null); + assertEquals("Auto", s.coreVersion); + assertNull(s.coreProfile); + } + + @Test + public void blankCoreVersionFallsBackToLegacy() { + XoopsSettingsState s = XoopsSettingsState.migrateForTest(" ", "2.5"); + assertEquals("2.5", s.coreVersion); + } +} diff --git a/test-fixtures/index_404_stub.php b/test-fixtures/index_404_stub.php new file mode 100644 index 0000000..6a50543 --- /dev/null +++ b/test-fixtures/index_404_stub.php @@ -0,0 +1,2 @@ +query($sql); + if (!$xoopsDB->isResultSet($result) || !$result instanceof \mysqli_result) { + throw new RuntimeException('Database query failed'); + } + while (list($sumIn, $sumOut) = $xoopsDB->fetchRow($result)) { + $unused = $sumIn + $sumOut; + } + + $result = $xoopsDB->query('SELECT 2'); + $row = $xoopsDB->fetchArray($result); // should warn — $result was reassigned + } +} diff --git a/whats-new.html b/whats-new.html index 37294d3..e96e59c 100644 --- a/whats-new.html +++ b/whats-new.html @@ -1,4 +1,19 @@ +

1.0.0 Alpha 3

+

Field-report fixes from wgSimpleAcc plus language-constant navigation and two new inspections.

+
    +
  • Inspections no longer duplicate every finding (PHP+HTML PSI)
    • +
  • isResultSet quick-fix is safe in Inspect Code batch apply; early-exit guards cover while (list = fetchRow)
    • +
  • ROOT_PATH guard: after namespace, accepts die, skips 404 stubs and admin/bootstrap entry points
    • +
  • Language constants from every language/**/*.php; Ctrl+B / Find Usages
    • +
  • Unregistered .tpl inspection; XOBJ_DTYPE_UNICODE_* rename quick-fix
    • +
  • Review hardening: Alpha 2 Core Version setting migrates; block templates in templates/blocks/, ['template'] = ... manifests and URLs in descriptions parse correctly; comment mask keeps strings and heredocs; commented-out define() is not indexed; no-op quick-fixes are not offered
    • +
+

1.0.0 Alpha 2

+
    +
  • On-demand Overview scan (no monorepo boot freeze); cancellable scans
    • +
  • Inspections under top-level XOOPS group
    • +

1.0.0 Alpha 1

First public alpha of XOOPS Support.