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

fix: fixes autocomplete not working in valid positions because of a Java iterable INTELLIJ-158 #113

Merged
merged 7 commits into from
Jan 8, 2025

Conversation

himanshusinghs
Copy link
Contributor

Description

AC was not working in cases where a valid MethodCallExpression was having a Java Iterable as an argument. This PR fixes that by handling the case of Java Iterable and traversing upwards to determine whether a caret position qualifies for AC or not.

Checklist

Open Questions

@himanshusinghs himanshusinghs added the no release notes It's a chore and doesn't require release notes. label Jan 8, 2025
@github-actions github-actions bot added fix Fixes a bug. and removed no release notes It's a chore and doesn't require release notes. fix Fixes a bug. labels Jan 8, 2025
@himanshusinghs himanshusinghs added the no release notes It's a chore and doesn't require release notes. label Jan 8, 2025
@github-actions github-actions bot added fix Fixes a bug. and removed fix Fixes a bug. no release notes It's a chore and doesn't require release notes. labels Jan 8, 2025
method.containingClass?.qualifiedName == JAVA_LIST_FQN

val isArrayAsListCall = method?.name == "asList" &&
method.containingClass?.qualifiedName == JAVA_ARRAYS_FQN
Copy link
Contributor

Choose a reason for hiding this comment

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

There are other ways to define iterables in Java, maybe instead of checking the class of the method, we can check the return type is a Collection or an array? Collections.singletonList is also another way.

I don't think we need to support all possible factory methods right now, but if the cost of supporting them is low I would try it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Discussed this over Slack - Checking the return type of the expression is the sure shot way for inferring wether the expression is a Java Iterable or not but then it would also "accidentally" cover cases for iterables that could potentially be modified such as iterables created with new ArrayList interface. For now we decided to rename the interface to isParseableJavaIterable to abstract away how we identify the iterable. It also now support Collections.singletonList (71a30ef)

@github-actions github-actions bot added fix Fixes a bug. and removed fix Fixes a bug. labels Jan 8, 2025
@github-actions github-actions bot added fix Fixes a bug. and removed fix Fixes a bug. labels Jan 8, 2025
Base automatically changed from fix/INTELLIJ-157 to main January 8, 2025 15:47
@github-actions github-actions bot added fix Fixes a bug. and removed fix Fixes a bug. labels Jan 8, 2025
Copy link

github-actions bot commented Jan 8, 2025

Coverage Report

Overall Project 78.34% -0.09%
Files changed 78.99%

File Coverage
JavaDriverDialectParser.kt 88.33% -0.76%

Copy link

github-actions bot commented Jan 8, 2025

🤖 Benchmark Comparison for Merge branch 'main' into fix/INTELLIJ-158

Benchmark Previous Current Change
com.mongodb.jbplugin.jmh.SampleBenchmark.init 2,664,355,353.95 ops/s 2,666,277,643.56 ops/s 0.07%

@himanshusinghs himanshusinghs merged commit 517c061 into main Jan 8, 2025
11 checks passed
@himanshusinghs himanshusinghs deleted the fix/INTELLIJ-158 branch January 8, 2025 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Fixes a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants