Skip to content

Rules to check migrations (POC)#74

Draft
SebSept wants to merge 1 commit into
glpi-project:developfrom
SebSept:phpstan-migrations
Draft

Rules to check migrations (POC)#74
SebSept wants to merge 1 commit into
glpi-project:developfrom
SebSept:phpstan-migrations

Conversation

@SebSept

@SebSept SebSept commented Apr 18, 2025

Copy link
Copy Markdown

While I was fixing a migration file, I've a POC to check migration files, so the implicit rules become explicit.

An exemple phpstan config could be, (important part is the rules includes).

# phpstan_migrations.neon.dist

parameters:
    level: 4
    bootstrapFiles:
        - stubs/db_config_classes.php
        - stubs/glpi_constants.php
    paths:
        - install/migrations/update_10.0.x_to_11.0.0/
    dynamicConstantNames:
        - GLPI_AJAX_DASHBOARD
        - GLPI_ALLOW_IFRAME_IN_RICH_TEXT
        - GLPI_CACHE_DIR
        - GLPI_CALDAV_IMPORT_STATE
        - GLPI_CENTRAL_WARNINGS
        - GLPI_CONFIG_DIR
        - GLPI_CRON_DIR
        - GLPI_DISABLE_ONLY_FULL_GROUP_BY_SQL_MODE
        - GLPI_DISALLOWED_UPLOADS_PATTERN
        - GLPI_DOC_DIR
        - GLPI_DOCUMENTATION_ROOT_URL
        - GLPI_ENVIRONMENT_TYPE
        - GLPI_FORCE_MAIL
        - GLPI_GRAPH_DIR
        - GLPI_INSTALL_MODE
        - GLPI_INVENTORY_DIR
        - GLPI_LOCAL_I18N_DIR
        - GLPI_LOCK_DIR
        - GLPI_LOG_DIR
        - GLPI_LOG_LVL
        - GLPI_MARKETPLACE_ALLOW_OVERRIDE
        - GLPI_MARKETPLACE_DIR
        - GLPI_MARKETPLACE_ENABLE
        - GLPI_MARKETPLACE_MANUAL_DOWNLOADS
        - GLPI_MARKETPLACE_PLUGINS_API_URI
        - GLPI_MARKETPLACE_PRERELEASES
        - GLPI_NETWORK_REGISTRATION_API_URL
        - GLPI_NETWORK_MAIL
        - GLPI_NETWORK_SERVICES
        - GLPI_PICTURE_DIR
        - GLPI_PLUGIN_DOC_DIR
        - GLPI_RSS_DIR
        - GLPI_SERVERSIDE_URL_ALLOWLIST
        - GLPI_SESSION_DIR
        - GLPI_STRICT_DEPRECATED
        - GLPI_TELEMETRY_URI
        - GLPI_TEXT_MAXSIZE
        - GLPI_THEMES_DIR
        - GLPI_TMP_DIR
        - GLPI_UPLOAD_DIR
        - GLPI_USER_AGENT_EXTRA_COMMENTS
        - GLPI_VAR_DIR
        - GLPI_WEBHOOK_ALLOW_RESPONSE_SAVING
        - PLUGINS_DIRECTORIES
        - TU_USER
        
rules:
    - GlpiProject\Tools\PHPStan\Rules\ForbidStaticDbFunctionsRule
    - GlpiProject\Tools\PHPStan\Rules\ForbidGlobalFunctionsRule

level may be set higher, that not the purpose of these PR.
Some other files may be needed in bootstrap.
String must be translated in english, that's just a fast POC.

@trasher

trasher commented Apr 18, 2025

Copy link
Copy Markdown
Contributor

I do not understand why we should forbid the use of those methods; not now.
It seems not really possible until we have a relevant replacement, which depends on a dependency injection management that still does not exist on GLPI core.

@SebSept

SebSept commented Apr 22, 2025

Copy link
Copy Markdown
Author

This is because this values are evaluated at runtime, so they are subject to change, return something different if a table name changes for example. This should be hardcoded so we are sure of the values, at anytime. This ensure the table xxx was created because it's hardcoded.
This is what @AdrienClairembault and @cedric-anne told me, and it make sense.

@cedric-anne

Copy link
Copy Markdown
Member

I do not understand why we should forbid the use of those methods; not now. It seems not really possible until we have a relevant replacement, which depends on a dependency injection management that still does not exist on GLPI core.

These rules specifically target migration files.

@AdrienClairembault AdrienClairembault left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ok for me but the errors should be in english.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants