Add Twig and Qiq template embed navigation#18
Open
suzumaze wants to merge 16 commits into
Open
Conversation
Goto-declaration from {{ var }} in a Twig template jumps to the
template of the resource embedded via #[Embed(rel: 'var')] on the
surrounding resource class.
- TemplateEngineSupport: interface for template engines with
accepts / resolveTemplates / resolveResourceClass /
extractVariableName
- TwigSupport: maps src/Resource/<X>.php <-> <twigPath>/<X>.html.twig
for each path in Settings.twigTemplatePaths (defaults:
src/Resource, var/templates); identifies VARIABLE_REFERENCE
composite elements inside Twig print blocks
- TemplateUtils: shared helpers for app-root resolution from a
PhpClass via the /src/Resource/ marker
- EmbedResolver: parses #[Embed(rel: 'X', src: 'app://self/X')]
attributes via regex, then maps the URI to a PhpClass under
src/Resource/
- EmbedTemplateReference: PsiReferenceBase with explicit
TextRange to avoid the missing ElementManipulator on
TwigVariableReference; resolve returns the embedded
template's PsiFile
- EmbedTemplateReferenceContributor: registers the reference
with HIGHER_PRIORITY so it wins over PhpStorm's built-in
Twig variable reference
- Settings: add twigTemplatePaths (src/Resource, var/templates)
- plugin.xml: add psi.referenceContributor (Twig),
notificationGroup (BEAR.Sunday), and depends on
com.jetbrains.twig
- build.gradle.kts: bundle com.jetbrains.twig
Render a navigation gutter icon next to each {{ var }} in a Twig
template that resolves to a #[Embed(rel: 'var')] on the surrounding
resource class. Clicking it jumps to the embedded resource's
template, complementing the existing goto-declaration support.
- EmbedTwigLineMarkerProvider: anchors a LineMarkerInfo on the leaf
identifier of a VARIABLE_REFERENCE composite, reusing TwigSupport
and EmbedResolver to resolve the embed src URI and target template
- plugin.xml: register codeInsight.lineMarkerProvider (Twig)
- icons: embed.svg / embed_dark.svg for the gutter marker
The (EditorImpl) editor cast in ResourceGotoDeclarationHandler and ResourceIndex.getFileByUri throws for injected fragments where the editor is an EditorWindowImpl (e.g. PHP injected into a Qiq tag). FileDocumentManager.getFile(editor.getDocument()) returns the same file without the cast and works for both regular and injected editors.
- QiqSupport implements TemplateEngineSupport. Maps var/qiq/template/<rel>.php <-> src/Resource/<rel>.php (bear/qiq-module 2.x). - EmbedQiqLineMarkerProvider draws the gutter icon on Qiq output tags whose expression is a stand-alone $this->X bound by #[Embed(rel: 'X')]. Registered for language="Qiq" so it runs on the host PSI once per tag; registering on language="PHP" produced two icons because the host's collectLineMarkersForInjected and the injected fragment's own pass both fired. Chained accesses ($this->X->Y, $this->X[...]) are rejected so the icon doesn't point at the sub-resource template instead of what's actually rendered. The variable name is taken via regex on the host text - the Qiq injector coalesces every host in a file into one PHP fragment, so walking injected PSI would surface unrelated $this->X. - EmbedQiqGotoDeclarationHandler is language-agnostic; Cmd+click on $this->X in any Qiq tag jumps to the embed target template. - EmbedResolver.resolveEmbeddedTemplates centralises the URI->class-> template resolution shared between the Twig and Qiq line marker providers. - Settings.qiqTemplatePath (default var/qiq/template) - single directory, matching QiqRenderer's constructor. - plugin.xml registers the EPs without depending on the Qiq Templates Support plugin: language="Qiq" stays unresolved when that plugin is absent and the EP is silently inactive, so the rest of the plugin keeps working. Verified with ./gradlew runIde (no Qiq plugin) in addition to ./gradlew runIdeWithQiq. - build.gradle.kts adds a test-only runIdeWithQiq task that installs the Qiq plugin into the sandbox; the published plugin does not depend on it.
WalkthroughTwig・Qiq テンプレートの埋め込みリソース参照に対する IDE ナビゲーション（GoTo宣言・参照解決・行マーカー）を実装。EditorImpl キャストを廃止して他エディタ実装との互換性を向上。build.gradle.kts に Twig 対応とテスト起動設定を追加しました。 Changes埋め込みテンプレートナビゲーション実装
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
src/main/java/idea/bear/sunday/template/EmbedTemplateReferenceContributor.java (1)
21-24: ⚡ Quick win参照プロバイダの登録対象を Twig 変数要素に限定してください。
Line 22 は Twig 言語の全 PSI 要素にマッチするため、不要な参照解決呼び出しが増えやすいです。
VARIABLE_REFERENCE相当の要素に絞ると、ハイライト時の負荷を下げられます。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/idea/bear/sunday/template/EmbedTemplateReferenceContributor.java` around lines 21 - 24, The reference provider is currently registered for all Twig PSI elements via PlatformPatterns.psiElement().withLanguage(TwigLanguage.INSTANCE), causing unnecessary resolution calls; change the pattern to target the Twig variable element (the VARIABLE_REFERENCE element type) instead. Update the registrar.registerReferenceProvider call so the pattern matches the Twig variable PSI element (e.g., use the Twig VARIABLE_REFERENCE element type in PlatformPatterns.psiElement(...).withLanguage(TwigLanguage.INSTANCE)) and keep using EmbedReferenceProvider and PsiReferenceRegistrar.HIGHER_PRIORITY.src/main/java/idea/bear/sunday/template/EmbedTwigLineMarkerProvider.java (1)
37-37: ⚡ Quick win要素型の判定で
toString()文字列比較を避けてください。Line 37 の
"VARIABLE_REFERENCE"直比較は壊れやすいので、Twig の要素型定数（または同等の型安全な判定）へ置き換える方が安全です。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/idea/bear/sunday/template/EmbedTwigLineMarkerProvider.java` at line 37, EmbedTwigLineMarkerProvider の条件で parent.getNode().getElementType().toString() を文字列比較している箇所は壊れやすいので、文字列比較をやめて要素型の定数（例: Twig の IElementType 定数）と型安全に比較するよう修正してください。具体的には EmbedTwigLineMarkerProvider 内の該当 if 条件を、toString() を使わず parent.getNode().getElementType() をそのまま Twig の要素型定数（例: VARIABLE_REFERENCE に対応する定数）と == か equals で比較するよう置き換え、利用している Twig ライブラリの TwigElementTypes（または同等の定数クラス）を参照するようにしてください。
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/idea/bear/sunday/template/EmbedQiqLineMarkerProvider.java`:
- Around line 55-57: The current regex and usage allow partial matches
(matcher.find()) so constructs like "{{= foo($this->x) }}" can produce
false-positive gutter icons; update EmbedQiqLineMarkerProvider to only match
whole-expression instances by changing the detection to require full-string
match (either switch from matcher.find() to matcher.matches() or anchor the
STANDALONE_THIS_PROPERTY pattern with ^...$) so that STANDALONE_THIS_PROPERTY
only returns true for a standalone "$this->X" expression; ensure any surrounding
whitespace/templating delimiters are accounted for if needed.
In `@src/main/java/idea/bear/sunday/template/EmbedResolver.java`:
- Around line 70-72: EmbedResolverの末尾スラッシュ処理で小文字の "index"
を連結してしまい大文字小文字が区別される環境でクラス解決が失敗しているので、classRel.endsWith("/") の分岐で連結する文字列を
"Index"（大文字I）に変更し、classRel に "Index" を追加するよう修正してください（対象シンボル: classRel,
EmbedResolver）。
In `@src/main/java/idea/bear/sunday/template/EmbedTemplateReference.java`:
- Around line 39-44: templates.get(0)
を固定で返しているため先頭が解決できないと他の候補を取りこぼします。support.resolveTemplates(embeddedClass)
の結果をループし、各 VirtualFile に対して PsiManager.getInstance(project).findFile(...)
を呼んで最初に非 null の PsiFile を返すように変更し、全て null なら最終的に null を返すようにしてください（参照箇所:
templates, templates.get(0), PsiManager.getInstance(project).findFile）。
In `@src/main/java/idea/bear/sunday/template/QiqSupport.java`:
- Around line 62-64: 現在のチェック only verifies that element is injected via
InjectedLanguageManager.getInstance(element.getProject()).getInjectionHost(element)
!= null, which allows non-Qiq injected PHP to pass; update the guard to also
confirm the injection host belongs to Qiq by calling your qiqHostFile(...)
predicate (or equivalent) before proceeding. In other words, replace or augment
the null-check in QiqSupport (around getInjectionHost(element)) so it returns
null unless getInjectionHost(element) is non-null AND qiqHostFile(hostFileOrPsi)
returns true; reference InjectedLanguageManager.getInstance,
getInjectionHost(element) and qiqHostFile(...) to locate and modify the
condition.
In `@src/main/java/idea/bear/sunday/template/TemplateEngineSupport.java`:
- Around line 26-33: The forFile method in TemplateEngineSupport currently
enumerates only TwigSupport.INSTANCE so Qiq cannot be selected; update the
candidate list in TemplateEngineSupport.forFile to include QiqSupport.INSTANCE
alongside TwigSupport.INSTANCE so the method can return QiqSupport when
QiqSupport.accepts(file, project) is true (i.e., add QiqSupport.INSTANCE to the
List.of(...) used in forFile).
---
Nitpick comments:
In
`@src/main/java/idea/bear/sunday/template/EmbedTemplateReferenceContributor.java`:
- Around line 21-24: The reference provider is currently registered for all Twig
PSI elements via
PlatformPatterns.psiElement().withLanguage(TwigLanguage.INSTANCE), causing
unnecessary resolution calls; change the pattern to target the Twig variable
element (the VARIABLE_REFERENCE element type) instead. Update the
registrar.registerReferenceProvider call so the pattern matches the Twig
variable PSI element (e.g., use the Twig VARIABLE_REFERENCE element type in
PlatformPatterns.psiElement(...).withLanguage(TwigLanguage.INSTANCE)) and keep
using EmbedReferenceProvider and PsiReferenceRegistrar.HIGHER_PRIORITY.
In `@src/main/java/idea/bear/sunday/template/EmbedTwigLineMarkerProvider.java`:
- Line 37: EmbedTwigLineMarkerProvider の条件で
parent.getNode().getElementType().toString()
を文字列比較している箇所は壊れやすいので、文字列比較をやめて要素型の定数（例: Twig の IElementType
定数）と型安全に比較するよう修正してください。具体的には EmbedTwigLineMarkerProvider 内の該当 if 条件を、toString()
を使わず parent.getNode().getElementType() をそのまま Twig の要素型定数（例: VARIABLE_REFERENCE
に対応する定数）と == か equals で比較するよう置き換え、利用している Twig ライブラリの
TwigElementTypes（または同等の定数クラス）を参照するようにしてください。
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 045da6d1-1712-4e57-b36a-75679b5d9d76
⛔ Files ignored due to path filters (2)
src/main/resources/icons/embed.svgis excluded by!**/*.svgsrc/main/resources/icons/embed_dark.svgis excluded by!**/*.svg
📒 Files selected for processing (17)
build.gradle.ktssrc/main/java/idea/bear/sunday/Settings.javasrc/main/java/idea/bear/sunday/index/ResourceIndex.javasrc/main/java/idea/bear/sunday/resource/ResourceGotoDeclarationHandler.javasrc/main/java/idea/bear/sunday/template/EmbedQiqGotoDeclarationHandler.javasrc/main/java/idea/bear/sunday/template/EmbedQiqLineMarkerProvider.javasrc/main/java/idea/bear/sunday/template/EmbedResolver.javasrc/main/java/idea/bear/sunday/template/EmbedTemplateReference.javasrc/main/java/idea/bear/sunday/template/EmbedTemplateReferenceContributor.javasrc/main/java/idea/bear/sunday/template/EmbedTwigLineMarkerProvider.javasrc/main/java/idea/bear/sunday/template/QiqSupport.javasrc/main/java/idea/bear/sunday/template/TemplateEngineSupport.javasrc/main/java/idea/bear/sunday/template/TemplateUtils.javasrc/main/java/idea/bear/sunday/template/TwigSupport.javasrc/main/resources/META-INF/plugin.xmlsrc/test/java/idea/bear/sunday/SettingsTest.javasrc/test/java/idea/bear/sunday/template/TemplateUtilsTest.java
URI templates ending with a trailing slash (e.g. "page://self/") resolve to the default Index resource. Linux file systems are case-sensitive, so appending "index" produced "resource/page/index.php" and failed to find the actual "Index.php" file. Use "Index" to match the PSR-4 class file name on case-sensitive systems.
The previous check only verified that the PHP fragment was *injected*, so a $this->X reference inside any non-Qiq injection (a HEREDOC, a different template engine plugin's injection, etc.) would be treated as a Qiq variable and produce a misleading EmbedTemplateReference. Route the check through qiqHostFile(), which additionally requires the host file to satisfy QiqSupport#accepts (correct extension and under the configured Qiq template directory).
forFile is the single entry point used by EmbedTemplateReferenceContributor to dispatch a file to its template engine. Without QiqSupport in the candidate list, .php files under the configured Qiq template directory never resolved a TemplateEngineSupport, so the reference-based navigation fell back to no-op even though the Qiq line marker / goto handler worked independently. Add QiqSupport.INSTANCE so the contributor path matches the line-marker path.
The previous pattern combined Matcher#find with negative lookaheads, which
still allowed a partial match against any $this->X occurrence inside a
larger expression. A tag like {{= foo($this->x) }} would match the inner
$this->x because the closing ')' satisfies the negative lookaheads, and the
gutter icon would mislead users into thinking the tag renders the embedded
resource directly.
Switch to a whitespace-tolerant pattern used with Matcher#matches so the
entire host text must be a stand-alone $this->X — any wrapping expression
fails to match.
…iFile TwigSupport.resolveTemplates returns multiple VirtualFile candidates when several twig.template_paths are configured. The previous resolve() only asked PsiManager about templates.get(0): if that file could not be resolved to a PsiFile (it had a missing PSI parser, was marked binary, or the project no longer indexed its containing directory), the reference returned null and the remaining candidates were silently ignored. Iterate the candidates and return the first one that resolves to a PsiFile. The behaviour matches EmbedResolver#resolveEmbeddedTemplates already used by the line markers, so reference- and gutter-navigation now agree.
The previous pattern matched every PsiElement under TwigLanguage, so the provider's getReferencesByElement ran on every leaf and composite Twig node during highlight passes — keywords, punctuation, whitespace included. Each call performed a TemplateEngineSupport#forFile dispatch and an extractVariableName check just to discard the element. Restrict the pattern to TwigElementTypes.VARIABLE_REFERENCE so the IDE only calls the provider on the composite element that actually backs a Twig variable, leaving extractVariableName's PRINT_BLOCK check as the second-line guard.
Stringifying an IElementType via toString() and comparing it against a literal couples both call sites to whatever debug-name the Twig plugin chooses for VARIABLE_REFERENCE. If JetBrains ever renames or namespaces the debug string (or returns a localised one) the check silently breaks, with no compile-time warning. Use == against TwigElementTypes.VARIABLE_REFERENCE, the same constant the Twig plugin uses to mark the node. TwigSupport.java already imports TwigElementTypes for PRINT_BLOCK, so this also aligns both checks in that file. EmbedTwigLineMarkerProvider.java picks up the matching import.
EmbedTemplateReference#resolve was iterating over VirtualFile candidates and calling PsiManager#findFile to produce a PsiFile, identical to the loop inside EmbedResolver#resolveEmbeddedTemplates. The earlier fix that added the loop here missed the existing helper. Delegate to resolveEmbeddedTemplates and take the first element. The single- candidate vs. all-candidates distinction is now encoded only at call sites: LineMarker providers consume the collection, the reference returns the head.
The host-file restriction rationale ("otherwise non-Qiq injections would be
misclassified") was duplicated three times: here, on qiqHostFile's JavaDoc,
and inline at the host's accepts() call. Keep the tag-kind WHY (which only
exists at this call site) and point readers at qiqHostFile for the rest.
The expanded STANDALONE_THIS_PROPERTY JavaDoc restated the wrapping-rejection rationale already documented in the class JavaDoc (paragraph 4). Keep only what is local to the pattern: that it is anchored via Matcher#matches and whitespace-tolerant. findEmbedReference's JavaDoc only paraphrased the method name and parameter names, so remove it entirely; the body is three lines.
TwigSupport.extractVariableName already verifies that its argument has a non-null AST node, is a Twig VARIABLE_REFERENCE composite element, and lives inside a Twig print block. The provider was repeating the first two of those checks (and a stale comment claimed they were a precondition for reusing the extractor). Remove the duplicated checks and call extractVariableName first — it acts as an early filter for any leaf whose parent is not a Twig variable. The TwigElementTypes import becomes unused with this change.
Contributor
Author
|
@coderabbitai Thanks for the review. All 7 actionable comments are addressed in separate commits below, with 4 additional cleanups surfaced by an internal self-review. CodeRabbit findings
Self-review cleanups
Verified
|
|
🐇 ✨ If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! ✅ Actions performedFull review triggered. |
Template resolution no longer depends on the configured Twig/Qiq directory settings. Both engines look the template up by file name via FilenameIndex and keep candidates under the resource's app root with the matching relative path, so any template directory works without configuration. Removes the now-unused twigTemplatePaths/qiqTemplatePath settings and the trimSlash helper.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/idea/bear/sunday/template/TemplateUtils.java`:
- Around line 67-86: TemplateUtils performs index/PSI access without dumb-mode
guards or read-action protection; add a DumbService check at the start of
filesNamed, findClass, and findClassByAbsolutePath to return an appropriate
empty/null when DumbService.isDumb(project) is true (so callers like
resolveEmbeddedTemplates used from Embed* providers get safe empty/null results
in dumb mode), and wrap PSI/index operations (PsiManager.findFile,
PsiTreeUtil.findChildOfType, FilenameIndex.getVirtualFilesByName,
LocalFileSystem.findFileByPath) inside
ApplicationManager.getApplication().runReadAction(...) to ensure read-action
safety; keep return semantics (filesNamed -> empty collection,
findClass/findClassByAbsolutePath -> null).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d375ac5c-789d-4cfd-ab3c-aedc761aeb5b
📒 Files selected for processing (4)
src/main/java/idea/bear/sunday/template/QiqSupport.javasrc/main/java/idea/bear/sunday/template/TemplateUtils.javasrc/main/java/idea/bear/sunday/template/TwigSupport.javasrc/test/java/idea/bear/sunday/template/TemplateUtilsTest.java
Comment on lines
+67
to
+86
| /** Project files (libraries excluded) with the given file name, looked up via the name index. */ | ||
| @NotNull | ||
| static Collection<VirtualFile> filesNamed(@NotNull Project project, @NotNull String fileName) { | ||
| return FilenameIndex.getVirtualFilesByName(fileName, GlobalSearchScope.projectScope(project)); | ||
| } | ||
|
|
||
| @Nullable | ||
| static PhpClass findClass(@NotNull Project project, @NotNull VirtualFile file) { | ||
| PsiFile psiFile = PsiManager.getInstance(project).findFile(file); | ||
| if (psiFile == null) { | ||
| return null; | ||
| } | ||
| return PsiTreeUtil.findChildOfType(psiFile, PhpClass.class); | ||
| } | ||
|
|
||
| @Nullable | ||
| static PhpClass findClassByAbsolutePath(@NotNull Project project, @NotNull String absolutePath) { | ||
| VirtualFile classFile = LocalFileSystem.getInstance().findFileByPath(absolutePath); | ||
| return classFile == null ? null : findClass(project, classFile); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# TemplateUtils.filesNamed / findClass の呼び出し元を特定
rg -nP --type=java -C3 '\b(filesNamed|findClass|findClassByAbsolutePath)\s*\('
# 行マーカー/コントリビュータ/GoToハンドラ内での DumbService ガード有無を確認
fd -e java -i -g '*LineMarker*' -g '*Contributor*' -g '*GotoDeclaration*' \
--exec rg -nP -C3 '(DumbService|isDumb|runReadAction|ReadAction)' {} \;Repository: bearsunday/idea-php-bearsunday-plugin
Length of output: 8574
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) TemplateUtils の各メソッドの呼び出し元を全特定
rg -n --type=java "TemplateUtils\.(filesNamed|findClass|findClassByAbsolutePath)\s*\(" src/main/java
# 2) DumbService / isDumb / runWhenSmart / runReadAction / ReadAction の出現箇所を、関係しそうなクラス周辺中心に確認
# （まず該当ファイル名を手早く抽出）
files=$(rg -n --type=java "TemplateUtils\.(filesNamed|findClass|findClassByAbsolutePath)\s*\(" src/main/java \
| cut -d: -f1 | sort -u)
echo "---- Related files ----"
echo "$files"
for f in $files; do
echo "==== $f : Dumb/readaction related context ===="
rg -n --type=java -C3 "(DumbService|getInstance\(\)\.runWhenSmart|isDumb\(|runWhenSmart|runReadAction|ReadAction\.run|ReadAction)" "$f" || true
done
# 3) TemplateUtils 自体の呼び出しに近い周辺（呼び出し元の可能性が高い箇所）を出す
for f in $files; do
echo "==== $f : call sites (TemplateUtils.*) ===="
rg -n --type=java -C3 "TemplateUtils\.(filesNamed|findClass|findClassByAbsolutePath)\s*\(" "$f"
doneRepository: bearsunday/idea-php-bearsunday-plugin
Length of output: 5334
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --type=java "(DumbService|isDumb\(|runWhenSmart|runReadAction|ReadAction)" \
src/main/java/idea/bear/sunday/template/QiqSupport.java \
src/main/java/idea/bear/sunday/template/TwigSupport.java \
src/main/java/idea/bear/sunday/template/EmbedResolver.java || true
# 呼び出し元の該当周辺を表示
for f in \
src/main/java/idea/bear/sunday/template/QiqSupport.java \
src/main/java/idea/bear/sunday/template/TwigSupport.java \
src/main/java/idea/bear/sunday/template/EmbedResolver.java
do
echo "==== context for $f ===="
rg -n --type=java "TemplateUtils\.(filesNamed|findClass|findClassByAbsolutePath)\s*\(" "$f"
# show ~40 lines around each match
rg -n --type=java -C40 "TemplateUtils\.(filesNamed|findClass|findClassByAbsolutePath)\s*\(" "$f"
doneRepository: bearsunday/idea-php-bearsunday-plugin
Length of output: 14518
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 呼び出し元探索: EmbedResolver.resolveEmbeddedTemplates を中心に
rg -n --type=java "EmbedResolver\.resolveEmbeddedTemplates\s*\(" src/main/java
# ついでに resolveTemplates / resolveResourceClass を呼んでいる箇所（LineMarker/GoTo の可能性がある）
rg -n --type=java "(\.resolveTemplates\s*\(|\.resolveResourceClass\s*\()" src/main/java/idea/bear/sunday/template
# DumbService / isDumb / runWhenSmart / runReadAction / ReadAction の有無（上記の呼び出し元候補周辺）
# まず呼び出し元候補ファイルを列挙してから周辺検索
candidates=$(rg -n --type=java "EmbedResolver\.resolveEmbeddedTemplates\s*\(" src/main/java | cut -d: -f1 | sort -u)
echo "Candidates:"
echo "$candidates"
for f in $candidates; do
echo "==== Dumb/ReadAction context in $f ===="
rg -n --type=java -C3 "(DumbService|getInstance\(\)\.runWhenSmart|isDumb\(|runWhenSmart|runReadAction|ReadAction)" "$f" || true
doneRepository: bearsunday/idea-php-bearsunday-plugin
Length of output: 2376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
files=(
src/main/java/idea/bear/sunday/template/EmbedQiqLineMarkerProvider.java
src/main/java/idea/bear/sunday/template/EmbedTwigLineMarkerProvider.java
src/main/java/idea/bear/sunday/template/EmbedQiqGotoDeclarationHandler.java
src/main/java/idea/bear/sunday/template/EmbedTemplateReference.java
src/main/java/idea/bear/sunday/template/EmbedTemplateReferenceContributor.java
)
for f in "${files[@]}"; do
echo "===== $f (size) ====="
wc -l "$f" || true
done
# マッチ箇所近傍を表示
for f in "${files[@]}"; do
echo "===== $f : EmbedResolver.resolveEmbeddedTemplates call context ====="
rg -n --type=java "EmbedResolver\.resolveEmbeddedTemplates" "$f" || true
rg -n --type=java -C8 "EmbedResolver\.resolveEmbeddedTemplates" "$f" || true
done
# クラス宣言（DumbAware等）と read action 関連の手がかり探し
for f in "${files[@]}"; do
echo "===== $f : class header / DumbAware / readAction patterns ====="
rg -n --type=java "class |DumbAware|DumbService|runWhenSmart|runReadAction|ReadAction" "$f" || true
done
# ついでに TemplateUtils 本体も Dumb/ReadAction がないか確認
echo "===== TemplateUtils.java patterns ====="
rg -n --type=java "(DumbService|getInstance\(\)\.runWhenSmart|runReadAction|ReadAction|dumb mode|IndexNotReadyException)" src/main/java/idea/bear/sunday/template/TemplateUtils.java || trueRepository: bearsunday/idea-php-bearsunday-plugin
Length of output: 6033
🏁 Script executed:
#!/bin/bash
set -euo pipefail
files=(
src/main/java/idea/bear/sunday/template/EmbedQiqLineMarkerProvider.java
src/main/java/idea/bear/sunday/template/EmbedTwigLineMarkerProvider.java
src/main/java/idea/bear/sunday/template/EmbedQiqGotoDeclarationHandler.java
src/main/java/idea/bear/sunday/template/EmbedTemplateReference.java
src/main/java/idea/bear/sunday/template/EmbedTemplateReferenceContributor.java
src/main/java/idea/bear/sunday/template/TemplateUtils.java
)
for f in "${files[@]}"; do
echo "===== $f (top 140 lines with numbers) ====="
nl -ba "$f" | head -n 140
done
# DumbService / ReadAction をより広く探索
rg -n --type=java "(DumbService|getInstance\(\)\.runWhenSmart|runWhenSmart|isDumb|IndexNotReadyException|ReadAction|readAction\()" src/main/java/idea/bear/sunday/template/Embed*.java src/main/java/idea/bear/sunday/template/TemplateUtils.java || trueRepository: bearsunday/idea-php-bearsunday-plugin
Length of output: 256
🏁 Script executed:
#!/bin/bash
set -euo pipefail
files=(
src/main/java/idea/bear/sunday/template/EmbedQiqLineMarkerProvider.java
src/main/java/idea/bear/sunday/template/EmbedTwigLineMarkerProvider.java
src/main/java/idea/bear/sunday/template/EmbedQiqGotoDeclarationHandler.java
src/main/java/idea/bear/sunday/template/EmbedTemplateReference.java
src/main/java/idea/bear/sunday/template/EmbedTemplateReferenceContributor.java
src/main/java/idea/bear/sunday/template/TemplateUtils.java
)
# まずテンプレートパッケージ内で Dumb/ReadAction 関連の出現を確認（広すぎる場合は template ディレクトリに限定）
rg -n --type=java "(DumbService|getInstance\(\)\.runWhenSmart|runWhenSmart|isDumb|IndexNotReadyException|ReadAction|readAction\(|NotNullLazyValue\.lazy|ReadAction\.compute|compute|executeOnPooledThread)" \
src/main/java/idea/bear/sunday/template || true
# 各ファイルの冒頭〜150行（クラス宣言や interface 実装を確認）
for f in "${files[@]}"; do
echo "===== $f : first 160 lines ====="
cat -n "$f" | head -n 160
done
# 直近で呼び出しが出ていた箇所周辺（LineMarker/resolve/GetTargets等）を追加で絞り込み表示
rg -n --type=java "NotNullLazyValue\.lazy|resolve\(\)|getTargets|getPsiElement|getElement|getActionText" \
src/main/java/idea/bear/sunday/template/EmbedQiqLineMarkerProvider.java \
src/main/java/idea/bear/sunday/template/EmbedTwigLineMarkerProvider.java \
src/main/java/idea/bear/sunday/template/EmbedQiqGotoDeclarationHandler.java \
src/main/java/idea/bear/sunday/template/EmbedTemplateReference.java \
src/main/java/idea/bear/sunday/template/EmbedTemplateReferenceContributor.java || true
echo "===== EmbedQiqLineMarkerProvider around EmbedResolver call ====="
cat -n src/main/java/idea/bear/sunday/template/EmbedQiqLineMarkerProvider.java | sed -n '60,110p'
echo "===== EmbedTwigLineMarkerProvider around EmbedResolver call ====="
cat -n src/main/java/idea/bear/sunday/template/EmbedTwigLineMarkerProvider.java | sed -n '35,90p'
echo "===== EmbedQiqGotoDeclarationHandler around EmbedResolver call ====="
cat -n src/main/java/idea/bear/sunday/template/EmbedQiqGotoDeclarationHandler.java | sed -n '25,80p'
echo "===== EmbedTemplateReference around resolve() ====="
cat -n src/main/java/idea/bear/sunday/template/EmbedTemplateReference.java | sed -n '1,90p'Repository: bearsunday/idea-php-bearsunday-plugin
Length of output: 31075
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# plugin.xml / plugin descriptor を探す
fd -t f "plugin.xml" . > /tmp/plugin_xmls.txt || true
echo "Found plugin.xml:"
cat /tmp/plugin_xmls.txt || true
# 重要クラス名を plugin.xml 内で検索
classes=(
"EmbedQiqLineMarkerProvider"
"EmbedTwigLineMarkerProvider"
"EmbedQiqGotoDeclarationHandler"
"EmbedTemplateReferenceContributor"
)
for c in "${classes[@]}"; do
echo "---- matches for $c ----"
rg -n --hidden --glob '!**/build/**' "$c" -S . $(cat /tmp/plugin_xmls.txt 2>/dev/null || true) || true
done
# DumbAware や dumb タグがあれば一緒に探索（該当拡張周辺だけでも絞る）
rg -n --hidden --glob '!**/build/**' "(DumbAware|dumbAware|DUMB_AWARE|dumb-aware)" . || trueRepository: bearsunday/idea-php-bearsunday-plugin
Length of output: 2534
🏁 Script executed:
#!/bin/bash
set -euo pipefail
f="src/main/resources/META-INF/plugin.xml"
# print a focused window around the reported line numbers
sed -n '1,120p' "$f" | cat -nRepository: bearsunday/idea-php-bearsunday-plugin
Length of output: 4022
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# plugin.xml の dumbAware / dumb-aware 指定有無
rg -n --type=xml -S "dumbAware|dumb-aware|DumbAware" src/main/resources/META-INF/plugin.xml || true
# 該当クラスが DumbAware を実装しているか
for f in \
src/main/java/idea/bear/sunday/template/EmbedQiqLineMarkerProvider.java \
src/main/java/idea/bear/sunday/template/EmbedTwigLineMarkerProvider.java \
src/main/java/idea/bear/sunday/template/EmbedQiqGotoDeclarationHandler.java \
src/main/java/idea/bear/sunday/template/EmbedTemplateReferenceContributor.java \
src/main/java/idea/bear/sunday/template/EmbedTemplateReference.java
do
echo "---- $f ----"
rg -n "DumbAware|DumbService" "$f" || true
doneRepository: bearsunday/idea-php-bearsunday-plugin
Length of output: 499
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --type=java "(DumbService|isDumb|getInstance\(\)\.runWhenSmart|runWhenSmart|runReadAction|ReadAction|IndexNotReadyException)" \
src/main/java/idea/bear/sunday/template/QiqSupport.java \
src/main/java/idea/bear/sunday/template/TwigSupport.java \
src/main/java/idea/bear/sunday/template/TemplateEngineSupport.java \
src/main/java/idea/bear/sunday/template/EmbedResolver.java || true
# TemplateEngineSupport の実装/抽象を見に行く
rg -n --type=java "class TemplateEngineSupport|interface TemplateEngineSupport|enum TemplateEngineSupport" \
src/main/java/idea/bear/sunday/template || true
# EmbedResolver 周辺も少し見る（resolveEmbeddedTemplates / resolveEmbeddedClass の文脈）
cat -n src/main/java/idea/bear/sunday/template/EmbedResolver.java | sed -n '1,180p'Repository: bearsunday/idea-php-bearsunday-plugin
Length of output: 6716
dumb mode（Index作成中）に FilenameIndex / PsiManager を触らないガードを追加してください
TemplateUtils.filesNamed（FilenameIndex.getVirtualFilesByName）および TemplateUtils.findClass/findClassByAbsolutePath（PsiManager.getInstance(...).findFile + PsiTreeUtil.findChildOfType）はインデックス/PSIアクセスを行いますが、EmbedResolver.resolveEmbeddedTemplates 経由で EmbedQiqLineMarkerProvider/EmbedTwigLineMarkerProvider（NotNullLazyValue.lazy の supplier）、EmbedQiqGotoDeclarationHandler#getGotoDeclarationTargets、EmbedTemplateReference#resolve から呼ばれており、DumbService/isDumb/runWhenSmart 等の dumb mode ガードや read action の明示がこのコード上に見当たりません。さらに plugin.xml でもこれら拡張の DumbAware 指定はありません。
dumb mode では（もしくは smart mode へ切り替えた上で）resolveEmbeddedTemplates 内の TemplateUtils 呼び出し部分を実行するようにガード（dumb 時は空返却/ null 返却）を入れてください。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/idea/bear/sunday/template/TemplateUtils.java` around lines 67 -
86, TemplateUtils performs index/PSI access without dumb-mode guards or
read-action protection; add a DumbService check at the start of filesNamed,
findClass, and findClassByAbsolutePath to return an appropriate empty/null when
DumbService.isDumb(project) is true (so callers like resolveEmbeddedTemplates
used from Embed* providers get safe empty/null results in dumb mode), and wrap
PSI/index operations (PsiManager.findFile, PsiTreeUtil.findChildOfType,
FilenameIndex.getVirtualFilesByName, LocalFileSystem.findFileByPath) inside
ApplicationManager.getApplication().runReadAction(...) to ensure read-action
safety; keep return semantics (filesNamed -> empty collection,
findClass/findClassByAbsolutePath -> null).
Contributor
Author
|
テンプレートの探索を設定ディレクトリ依存から @coderabbitai review |
|
The PR description image shows the JetBrains IDE with
✅ Actions performedReview triggered.
|
Member
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/idea/bear/sunday/index/ResourceIndex.java (1)
100-111:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win注入フラグメント時は
getFileがnullになり得るため分岐意図を確認IntelliJ では
FileDocumentManager#getFile(Document)は、Injected なDocumentWindow等（VirtualFile に紐づかない Document）だとnullを返すのが仕様です。そのため現状はeditFile == nullとなり、/src/Resource/Page判定を行えず常に"App"側にフォールバックする可能性があります。ホスト（テンプレート）ファイルのパスで"Page"判定したいなら、InjectedLanguageManager等でホストに戻してから VirtualFile を解決する対応が必要です。意図として"App"固定でよいならそのままでOKです。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/idea/bear/sunday/index/ResourceIndex.java` around lines 100 - 111, The current branch in ResourceIndex checking FileDocumentManager.getFile(editor.getDocument()) can receive null for injected Documents, causing the logic that appends "Page" vs "App" to always fall back to "App"; update the branch to resolve the host file when editor's document is injected by using InjectedLanguageManager.getInstance(project).getTopLevelFile(editor.getDocument()) or otherwise mapping to the host VirtualFile before calling FileDocumentManager.getFile, so that the path check against project.getBasePath() + "/src/Resource/Page" correctly detects host template files; if the intent is to always treat injected fragments as "App" leave as-is but add a clear comment in ResourceIndex near the editor / editFile handling explaining the choice.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/main/java/idea/bear/sunday/index/ResourceIndex.java`:
- Around line 100-111: The current branch in ResourceIndex checking
FileDocumentManager.getFile(editor.getDocument()) can receive null for injected
Documents, causing the logic that appends "Page" vs "App" to always fall back to
"App"; update the branch to resolve the host file when editor's document is
injected by using
InjectedLanguageManager.getInstance(project).getTopLevelFile(editor.getDocument())
or otherwise mapping to the host VirtualFile before calling
FileDocumentManager.getFile, so that the path check against
project.getBasePath() + "/src/Resource/Page" correctly detects host template
files; if the intent is to always treat injected fragments as "App" leave as-is
but add a clear comment in ResourceIndex near the editor / editFile handling
explaining the choice.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 23e526cc-9e1e-4680-8bd1-9943c515114f
⛔ Files ignored due to path filters (2)
src/main/resources/icons/embed.svgis excluded by!**/*.svgsrc/main/resources/icons/embed_dark.svgis excluded by!**/*.svg
📒 Files selected for processing (15)
build.gradle.ktssrc/main/java/idea/bear/sunday/index/ResourceIndex.javasrc/main/java/idea/bear/sunday/resource/ResourceGotoDeclarationHandler.javasrc/main/java/idea/bear/sunday/template/EmbedQiqGotoDeclarationHandler.javasrc/main/java/idea/bear/sunday/template/EmbedQiqLineMarkerProvider.javasrc/main/java/idea/bear/sunday/template/EmbedResolver.javasrc/main/java/idea/bear/sunday/template/EmbedTemplateReference.javasrc/main/java/idea/bear/sunday/template/EmbedTemplateReferenceContributor.javasrc/main/java/idea/bear/sunday/template/EmbedTwigLineMarkerProvider.javasrc/main/java/idea/bear/sunday/template/QiqSupport.javasrc/main/java/idea/bear/sunday/template/TemplateEngineSupport.javasrc/main/java/idea/bear/sunday/template/TemplateUtils.javasrc/main/java/idea/bear/sunday/template/TwigSupport.javasrc/main/resources/META-INF/plugin.xmlsrc/test/java/idea/bear/sunday/template/TemplateUtilsTest.java
Member
|
@claude review |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🙇 bearsunday メンテナーの皆様、本 PR のレビューをお願いいたします。
概要
#[Embed]でリソースを埋め込むと、テンプレートには{{ embedded }}（Twig）/{{= $this->embedded }}（Qiq）のような短い変数参照だけが残ります。本 PR は、その変数から「実際に描画されるテンプレート」へ IDE 上でワンクリックでジャンプできるようにします。機能
Twig (
madapaja/twig-module){{ X }}を Cmd+click → 埋め込みリソースのテンプレートへジャンプ#[Embed(rel: 'X')]に対応する{{ X }}の行にガターアイコンQiq (
bear/qiq-module2.x)$this->Xを Cmd+click → 埋め込みリソースのテンプレートへジャンプ#[Embed(rel: 'X')]に対応する{{= $this->X }}出力タグの行にガターアイコンPsiReference（「この変数はこのファイルを指す」と IDE に教える参照）で実現しています。一方 Qiq の$this->Xは PHP のFieldReferenceで、ここに参照を上書きすると Qiq プラグイン／PHP 本来のフィールド解決と競合してしまいます。それを避けるため Qiq では参照ではなくGotoDeclarationHandler（クリック時のジャンプ専用）を採用しており、その性質上ホバープレビューは出ません（ジャンプは可能です）。テンプレートの場所は自動検出
テンプレートのディレクトリ（
var/templates、var/twigなど）はプロジェクトごとに異なります。本 PR ではディレクトリを決め打ちせず、IDE のファイル索引（FilenameIndex）から名前で検索し、リソースと同じ相対パスにあるテンプレートを採用します。設定は不要です。Qiq プラグインは任意
Qiq Templates Support プラグイン（
io.github.jingu.idea-qiq-plugin）はコンパイル時依存にしていません。未インストールの環境では Qiq 関連機能が無効化されるだけで、プラグイン本体は通常通り動作します。検証
./gradlew test── ユニットテスト./gradlew runIde── Twig（Qiq プラグインなし）./gradlew runIdeWithQiq── Qiq（Qiq プラグインあり）Summary by CodeRabbit
New Features
Bug Fixes
Tests