minor: do not check details if checker suppression is unstable #12211
Conversation
nrmancuso
force-pushed
the
checker-hack
branch
3 times, most recently
from
7d12f2e
to
f631e97
Compare
|
Unfortunately, this will not work, we got a new changing variable:
|
nrmancuso
force-pushed
the
checker-hack
branch
from
f631e97
to
5e6e173
Compare
nrmancuso
force-pushed
the
checker-hack
branch
2 times, most recently
from
b5280cb
to
a71a029
Compare
.ci/checker-framework.groovy
Outdated
| result = (fileName != null ? fileName.hashCode() : 0) | ||
| result = 31 * result + (specifier != null ? specifier.hashCode() : 0) | ||
| result = 31 * result + (message != null ? message.hashCode() : 0) | ||
| result = 31 * result + (details != null ? details.hashCode() : 0) |
There was a problem hiding this comment.
This won't work if hashCode is actually required and used.
If two objects are equal according to equals() method, then their hash code must be same.
There was a problem hiding this comment.
As long as you override hashCode and don't implement similar logic between it and equals, it will be an issue. Removing details and message from just this method won't "Fix" the underlying issue.
Is this hashCode actually used/needed?
There was a problem hiding this comment.
No, I just figured codenarc or something else would have a warning about overriding equals but not hashcode.
There was a problem hiding this comment.
I've removed it, let's see if there are any warnings.
There was a problem hiding this comment.
Another question, do we actually have a reason for it to be equals or can it be it's own thing like fuzzyEquals?
There was a problem hiding this comment.
https://github.com/checkstyle/checkstyle/blob/master/.ci/checker-framework.groovy#L163
final Set<CheckerFrameworkError> errors = new HashSet<>()
It is a hash object. You will need a valid hashCode and equals.
There was a problem hiding this comment.
Another question, do we actually have a reason for it to be
equalsor can it be it's own thing likefuzzyEquals?
Set.contains relies on equals.
nrmancuso
force-pushed
the
checker-hack
branch
2 times, most recently
from
2428c4e
to
aaf78cf
Compare
nrmancuso
force-pushed
the
checker-hack
branch
from
aaf78cf
to
c0bb9d2
Compare
we can not ignore message, better to apply regexp to it to remove variable part. |
|
Closing in favor of #12215 |
This is a quick hack until we come up with better idea for #12210