* NEW: Add extrafields support to Link class
Link now declares isextrafieldmanaged and wires fetch_optionals()/
insertExtraFields()/deleteExtraFields() into fetch(), fetchAll(),
create(), update() and delete(), matching the llx_links_extrafields
table added previously.
* FIX: Several bugs in Link class review
- create(): wrong duplicate-record error message (copy-pasted from
Societe, referenced undefined $this->name) replaced by the generic
ErrorDuplicateField, consistent with update().
- create()/update(): missing "NoURL" translation key replaced by the
standard ErrorFieldRequired pattern.
- fetch(): guard against running an unfiltered query (no rowid, no
hashforshare) that could silently return an arbitrary link; a caller
in actions_linkedfiles.inc.php could hit this when 'linkid' was
missing from the request.
- actions_linkedfiles.inc.php: check fetch() result with `> 0` instead
of a truthy test, since -1 (error) is truthy in PHP.
- delete(): add User type hint (consistent with create()/update()) and
a $notrigger parameter to optionally skip the LINK_DELETE trigger.
- update(): fix copy-pasted docblock ("third party" -> "link").
* NEW: Add PHPUnit test for Link class
Covers create/fetch/update/fetchAll/count/delete, plus regression
tests for the two bugs fixed in the previous commit: create() rejects
an empty url, and fetch() rejects a call with neither rowid nor
hashforshare instead of returning an arbitrary record.
* FIX Phan false positive on $object in actions_linkedfiles.inc.php
Phan's ambient type inference for the loosely-typed global $object in
this shared include file was picking up CommonSocialNetworks (an
unrelated trait, not even a class), reported as undeclared
->id/->entity/->addThumbs()/->delThumbs() in a real CI Phan run on
this branch, once this file was analyzed on its own via the
changed-files file-list (this file has no prior baseline entry, so it
was apparently never previously exercised in isolation like this).
Force $object's type explicitly to CommonObject via the same
@phan-var-force string-literal idiom this file already uses for
$upload_dir/$upload_dirold/$confirm/$forceFullTextIndexation -
CommonObject genuinely declares addThumbs()/delThumbs(), which
resolves those two errors outright. $id/$entity remain reported as
PhanUndeclaredProperty (CommonObject itself does not declare them,
only its concrete subclasses do at runtime) - baseline-suppress that
for this file the same way it is already suppressed for the sibling
shared-include files actions_addupdatedelete.inc.php,
actions_massactions.inc.php and actions_sendmails.inc.php, which have
the exact same $object typing situation.
# Qual: Suppress PhanUndeclaredProperty warnings in commonobject.class.php
Ignore PhanUndeclaredProperty notices line by line to avoid new ones slipping in.
- Add @phan-suppress-next-line comments for PhanUndeclaredProperty warnings
- Add comments explaining the suppressions
- Update Phan baseline
Co-authored-by: Laurent Destailleur <eldy@destailleur.fr>
* FIX pre-commit PHPStan hook errors on commits with no file in scope
phpstan.neon.dist only analyzes htdocs/ and scripts/, but the
pre-commit wrapper passed every staged file straight to phpstan
regardless of path. A commit touching only files outside that scope
(e.g. test/phpunit/) made phpstan exit with "No files found to
analyse", failing the hook for reasons unrelated to the change being
committed. Filter to htdocs/ and scripts/ first, and skip cleanly
when nothing remains.
* NEW: Add phpunit test for Opensurveysondage class
Add a CRUD test (create/fetch/update/delete) for the Opensurveysondage
class, which had no test coverage yet.
Note: create()'s return value is not used to identify the created
record - Opensurveysondage::create() returns $this->id, but this
class never sets $this->id (the real primary key is the caller-
supplied $this->id_sondage string), so the return value carries no
information. The test uses errors/re-fetch to check success instead.
* NEW: Auto-activate module opensurvey in OpensurveysondageTest if needed
Activate module opensurvey in setUpBeforeClass() when it is not
already enabled, so the test does not depend on the environment's
module configuration.
Note: this activation is real and persists after the test run - it
is not undone by the transaction rollback in tearDownAfterClass().
Activating a module re-runs its SQL install scripts (CREATE/ALTER
TABLE), which causes an implicit commit in MySQL/InnoDB, same as an
admin enabling it from Setup > Modules would do.
* Qual: Remove SQL injection suppressions from Phan baseline
# Qual: Remove SQL injection suppressions from Phan baseline
- Removed SqlInjection suppressions from baseline.txt
- Removed SqlInjectionUnquotedEscape suppressions from baseline.txt
* Qual: Improve SQL query escaping and formatting in contact list
# Qual: Improve SQL query escaping and formatting in contact list
Improved the SQL query escaping for compatibility with SqlInjection.
Does not fix the inappropriate/incomplete escaping through usage of the 'escapeforlike' for REGEX or PGSQL '~'.
* Qual: Add SQLInjection suppression comments
# Qual: Add SQLInjection suppression comments
Added SQLInjection suppression directives:
- In modules_import.class.php, for json_encode operations.
- In html.formproduct.class.php, for expression "sanitized" by $db->order().
- In html.formwebportal.class.php, for the WHERE clause conditions (supposes safe argument).
---------
Co-authored-by: Laurent Destailleur <eldy@destailleur.fr>
* QUAL: Add SqlInjectionPlugin to detect unsafe SQL variable usage
# QUAL: Add SqlInjectionPlugin to detect unsafe SQL variable usage
This commit adds a new SqlInjectionPlugin to the Phan configuration, which helps detect unsafe SQL variable usage in the Dolibarr codebase. The plugin checks for variables used in sql expressions that are not properly escaped, cast, or protected by safe methods.
* Qual: Add SqlInjection exceptions to Phan baseline
* Qual: Add SqlInjection exceptions to Phan baseline
* Qual: Update baseline.txt to remove SqlInjection suppressions
The SqlInjection suppressions have been removed from the baseline.txt file.
This reenables the notification of "unfixed" cases.
* Qual: SQL injection detection for missing quotes on escaped strings
# Qual: SQL injection detection for missing quotes on escaped strings
In summary: report lines with missing quotes as in `" WHERE ".$db->escape($value)." OR "`.
* Qual: Fix code for SqlInjection analysis
# Qual: Fix code for SqlInjection analysis
Rewrite some escape calls, change escape() in sanitize(), rename variable, ignore SqlInjection notices.
* Qual: Update baseline
Allow some SqlInjection* notices, to be fixed later or already fixed in develop.
# Qual: Fix Phan notices
Some phan notices are not reported in ci.
This fixes new ones that appeared:
- Update imap.phan_php to provide IMAP related typing info;
- Add typing info to variables;
- Decleare some variables global (htdocs/admin/dict.php) as reused in function;
- Cast potential null to string;
- Adjust param typing to correspond to default null value (getOnlineSignatureUrl);
- Check that parameter is object as expected by function all (dol_format_address);
* Qual: Generate phan specific changed php files list
# Qual: Generate phan specific changed php files list
Uses multiple parts of the Phan configuration to select and exclude files
for phan analysis.
* avoid catching special characters in regexes as indicators of french
* Qual: Translate french to english
* Fix#38697 catch per-email exceptions in EmailCollector
The foreach over richarrayofemail in EmailCollector::doCollectOneCollector
had no per-iteration exception guard. On PHP 8, mysqli throws on query
errors, so a single poison email - a duplicate UNIQUE INSERT, an invalid
date, etc. - aborted the whole run mid-loop. datelastresult was never
updated, the processing flag stayed set and every remaining email was
skipped, leaving the collector stuck until cleared manually.
Wrap the body in try { ... } catch (Throwable $e) so a failing email is
rolled back, logged and the loop continues. The rest of the batch
completes and the collector lock is released.
Signed-off-by: Dolicraft <contact@dolicraft.com>
* Fix#38697 reindent foreach body wrapped in try
The previous commit wrapped the per-email loop body in try/catch but did
not bump the inner indentation, so pre-commit/phpcs flagged ~1.6k tab
count mismatches. Add the missing inner tab on the wrapped range.
Signed-off-by: Dolicraft <contact@dolicraft.com>
* Fix#38697 update codespell ignore tab count after reindent
The two existing udate (IMAP overview field) entries were ignored with
five tabs of indent. The previous reindent commit added one more tab
inside the wrapped try block, so the ignore lines no longer match the
source lines. Re-add the missing tab on the two ignore entries.
Signed-off-by: Dolicraft <contact@dolicraft.com>
---------
Signed-off-by: Dolicraft <contact@dolicraft.com>
Co-authored-by: Dolicraft <contact@dolicraft.com>
* [COREMB] Centralize myobject/mymodule substitutions via NamingContract value object
Introduce NamingContract (immutable value object) and NamingContractValidator
to replace 6 scattered ad-hoc $arrayreplacement blocks across index.php and
modulebuilder.lib.php. Fixes a critical bug where MYOBJECT token was commented
out in initobject, leaving generated class files with unsubstituted placeholders.
- NamingContract: canonical ordered substitution map (12 tokens with object,
7 module-only), applyTo() via str_replace (not make_substitutions, to avoid
processing Dolibarr __(key)__ patterns in raw templates), applyToFilename()
for lowercase-only filename substitution
- StrictNamingContractValidator: post-generation scan for residual myobject/
mymodule tokens, skipping /* BEGIN/END MODULEBUILDER */ structural markers
- modulebuilderValidateGeneratedFile() helper: non-blocking warnings with
XSS-safe output via dol_escape_htmltag()
- Collision guard: InvalidArgumentException if module === object name
(case-insensitive), wrapped in try-catch at all instantiation sites
- 24 PHPUnit tests, 52 assertions
* fix(initobject): apply object substitution to all module PHP files
After initmodule creates the module structure, files like index.php,
lib/module.lib.php, admin/setup.php and class/api_module.class.php
still contain myobject/mymodule placeholders. initobject now scans
ALL PHP files in the module directory and applies the full NamingContract
substitution, and deletes the orphan stats/myobject_index.php placeholder.
* fix(initmodule): prevent array_merge from renumbering the '500000' key
PHP casts numeric string keys like '500000' to int(500000) internally.
array_merge() then renumbers integer keys starting from 0, turning
'500000' into key 0. make_substitutions() then calls str_replace(0, ...)
which casts 0 to the string '0', replacing EVERY zero digit in every
generated file with the idmodule value.
Fix: replace the bare '500000' key with '$this->numero = 500000' so that
array_merge sees a non-numeric string key and preserves it as-is.
* Fix initapi: apply header substitution after addObjectsToApiFile
addObjectsToApiFile substitutes properties/constructor/includes via regex
but leaves PHPDoc header and class declaration with mymodule/myobject tokens.
Fix: apply $arrayreplacement (minus 'MYOBJECT' uppercase key) after
addObjectsToApiFile to resolve the remaining residuals.
'MYOBJECT' is excluded to preserve the /* BEGIN MODULEBUILDER API MYOBJECT */
placeholder that addObjectsToApiFile relies on for future object additions.
* Move NamingContractTest to core test directory
NamingContract and NamingContractValidator are core modulebuilder classes;
their tests belong alongside other Dolibarr core tests in test/phpunit/,
not in htdocs/modulebuilder/test/phpunit/ (the custom-module pattern).
Updated require_once paths accordingly.
* fix useless file
* fix(phpcs): add missing @param descriptions and function docblocks
- NamingContract: merge adjacent string literals in exception message,
add description to @param $content in applyTo()
- NamingContractValidator: add descriptions to all bare @param tags
in interface and StrictNamingContractValidator methods
- NamingContractTest: add /** @return void */ docblocks to all test
methods, merge unnecessary string concat on template variable
* fix(phan/phpstan): remove readonly, add baselines for test file
- NamingContract: replace readonly properties with plain public string
(readonly requires PHP 8.1, phan minimum target is PHP 7.2)
- dev/tools/phan/baseline.txt: add NamingContractTest.php suppression
for PhanUndeclaredMethod (PHPUnit stubs not available in phan scope)
- NamingContractTest.php: add @phpstan-ignore class.notFound on class
declaration and method.notFound on each assertion call (PHPUnit not
in phpstan bootstrap, consistent with CommonClassTest.class.php)
* fix(phan/phpstan): suppress typed property warnings, NoopNew, class.notFound
- baseline.txt: add PhanCompatibleTypedProperty for NamingContract.class.php
(typed properties require PHP 7.4, phan min target is 7.2 — warning only
but phan exits 1 on any issue including warnings)
- baseline.txt: add PhanNoopNew for NamingContractTest.php (constructor
called for exception side-effect in guard tests is intentional)
- NamingContractTest.php: move @phpstan-ignore class.notFound to inline
comment on class declaration (docblock placement not picked up by phpstan)
* fix(phan/phpstan): add missing phan suppress on index.php l.574, fix phpstan class docblock
- index.php l.574: add @phan-suppress-next-line PhanPluginSuspiciousParamPosition
(same pattern as l.577 and l.587 for identical dolReplaceInFile calls
where local variable name $destfile matches parameter #3 of the function)
- NamingContractTest.php: use standalone /** @phpstan-ignore class.notFound */
docblock before class declaration (inline // comment not picked up by phpstan
on class declarations; separate docblock matches CommonClassTest.class.php pattern)
* Update baseline.txt
* fix(phan): add phan-file-suppress on NamingContractTest for PHPUnit undeclared methods
* fix(phan): suppress PhanNoopNew on expectException patterns in NamingContractTest
---------
Co-authored-by: Laurent Destailleur <eldy@destailleur.fr>
# Qual: Add missing sqlite3 stub for Phan
About 43 Phan notices on https://cti.dolibarr.org are present
because of missing information about SQLite3.
This stub and configuration change adds the information.
Co-authored-by: Laurent Destailleur <eldy@destailleur.fr>
* Qual: Fix Phan notices, set PHP min to 7.2 to avoid some
* Qual: Fix Phan and PHPStan notices
* Update apstats.php
---------
Co-authored-by: Laurent Destailleur <eldy@destailleur.fr>
# Qual: Update Phan baseline and config using global variables for dolibarr_*
- Add new global variables to config.php to improve static analysis
- Remove dolibarr_* global variable references in filefunc.inc.php
- Update Phan baseline
# Qual: Fix PHPStan for ldap, pgsql, odtphp
- Adjust typing in ldap and pgsql class to fix PHPStan notices;
- Exclude all files for odtphp from analysis (external lib);
- Update PHPStan baseline.
* Qual: Update GETPOST calls to handle hint array type
# Qual: Update GETPOST calls to handle hint array type
- Updated GETPOST calls for 'amounts' and 'multicurrency_amounts' to explicitly handle array inputs by adding the 'array' type parameter
This fixes PHPStan notices where expressions expect an array.
* Qual: Set type for status label mapping in LignePrelevement (PHPStan)
# Qual: Set type for status label mapping in LignePrelevement (PHPStan)
Added type hint to property `$labelStatus`
* Qual: Add type hints to datec and datep properties in PaymentSocialContribution
# Qual: Add type hints to datec and datep properties in PaymentSocialContribution
Added type hints to datec and datep properties.
* Qual: Add type hint to MIME mapping for image file extensions
# Qual: Add type hint to MIME mapping for image file extensions
Added type hint to `$image_types`
* Qual: Better type hint for getAllItemsLinkedByObjectID method
# Qual: Better type hint for getAllItemsLinkedByObjectID method
The return type annotation for the getAllItemsLinkedByObjectID method has been updated from 'array|int' to 'string[]|int'.
* Qual: Better type hint for fetchAll return value
# Qual: Better type hint for fetchAll return value
Suggest thet fetchAll returns list of objects
* Qual: Remove array from return value hint
* Qual: Add missing type hints to Notify
* Qual: Cast resource IDs to integers in getAllResourcesOfWorkstation
# Qual: Cast resource IDs to integers in getAllResourcesOfWorkstation
Ensure returned value is list of ints as parent::getAllItemsLinkedByObjectID does not ensure value type.
* Qual: Ensure correct return type for getAllGroupsOfWorkstation
* Qual: Improve typing hints
* Qual: Improve typing hints, ignore some notices
* Qual: Fix argument type
* Qual: Improve type hints
* Qual: Improve typing hints
* Qual: Add missing type hint
* Qual: Update Phan baseline
* Qual: Remove redundant exit (PHPStan, Phan)
* Qual: Fix phpstan notice with cast
* Qual: Fix initialiser (phpstan) and narrow some types
* Qual: Fix typing for object->type field (phpstan)
* Qual: Update preg_quote usage in step2.php
# Qual: Ignore false phpstan positive for preg_match
Add comment to ignore phpstan warning for preg_match argument
* Qual: Extend accepted types for arrayoptions (phpstan)
* Qual: Fix type hint typo
* Qual: Fix type for contact_id assignment
* Qual: Add menuonly as allowed reloadmode
* Qual: Add PHPStan ignore comments for argument.type
* Qual: Ignore unused parameter in constructors (PHPStan)
* Qual: Add type hint for $TMsg (PHPStan)
* Qual: Cast array values to match expected type (PHPStan)
* Qual: Cast $remise_percent_ligne to float (PHPStan)
* Qual: Add array specifier to GETPOST for correct type (PHPStan)
* Qual: Update sumSituation() argument types
* Qual: Cast value to string for printStdColumnContent (PHPStan)
* Qual: Ignore PHPStan notice for autoload function return value
* Qual: Fix type of argument to Menubase::recur (PHPStan)
* Qual: Fix return type hint (PHPStan)
* Qual: Add $dolibarr_main_db_type as default global
* Qual: Update Phan baseline (due to PHPStan fix)
* Qual: Ignore PhanUndeclaredProperty for specific lines
* Fix: Update payment mode reference in PDF generation
# Fix: Update payment mode reference in PDF generation
The payment mode reference in the PDF generation was incorrectly using `type_payment_code`, an unknown property in Salary.
It has been updated to use `type_code` as used elsewhere for the same purpose.
* Qual: Typing of several DDLUpdateField field_desc items made optional
# Qual: Typing of several DDLUpdateField field_desc items made optional
Not all fields are required for updateing the DB typing, and ExtraFields was not providing all of them.
* Qual: Update typing for phan notices
# Qual: Update typing for phan notices
Fix typing mismatches
* Qual: Fix extrafields typing in objectline_create.tpl.php
# Qual: Fix extrafields typing in objectline_create.tpl.php
- Update typing for phan
* Qual: Fix typing for Phan & re-use encrypted password when requested
# Qual: Fix typing for Phan & re-use encrypted password when requested
- Added a cast to string for the crypted password to hint phan that it returns the string version (not the array)
- Added a check to ensure the password is not already crypted before generating a new password.
- Updated the type hints for parameters `$changelater`, `$notrigger`, `$nosyncmember`, and `$passwordalreadycrypted` to include the range of possible values.
- Updated the return type hint for the `setPassword` method to include the range of possible error values.
* Qual: Fix file argument to showPreview() calls
# Qual: Fix file argument to showPreview() calls
Fix argument type mismatches by adjusting '$file' arguments in showPreview calls.
* Qual: Update login_c type to string
# Qual: Update login_c type to string
login_c is username from db.
* Qual: Update type hints in treeview.lib.php
# Qual: Update type hints in treeview.lib.php
Update type hints for static analysis.
* Qual: Add null check for objecttmp in selectForFormsList
# Qual: Add null check for objecttmp in selectForFormsList
Add null check for objecttmp before calling selectForFormsList to ensure static analysis
* Qual: Fix phan through null check on object argument
# Qual: Fix phan through null check on object argument
- Do null check to avoid null parameter (fix phan);
- Set more specific type for argument.
* Qual: Change type hint of $remise_percent parameter to float
# Qual: Change type hint of $remise_percent parameter to float
DB type is float, so correct arg type to fix phan notice.
* Qual: Ignore some phan notices
The descriptions for the linkedObjectsFullLoaded and oldcopy properties have been updated to avoid modification by code formatting tools
* Qual: Update phan baseline
* Qual: Cast remise_percent to float to match typing requirements
* Qual: Cast label to string to help static analysis
* Qual: Update date_cancel property type and assignment
To fix phan notices:
- The date_cancel property type was changed from 'int|string' to 'int|""' to match return type from jdate().
- Additionally, the assignment of date_cancel was updated to use dol_now() instead of $this->db->idate(dol_now()).
* Qual: Provide the expected user_id instead of the User object
Changed the user fetch logic from using `user_author` to `user_creation_id`
* Qual: Correct bool argument from 0 to false ($fill)
* Qual/Fix: Correct date converted to dol_print_date
* Qual: Fix argument type for addline()
* Qual: Update Phan baseline.txt
