Refactor for Error Message class use #284
Conversation
| /** Contains error message string to be displayed and the message type from {@link MessageTypes}. */ | ||
| public class ErrorMessage { | ||
|
|
||
| static Config config; | ||
| static NullAway nullAway; |
There was a problem hiding this comment.
No static fields please; they can lead to memory leaks and data races. Also we shouldn't be keeping a pointer to the NullAway object here. We should pass in just the state that we need.
There was a problem hiding this comment.
Done 👍
Separated the code into new ErrorBuilder class.
|
|
||
| private final Config config; | ||
| private final String checkerName; | ||
| private final Set<String> allNames; |
There was a problem hiding this comment.
What is this field? Pls document
| public class ErrorBuilder { | ||
|
|
||
| private final Config config; | ||
| private final String checkerName; |
There was a problem hiding this comment.
Should this be maybe suppressionName? Looks like it's just used when generating a @SuppressWarnings. Pls also add docs
| @@ -48,8 +48,6 @@ | |||
|
|
|||
| private final DataFlow dataFlow; | |||
|
|
|||
| private static String OPTIONAL_PATH = "java.util.Optional"; | |||
|
Not a full review, but, eyeballing it, it looks fine to me, except the PR title/commit message has a typo: "Mesaage". |
shubhamugare
changed the title
Oops! 😅 |
Made following changes:
Moved all the methods which are related to creating error message to
ErrorBuilderclass.Used Error Message object as a parameter for
createErrorDescription.Some minor nits.