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

Introduce MongoDB Refaster rules #1214

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Introduce MongoDB Refaster rules #1214

wants to merge 2 commits into from

Conversation

werli
Copy link
Member

@werli werli commented Jun 12, 2024

Opening this PR as draft to gather feedback on:

  1. Do we want MongoDB-related Refaster rules in the first place?
  2. This specific rule - I'll open a thread in the code for both.

@werli werli added the question Further information is requested label Jun 12, 2024
Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

@@ -177,7 +177,7 @@
<dependency>
<groupId>org.mongodb</groupId>
<artifactId>mongodb-driver-core</artifactId>
<scope>test</scope>
<scope>provided</scope>
Copy link
Member Author

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?

Copy link
Member

Choose a reason for hiding this comment

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

Comment on lines +19 to +30
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);
}
}
Copy link
Member Author

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?

Copy link
Member

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 💪🏻 !

Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link
Member

@rickie rickie left a 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>
Copy link
Member

Choose a reason for hiding this comment

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

Comment on lines +19 to +30
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);
}
}
Copy link
Member

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 💪🏻 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Development

Successfully merging this pull request may close these issues.

2 participants