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

Add suggestion for explicit_write lint #3450

Merged
merged 7 commits into from Dec 12, 2018

Conversation

Projects
None yet
4 participants
@phansch
Copy link
Collaborator

phansch commented Nov 23, 2018

Closes #2083

@phansch phansch changed the title Add suggestion for explicit_write lint WIP: Add suggestion for explicit_write lint Nov 23, 2018

@phansch
Copy link
Collaborator

phansch left a comment

need to fix one leftover debug variable name

Show resolved Hide resolved clippy_lints/src/explicit_write.rs Outdated

@phansch phansch force-pushed the phansch:structured_sugg_for_explicit_write branch 2 times, most recently from 7fe226f to 13819ce Nov 23, 2018

@phansch phansch changed the title WIP: Add suggestion for explicit_write lint Add suggestion for explicit_write lint Nov 24, 2018

@@ -83,32 +84,57 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
} else {
""
};

// We need to remove the last trailing newline from the string because the
// underlying `fmt::write` function doesn't know wether `println!` or `print!` was

This comment has been minimized.

@detrumi

detrumi Nov 28, 2018

Contributor

Typo in whether

// used.
let mut write_output: String = write_output.to_string();
if write_output.ends_with('\n') {
write_output.truncate(write_output.len() - 1)

This comment has been minimized.

@detrumi

detrumi Nov 28, 2018

Contributor

How about write_output.pop()?

// We need to remove the last trailing newline from the string because the
// underlying `fmt::write` function doesn't know wether `println!` or `print!` was
// used.
let mut write_output: String = write_output_string(write_args).unwrap();

This comment has been minimized.

@detrumi

detrumi Nov 28, 2018

Contributor

I'm not sure if this function can fail on some inputs, but if there are cases where this fails, it should probably fall back to not using a suggestion. Otherwise, maybe expect() would be better?

@phansch phansch force-pushed the phansch:structured_sugg_for_explicit_write branch from b3d41bf to c84a17d Nov 29, 2018

@phansch

This comment was marked as resolved.

Copy link
Collaborator

phansch commented Dec 7, 2018

@bors try

@bors

This comment was marked as resolved.

Copy link
Contributor

bors commented Dec 12, 2018

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

phansch added some commits Nov 23, 2018

Address review feedback
* Fix typo
* Handle None value instead of using `unwrap()`
* `pop()` instead of `x.truncate(x.len() - 1)`

@phansch phansch force-pushed the phansch:structured_sugg_for_explicit_write branch from 0933b7a to 194acaf Dec 12, 2018

@flip1995
Copy link
Collaborator

flip1995 left a comment

travis is currently failing (https://travis-ci.org/rust-lang/rust-clippy/jobs/467013498#L152). I'm working on a fix. If travis is working again r=me.

@flip1995

This comment has been minimized.

Copy link
Collaborator

flip1995 commented Dec 12, 2018

@bors rollup

@flip1995

This comment has been minimized.

Copy link
Collaborator

flip1995 commented Dec 12, 2018

Let's queue this

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Dec 12, 2018

📌 Commit 194acaf has been approved by flip1995

bors added a commit that referenced this pull request Dec 12, 2018

Auto merge of #3450 - phansch:structured_sugg_for_explicit_write, r=f…
…lip1995

Add suggestion for explicit_write lint

Closes #2083
@bors

This comment has been minimized.

Copy link
Contributor

bors commented Dec 12, 2018

⌛️ Testing commit 194acaf with merge 7c823ca...

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Dec 12, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: flip1995
Pushing 7c823ca to master...

@bors bors merged commit 194acaf into rust-lang:master Dec 12, 2018

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details

@phansch phansch deleted the phansch:structured_sugg_for_explicit_write branch Dec 12, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment