-
Notifications
You must be signed in to change notification settings - Fork 38
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 TypeMemberOrder
bug checker
#636
base: master
Are you sure you want to change the base?
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
00eebf3
to
ebb9041
Compare
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
Verifying that the whitespace is untouched maybe a bit harder than I thought because I looked into Indentation of replaced code might cause some challenge. |
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
ca7ada8
to
11816d9
Compare
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
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.
There were some imports from ASTHelpers
that we usually do not statically import :).
Added a commit! The setup looks nice, didn't get till the end, but already flushing my comments 😄.
error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MemberOrdering.java
Outdated
Show resolved
Hide resolved
error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MemberOrdering.java
Outdated
Show resolved
Hide resolved
error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MemberOrdering.java
Outdated
Show resolved
Hide resolved
error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MemberOrdering.java
Outdated
Show resolved
Hide resolved
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MemberOrdering.java
Outdated
Show resolved
Hide resolved
(Okay apparently I had to rebuild, the test locally passed 😄, will take a look).
@benhalasi I would add a testcase with an annotation in that case 😉 .
When using the suppression for |
Kudos, SonarCloud Quality Gate passed! |
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
I reverted the commit as I'd need some more time to properly fix this. I'm wondering however, maybe we should use a different method altogether. In the |
2fcedcd
to
291a517
Compare
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
1 similar comment
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MemberOrdering.java
Outdated
Show resolved
Hide resolved
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
To include comments, we can use |
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
ee2cb5e
to
b0a23ba
Compare
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
Thanks oxkitsune for the Apart from the minor questions below, the only big thing to tackle is verifying that this checker reserves whitespaces or deciding that we don't verify it. |
error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MemberOrdering.java
Outdated
Show resolved
Hide resolved
error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MemberOrdering.java
Outdated
Show resolved
Hide resolved
error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MemberOrdering.java
Outdated
Show resolved
Hide resolved
error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MemberOrdering.java
Outdated
Show resolved
Hide resolved
…TypeMemberOrdering
df1d962
to
8d2c426
Compare
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
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.
Sorry I didn't put comments everywhere. The main idea behind the changes is that we now do only one "scan" and collect only the members that we can actually change. After that we check is the list sorted, and if not we do the same as before. But now, we know for sure that we only have items that we can and want to switch around :). Makes it a bit easier as we don't need to check for empty/present optionals and have all the data.
So there is still a specific case not working and I didn't finish reviewing all the tests yet, but man this will be an epic check.
Oh and we should still move this to experimental :).
Credits to @Stephan202 as we paired on this.
return Position.NOPOS; | ||
} | ||
|
||
/* We return the source code position of the first token that follows the first left brace. */ |
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.
Didn't go through it with the debugger yet, but we mean the {
of the classTree
right? In that case we can also say
/* We return the source code position of the first token that follows the first left brace. */ | |
/* Return the source code position of the first token that comes after the first curly bracket. */ |
`
ClassMemberOrdering
bug checkerTypeMemberOrder
bug checker
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
2f0bef9
to
50ee382
Compare
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
Quality Gate passedIssues Measures |
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed. |
#595
Suggested commit message