Skip to content

fix: skip insights for queries with unsupported commands / operations INTELLIJ-316 #216

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

himanshusinghs
Copy link
Contributor

@himanshusinghs himanshusinghs commented May 22, 2025

Description

Primarily this PR targets skipping the insights generation for queries that contains commands or operations that are not yet supported by our parsers. Earlier we were ignoring such commands / operations but now we are marking them as UNKNOWN and later using that information to decide whether to skip insight generation / inlay decoration for such queries or not.

The logic to detect whether the query has UNSUPPORTED operations or not, has also been modified. Earlier it was only looking in Filters component and instead it should have been looking at any component that has children for the Named operations, now it does as stated.

Additionally there are two other but important changes:

  1. Reverting the change we did in fix(java): ability to parse .iterator queries written in the MongoDB java driver INTELLIJ-235 #163 for INTELLIJ-235. The reason being, we had always supported queries chained with .iterator calls or any other mongo iterable calls. Only the test case had to target the right query which we are doing now in this commit.
  2. We should not be running inspections when the DataSource is not connected. That is what the second commit fixes.

Checklist

Open Questions

@himanshusinghs himanshusinghs marked this pull request as draft May 22, 2025 15:05
Copy link

github-actions bot commented May 22, 2025

Coverage Report

Overall Project 77.88% -0.12%
Files changed 88.92%

File Coverage
Named.kt 97.64% -0.59%
ProjectStageParser.kt 93.6%
AggregationStagesParser.kt 93.02% -1.16%
SpringCriteriaDialectParser.kt 91.64%
GroupStageParser.kt 91.41% -0.32%
SortStageParser.kt 91.1% -3.82% 🚫
AddFieldsStageParser.kt 91.01% -0.29%
JavaDriverDialectParser.kt 88.6% -0.49%
Named.kt 88.26% -4.69%
AbstractMongoDbInspectionBridge.kt 75.6% -0.96%

Earlier we were incorrectly identifying .iterator call always as a
FIND_MANY command. In reality .iterator could be chained on any
expression returning a MongoIterable.

This change, removes explicit identification of .iterator as FIND_MANY
command. IntelliJ eventually asks for a command for the right expression
which is generally the bottom most expression in the call chain.

The tests has been modified to replicate what IntelliJ does.
@himanshusinghs himanshusinghs marked this pull request as ready for review May 23, 2025 14:47
@himanshusinghs himanshusinghs changed the title fix: identify command behind .iterator correctly INTELLIJ-316 fix: skip insights for queries with unsupported commands / operations INTELLIJ-316 May 26, 2025
A command is an identified mongodb command such as $match. There are a
few aggregate commands that our parsers do not support, in our case
anything other than match, group, unwind, sort, project, addFields, at
the time of this commit.

Earlier the parser was skipping the command that it does not support
but now we identify them and mark them as UNKNOWN so that later when we
are making decision wether to run inspection on the query or not, we can
decide that on the basis of whether the query has UNKNOWN command or
not.

The unknown operation detection logic has also been changed in this
commit. Earlier it was only looking in Filters component and instead it
should be looking at any component that has children for the Named
operations.
@himanshusinghs himanshusinghs added the skip-spec-check Discard any check on the MQL specification. label May 26, 2025
@himanshusinghs himanshusinghs requested a review from Copilot May 27, 2025 09:10
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves the parser’s handling of unsupported commands and operations by consistently marking them as UNKNOWN, which ensures that insights are skipped for such unsupported queries. It also reverts a previous change regarding chaining queries and adds a data source connectivity check to avoid running inspections when disconnected.

  • Skips insight generation for unsupported query commands/operations.
  • Returns UNKNOWN nodes instead of null for unrecognized methods in aggregation stage parsing.
  • Adds a connectivity check in inspections to prevent running when the data source is not connected.

Reviewed Changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.

File Description
packages/mongodb-dialects/spring-criteria/* Updated test and parser logic for aggregation stages returning UNKNOWN for unsupported operations.
packages/mongodb-dialects/java-driver/* Modified JavaDriverDialectParser to consistently return UNKNOWN nodes and updated tests for unsupported operations.
packages/jetbrains-plugin/* Added a connectivity check in inspections and updated related tests and integrations.
packages/jetbrains-plugin/src/test/kotlin/com/mongodb/jbplugin/fixtures/* Minor adjustments to account for new import patterns in integration tests.
Comments suppressed due to low confidence (1)

packages/mongodb-dialects/java-driver/src/main/kotlin/com/mongodb/jbplugin/dialects/javadriver/glossary/JavaDriverDialectParser.kt:933

  • Ensure that the allCallExpressions list contains at least two elements before accessing lastIndex - 1 to avoid potential IndexOutOfBounds errors in chained method calls.
val lastCallExpression = allCallExpressions.getOrNull(allCallExpressions.lastIndex - 1)

We regressed in one of the earlier commits about how we group our
inspections in Settings -> Inspections panel in IntelliJ. This commits
fixes that.

The ideal grouping for our inspection is to be under one path which,
earlier, we were calling "MongoDB". That is one thing we changed in this
commit, populated groupPath metadata in plugin.xml for top level
grouping. Then we have subgrouped inspections in their categories using
groupKey metadata in plugin.xml.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-spec-check Discard any check on the MQL specification.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants