-
Notifications
You must be signed in to change notification settings - Fork 299
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
Fix another bug with Lombok equals() parameter annotations #880
base: master
Are you sure you want to change the base?
Changes from 7 commits
1ccfab4
5d50e8d
bad9d8a
ddb321c
d5ae980
e3d53ba
7091a25
210d2b6
7092ef6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -129,9 +129,10 @@ public Description createErrorDescription( | |
checkName = INITIALIZATION_CHECK_NAME; | ||
} | ||
|
||
// Mildly expensive state.getPath() traversal, occurs only once per potentially | ||
// reported error. | ||
if (hasPathSuppression(state.getPath(), checkName)) { | ||
// Mildly expensive state.getPath() traversal, occurs twice per potentially reported error. We | ||
// can get rid of the check for "all" once we require Error Prone 2.22.0 or higher. | ||
Comment on lines
+132
to
+133
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is what I was talking about on the EP support policy, we probably want to remove this once we drop support for older EP versions (and we can have an issue to track it due to performance implications) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a very good question. Right now, we are stuck supporting a very old EP version (2.10.0) since we still support running NullAway on JDK 8, and that was the last EP version that supported running on JDK 8. At this point, I think we should make a plan for dropping JDK 8 support in the near-ish future, and then we could bump our minimum EP version to something much more recent. As to what policy we should adhere to after that, I'm thinking something like we will typically support an EP version for X months after it was released. For reference, 2.10.0 was released on Nov. 4, 2021, so we are just past two years since it came out. Maybe we could generally aim for supporting an EP release for 12-18 months? Not sure of the right number, and we don't need a hard-and-fast rule. But I agree we should have something in mind. I've added a comment on #634 regarding dropping JDK 8 support. And I've opened #882 regarding support for older Error Prone versions. |
||
TreePath path = state.getPath(); | ||
if (hasPathSuppression(path, checkName) || hasPathSuppression(path, "all")) { | ||
return Description.NO_MATCH; | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
package com.uber.lombok; | ||
|
||
import javax.annotation.Nullable; | ||
import lombok.Data; | ||
import lombok.EqualsAndHashCode; | ||
|
||
@Data | ||
@EqualsAndHashCode(callSuper = false) | ||
public class SimpleDataSub extends SimpleDataSuper { | ||
@Nullable private String field2; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
package com.uber.lombok; | ||
|
||
import javax.annotation.Nullable; | ||
import lombok.Data; | ||
|
||
@Data | ||
public class SimpleDataSuper { | ||
@Nullable private String field1; | ||
} |
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 is not required with the latest Error Prone version (the issue was fixed), and the check did not exist in the oldest Error Prone version we support, so just delete