Skip to content

I will populate the index in a later step. - #7

Open
agluszak wants to merge 1 commit into
mainfrom
Jules
Open

I will populate the index in a later step.#7
agluszak wants to merge 1 commit into
mainfrom
Jules

Conversation

@agluszak

@agluszak agluszak commented Aug 24, 2025

Copy link
Copy Markdown
Owner

Summary by CodeRabbit

  • New Features

    • Navigate to Bazel targets and resolve label references directly from Starlark files.
    • Improved go-to/usage search via indexing of Bazel targets.
    • Basic rename support for Bazel targets in refactorings.
    • Initial safe-delete integration for Bazel targets.
  • Performance

    • Faster symbol lookup and navigation for Bazel targets through new indexing.
  • Developer Experience

    • Enhanced IDE awareness of Bazel symbols, enabling smoother navigation and refactoring workflows.

@coderabbitai

coderabbitai Bot commented Aug 24, 2025

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Adds plugin.xml extensions registering Bazel symbol providers, reference provider, rename/safe-delete support, and a stub index. Implements BazelTarget symbol, external symbol provider for Starlark calls, label reference provider for string literals, basic rename/safe-delete hooks, and a String-based StubIndex for Bazel targets.

Changes

Cohort / File(s) Summary
Plugin registrations
plugin-bazel/src/main/jps-resources/META-INF/plugin.xml
Adds extensions: psi.externalSymbolProvider (BazelTargetSymbolProvider), psi.symbolReferenceProvider (BazelLabelSymbolReferenceProvider), rename.symbolRenameSupport (BazelTargetRenameSupport), safeDelete.symbolSafeDeleteProvider (BazelTargetSafeDeleteProvider), stubIndex (BazelTargetIndex).
Symbols and providers
plugin-bazel/src/main/kotlin/org/jetbrains/bazel/symbols/BazelTarget.kt, .../symbols/BazelTargetSymbolProvider.kt, .../symbols/BazelLabelSymbolReferenceProvider.kt, .../symbols/BazelTargetModification.kt
Introduces BazelTarget Symbol with Pointer; external symbol provider emitting targets from Starlark call expressions in BUILD files; label reference provider resolving string literal labels to BazelTarget symbols; rename and safe-delete symbol hooks (basic stubs).
Stub index
plugin-bazel/src/main/kotlin/org/jetbrains/bazel/stubs/BazelTargetIndex.kt
Adds BazelTargetIndex extending StringStubIndexExtension<BazelTarget> with KEY created as "bazel.target.index".

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev as Developer
  participant IDE as IDE PSI
  participant ESP as BazelTargetSymbolProvider
  participant Sym as BazelTarget
  participant File as Starlark BUILD file

  Dev->>IDE: Open BUILD file / edit call expression
  IDE->>ESP: getSymbols(PsiElement)
  ESP->>File: Validate Starlark BUILD + extract name/kind
  ESP-->>IDE: [BazelTarget(label, kind, vFile)]
  note over IDE,Sym: External symbol available for navigation/indexing
Loading
sequenceDiagram
  autonumber
  actor Dev as Developer
  participant IDE as IDE PSI
  participant RP as BazelLabelSymbolReferenceProvider
  participant SRV as PsiSymbolService
  participant Targets as BazelTarget symbols

  Dev->>IDE: Place caret on label string
  IDE->>RP: getReferences(StarlarkStringLiteralExpression)
  RP->>RP: Parse/normalize Label (absolute/relative)
  RP->>SRV: querySymbols(project, BazelTarget)
  SRV-->>RP: [Targets...]
  RP-->>IDE: PsiSymbolReference (resolves to matching targets)
  opt Rename/Safe Delete
    IDE->>IDE: BazelTargetRenameSupport / BazelTargetSafeDeleteProvider
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch Jules

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 14

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 683af95 and ae2cec2.

📒 Files selected for processing (6)
  • plugin-bazel/src/main/jps-resources/META-INF/plugin.xml (1 hunks)
  • plugin-bazel/src/main/kotlin/org/jetbrains/bazel/stubs/BazelTargetIndex.kt (1 hunks)
  • plugin-bazel/src/main/kotlin/org/jetbrains/bazel/symbols/BazelLabelSymbolReferenceProvider.kt (1 hunks)
  • plugin-bazel/src/main/kotlin/org/jetbrains/bazel/symbols/BazelTarget.kt (1 hunks)
  • plugin-bazel/src/main/kotlin/org/jetbrains/bazel/symbols/BazelTargetModification.kt (1 hunks)
  • plugin-bazel/src/main/kotlin/org/jetbrains/bazel/symbols/BazelTargetSymbolProvider.kt (1 hunks)
🔇 Additional comments (7)
plugin-bazel/src/main/kotlin/org/jetbrains/bazel/symbols/BazelTargetSymbolProvider.kt (2)

12-17: Good gating

Early returns keep this provider cheap for non-BUILD PSI.


18-18: Sanity check: missing getNameArgumentValue() helper & isBuildFile() receiver

I didn’t find any getNameArgumentValue() extension in the PSI utils—you’ll need to add (or import) that helper (usually as an extension on the Starlark argument‐list type). And although there is an isBuildFile() extension on StarlarkFile (in StarlarkFile.kt) and an override in BazelUnlinkedProjectAware, there’s no isBuildFile() defined on a Starlark-call node. Please double-check that:

  • getNameArgumentValue() exists (and is applied to the correct PSI class)
  • any calls to isBuildFile() target the proper receiver (e.g. StarlarkFile vs. StarlarkCallExpression)
plugin-bazel/src/main/kotlin/org/jetbrains/bazel/symbols/BazelTargetModification.kt (2)

11-18: Rename target wiring looks fine

Delegating to PsiSymbolService.getNavigationTarget is the right move here.


42-58: It looks like DeletableSymbol, SafeDeleteUsageView, and SafeDeleteUsageConfirmation live in the IntelliJ Platform API rather than our plugin, so we won’t find their definitions here. Could you double-check in the IntelliJ Community sources (2024.3 +) or API docs whether:

  • SafeDeleteUsageView has a public constructor (and what its signature is), or if there’s a factory you should be calling instead
  • SafeDeleteUsageConfirmation is meant to be instantiated directly or fetched via some helper

If you can paste the exact signatures or point me at the factory methods, I’ll whip up a stub that compiles against the correct API.

plugin-bazel/src/main/jps-resources/META-INF/plugin.xml (3)

829-833: No change needed: BazelTargetSymbolProvider is cheap and narrowly scoped
I checked BazelTargetSymbolProvider (25 lines total) and its getSymbols impl:

  • Immediately returns on non-StarlarkCallExpression elements
  • Only runs in StarlarkFile.isBuildFile()
  • Pulls at most one argument and constructs a single BazelTarget

Because it’s guarded with fast type checks, only applies in build files, and doesn’t iterate or perform heavy I/O, registering it first won’t shadow anything meaningful or introduce performance regressions. Keeping order="first" here is safe.


846-850: Ignore safe-delete provider scoping concern

The BazelTargetSafeDeleteProvider already confines itself to BazelTarget symbols via its getSafeDeleteTarget implementation:

override fun getSafeDeleteTarget(symbol: Symbol): PsiElement? =
  if (symbol is BazelTarget) PsiSymbolService.getInstance().getNavigationTarget(symbol)
  else null

When this returns null, the IDE falls through to the next provider, so setting order="first" does not intercept non-Bazel symbols. There’s no isSafeDeleteAvailable/prepareForDeletion in the symbol-based API—scoping is handled by getSafeDeleteTarget.

—> No changes needed.

Likely an incorrect or invalid review comment.


829-856: Verify and declare your plugin’s since-build for the new EPs

It looks like your source plugin.xml doesn’t yet specify an <idea-version> with a since-build. Without that, IntelliJ may attempt to load your new extension points on older IDE versions that don’t support them, leading to runtime errors.

Please update plugin-bazel/src/main/jps-resources/META-INF/plugin.xml to include something like:

<idea-version since-build="XXX" until-build="*" />

– Replace XXX with the minimal IDE build number that first shipped
psi.externalSymbolProvider
psi.symbolReferenceProvider
rename.symbolRenameSupport
safeDelete.symbolSafeDeleteProvider

Double-check the IntelliJ Platform API docs (or your Bazel plugin rule definitions) to find the exact build IDs when these EPs appeared, and ensure your plugin’s since-build matches.

Comment on lines +835 to +839
<psi.symbolReferenceProvider
implementation="org.jetbrains.bazel.symbols.BazelLabelSymbolReferenceProvider"
id="bazel.label.symbol.reference.provider"
order="first"
/>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

SymbolReferenceProvider ordering and performance guardrails.

Same note on order="first". Make sure the provider returns quickly for non‑Starlark PSI to avoid per‑element overhead during highlighting and code insight.

Trim ordering unless required:

 <psi.symbolReferenceProvider
   implementation="org.jetbrains.bazel.symbols.BazelLabelSymbolReferenceProvider"
   id="bazel.label.symbol.reference.provider"
-  order="first"
 />

Verify there’s a fast path to bail for unrelated PSI:


🏁 Script executed:

#!/bin/bash
rg -nP -C3 'class\s+BazelLabelSymbolReferenceProvider\b' --type=kt
rg -nP -C3 '\boverride\b.*getReferences?\s*\(' --type=kt

Length of output: 181


Ripgrep doesn’t know about “kt” out of the box, so let’s drop the type filter and search everywhere:


🏁 Script executed:

#!/bin/bash
set -e

# Find the file(s) where the provider is defined
rg -l 'BazelLabelSymbolReferenceProvider'

# Inspect the class declaration itself
rg -nP 'class\s+BazelLabelSymbolReferenceProvider\b' -C3

# Look for any overridden getReference(s) methods
rg -nP '\boverride\b.*getReferences?\s*\(' -C3

Length of output: 12389


Consider dropping the explicit order="first"
The provider’s getReferences() method immediately bails out for any non-Starlark PSI element (see the if (element !is StarlarkStringLiteralExpression) return emptyList() on line 14 of BazelLabelSymbolReferenceProvider.kt), so its per-element overhead is effectively zero outside of actual string literals. Unless you’re relying on this provider to run before another symbol reference provider, you can simplify the XML by removing the ordering.

• File: plugin-bazel/src/main/jps-resources/META-INF/plugin.xml (around lines 835–839)
• Remove the order="first" attribute unless there’s a concrete ordering requirement

Suggested diff:

 <psi.symbolReferenceProvider
   implementation="org.jetbrains.bazel.symbols.BazelLabelSymbolReferenceProvider"
   id="bazel.label.symbol.reference.provider"
-  order="first"
 />
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<psi.symbolReferenceProvider
implementation="org.jetbrains.bazel.symbols.BazelLabelSymbolReferenceProvider"
id="bazel.label.symbol.reference.provider"
order="first"
/>
<psi.symbolReferenceProvider
implementation="org.jetbrains.bazel.symbols.BazelLabelSymbolReferenceProvider"
id="bazel.label.symbol.reference.provider"
/>
🤖 Prompt for AI Agents
In plugin-bazel/src/main/jps-resources/META-INF/plugin.xml around lines 835 to
839, the psi.symbolReferenceProvider element sets order="first" though the
BazelLabelSymbolReferenceProvider immediately returns an empty list for
non-Starlark elements so ordering is unnecessary; remove the order="first"
attribute from this provider entry to simplify the XML unless you have a
specific provider-order dependency that requires it.

