Skip to content
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

Error Condition will Never Occur #300

Closed
nathannaveen opened this issue Jan 6, 2023 · 1 comment · Fixed by #301
Closed

Error Condition will Never Occur #300

nathannaveen opened this issue Jan 6, 2023 · 1 comment · Fixed by #301

Comments

@nathannaveen
Copy link
Contributor

I was just testing marshalToMap() in internal/signalio/helpers.go and realized that the error statement will never be used.

if s, err := marshalValue(v); err != nil {
return nil, fmt.Errorf("failed to write field %s: %w", k, err)

The error statements are never used because marshalValue() in internal/signalio/csv.go only returns an error when the value passed to it is a map, slice, or struct (The case defaults for any type that is not an int, bool, int16, time.Time, nil and so on). But since it is called with the key value of signal.SetAsMap(s, true) where s is a signal.Set. Set is:

func (s *Field[T]) Set(v T) {
s.value = v
s.set = true
}

Set takes a parameter v which is of type SupportedType. Since SupportedType does not include maps, slices, and structs, marshalValue will never return an error.

nathannaveen added a commit to nathannaveen/criticality_score that referenced this issue Jan 6, 2023
- Included tests for `internal/signalio/helpers`
- Fixes ossf#300

Signed-off-by: nathannaveen <42319948+nathannaveen@users.noreply.github.com>
nathannaveen added a commit to nathannaveen/criticality_score that referenced this issue Jan 6, 2023
- Included tests for `internal/signalio/helpers`
- Fixes ossf#300

Signed-off-by: nathannaveen <42319948+nathannaveen@users.noreply.github.com>
@nathannaveen
Copy link
Contributor Author

This is invalid because of the example given by @calebbrown: #301 (comment)

calebbrown pushed a commit that referenced this issue Apr 24, 2023
* Included Tests for internal/signalio/helpers

- Included tests for `internal/signalio/helpers`
- Fixes #300

Signed-off-by: nathannaveen <42319948+nathannaveen@users.noreply.github.com>

* Updated based on code review

Signed-off-by: nathannaveen <42319948+nathannaveen@users.noreply.github.com>

* Included more tests

Signed-off-by: nathannaveen <42319948+nathannaveen@users.noreply.github.com>

---------

Signed-off-by: nathannaveen <42319948+nathannaveen@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant