fix: do not use a pointer receiver for Status.MarshalJSON #436
Conversation
Signed-off-by: knqyf263 <[email protected]>
There was a problem hiding this comment.
I tested this change in aquasecurity/trivy#7463
status field in ExperimentalModifiedFindings is marshaled correctly
| @@ -58,7 +58,7 @@ func (s *Status) Index() int { | |||
| return int(*s) | |||
| } | |||
|
|
|||
| func (s *Status) MarshalJSON() ([]byte, error) { | |||
| func (s Status) MarshalJSON() ([]byte, error) { | |||
There was a problem hiding this comment.
Now struct Status has methods on both value (UnmarshalJSON) and pointer (String, Index and UnmarshalJSON) receivers.
Should we change receivers for other functions too?
There was a problem hiding this comment.
But in this case we can't set Status to UnmarshalJSON, or am I wrong?
There was a problem hiding this comment.
it seems it's a common practice to use value receiver for UnmarshalJSON and pointer for other methods:
https://github.com/CycloneDX/cyclonedx-go/blob/6f53207eded10e9e2a362772b253621840e7ad94/cyclonedx_json.go
There was a problem hiding this comment.
There was a problem hiding this comment.
Yes, MarshalJSON and UnmarshalJSON should be excluded from the linter rule.
https://youtrack.jetbrains.com/issue/GO-13587
Description
In MarshalJSON, using a pointer receiver was not supposed to be marshalled correctly, but in Trivy, the output was correct (I did not investigate the reason in detail), and furthermore, to suppress the following warning, I used a pointer.
However, it seems that
Statuswas only correctly marshalled by chance because DetectedVulnerability was a slice. We actually found a case whereDetectedVulnerabilitywas not correctly marshalled whenDetectedVulnerabilitywas not a slice, so this PR will correct this.Translated with DeepL.com (free version)