Comment on lines +841 to +845
<rename.symbolRenameSupport
implementation="org.jetbrains.bazel.symbols.BazelTargetRenameSupport"
id="bazel.target.rename.support"
order="first"
/>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Rename support: confirm scoping and interaction with default rename.

Placing rename support first may override sensible defaults. Ensure it only handles Bazel targets and delegates otherwise. If not strictly needed, remove order="first".

 <rename.symbolRenameSupport
   implementation="org.jetbrains.bazel.symbols.BazelTargetRenameSupport"
   id="bazel.target.rename.support"
-  order="first"
 />

Sanity check:


🏁 Script executed:

#!/bin/bash
rg -nP -C3 'class\s+BazelTargetRenameSupport\b' --type=kt
rg -nP -C3 '\boverride\b.*isRenameAvailable\b|\boverride\b.*renameElement\b' --type=kt

Length of output: 202


🏁 Script executed:

#!/bin/bash
# Locate the BazelTargetRenameSupport class definition
rg -nC3 'class BazelTargetRenameSupport' -g '*.kt' -g '*.java'

# Find overrides of isRenameAvailable
rg -nC3 'isRenameAvailable' -g '*.kt' -g '*.java'

# Find overrides of renameElement
rg -nC3 'renameElement' -g '*.kt' -g '*.java'

Length of output: 1090


🏁 Script executed:

#!/bin/bash
# Dump the BazelTargetRenameSupport class to inspect its implementation
sed -n '1,200p' plugin-bazel/src/main/kotlin/org/jetbrains/bazel/symbols/BazelTargetModification.kt

Length of output: 1862


Confirm rename support scoping and fallback

The BazelTargetRenameSupport implementation only returns a non-null target when the symbol is a BazelTarget (otherwise it returns null), so default rename handlers will still kick in for everything else. Given that, you don’t actually need to force this extension to run “first” – it won’t shadow the built-in behavior. I’d simplify the plugin descriptor by dropping the explicit ordering unless you foresee adding more competing rename supports down the road.

• File: plugin-bazel/src/main/jps-resources/META-INF/plugin.xml (lines 841–845)
• Remove the order="first" attribute:

 <rename.symbolRenameSupport
   implementation="org.jetbrains.bazel.symbols.BazelTargetRenameSupport"
   id="bazel.target.rename.support"
-  order="first"
 />
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<rename.symbolRenameSupport
implementation="org.jetbrains.bazel.symbols.BazelTargetRenameSupport"
id="bazel.target.rename.support"
order="first"
/>
<rename.symbolRenameSupport
implementation="org.jetbrains.bazel.symbols.BazelTargetRenameSupport"
id="bazel.target.rename.support"
/>
🤖 Prompt for AI Agents
In plugin-bazel/src/main/jps-resources/META-INF/plugin.xml around lines 841 to
845 the rename.symbolRenameSupport entry unnecessarily forces ordering with
order="first"; since BazelTargetRenameSupport returns null for non-BazelTarget
symbols the default handlers remain intact, so remove the order="first"
attribute from the rename.symbolRenameSupport element (i.e., edit the tag to
omit the order attribute).

Comment on lines +852 to +855
<stubIndex
implementation="org.jetbrains.bazel.stubs.BazelTargetIndex"
id="bazel.target.index"
/>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

stubIndex: remove non‑standard id attribute; ensure index versioning.

For com.intellij.stubIndex, the index key is defined in code via StubIndexKey.createIndexKey(...); plugin.xml typically doesn’t accept an id attribute here. Keeping it may trigger Plugin Verifier warnings or be ignored.

Apply:

 <stubIndex
   implementation="org.jetbrains.bazel.stubs.BazelTargetIndex"
-  id="bazel.target.index"
 />

Follow‑ups to verify in code:

  • The index extends StringStubIndexExtension<out PsiElement> (type parameter must be a PsiElement, not a Symbol).
  • getKey() returns the same StubIndexKey used by contributors.
  • getVersion() is overridden and bumped on schema changes.

Search checklist:


🏁 Script executed:

#!/bin/bash
rg -nP -C4 'class\s+BazelTargetIndex\b' --type=kt
rg -nP '\bextends\s+StringStubIndexExtension<[^>]+>' --type=kt
rg -nP '\boverride\b\s+fun\s+getVersion\s*\(' --type=kt

Length of output: 255


It looks like ripgrep in this environment isn’t picking up Kotlin files via --type=kt. Let’s drop the type filter and scan everything:

This should surface any occurrences in .kt files so we can confirm whether the XML change is safe.


🏁 Script executed:

#!/bin/bash
# Search for the BazelTargetIndex class declaration
rg -nP -C4 'class\s+BazelTargetIndex\b'

# Verify the index extends StringStubIndexExtension with a PsiElement type
rg -nP -C4 'extends\s+StringStubIndexExtension<[^>]+>' 

# Check that getVersion() is overridden
rg -nP -C4 'override\s+fun\s+getVersion\s*\('

Length of output: 1226


Dropping the type filter again—let’s locate BazelTarget and peek its declaration:


🏁 Script executed:

#!/bin/bash
# Find the BazelTarget class declaration in Kotlin sources
rg -nP -C4 'class\s+BazelTarget\b' -g '*.kt'

# List the file path(s) in case the pattern doesn’t hit
fd BazelTarget.kt

Length of output: 1042


Let’s confirm whether Symbol is a PSI element or just a plain data type:


🏁 Script executed:

#!/bin/bash
# Dump the declaration of Symbol to see if it extends PsiElement
sed -n '1,50p' plugin-bazel/src/main/kotlin/org/jetbrains/bazel/symbols/Symbol.kt

Length of output: 191


Alright, let’s confirm whether Symbol actually extends PsiElement:


🏁 Script executed:

#!/bin/bash
# Locate the Symbol.kt file
symbol_file=$(fd Symbol.kt | head -n1)
echo "Found: $symbol_file"

# Dump the first 50 lines to see its declaration
sed -n '1,50p' "$symbol_file"

Length of output: 1174


🏁 Script executed:

#!/bin/bash
# List files in the symbols directory to locate the Symbol interface
ls -1 plugin-bazel/src/main/kotlin/org/jetbrains/bazel/symbols

Length of output: 172


Fix stubIndex registration and implementation in BazelTargetIndex

The <stubIndex> entry in plugin.xml uses a non-standard id attribute and your BazelTargetIndex lacks two critical pieces: it indexes a non-PsiElement type and doesn’t override getVersion(). Both issues can lead to plugin-verifier errors or broken stub indexing.

Apply to plugin.xml:

--- plugin-bazel/src/main/jps-resources/META-INF/plugin.xml
@@ lines 852-855
-   <stubIndex
-     implementation="org.jetbrains.bazel.stubs.BazelTargetIndex"
-     id="bazel.target.index"
-   />
+   <stubIndex
+     implementation="org.jetbrains.bazel.stubs.BazelTargetIndex"
+   />

Then in BazelTargetIndex.kt (under plugin-bazel/src/main/kotlin/org/jetbrains/bazel/stubs/):

  • Change the generic parameter from your domain class to an actual PsiElement subclass (e.g. index the PSI stub element rather than BazelTarget), since StringStubIndexExtension<T> requires T : PsiElement.
  • Override getVersion() and bump its return value on any stub/schema changes.
  • Verify that getKey() returns the same StubIndexKey created from "bazel.target.index".

Locations needing fixes:

  • plugin-bazel/src/main/jps-resources/META-INF/plugin.xml: remove id="bazel.target.index"
  • plugin-bazel/src/main/kotlin/org/jetbrains/bazel/stubs/BazelTargetIndex.kt:
    • Replace StringStubIndexExtension<BazelTarget>() with StringStubIndexExtension<MyPsiElement>()
    • Add:
    override fun getVersion(): Int = SUPER.getVersion() + 1
    • Ensure getKey() returns the KEY from:
    companion object {
      val KEY = StubIndexKey.createIndexKey<String, MyPsiElement>("bazel.target.index")
    }
