Fix camelCase resource URI goto (#11) and add AOP interceptor goto (#19)#20
Merged
Conversation
Resource URI goto reconstructed the resource class path with WordUtils.capitalizeFully(), which lower-cases every character after the first of each segment. A camelCase URI such as app://self/blogPosting was therefore resolved to Blogposting.php instead of BlogPosting.php and the goto silently failed. Switch to WordUtils.capitalize(), which only upper-cases the first letter of each '/' or '-' delimited segment and preserves the rest, so both camelCase and hyphenated URIs resolve correctly. The pure URI -> relative path transformation is extracted into UriUtil.toResourceRelativePath() and covered by regression tests. https://claude.ai/code/session_015g4MGXB3hgEjhBi74CLPa9
Ctrl/Cmd-click on a Ray.Aop binding attribute (e.g. #[Transactional]) now jumps to the interceptor class(es) bound to it in a module via bindInterceptor(). A new file-based index (InterceptorBindingIndex) scans modules for bindInterceptor(..., $this->matcher->annotatedWith(X::class), [Interceptor::class]) calls and maps the annotation FQN to its bound interceptor FQNs. InterceptorGotoDeclarationHandler resolves the clicked attribute FQN, looks it up in the index and navigates to the interceptor classes via PhpIndex. https://claude.ai/code/session_015g4MGXB3hgEjhBi74CLPa9
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughResource URI 解決を統一、ProjectUtil でのプロジェクトディレクトリ推定を導入し、Ray.Aop の bindInterceptor をインデックス化して属性からバウンドインターセプタへナビゲート可能にした変更群。キャメルケース URI 対応も含む。 ChangesResource URI 統一と Interceptor Navigation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
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: 1
🧹 Nitpick comments (1)
src/main/java/idea/bear/sunday/aop/InterceptorBindingIndexUtil.java (1)
42-42: 💤 Low valuePHP のメソッド名は大文字小文字を区別しないため、比較を大文字小文字非依存にするのが安全です
InterceptorBindingIndexUtilではcall.getName()をBIND_INTERCEPTOR ("bindInterceptor")、matcher.getName()をANNOTATED_WITH ("annotatedWith")でString#equals（大文字小文字区別）比較しています（42, 73行目）。PHP はメソッド名が大文字小文字を区別しないため、別表記で呼ばれるとインデックスが取りこぼす可能性があるので、equalsIgnoreCaseや正規化（例:toLowerCase(Locale.ROOT)）での比較を検討してください。🤖 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/aop/InterceptorBindingIndexUtil.java` at line 42, InterceptorBindingIndexUtil currently compares call.getName() to BIND_INTERCEPTOR ("bindInterceptor") and matcher.getName() to ANNOTATED_WITH ("annotatedWith") using case-sensitive String#equals; change these comparisons to be case-insensitive (e.g., use String.equalsIgnoreCase or normalize both sides with toLowerCase(Locale.ROOT)) so PHP method-name case-variants do not miss matches — update the comparisons where call.getName() and matcher.getName() are checked against BIND_INTERCEPTOR and ANNOTATED_WITH respectively.
🤖 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/index/ResourceIndex.java`:
- Line 84: The code in ResourceIndex.java uses the deprecated
Project#getBaseDir() when resolving the target file (VirtualFile targetFile =
project.getBaseDir().findFileByRelativePath(relPath)); replace that call by
first calling com.intellij.openapi.project.ProjectUtil.guessProjectDir(project),
then use the returned VirtualFile to call findFileByRelativePath(relPath); also
add null checks for the guessed project dir before calling
findFileByRelativePath to avoid NPEs and handle the case where the project dir
is absent.
---
Nitpick comments:
In `@src/main/java/idea/bear/sunday/aop/InterceptorBindingIndexUtil.java`:
- Line 42: InterceptorBindingIndexUtil currently compares call.getName() to
BIND_INTERCEPTOR ("bindInterceptor") and matcher.getName() to ANNOTATED_WITH
("annotatedWith") using case-sensitive String#equals; change these comparisons
to be case-insensitive (e.g., use String.equalsIgnoreCase or normalize both
sides with toLowerCase(Locale.ROOT)) so PHP method-name case-variants do not
miss matches — update the comparisons where call.getName() and matcher.getName()
are checked against BIND_INTERCEPTOR and ANNOTATED_WITH respectively.
🪄 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: c35fad96-56e5-4242-9fdb-34be70b3a88e
📒 Files selected for processing (10)
CHANGELOG.mdREADME.mdsrc/main/java/idea/bear/sunday/aop/InterceptorBindingExternalizer.javasrc/main/java/idea/bear/sunday/aop/InterceptorBindingIndex.javasrc/main/java/idea/bear/sunday/aop/InterceptorBindingIndexUtil.javasrc/main/java/idea/bear/sunday/aop/InterceptorGotoDeclarationHandler.javasrc/main/java/idea/bear/sunday/index/ResourceIndex.javasrc/main/java/idea/bear/sunday/util/UriUtil.javasrc/main/resources/META-INF/plugin.xmlsrc/test/java/idea/bear/sunday/util/UriUtilTest.java
- Match PHP method names case-insensitively when indexing interceptor bindings (bindInterceptor / annotatedWith), since PHP method names are case-insensitive - Replace deprecated Project#getBaseDir() with ProjectUtil.guessProjectDir() and guard against a null project dir in ResourceIndex.getFileByUri() https://claude.ai/code/session_015g4MGXB3hgEjhBi74CLPa9
Follow-up to the CodeRabbit review: replace the deprecated Project#getBaseDir() with ProjectUtil.guessProjectDir() (plus null guards) in the router, SQL and JSON Schema goto handlers and in the resource index filter, so the whole plugin uses the recommended API consistently. Bump the resource index version accordingly. https://claude.ai/code/session_015g4MGXB3hgEjhBi74CLPa9
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 (2)
src/main/java/idea/bear/sunday/annotation/AnnotationOrAttributeGotoDeclarationHandler.java (1)
117-119:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
psiManager.findFileの null チェックが欠落しています。
findFileはnullを返す可能性があり、null要素を含む配列を返すと後続のナビゲーション処理で NPE を引き起こす恐れがあります。jsonSchemaGoto（167-169行）では null チェックを行っており、こちらと整合させるべきです。🛡️ 提案する修正
PsiFile psiFile = psiManager.findFile(targetFile); - psiElements.add(psiFile); + if (psiFile != null) { + psiElements.add(psiFile); + }🤖 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/annotation/AnnotationOrAttributeGotoDeclarationHandler.java` around lines 117 - 119, In AnnotationOrAttributeGotoDeclarationHandler, add a null-check around the result of psiManager.findFile so you don't add null into psiElements: after calling psiManager.findFile(targetFile) verify psiFile != null and only then call psiElements.add(psiFile); mirror the null-check pattern used in jsonSchemaGoto to ensure navigation code won’t encounter NPEs when iterating psiElements.src/main/java/idea/bear/sunday/router/RouterGotoDeclarationHandler.java (1)
71-72:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
psiManager.findFileの戻り値を null チェックしてください。
findFileはnullを返し得るため、nullをpsiElementsに追加すると返却配列にnull要素が混入し、ナビゲーション時に NPE を招く可能性があります。🛡️ 提案する修正
PsiFile pfiFile = psiManager.findFile(targetFile); - psiElements.add(pfiFile); + if (pfiFile != null) { + psiElements.add(pfiFile); + }🤖 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/router/RouterGotoDeclarationHandler.java` around lines 71 - 72, findFile の戻り値を null チェックせずに psiElements に追加しているため null が混入し NPE を招く可能性があります; 呼び出し箇所の psiManager.findFile(targetFile) の結果を PsiFile pfiFile で受け取った後に null チェックを行い、非 null のときのみ psiElements.add(pfiFile) するよう修正してください（つまり PsiFile pfiFile の null 判定を追加してから psiElements へ追加する）。
🤖 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/annotation/AnnotationOrAttributeGotoDeclarationHandler.java`:
- Around line 117-119: In AnnotationOrAttributeGotoDeclarationHandler, add a
null-check around the result of psiManager.findFile so you don't add null into
psiElements: after calling psiManager.findFile(targetFile) verify psiFile !=
null and only then call psiElements.add(psiFile); mirror the null-check pattern
used in jsonSchemaGoto to ensure navigation code won’t encounter NPEs when
iterating psiElements.
In `@src/main/java/idea/bear/sunday/router/RouterGotoDeclarationHandler.java`:
- Around line 71-72: findFile の戻り値を null チェックせずに psiElements に追加しているため null が混入し
NPE を招く可能性があります; 呼び出し箇所の psiManager.findFile(targetFile) の結果を PsiFile pfiFile
で受け取った後に null チェックを行い、非 null のときのみ psiElements.add(pfiFile) するよう修正してください（つまり
PsiFile pfiFile の null 判定を追加してから psiElements へ追加する）。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c21e90c4-fa98-435c-b3f8-b9bfef9c3c70
📒 Files selected for processing (5)
CHANGELOG.mdsrc/main/java/idea/bear/sunday/annotation/AnnotationOrAttributeGotoDeclarationHandler.javasrc/main/java/idea/bear/sunday/aop/InterceptorBindingIndexUtil.javasrc/main/java/idea/bear/sunday/index/ResourceIndex.javasrc/main/java/idea/bear/sunday/router/RouterGotoDeclarationHandler.java
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/idea/bear/sunday/aop/InterceptorBindingIndexUtil.java
- src/main/java/idea/bear/sunday/index/ResourceIndex.java
PsiManager#findFile is nullable; sqlGoto and the router goto added its result to the targets list without a null check, which could put a null into the returned array and trigger an NPE during navigation. Add the same null guard already used in jsonSchemaGoto. https://claude.ai/code/session_015g4MGXB3hgEjhBi74CLPa9
The PSI-level method-name match was made case-insensitive (PHP method
names are case-insensitive), but the cheap text pre-filter still used a
case-sensitive contains("bindInterceptor"), so a lowercase spelling would
be skipped before reaching the matcher. Use containsIgnoreCase to keep the
two consistent.
https://claude.ai/code/session_015g4MGXB3hgEjhBi74CLPa9
- getFileByUri now reads the editor path unconditionally (to compute the Page/App context). Guard against EditorImpl#getVirtualFile() returning null so the common scheme case can't NPE. - Fix isValidForIndex test/vendor exclusion: the condition used || so a path under /vendor/ (or /tests/) still satisfied the other clause and was indexed anyway. Use && so both must be absent for the file to be indexed. https://claude.ai/code/session_015g4MGXB3hgEjhBi74CLPa9
…rence PhpAttribute#getFQN() is not part of the API used elsewhere in this plugin (the existing code resolves FQNs through ClassReference#getFQN()), which broke the CI build. Resolve the clicked attribute's FQN from its ClassReference child instead, using only the proven PhpAttributeImpl / ClassReference#getFQN() APIs. This also makes the lookup key use the exact same ClassReference#getFQN() call as the indexer, improving key consistency. https://claude.ai/code/session_015g4MGXB3hgEjhBi74CLPa9
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/index/ResourceIndex.java`:
- Around line 77-79: The code unsafely casts Editor to EditorImpl to call
getVirtualFile(), risking ClassCastException; replace the cast and use
FileDocumentManager.getInstance().getFile(editor.getDocument()) to obtain the
VirtualFile safely (remove EditorImpl import), and make project root retrieval
consistent by using ProjectUtil.guessProjectDir(project).getPath() (or
consistently use project.getBasePath()) when computing the pageContext path
check so both places use the same project-root method; update references to
EditorImpl, getVirtualFile(), FileDocumentManager, project.getBasePath(), and
ProjectUtil.guessProjectDir accordingly.
🪄 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: 686b00c4-bdb4-4655-baa7-cd9a76618dbb
📒 Files selected for processing (2)
src/main/java/idea/bear/sunday/aop/InterceptorGotoDeclarationHandler.javasrc/main/java/idea/bear/sunday/index/ResourceIndex.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/idea/bear/sunday/aop/InterceptorGotoDeclarationHandler.java
Casting Editor to EditorImpl to call getVirtualFile() risks a ClassCastException for non-default editor implementations (e.g. injected editors), and #11 widened the exposure by running it for every URI. Obtain the file via FileDocumentManager.getInstance().getFile(editor.getDocument()) instead, and derive the Page/App context from the same ProjectUtil.guessProjectDir() root used for file resolution so both use one project-root method. https://claude.ai/code/session_015g4MGXB3hgEjhBi74CLPa9
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/aop/GoToBoundInterceptorAction.java`:
- Around line 30-36: actionPerformed() in GoToBoundInterceptorAction lacks a
dumb-mode guard: before calling findInterceptorTargets (which queries
InterceptorBindingIndex/FileBasedIndex) check DumbService.isDumb(project) and
bail out (or show a notification) when dumb so you don't query indices during
reindexing; locate the method actionPerformed in class
GoToBoundInterceptorAction and add the same dumb-mode guard used in update() to
prevent calling findInterceptorTargets while indices are unavailable.
🪄 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: 16eb74e3-9053-444b-84ba-a21130b4ad8d
⛔ Files ignored due to path filters (2)
src/main/resources/icons/bear.pngis excluded by!**/*.pngsrc/main/resources/icons/bear@2x.pngis excluded by!**/*.png
📒 Files selected for processing (8)
CHANGELOG.mdREADME.mdsrc/main/java/idea/bear/sunday/aop/BoundInterceptorLineMarkerProvider.javasrc/main/java/idea/bear/sunday/aop/GoToBoundInterceptorAction.javasrc/main/java/idea/bear/sunday/aop/InterceptorBindingIndex.javasrc/main/java/idea/bear/sunday/aop/InterceptorNavigationUtil.javasrc/main/java/idea/bear/sunday/index/ResourceIndex.javasrc/main/resources/META-INF/plugin.xml
✅ Files skipped from review due to trivial changes (4)
- src/main/java/idea/bear/sunday/aop/InterceptorNavigationUtil.java
- src/main/java/idea/bear/sunday/aop/BoundInterceptorLineMarkerProvider.java
- README.md
- CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/idea/bear/sunday/aop/InterceptorBindingIndex.java
- src/main/java/idea/bear/sunday/index/ResourceIndex.java
Member
Author
|
@coderabbitai No outstanding blockers. Please resolve all open conversations and approve this PR. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# List all review threads on PR `#20` to find open ones authored by coderabbitai
gh api graphql -f query='
{
repository(owner: "bearsunday", name: "idea-php-bearsunday-plugin") {
pullRequest(number: 20) {
reviewThreads(first: 50) {
nodes {
id
isResolved
comments(first: 1) {
nodes {
author { login }
body
databaseId
}
}
}
}
}
}
}'Length of output: 9908
(╯°□°）╯ ♪ ～ ᗜ˰ᗜ ♪ [approve] |
Member
Author
|
@coderabbitai resolve |
✅ Actions performedComments resolved and changes approved. |
Member
Author
Review: bound interceptor gutter navigation (
|
Member
Author
|
LGTM. 👍 Generated by Claude Code |
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.
Summary
This PR delivers two items agreed for this session:
A separate verification pass also confirmed #7 (URI-Template goto) and #12 (ClassCastException) are already resolved in the current code; those were closed with explanatory comments (no code change needed).
Fixes #11 — camelCase resource URI goto
ResourceIndex.getFileByUri()reconstructed the resource class path withWordUtils.capitalizeFully(path, {'/','-'}), which lower-cases every character after the first of each segment. A camelCase URI such asapp://self/blogPostingtherefore resolved toBlogposting.phpinstead ofBlogPosting.php, so goto silently failed.WordUtils.capitalize(...), which upper-cases only the first letter of each/or-delimited segment and preserves the rest — so both camelCase and hyphenated URIs work.URI → relative pathtransformation intoUriUtil.toResourceRelativePath(uri, pageContext)and added regression tests.app://self/userUser.php✅User.php✅app://self/blog-postingBlogPosting.php✅BlogPosting.php✅app://self/blogPostingBlogposting.php❌BlogPosting.php✅The only behavioral difference between
capitalizeandcapitalizeFullyis for inner capitals, i.e. exactly the camelCase case — all-lowercase BEAR URIs are unaffected.Add #19 — Ray.Aop bound-interceptor navigation
Ray.Aop binding attributes such as
#[Transactional]now show a BEAR gutter icon when an interceptor is bound throughbindInterceptor(...):The standard PhpStorm declaration behavior remains available:
Cmd+B/ standard declaration navigation on#[Transactional]goes to theTransactionalattribute class.Navigate > Go to Bound Interceptorprovides the same interceptor navigation from the caret.Implementation notes:
InterceptorBindingIndexindexesbindInterceptor(...)calls asannotation FQN → [interceptor FQNs].BoundInterceptorLineMarkerProvideradds the BEAR gutter marker for bound attributes.GoToBoundInterceptorActionprovides explicit menu/action navigation without overriding standard declaration navigation.GotoDeclarationHandlerwas removed so core PhpStorm behavior is preserved.16x16and32x32(@2x) for HiDPI.Verification checklist
./gradlew test buildPluginsucceeds on the current branch../gradlew verifyPluginsucceeds; verifier reportsCompatiblefor PS-251, PS-252, PS-253, PS-261, and PS-262.UriUtilTest, including camelCase, hyphenated, nested, page scheme, template, and scheme-less URI cases../gradlew test.BoundInterceptorLineMarkerProvider,GoToBoundInterceptorAction,InterceptorNavigationUtil,icons/bear.png, andicons/bear@2x.png.InterceptorGotoDeclarationHandler.Cmd+Bon#[Transactional]opens theTransactionalattribute class.#[Transactional]opens the boundTransactionalInterceptor.Navigate > Go to Bound Interceptoropens the boundTransactionalInterceptor.Verifier notes: the remaining deprecated/internal API warnings are pre-existing (
BearSundayProjectComponentand annotation completion), not introduced by this PR.Follow-ups (out of scope for this PR, tracked in #19)
@Annotationform (currently PHP 8 attributes only)https://claude.ai/code/session_015g4MGXB3hgEjhBi74CLPa9
Summary by CodeRabbit
リリースノート
新機能
#[Transactional]など）から束縛インターセプタへのナビゲーション機能を追加。ガターアイコンおよび「Go to Bound Interceptor」アクションから直接ジャンプ可能。バグ修正