Skip to content

Commit f0e4241

Browse files
marcoscanomarcoscano
authored
Include a PHPCS sniffer (#2)
* Scaffold an initial sniffer for template files * Fix the sniffer * Do not error out in cases we know the field access is not an issue
1 parent adbf22a commit f0e4241

File tree

3 files changed

+204
-11
lines changed

3 files changed

+204
-11
lines changed

‎README.md

+110-11
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,16 @@
11
# Cacheability Metadata Checker (CMC)
22

3-
A Drupal module that helps developers identify missing cache tags in their pages by tracking loaded entities and verifying their cache tags are properly bubbled up to the response.
4-
5-
## Overview
6-
7-
The Cacheability Metadata Checker monitors entity loads during page requests and verifies that all cache tags from those entities are present in the final response. This helps ensure proper cache invalidation by catching situations where entity cache tags are accidentally stripped or forgotten.
3+
A Drupal module that helps developers identify missing cache tags in their pages by tracking loaded entities and
4+
verifying their cache tags are properly bubbled up to the response.
85

96
## Features
107

11-
- Tracks loaded content entities and their cache tags
8+
- Tracks loaded entities and their cache tags, compares with tags bubbled to the HTTP response object
129
- Configurable operation modes (disabled, display errors, strict)
1310
- Option to check only front-end pages or include admin pages
1411
- Ability to skip specific URLs
1512
- API for other modules to exclude certain entities from tracking
13+
- Custom PHPCS sniff for Twig template best practices
1614

1715
## Configuration
1816

@@ -26,7 +24,8 @@ Visit `/admin/config/development/cmc` to configure the module. The following opt
2624

2725
### Skip Admin Pages
2826

29-
When enabled (default), the module only checks pages using the default theme. When disabled, it also checks admin pages using the admin theme.
27+
When enabled (default), the module only checks pages using the default theme. When disabled, it also checks admin
28+
pages using the admin theme.
3029

3130
### Skip URLs
3231

@@ -53,7 +52,107 @@ function mymodule_cmc_skip_tracking(EntityInterface $entity): bool {
5352

5453
## Best Practices
5554

56-
1. Enable "Strict" mode during development to catch cache tag issues early
57-
2. Use "Display errors" mode on staging environments for visual feedback
58-
3. Disable the module in production environments
59-
4. Consider excluding admin pages if you're only concerned with front-end caching
55+
1. Enable "Strict" mode during development to catch cache tag issues early.
56+
2. Use "Display errors" mode on staging environments for visual feedback or to use tools such as VisualDiff tests.
57+
3. Never enable the module in production environments.
58+
4. Consider excluding admin pages to avoid unrelated issues when developing the front-end theme.
59+
60+
## Coding Standards
61+
62+
The module includes a custom PHPCS sniff that helps maintain Drupal best practices in Twig templates:
63+
64+
### Direct Field Access Sniff
65+
66+
This sniff detects direct field access in Twig templates using patterns like `node.field_*`, `content.field_*`, etc.
67+
This pattern is discouraged because it may introduce caching bugs by leaving out the proper cache tags in the top-level
68+
render array.
69+
70+
#### Installation
71+
72+
1. Install development dependencies:
73+
```bash
74+
composer require --dev drupal/coder squizlabs/php_codesniffer
75+
```
76+
77+
2. Register the custom standard:
78+
```bash
79+
vendor/bin/phpcs --config-set installed_paths /path/to/web/modules/custom/cmc/phpcs
80+
```
81+
Note: the example above will set your `installed_paths` to include only the path to this module's sniffers, which
82+
is likely not what you want. You can pass a comma-separated list of paths to the above command, or include the line
83+
below in your `phpcs.xml.dist` configuration file:
84+
85+
```xml
86+
<config name="installed_paths" value="../../drupal/coder/coder_sniffer,../../sirbrillig/phpcs-variable-analysis,../../slevomat/coding-standard,../../../web/modules/custom/cmc/phpcs" />
87+
```
88+
89+
3. Add the standard to your `phpcs.xml.dist`:
90+
```xml
91+
<rule ref="web/modules/custom/cmc/phpcs/ruleset.xml"/>
92+
```
93+
94+
Also make sure you allow `twig` as one of the extensions to be sniffed, in case you have an `<arg name="extensions">`
95+
config value.
96+
97+
4. Run the sniffer:
98+
```bash
99+
vendor/bin/phpcs /path/to/your/templates `--standard=CMC`
100+
```
101+
102+
If you included the standard in your `phpcs.xml.dist` configuration file, you can omit the `--standard=CMC` flag.
103+
104+
105+
#### Skip Conditions
106+
107+
The sniff will automatically skip the warning under these conditions:
108+
109+
1. If the template renders the full `content` variable:
110+
```twig
111+
{{ content }}
112+
```
113+
114+
2. If the template renders cache metadata explicitly:
115+
```twig
116+
{{ content['#cache'] }}
117+
```
118+
119+
3. If you explicitly opt-out using the special comment:
120+
```twig
121+
{# cmc_direct_field_access_sniff_opt_out #}
122+
```
123+
124+
#### Example Usage
125+
126+
❌ Not recommended:
127+
```twig
128+
{# Direct field access will trigger warning #}
129+
{{ node.field_image }}
130+
{{ content.field_tags }}
131+
{{ media.field_media_image }}
132+
```
133+
134+
✅ Recommended:
135+
```twig
136+
{# Render the full content variable #}
137+
{{ content }}
138+
139+
{# Render cache metadata & assets explicitly #}
140+
{{ node.field_image }}
141+
{{ {'#cache': content['#cache'], '#attached': content['#attached']} }}
142+
143+
{# Or opt-out if you know this is being handled elsewhere #}
144+
{# cmc_direct_field_access_sniff_opt_out #}
145+
{{ node.field_special_case }}
146+
```
147+
148+
Example warning:
149+
```
150+
FILE: /path/to/template.html.twig
151+
--------------------------------------------------------------------------------
152+
FOUND 1 WARNING AFFECTING 1 LINE
153+
--------------------------------------------------------------------------------
154+
15 | WARNING | Direct field access using "node.field_image" is not recommended.
155+
| | Try to always render full render arrays that come from the backend.
156+
--------------------------------------------------------------------------------
157+
```
158+

‎phpcs/Sniffs/Twig/DirectFieldAccessSniff.php

+84
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Drupal\cmc\Sniffs\Twig;
6+
7+
use PHP_CodeSniffer\Files\File;
8+
use PHP_CodeSniffer\Sniffs\Sniff;
9+
10+
/**
11+
* Detects direct field access in Twig templates.
12+
*/
13+
class DirectFieldAccessSniff implements Sniff {
14+
15+
/**
16+
* {@inheritdoc}
17+
*/
18+
public function register(): array {
19+
return [T_INLINE_HTML];
20+
}
21+
22+
/**
23+
* {@inheritdoc}
24+
*/
25+
public function process(File $phpcsFile, $stackPtr) {
26+
// Only process .twig files.
27+
$fileExtension = strtolower(substr($phpcsFile->getFilename(), -5));
28+
if ($fileExtension !== '.twig') {
29+
return;
30+
}
31+
32+
$tokens = $phpcsFile->getTokens();
33+
$content = $tokens[$stackPtr]['content'];
34+
35+
// Look for content.field_*, node.field_*, media.field_*, etc.
36+
if (preg_match('/[a-z]+\.field_[a-z]+/', $content, $matches)) {
37+
// Do not error out if this is a case we know it's not an issue.
38+
$file_content = $phpcsFile->getTokensAsString(0, count($tokens));
39+
if ($this->shouldSkip($file_content)) {
40+
return;
41+
}
42+
43+
$error = 'Direct field access using "%s" is not recommended. Try to always render full render arrays that come from the backend.';
44+
$data = [$matches[0]];
45+
$phpcsFile->addWarning($error, $stackPtr, 'DirectFieldAccess', $data);
46+
}
47+
}
48+
49+
/**
50+
* Detects whether we should skip an identified failure.
51+
*
52+
* @param string $file_content
53+
* The full contents of the file being sniffed.
54+
*
55+
* @return bool
56+
* TRUE if at least one of the below conditions are met:
57+
* 1- If {{ content }} is being printed in the same template;
58+
* 2- If {{ content['#cache'] }} is being printed as-is;
59+
* 3- If devs assume responsibility and opt-out with the special syntax
60+
* {# cmc_direct_field_access_sniff_opt_out #}.
61+
* Will return FALSE otherwise.
62+
*/
63+
private function shouldSkip(string $file_content): bool {
64+
$matches = [];
65+
// Pattern {{ content }}.
66+
if (preg_match('/{{(\s)?content(\s)?}}/', $file_content, $matches)) {
67+
return TRUE;
68+
}
69+
// Pattern {{ content['#cache'] }}.
70+
$skip_pattern = <<<TEXT
71+
{{(\s)?content\[('|")#cache('|")\](\s)?}}
72+
TEXT;
73+
if (preg_match($skip_pattern, $file_content, $matches)) {
74+
return TRUE;
75+
}
76+
// Pattern {# cmc_direct_field_access_sniff_opt_out #}.
77+
if (preg_match('/{#(\s)cmc_direct_field_access_sniff_opt_out(\s)?#}/', $file_content, $matches)) {
78+
return TRUE;
79+
}
80+
81+
return FALSE;
82+
}
83+
84+
}

‎phpcs/ruleset.xml

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?xml version="1.0"?>
2+
<ruleset name="CMC">
3+
<description>Custom coding standards for the CMC module.</description>
4+
5+
<!-- Include our custom sniffs -->
6+
<rule ref="./Sniffs/Twig/DirectFieldAccessSniff.php"/>
7+
8+
<!-- Define file extensions to check -->
9+
<arg name="extensions" value="html.twig"/>
10+
</ruleset>

0 commit comments

Comments
 (0)