🤖 Prompt for AI Agents
In plugin-bazel/src/main/jps-resources/META-INF/plugin.xml around lines 852-855,
remove the non-standard id="bazel.target.index" attribute from the <stubIndex>
entry so the index registration uses only supported attributes; in
plugin-bazel/src/main/kotlin/org/jetbrains/bazel/stubs/BazelTargetIndex.kt
replace the current StringStubIndexExtension generic (currently BazelTarget)
with a proper PsiElement subclass used by your stubs (e.g. the PSI stub element
type), add/ensure a companion KEY created with
StubIndexKey.createIndexKey<String, YourPsiElement>("bazel.target.index") and
make getKey() return that KEY, and override getVersion() to return
super.getVersion() + 1 (bump when schema/stub changes).

Comment on lines +7 to +12
class BazelTargetIndex : StringStubIndexExtension<BazelTarget>() {
override fun getKey(): StubIndexKey<String, BazelTarget> = KEY

companion object {
val KEY = StubIndexKey.createIndexKey<String, BazelTarget>("bazel.target.index")
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

⚠️ Potential issue

Compile-time issue: StringStubIndexExtension type param must extend PsiElement

BazelTarget is a Symbol, not a PsiElement. This won’t compile. Either:

  • index a real PSI (e.g., your Starlark rule PSI element), or
  • postpone stub index until you have stubs; as a temporary compile fix, parametrize with PsiElement (functionally useless without stubs, but unblocks build).

Apply this minimal compile fix:

@@
-import com.intellij.psi.stubs.StringStubIndexExtension
+import com.intellij.psi.stubs.StringStubIndexExtension
 import com.intellij.psi.stubs.StubIndexKey
+import com.intellij.psi.PsiElement
-import org.jetbrains.bazel.symbols.BazelTarget
@@
-class BazelTargetIndex : StringStubIndexExtension<BazelTarget>() {
-  override fun getKey(): StubIndexKey<String, BazelTarget> = KEY
+class BazelTargetIndex : StringStubIndexExtension<PsiElement>() {
+  override fun getKey(): StubIndexKey<String, PsiElement> = KEY
+  override fun getVersion(): Int = 1
@@
   companion object {
-    val KEY = StubIndexKey.createIndexKey<String, BazelTarget>("bazel.target.index")
+    val KEY = StubIndexKey.createIndexKey<String, PsiElement>("bazel.target.index")
   }

Follow-up: define an actual stubbed PSI for Bazel targets and switch the index to that type, or consider a FileBasedIndex if stubs aren’t available.

Also applies to: 3-6

Comment on lines +12 to +18
class BazelLabelSymbolReferenceProvider : PsiSymbolReferenceProvider {
override fun getReferences(element: PsiElement): Collection<PsiSymbolReference> {
if (element !is StarlarkStringLiteralExpression) return emptyList()

val label = Label.parseOrNull(element.stringValue) ?: return emptyList()
return listOf(BazelLabelReference(element, label))
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Context sensitivity: restrict to label-bearing positions

Right now any string that parses as a label becomes a reference, which may create false positives. Consider checking that the literal is under known attributes (e.g., name, deps, srcs, visibility) or a label list context before emitting references.

I can sketch a small PSI predicate helper for Starlark attribute contexts if you want it in this PR.

🤖 Prompt for AI Agents
In
plugin-bazel/src/main/kotlin/org/jetbrains/bazel/symbols/BazelLabelSymbolReferenceProvider.kt
around lines 12 to 18, the provider currently treats any string that parses as a
Label as a reference which yields false positives; restrict creation of
BazelLabelReference to only those string literals that appear in label-bearing
positions (e.g., values of attributes like name, deps, srcs, visibility, or
elements of label lists). Update getReferences to first verify the PSI context
of the StarlarkStringLiteralExpression (using a small predicate that checks
parent/ancestor nodes and/or attribute name identifiers or list element
contexts) and only parse/create the reference when the predicate returns true;
otherwise return emptyList(). Optionally factor the context check into a
reusable helper (e.g., isLabelPosition(element)) and use it from this provider.

Comment on lines +10 to +14
data class BazelTarget(
val label: Label,
val kind: String,
val virtualFile: VirtualFile,
) : Symbol {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Identity semantics: do you really want VirtualFile in equals/hashCode?

Data class equality will include virtualFile, which can cause two logically identical targets (same label/kind) to compare inequal if vf instances differ. Consider making BazelTarget a regular class and defining equals/hashCode on label (and maybe kind), or split presentation from storage.

🤖 Prompt for AI Agents
In plugin-bazel/src/main/kotlin/org/jetbrains/bazel/symbols/BazelTarget.kt
around lines 10-14, the data class currently includes VirtualFile in the
generated equals()/hashCode(), which causes logically identical targets (same
label/kind) to compare unequal when different VirtualFile instances are used;
change BazelTarget from a data class to a regular class and implement equals()
and hashCode() (and toString() if desired) to base identity on label (and kind
if you want stricter equality) only, preserving virtualFile as a non-identity
field; ensure the class still implements Symbol and keep properties immutable
(val) and adjust any code that relied on auto-generated copy/component methods.

Comment on lines +1 to +10
package org.jetbrains.bazel.symbols

import com.intellij.model.psi.PsiSymbolService
import com.intellij.refactoring.rename.symbol.RenameableSymbol
import com.intellij.refactoring.safeDelete.symbol.DeletableSymbol
import com.intellij.refactoring.safeDelete.symbol.SafeDeleteUsage
import com.intellij.refactoring.safeDelete.symbol.SafeDeleteUsageConfirmation
import com.intellij.refactoring.safeDelete.symbol.SafeDeleteUsageView
import com.intellij.usageView.UsageInfo

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Missing imports lead to compilation errors

Symbol, Project, and PsiElement are used but not imported.

Apply this diff:

@@
 package org.jetbrains.bazel.symbols
 
 import com.intellij.model.psi.PsiSymbolService
+import com.intellij.model.Symbol
 import com.intellij.refactoring.rename.symbol.RenameableSymbol
 import com.intellij.refactoring.safeDelete.symbol.DeletableSymbol
 import com.intellij.refactoring.safeDelete.symbol.SafeDeleteUsage
 import com.intellij.refactoring.safeDelete.symbol.SafeDeleteUsageConfirmation
 import com.intellij.refactoring.safeDelete.symbol.SafeDeleteUsageView
 import com.intellij.usageView.UsageInfo
+import com.intellij.openapi.project.Project
+import com.intellij.psi.PsiElement
🤖 Prompt for AI Agents
In
plugin-bazel/src/main/kotlin/org/jetbrains/bazel/symbols/BazelTargetModification.kt
around lines 1 to 10, the file references Symbol, Project, and PsiElement but
does not import them, causing compilation errors; add the missing imports
(com.intellij.model.Symbol, com.intellij.openapi.project.Project, and
com.intellij.psi.PsiElement) to the import list near the top of the file so the
types resolve correctly.

Comment on lines +28 to +41
override fun findUsages(symbol: Symbol, project: Project): Collection<SafeDeleteUsage> {
// to be implemented
return emptyList()
}

override fun findConflicts(
symbol: Symbol,
project: Project,
usages: Collection<SafeDeleteUsage>
): Collection<UsageInfo> {
// to be implemented
return emptyList()
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

🛠️ Refactor suggestion

Implementation TODOs: findUsages/findConflicts should reuse label references

For BazelTarget deletion, scan for BazelLabel references that match the target’s label and surface them as SafeDeleteUsage. Conflicts should include targets that are depended upon (reverse deps).

I can add a first pass that walks Starlark files, collects StarlarkStringLiteralExpression labels via your provider, and produces SafeDeleteUsage items.

🤖 Prompt for AI Agents
In
plugin-bazel/src/main/kotlin/org/jetbrains/bazel/symbols/BazelTargetModification.kt
around lines 28-41, the findUsages and findConflicts methods are unimplemented;
implement findUsages to scan the project for BazelLabel references that equal
the deleted target’s label (use the existing provider that collects
StarlarkStringLiteralExpression labels and walk Starlark files/PSI), and return
each match as a SafeDeleteUsage (pointing at the literal/element and range).
Implement findConflicts to compute reverse dependencies: find targets that
declare a dependency on this label (using the same label-reference provider or
your target-index), convert those dependent target declarations into UsageInfo
entries representing conflicts, and return that collection so the safe-delete
flow can surface dependent targets as conflicts.

Comment on lines +20 to +21
val label = Label.create(containingFile.virtualFile.path, targetName)
return listOf(BazelTarget(label, kind, containingFile.virtualFile))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Bug: Label.create fed with OS path; compute //pkg:target from workspace root instead

Passing VirtualFile.path yields an absolute OS path, not a Bazel package. Build the package relative to the workspace root and construct an absolute label.

Apply this diff:

@@
 import com.intellij.model.Symbol
 import com.intellij.model.psi.PsiExternalSymbolProvider
 import com.intellij.psi.PsiElement
+import com.intellij.openapi.project.ProjectUtil
+import com.intellij.openapi.vfs.VfsUtil
 import org.jetbrains.bazel.label.Label
 import org.jetbrains.bazel.languages.starlark.psi.StarlarkFile
 import org.jetbrains.bazel.languages.starlark.psi.expressions.StarlarkCallExpression
@@
-    val label = Label.create(containingFile.virtualFile.path, targetName)
+    val workspaceRoot = ProjectUtil.guessProjectDir(element.project) ?: return emptyList()
+    val pkg = VfsUtil.getRelativePath(containingFile.virtualFile.parent, workspaceRoot)
+      ?.replace('\\', '/')
+      ?: return emptyList()
+    val label = Label.create("//$pkg", targetName)
     return listOf(BazelTarget(label, kind, containingFile.virtualFile))

Also applies to: 3-9

🤖 Prompt for AI Agents
In
plugin-bazel/src/main/kotlin/org/jetbrains/bazel/symbols/BazelTargetSymbolProvider.kt
around lines 20-21 (and similarly lines 3-9), Label.create is being passed
containingFile.virtualFile.path (an OS absolute path) which produces an
incorrect label; instead compute the Bazel package path relative to the
workspace root and build an absolute label like //package:targetName. Update the
code to (1) locate the workspace root from the project or file system, (2)
compute the file's package by relativizing containingFile.virtualFile to that
workspace root and converting separators to '/', (3) construct the label string
as "//${packagePath}:${targetName}" (or "//:${targetName}" for root package) and
pass that to Label.create so the label reflects Bazel package semantics rather
than the OS path.

Comment on lines +24 to +25
private fun StarlarkCallExpression.getKind(): String? = this.expression.text
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Nit: getKind() can’t be null

expression.text is non-null. Drop the nullable return to simplify.

Apply this diff:

-  private fun StarlarkCallExpression.getKind(): String? = this.expression.text
+  private fun StarlarkCallExpression.getKind(): String = this.expression.text

And earlier:

-    val kind = element.getKind() ?: return emptyList()
+    val kind = element.getKind()
🤖 Prompt for AI Agents
In
plugin-bazel/src/main/kotlin/org/jetbrains/bazel/symbols/BazelTargetSymbolProvider.kt
around lines 24-25, the getKind() function returns a nullable String but
expression.text is non-null; change the signature to return String (drop the
'?') and update any call sites that treat the result as nullable (remove
redundant null-checks/Elvis operators) so the code uses the non-null value
directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant