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

new lint: read_line_without_trim #10970

Merged
merged 2 commits into from
Jul 5, 2023
Merged

Conversation

y21
Copy link
Member

@y21 y21 commented Jun 16, 2023

This adds a new lint that checks for calls to Stdin::read_line with a reference to a string that is then attempted to parse into an integer type without first trimming it, which is always going to fail at runtime.
This is something that I've seen happen a lot to beginners, because it's easy to run into when following the example of chapter 2 in the book where it shows how to program a guessing game.
It would be nice if we could point beginners to clippy and tell them "let's see what clippy has to say" and have clippy explain to them why it fails 👀

I think this lint can later be "generalized" to work not just for Stdin but also any BufRead (which seems to be where the guarantee about the trailing newline comes from) and also, matching/comparing it to a string slice that doesn't end in a newline character (e.g. input == "foo" is always going to fail)

changelog: new lint: [read_line_without_trim]

@rustbot
Copy link
Collaborator

rustbot commented Jun 16, 2023

r? @giraffate

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jun 16, 2023
@bors
Copy link
Contributor

bors commented Jul 1, 2023

☔ The latest upstream changes (presumably #11012) made this pull request unmergeable. Please resolve the merge conflicts.

@y21 y21 force-pushed the read_line_without_trim branch from 3df67cc to 8000b44 Compare July 2, 2023 15:01
@y21 y21 force-pushed the read_line_without_trim branch from 8000b44 to 573bd75 Compare July 3, 2023 20:51
@y21 y21 force-pushed the read_line_without_trim branch from 573bd75 to 85f535b Compare July 3, 2023 20:57
@giraffate
Copy link
Contributor

@bors r+

Thanks!

@bors
Copy link
Contributor

bors commented Jul 5, 2023

📌 Commit 85f535b has been approved by giraffate

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jul 5, 2023

⌛ Testing commit 85f535b with merge 1e656d8...

@bors
Copy link
Contributor

bors commented Jul 5, 2023

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: giraffate
Pushing 1e656d8 to master...

1 similar comment
@bors
Copy link
Contributor

bors commented Jul 5, 2023

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: giraffate
Pushing 1e656d8 to master...

@bors bors merged commit 1e656d8 into rust-lang:master Jul 5, 2023
bors added a commit that referenced this pull request Feb 27, 2024
[`read_line_without_trim`]: detect string literal comparison and `.ends_with()` calls

This lint now also realizes that a comparison like `s == "foo"` and calls such as `s.ends_with("foo")` will fail if `s` was initialized by a call to `Stdin::read_line` (because of the trailing newline).

changelog: [`read_line_without_trim`]: detect string literal comparison and `.ends_with()` calls

r? `@giraffate` assigning you because you reviewed #10970 that added this lint, so this is kinda a followup PR ^^
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