Skip to content
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

feat(spring): add support for the unwind aggregation stage INTELLIJ-176 #120

Merged
merged 2 commits into from
Jan 17, 2025

Conversation

kmruiz
Copy link
Contributor

@kmruiz kmruiz commented Jan 17, 2025

Description

Checklist

Open Questions

@github-actions github-actions bot added the feat label Jan 17, 2025
Copy link

github-actions bot commented Jan 17, 2025

Coverage Report

Overall Project 78.87% -0.05%
Files changed 84.26%

File Coverage
SpringCriteriaDialectParser.kt 88.57%
UnwindStageParser.kt 84.82% -15.18%

Copy link

🤖 Benchmark Comparison for chore: add support for unwind

Benchmark Previous Current Change
com.mongodb.jbplugin.jmh.SampleBenchmark.init 2,600,565,477.62 ops/s 2,588,223,764.27 ops/s -0.47%

@kmruiz kmruiz marked this pull request as ready for review January 17, 2025 16:08
@kmruiz kmruiz requested a review from himanshusinghs January 17, 2025 16:08
@github-actions github-actions bot added feat and removed feat labels Jan 17, 2025
Copy link
Contributor

@himanshusinghs himanshusinghs left a comment

Choose a reason for hiding this comment

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

In addition to the parser test, maybe we should extend our current inspection and autocomplete test to also check for cases in unwind.

override fun isSuitableForFieldAutoComplete(
methodCall: PsiMethodCallExpression,
method: PsiMethod
) = true
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the method name is a little confusing but the idea of this method isSuitableForFieldAutoComplete is for parsers to check and confirm, based on their understanding, if the provided method call is a good target for autocompleter to add field autocomplete.

So it will most likely have the same logic as canParse but might also differ (for example it does differ in project parser). In this case, I think we can safely defer to canParse.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah cool, I'll fix it then! I think eventually we will need to add more information to this method then, because not all arguments in a method call expression are suitable for autocomplete. But this is something we can do later.

@github-actions github-actions bot added feat and removed feat labels Jan 17, 2025
@kmruiz kmruiz merged commit a3328ea into main Jan 17, 2025
9 of 10 checks passed
@kmruiz kmruiz deleted the feat/INTELLIJ-176 branch January 17, 2025 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants