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

chore: basic parser for queries with the java driver INTELLIJ-19 #15

Merged
merged 10 commits into from
Jul 15, 2024

Conversation

kmruiz
Copy link
Contributor

@kmruiz kmruiz commented Jul 11, 2024

Description

Checklist

Open Questions

@kmruiz kmruiz self-assigned this Jul 11, 2024
@github-actions github-actions bot added the no release notes It's a chore and doesn't require release notes. label Jul 11, 2024
@kmruiz kmruiz marked this pull request as ready for review July 11, 2024 17:03
Copy link

github-actions bot commented Jul 11, 2024

Coverage Report

Overall Project 87.86% -4.97%
Files changed 86.87%

File Coverage
JavaDriverDialectParser.kt 89.72% -10.28%
NamespaceExtractor.kt 87.55%
PsiMdbTreeUtil.kt 87.03% -8.78%

@kmruiz kmruiz changed the title chore: basic parser for queries with the java driver chore: basic parser for queries with the java driver INTELLIJ-19 Jul 12, 2024
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.

Looking good! I understood the flow, syntax is still a little alien but I will get used to it :D

return false
}

override fun attachment(source: PsiElement): PsiElement = source.findMongoDbCollectionReference()!!
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it common to do !! towards the end of a method call. I am assuming this is a non-null assertion. If yes, does the TS recommendation hold true here as well - to make it verbose, add some conditional check instead of doing this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Usually you don't need to do it, but in this case the check that the node exists is happening in another method (the isCandidate) so it's fine and if someone is not using the API correctly they will see a NPE really fast.

val namespace = NamespaceExtractor.extractNamespace(source)
val collectionReference = namespaceComponent(namespace)

val currentCall = source as PsiMethodCallExpression? ?: return Node(source, listOf(collectionReference))
Copy link
Contributor

Choose a reason for hiding this comment

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

Another question similar to above - how safe is it to do this - source as PsiMethodCallExpression. I am assuming that this is type casting. Would it be better if we do some instanceOf check or there's no such thing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This specific cast is like an instance of and cast at the same time. Being a nullable type, if the casting is not successful it will return null, going through the return expression on the right side.

Copy link
Contributor

Choose a reason for hiding this comment

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

ohh that's interesting - thanks for the context

Comment on lines +181 to +183
public FindIterable<Document> findBookById(ObjectId id) {
return this.collection.find(Filters.eq("_id", id));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh now I understand! I was wondering when would filter be a callable expression with key and value pairs. Totally forgot that this is quite common when writing Java.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah all key/value operators have the same syntax in the driver, that is really useful to just implement the pattern once without needing to check the specific operator.

}
""",
)
fun `supports vararg operators`(psiFile: PsiFile) {
Copy link
Contributor

Choose a reason for hiding this comment

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

TIL: vararg is actually a term in Kotlin

@@ -109,10 +114,11 @@ fun Class<*>.toBsonType(): BsonType {
Int::class.javaObjectType -> BsonAnyOf(BsonNull, BsonInt32)
CharSequence::class.java, String::class.java -> BsonAnyOf(BsonNull, BsonString)
Date::class.java, Instant::class.java, LocalDate::class.java, LocalDateTime::class.java ->
BsonAnyOf(BsonNull, BsonDate)
BsonAnyOf(BsonNull, BsonDate)
Copy link
Contributor

Choose a reason for hiding this comment

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

qq: is there something like prettier for kotlin projects. That probably should help with the formatting changes here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah we have spotless in the Gradle project that does the formatting for us, but for some reason I need to check, it sometimes does weird things on the precommit hook.

@kmruiz kmruiz merged commit 6ba684e into main Jul 15, 2024
11 checks passed
@kmruiz kmruiz deleted the chore/intellij-19 branch July 15, 2024 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no release notes It's a chore and doesn't require release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants