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

Improved wording of or_fun_call lint #13528

Merged
merged 1 commit into from
Oct 11, 2024
Merged

Improved wording of or_fun_call lint #13528

merged 1 commit into from
Oct 11, 2024

Conversation

zvavybir
Copy link
Contributor

@zvavybir zvavybir commented Oct 10, 2024

The current wording (e.g. use of `ok_or` followed by a function call) is potentially confusing (at least it confused me) by suggesting that the function that follows the (in this case) ok_or is the problem and not the function that is an argument to it.

The code in my program that triggered the confusing message is the following:

let file_id = buf
    .lines()
    .next()
    .ok_or((
        InternalError::ProblemReadingFromInbox,
        anyhow!("No first line in inbox response ({file:?}): {buf:?}"),
    ))
    .html_context(stream, lang)?;

I thought that html_context was the problem and that I should do something along the following lines:

let file_id = buf
    .lines()
    .next()
    .ok_or_else(
        (
            InternalError::ProblemReadingFromInbox,
            anyhow!("No first line in inbox response ({file:?}): {buf:?}"),
        ),
        html_context(stream, lang),
    )?

This is of course wrong. My confusion was only cleared up through the help message indicating what I should try instead.

If someone has a better idea of a replacement wording (currently e.g. function call inside of `ok_or`), I'm all ears.

changelog: none

Signed-off-by: Matthias Kaak <zvavybir@zvavybir.eu>
@rustbot
Copy link
Collaborator

rustbot commented Oct 10, 2024

r? @Alexendoo

rustbot has assigned @Alexendoo.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Oct 10, 2024
@Alexendoo
Copy link
Member

Alexendoo commented Oct 11, 2024

Yeah that's better, thanks!

@bors r+

@bors
Copy link
Collaborator

bors commented Oct 11, 2024

📌 Commit 8c46c49 has been approved by Alexendoo

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Oct 11, 2024

⌛ Testing commit 8c46c49 with merge 29a0616...

bors added a commit that referenced this pull request Oct 11, 2024
Improved wording of or_fun_call lint

The current wording (e.g. ``use of `ok_or` followed by a function call``) is potentially confusing (at least it confused me) by suggesting that the function that follows the (in this case) `ok_or` is the problem and not the function that is an argument to it.

The code in my program that triggered the confusing message is the following:
```rust
let file_id = buf
    .lines()
    .next()
    .ok_or((
        InternalError::ProblemReadingFromInbox,
        anyhow!("No first line in inbox response ({file:?}): {buf:?}"),
    ))
    .html_context(stream, lang)?;
```
I thought that `html_context` was the problem and that I should do something along the following lines:
```rust
let file_id = buf
    .lines()
    .next()
    .ok_or_else(
        (
            InternalError::ProblemReadingFromInbox,
            anyhow!("No first line in inbox response ({file:?}): {buf:?}"),
        ),
        html_context(stream, lang),
    )?
```
This is of course wrong.  My confusion was only cleared up through the help message indicating what I should try instead.

If someone has a better idea of a replacement wording (currently e.g. ``` function call inside of `ok_or` ```), I'm all ears.

changelog [`or_fun_call`]: Improved confusing wording
@bors
Copy link
Collaborator

bors commented Oct 11, 2024

💔 Test failed - checks-action_test

@Alexendoo
Copy link
Member

@bors retry

@bors
Copy link
Collaborator

bors commented Oct 11, 2024

⌛ Testing commit 8c46c49 with merge b85f632...

@bors
Copy link
Collaborator

bors commented Oct 11, 2024

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Alexendoo
Pushing b85f632 to master...

@bors bors merged commit b85f632 into rust-lang:master Oct 11, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants