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: tests and minor suggestions for INTELLIJ-128 #101

Merged
merged 2 commits into from
Nov 25, 2024

Conversation

himanshusinghs
Copy link
Contributor

Description

Checklist

Open Questions

val (constant, value) = element.tryToResolveAsConstant()
return HasValueReference(
when {
constant && value is String -> HasValueReference.Computed(
element,
type = ComputedBsonType(
BsonAny,
BsonString,
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 is the same suggestion as in the original PR

Copy link
Contributor

@kmruiz kmruiz Nov 25, 2024

Choose a reason for hiding this comment

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

Ah yes, I remember answering there. The thing is that while how we define the expression is a string, the type of the value is not a string right? For example "$year" is not a string, is an int32, so using a string here is not accurate. I feel that until we can infer the type of the expression, we should just assume any. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

keeping BsonAny here because the actual type is not relevant for now, will revert this particular change

@himanshusinghs himanshusinghs merged commit d8a9dc0 into feat/INTELLIJ-128 Nov 25, 2024
1 check passed
@himanshusinghs himanshusinghs deleted the feat/INTELLIJ-128-tests-followup branch November 25, 2024 16:05
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.

2 participants