Skip to content

[Dropzone][Toolkit] Apply Twig CS Fixer - #3654

Merged
Kocal merged 1 commit into
symfony:3.xfrom
seb-jean:fix/twig-cs-fixer-variable-naming
Jun 5, 2026
Merged

Kocal merged 1 commit into
symfony:3.xfrom
seb-jean:fix/twig-cs-fixer-variable-naming

Conversation

@seb-jean

@seb-jean seb-jean commented Jun 2, 2026

Copy link
Copy Markdown
Contributor
Q A
Bug fix? no
New feature? no
Deprecations? no
Documentation? no
Issues Fix #...
License MIT

I added $config->allowNonFixableRules(); in the .twig-cs-fixer.dist.php file to surface errors that aren't automatically fixable, then fixed those errors.

Here is an overview of the errors:

php vendor/bin/twig-cs-fixer lint
 KO /home/sebastien/ux-twig-cs-fixer/src/Dropzone/templates/form_theme.html.twig
 ------- -----------------------------------------------------------------------------------------------------------------------------
  ERROR   1    | {% block dropzone_widget -%}
          2    |     {%- set dataController = (attr['data-controller']|default('') ~ ' symfony--ux-dropzone--dropzone')|trim -%}
          >>   | The var name must use snake_case; expected data_controller.
          3    |     {%- set attr = attr|merge({'data-controller': '', class: (attr.class|default('') ~ ' dropzone-input')|trim}) -%}
 ------- -----------------------------------------------------------------------------------------------------------------------------

 KO /home/sebastien/ux-twig-cs-fixer/src/TwigComponent/templates/Collector/twig_component.html.twig
 ------- -------------------------------------------------------------------------------------
  ERROR   133  | {% set text %}
          134  |     {% for _component in collector.components %}
          >>   | The var name must use snake_case; expected component.
          135  |         <div class="sf-toolbar-info-piece">
 ------- -------------------------------------------------------------------------------------
  ERROR   250  | <div class="twig-component-renders">
          251  |     {% set _memory = null %}
          >>   | The var name must use snake_case; expected memory.
          252  |     {% for render in collector.renders %}
 ------- -------------------------------------------------------------------------------------
  ERROR   268  | <th class="cell-right renderTime">
          269  |     {% set _render_memory = render.render_memory|default(0) / 1024 / 1024 %}
          >>   | The var name must use snake_case; expected render_memory.
          270  |     <span class="{{ _render_memory == _memory ? 'text-muted' }}">
 ------- -------------------------------------------------------------------------------------
  ERROR   273  |     <span class="text-muted text-small">MiB</span>
          274  |     {% set _memory = _render_memory %}
          >>   | The var name must use snake_case; expected memory.
          275  | </th>
 ------- -------------------------------------------------------------------------------------

 KO /home/sebastien/ux-twig-cs-fixer/src/Toolkit/kits/flowbite-4/dropdown/templates/components/Dropdown.html.twig
 ------- ------------------------------------------------------------------------
  ERROR   10   |
          11   | {%- set _dropdown_id = 'dropdown-' ~ id -%}
          >>   | The var name must use snake_case; expected dropdown_id.
          12   | {%- set _dropdown_trigger_id = _dropdown_id ~ '-trigger' -%}
 ------- ------------------------------------------------------------------------
  ERROR   11   | {%- set _dropdown_id = 'dropdown-' ~ id -%}
          12   | {%- set _dropdown_trigger_id = _dropdown_id ~ '-trigger' -%}
          >>   | The var name must use snake_case; expected dropdown_trigger_id.
          13   |
 ------- ------------------------------------------------------------------------

 KO /home/sebastien/ux-twig-cs-fixer/src/Toolkit/kits/flowbite-4/dropdown/templates/components/Dropdown/Sub.html.twig
 ------- ------------------------------------------------------------------------
  ERROR   13   |
          14   | {%- set _dropdown_id = 'dropdown-sub-' ~ id -%}
          >>   | The var name must use snake_case; expected dropdown_id.
          15   | {%- set _dropdown_trigger_id = _dropdown_id ~ '-trigger' -%}
 ------- ------------------------------------------------------------------------
  ERROR   14   | {%- set _dropdown_id = 'dropdown-sub-' ~ id -%}
          15   | {%- set _dropdown_trigger_id = _dropdown_id ~ '-trigger' -%}
          >>   | The var name must use snake_case; expected dropdown_trigger_id.
          16   | {%- set _dropdown_content = block('content') -%}
 ------- ------------------------------------------------------------------------
  ERROR   15   | {%- set _dropdown_trigger_id = _dropdown_id ~ '-trigger' -%}
          16   | {%- set _dropdown_content = block('content') -%}
          >>   | The var name must use snake_case; expected dropdown_content.
          17   |
 ------- ------------------------------------------------------------------------
 
 [...]

@seb-jean
seb-jean requested a review from Kocal as a code owner June 2, 2026 20:23
@seb-jean
seb-jean force-pushed the fix/twig-cs-fixer-variable-naming branch 2 times, most recently from 9342f59 to efbff5b Compare June 2, 2026 20:32

<div
id="{{ _dropdown_id }}"
id="{{ dropdown_id }}"

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.

Please keep _ prefix for Twig variables declared inside components, these variables "overflow" in the user's templates and we want to reduce the risk of collision

{%- props as = 'a' -%}

{% set attributesDefaults = {} %}
{% set attributes_defaults = {} %}

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.

Suggested change
{% set attributes_defaults = {} %}
{% set _attributes_defaults = {} %}

Comment thread src/Toolkit/kits/flowbite-4/dropdown/templates/components/Dropdown/Sub.html.twig Outdated
@carsonbot carsonbot added Status: Needs Work Additional work is needed and removed Status: Needs Review Needs to be reviewed labels Jun 3, 2026
@seb-jean
seb-jean force-pushed the fix/twig-cs-fixer-variable-naming branch from efbff5b to 91d6dee Compare June 3, 2026 15:13
@carsonbot carsonbot added Status: Needs Review Needs to be reviewed and removed Status: Needs Work Additional work is needed labels Jun 3, 2026
@seb-jean
seb-jean force-pushed the fix/twig-cs-fixer-variable-naming branch 2 times, most recently from a2f520a to fe5ba23 Compare June 3, 2026 15:36
@seb-jean seb-jean changed the title [Dropzone][Toolkit][TwigComponent] Apply Twig CS Fixer [Dropzone][Toolkit] Apply Twig CS Fixer Jun 3, 2026
@Kocal

Kocal commented Jun 3, 2026

Copy link
Copy Markdown
Member

There are no modifications on the Twig-CS-Fixer configuration file?

@seb-jean
seb-jean force-pushed the fix/twig-cs-fixer-variable-naming branch 2 times, most recently from 6e2358b to ce334f7 Compare June 3, 2026 16:02
@seb-jean

seb-jean commented Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

There are no modifications on the Twig-CS-Fixer configuration file?

@VincentLanglet, I'd like your opinion on this. Do you need to modify https://github.com/VincentLanglet/Twig-CS-Fixer to add two rules:

Or should we modify the configuration of our .twig-cs-fixer.dist.php file?

I added $config->allowNonFixableRules(); to the .twig-cs-fixer.dist.php file.

@VincentLanglet

Copy link
Copy Markdown

There are no modifications on the Twig-CS-Fixer configuration file?

@VincentLanglet, I'd like your opinion on this. Do you need to modify VincentLanglet/Twig-CS-Fixer to add two rules:

  • _ prefix for Twig variables declared inside components

The rule already has an option "optionalPrefix" we should allow you to use _ as a an optional prefix.

I'm not sure it should be change in the generic ruleset cause this seems to be a specific rule used by symfony/ux which is not recommended in the Twig coding standard.

Definetly something custom to this repo.

Or should we modify the configuration of our .twig-cs-fixer.dist.php file?

Yes, that's the way

@seb-jean
seb-jean marked this pull request as draft June 3, 2026 20:58
@seb-jean
seb-jean force-pushed the fix/twig-cs-fixer-variable-naming branch 3 times, most recently from 69be75f to c1d734f Compare June 4, 2026 05:02
@seb-jean

seb-jean commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

There are no modifications on the Twig-CS-Fixer configuration file?

@VincentLanglet, I'd like your opinion on this. Do you need to modify VincentLanglet/Twig-CS-Fixer to add two rules:

  • _ prefix for Twig variables declared inside components

The rule already has an option "optionalPrefix" we should allow you to use _ as a an optional prefix.

I'm not sure it should be change in the generic ruleset cause this seems to be a specific rule used by symfony/ux which is not recommended in the Twig coding standard.

Definetly something custom to this repo.

Or should we modify the configuration of our .twig-cs-fixer.dist.php file?

Yes, that's the way

Thanks @VincentLanglet for your feedback!

I've updated the .twig-cs-fixer.dist.php configuration with:

  • The optionalPrefix: '_' option for component-internal variables
  • A custom VariableNameRule (in tools/twig-cs-fixer/) that replaces the original rule to accept both snake_case and camelCase per segment, since some Toolkit variables mix both conventions (e.g. _tooltip_wrapperId)

@seb-jean
seb-jean marked this pull request as ready for review June 4, 2026 05:09
@Kocal
Kocal force-pushed the fix/twig-cs-fixer-variable-naming branch from c1d734f to 89facc0 Compare June 5, 2026 10:28
@carsonbot carsonbot added Status: Reviewed Has been reviewed by a maintainer and removed Status: Needs Review Needs to be reviewed labels Jun 5, 2026
@carsonbot carsonbot changed the title [Dropzone][Toolkit] Apply Twig CS Fixer [Dropzone][Toolkit][TwigComponent] Apply Twig CS Fixer Jun 5, 2026
@carsonbot carsonbot changed the title [Dropzone][Toolkit][TwigComponent] Apply Twig CS Fixer [Dropzone][Toolkit] Apply Twig CS Fixer Jun 5, 2026
@Kocal

Kocal commented Jun 5, 2026

Copy link
Copy Markdown
Member

Thank you @seb-jean.

@Kocal
Kocal merged commit 71b5747 into symfony:3.x Jun 5, 2026
32 of 33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Dropzone Status: Reviewed Has been reviewed by a maintainer Toolkit

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants