-
Notifications
You must be signed in to change notification settings - Fork 39
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
Introduce MongoDB Refaster rules #1214
base: master
Are you sure you want to change the base?
Conversation
Looks good. No mutations were possible for these changes. |
@@ -177,7 +177,7 @@ | |||
<dependency> | |||
<groupId>org.mongodb</groupId> | |||
<artifactId>mongodb-driver-core</artifactId> | |||
<scope>test</scope> | |||
<scope>provided</scope> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Unrelated to this line)
Do we want MongoDB-related Refaster rules in the first place?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, sounds like a nice idea to me. We also have a BugPattern that's MongoDB related: https://github.com/PicnicSupermarket/error-prone-support/blob/master/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MongoDBTextFilterUsage.java.
static final class Eq { | ||
@BeforeTemplate | ||
Bson before(String field, Enum<?> value) { | ||
return eq(field, value.toString()); | ||
} | ||
|
||
@AfterTemplate | ||
@UseImportPolicy(STATIC_IMPORT_ALWAYS) | ||
Bson after(String field, Enum<?> value) { | ||
return eq(field, value); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This rule works if Enum#toString
and the encoded value are equivalent.
This may not always be the case. Is there a way in Error Prone to identify e.g. whether Enum#toString
is overwritten?
If we cannot guarantee it, is a BugChecker
the better choice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might be able to use a Matcher, we have a few examples here: https://github.com/PicnicSupermarket/error-prone-support/tree/master/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers.
I think we can infer something like you are describing in a Matcher 💪🏻 !
Looks good. No mutations were possible for these changes. |
Quality Gate passedIssues Measures |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bit slow, but here is a response :).
@@ -177,7 +177,7 @@ | |||
<dependency> | |||
<groupId>org.mongodb</groupId> | |||
<artifactId>mongodb-driver-core</artifactId> | |||
<scope>test</scope> | |||
<scope>provided</scope> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, sounds like a nice idea to me. We also have a BugPattern that's MongoDB related: https://github.com/PicnicSupermarket/error-prone-support/blob/master/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MongoDBTextFilterUsage.java.
static final class Eq { | ||
@BeforeTemplate | ||
Bson before(String field, Enum<?> value) { | ||
return eq(field, value.toString()); | ||
} | ||
|
||
@AfterTemplate | ||
@UseImportPolicy(STATIC_IMPORT_ALWAYS) | ||
Bson after(String field, Enum<?> value) { | ||
return eq(field, value); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might be able to use a Matcher, we have a few examples here: https://github.com/PicnicSupermarket/error-prone-support/tree/master/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers.
I think we can infer something like you are describing in a Matcher 💪🏻 !
Opening this PR as draft to gather feedback on: