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

Disable the map_unwrap_or lint #7524

Merged
merged 1 commit into from
Sep 26, 2024
Merged

Disable the map_unwrap_or lint #7524

merged 1 commit into from
Sep 26, 2024

Conversation

abey79
Copy link
Member

@abey79 abey79 commented Sep 26, 2024

What

map_or(default_value, |val| f(val)) is imo strictly worse than map(|val| f(val)).unwrap_or(default_value), especially in a context of chained calls where having each on a different line is better. So this PR disables this lint.

Relevant slack thread: https://rerunio.slack.com/archives/C0440MD5UAV/p1724749276769059

Arguably, the self-admitted known limitations are much worse than the supposed problem this lint is supposed to fix: https://rust-lang.github.io/rust-clippy/stable/index.html#/map_unwrap_or

Checklist

  • I have read and agree to Contributor Guide and the Code of Conduct
  • I've included a screenshot or gif (if applicable)
  • I have tested the web demo (if applicable):
  • The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG
  • If applicable, add a new check to the release checklist!
  • If have noted any breaking changes to the log API in CHANGELOG.md and the migration guide

To run all checks from main, comment on the PR with @rerun-bot full-check.

@abey79 abey79 added 🦀 Rust API Rust logging API 🧑‍💻 dev experience developer experience (excluding CI) exclude from changelog PRs with this won't show up in CHANGELOG.md labels Sep 26, 2024
Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the brevity of map_or, but I also think it’s weirdly backwards - it says “map, or” but then does the opposite.

As long as you don’t plan on aggressively changing stuff back :D ✅

@abey79
Copy link
Member Author

abey79 commented Sep 26, 2024

No war against map_or. If it avoids a line break, fine 🤷🏻 I just want clippy to leave me alone with my map().unwrap_or() :)

@abey79 abey79 merged commit 0d0277d into main Sep 26, 2024
37 checks passed
@abey79 abey79 deleted the antoine/no-map-unwrap-or branch September 26, 2024 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧑‍💻 dev experience developer experience (excluding CI) exclude from changelog PRs with this won't show up in CHANGELOG.md 🦀 Rust API Rust logging API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants