Problem/Motivation

The cache pattern markup_only (modules/audit_performance/src/Plugin/AuditAnalyzer/PerformanceAnalyzer.php) reports "Render array with only #markup - missing cache metadata" for any return ['#markup' => ...]; in a file where hasCacheMetadataInClass() finds no '#cache' string. This produces notices where no cache metadata is needed or where it is already handled:

- Value objects implementing MarkupInterface and RenderableInterface whose toRenderable() returns ['#markup' => $this]: the markup is built from constructor arguments, and its cacheability belongs to the code that renders the object.
- Static content: return ['#markup' => '


']; or return ['#markup' => $this->t('No results.')];. These only vary by the cache contexts the renderer always adds to every render array (renderer.config required_cache_contexts: languages:language_interface, theme, user.permissions).
- Classes that set cache metadata through the cacheable metadata API instead of a literal '#cache' key: CacheableMetadata::createFromObject($node)->applyTo($build), $this->renderer->addCacheableDependency($build, $node), addCacheTags(), addCacheContexts(), mergeCacheMaxAge().

Steps to reproduce

1. Add a class implementing MarkupInterface and RenderableInterface whose toRenderable() returns ['#markup' => $this].
2. Run drush audit:run performance --format=json.
3. MARKUP_WITHOUT_CACHE is reported.

Proposed resolution

- In the markup_only pattern, do not match string literals and t() / $this->t() of a single literal (a possessive whitespace match before the negative lookahead, so leading spaces cannot bypass it).
- In hasCacheMetadataInClass() (which already receives the comment-stripped content), also accept CacheableMetadata and ->addCacheableDependency(), ->addCacheTags(), ->addCacheContexts(), ->mergeCacheMaxAge() as cache handling, and skip classes implementing MarkupInterface or RenderableInterface.

Dynamic markup without any cache handling in the class is still reported, also when '#cache' only appears in a comment.

Remaining tasks

- Update the pattern and the helper.
- Unit test with a data provider through scanFileForPatterns() and the shipped pattern: dynamic markup in a controller and '#cache' only in a comment => reported; string literal, translated literal, '#cache' in the class, CacheableMetadata::applyTo(), addCacheableDependency() and a MarkupInterface value object => not reported.

User interface changes

Fewer MARKUP_WITHOUT_CACHE notices for code that needs no cache metadata or already sets it.

API changes

None.

Data model changes

None.

Comments

trebormc created an issue. See original summary.

  • trebormc committed 3013da21 on 1.x
    Issue #3630484 by trebormc: Skip static markup, MarkupInterface value...
trebormc’s picture

Status: Active » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.