Skip to content

False positive 'option-map-unit-fn' for std::process::exit #5180

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

Closed
jyn514 opened this issue Feb 16, 2020 · 5 comments · Fixed by #5290 or #5292
Closed

False positive 'option-map-unit-fn' for std::process::exit #5180

jyn514 opened this issue Feb 16, 2020 · 5 comments · Fixed by #5290 or #5292
Labels
C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages good-first-issue These issues are a good way to get started with Clippy L-suggestion Lint: Improving, adding or fixing lint suggestions

Comments

@jyn514
Copy link
Member

jyn514 commented Feb 16, 2020

warning: called `map(f)` on an Option value where `f` is a unit function
   --> src/bin/rcc.rs:124:9
    |
124 |         unsafe { rccjit.run_main() }.map(std::process::exit);
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-
    |         |
    |         help: try this: `if let Some(_) = unsafe { rccjit.run_main() } { std::process::exit(_) }`

This suggestion makes no sense: you can't pass _ to a function. Furthermore, exit() is not a unit function, it takes i32.

cargo clippy -V: clippy 0.0.212 (e8642c7 2020-01-06) (but also happens in stable)

@flip1995
Copy link
Member

Furthermore, exit() is not a unit function, it takes i32.

The unit functions refers to the return value, not the param. ! is treated like the unit type in this lint apparently. The suggestion uses _ as a placeholder. Maybe we could use a or something similar as a placeholder instead.

@flip1995 flip1995 added L-suggestion Lint: Improving, adding or fixing lint suggestions good-first-issue These issues are a good way to get started with Clippy C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages labels Feb 16, 2020
@jyn514
Copy link
Member Author

jyn514 commented Feb 17, 2020

The unit functions refers to the return value, not the param.

I see, that makes more sense. That wasn't very clear from the suggestion, maybe it could say 'f is a function returning unit type' instead?

bors added a commit that referenced this issue Mar 9, 2020
Improve error messages for {option,result}_map_unit_fn

Instead of saying "unit function", use the phrase the description uses: "function that returns the unit type".

Fixes #5180.

changelog: Improve error messages for {option,result}_map_unit_fn
@bors bors closed this as completed in ab6e709 Mar 9, 2020
@jyn514
Copy link
Member Author

jyn514 commented Mar 9, 2020

That PR (#5292) didn't address my main concern, which is that 'unit function' is not very clear. Can this be reopened?

@flip1995 flip1995 reopened this Mar 9, 2020
@flip1995
Copy link
Member

flip1995 commented Mar 9, 2020

#5290 addressed this comment #5180 (comment)

With #5292, this issue should be resolved completely, right?

@jyn514
Copy link
Member Author

jyn514 commented Mar 9, 2020

Oops, I didn't notice that #5290 had been merged. In that case I'm happy with the fix :)

@jyn514 jyn514 closed this as completed Mar 9, 2020
bors added a commit that referenced this issue Mar 9, 2020
Improve placeholder in map_unit_fn

Instead of using `_` as a default placeholder use `a`.

fixes #5180
changelog: Improve default placeholder in map_unit_fn
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages good-first-issue These issues are a good way to get started with Clippy L-suggestion Lint: Improving, adding or fixing lint suggestions
Projects
None yet
2 participants