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

Jump to bottom

Suggest appropriate syntax on missing lifetime specifier in return type #55173

Merged
merged 3 commits into from Oct 25, 2018
Merged

Suggest appropriate syntax on missing lifetime specifier in return type #55173

merged 3 commits into from Oct 25, 2018

Conversation

estebank
Copy link
Contributor

Suggest using 'static when a lifetime is missing in the return type
with a structured suggestion instead of a note.

Fix #55170.

@rust-highfive
Copy link
Collaborator

r? @oli-obk

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 18, 2018
oli-obk
oli-obk reviewed Oct 18, 2018
View reviewed changes
src/librustc/middle/resolve_lifetime.rs Outdated
help!(db, "consider giving it a 'static lifetime");
let msg = "consider giving it a 'static lifetime";
match self.tcx.sess.source_map().span_to_snippet(span) {
Ok(ref snippet) if snippet == "&" => db.span_suggestion_with_applicability(
Copy link
Contributor

Choose a reason for hiding this comment

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

This match has some formattings that threw me off. I think it would be easier on the eyes to have something along the lines of

Ok(ref snippet) => {
    let (sugg, app) = match &snippet[..] {
        "&" => ("&'static ".to_owned(), Applicability::MachineApplicable),
        "'_" => ("'static ".to_owned(), Applicability::MachineApplicable),
        snippet => (format!("{} + 'static", snippet), Applicability::MaybeIncorrect),
    };
    db.span_suggestion_with_applicability(span, msg, sugg, app);
}

oli-obk
oli-obk requested changes Oct 18, 2018
View reviewed changes
src/librustc/middle/resolve_lifetime.rs Outdated
lifetime"
);
let msg = "consider giving it an explicit bounded or 'static lifetime";
match self.tcx.sess.source_map().span_to_snippet(span) {
Copy link
Contributor

Choose a reason for hiding this comment

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

same here, in fact, this could probably be unified into a function

src/test/ui/associated-types/bound-lifetime-in-binding-only.elision.stderr Outdated
| ^ expected lifetime parameter
| ^
| |
| expected lifetime parameter
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe remove the "expected lifetime parameter" message completely? It already says "missing lifetime specifier" in the error message

@rust-highfive

This comment has been minimized.

Sign in to view
@estebank
Copy link
Contributor Author

@oli-obk addressed review comments

oli-obk
oli-obk reviewed Oct 19, 2018
View reviewed changes
src/librustc/middle/resolve_lifetime.rs Outdated
fn suggest_lifetime(&self, db: &mut DiagnosticBuilder<'_>, span: Span, msg: &str) -> bool {
match self.tcx.sess.source_map().span_to_snippet(span) {
Ok(ref snippet) => {
let (sugg, applicability) = if &snippet[..] == "&" {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
let (sugg, applicability) = if &snippet[..] == "&" {
let (sugg, applicability) = if snippet == "&" {

@oli-obk
Copy link
Contributor

oli-obk commented Oct 19, 2018

r=me with nits applied

@estebank
Copy link
Contributor Author

@bors r=oli-obk

@bors
Copy link
Contributor

bors commented Oct 19, 2018

📌 Commit 3c096ec2f46542adf968139ce8709c8e690b6b1b has been approved by oli-obk

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 19, 2018
@bors

This comment has been minimized.

Sign in to view
@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 20, 2018
estebank and others added 3 commits October 22, 2018 14:54
@estebank
Suggest appropriate syntax on missing lifetime specifier in return type
e1e52eb 
Suggest using `'static` when a lifetime is missing in the return type
with a structured suggestion instead of a note.
@estebank
review comments
d0bd69a
@oli-obk @estebank
[review comments] modify test and clean up code
dd91c8f 
Co-Authored-By: estebank <esteban@kuber.com.ar>
@estebank
Copy link
Contributor Author

@bors r=oli-obk rollup

@bors
Copy link
Contributor

bors commented Oct 22, 2018

📌 Commit dd91c8f has been approved by oli-obk

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 22, 2018
pietroalbini added a commit to pietroalbini/rust that referenced this pull request Oct 23, 2018
@pietroalbini
Rollup merge of rust-lang#55173 - estebank:suggest-static, r=oli-obk
830c6b0 
Suggest appropriate syntax on missing lifetime specifier in return type

Suggest using `'static` when a lifetime is missing in the return type
with a structured suggestion instead of a note.

Fix rust-lang#55170.
@pietroalbini pietroalbini mentioned this pull request Oct 23, 2018
Closed
kennytm added a commit to kennytm/rust that referenced this pull request Oct 24, 2018
@kennytm
Rollup merge of rust-lang#55173 - estebank:suggest-static, r=oli-obk
942a162 
Suggest appropriate syntax on missing lifetime specifier in return type

Suggest using `'static` when a lifetime is missing in the return type
with a structured suggestion instead of a note.

Fix rust-lang#55170.
@kennytm kennytm mentioned this pull request Oct 24, 2018
Closed
pietroalbini added a commit to pietroalbini/rust that referenced this pull request Oct 25, 2018
@pietroalbini
Rollup merge of rust-lang#55173 - estebank:suggest-static, r=oli-obk
f50e285 
Suggest appropriate syntax on missing lifetime specifier in return type

Suggest using `'static` when a lifetime is missing in the return type
with a structured suggestion instead of a note.

Fix rust-lang#55170.
@pietroalbini pietroalbini mentioned this pull request Oct 25, 2018
Merged
bors added a commit that referenced this pull request Oct 25, 2018
@bors
Auto merge of #55347 - pietroalbini:rollup, r=pietroalbini
8ec22e7 
Rollup of 22 pull requests

Successful merges:

 - #53507 (Add doc for impl From for Waker)
 - #53931 (Gradually expanding libstd's keyword documentation)
 - #54965 (update tcp stream documentation)
 - #54977 (Accept `Option<Box<$t:ty>>` in macro argument)
 - #55138 (in which unused-parens suggestions heed what the user actually wrote)
 - #55173 (Suggest appropriate syntax on missing lifetime specifier in return type)
 - #55200 (Documents `From` implementations for `Stdio`)
 - #55245 (submodules: update clippy from 5afdf8b to b1d0343)
 - #55247 (Clarified code example in char primitive doc)
 - #55251 (Fix a typo in the documentation of RangeInclusive)
 - #55253 (only issue "variant of the expected type" suggestion for enums)
 - #55254 (Correct trailing ellipsis in name_from_pat)
 - #55269 (fix typos in various places)
 - #55282 (Remove redundant clone)
 - #55285 (Do some copy editing on the release notes)
 - #55291 (Update stdsimd submodule)
 - #55296 (Set RUST_BACKTRACE=0 for rustdoc-ui/failed-doctest-output.rs)
 - #55306 (Regression test for #54478.)
 - #55328 (Fix doc for new copysign functions)
 - #55340 (Operands no longer appear in places)
 - #55345 (Remove is_null)
 - #55348 (Update RELEASES.md after destabilization of non_modrs_mods)

Failed merges:

r? @ghost
@bors bors merged commit dd91c8f into rust-lang:master Oct 25, 2018
@estebank estebank mentioned this pull request Nov 7, 2018
Closed
@estebank estebank deleted the suggest-static branch November 9, 2023 05:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oli-obk oli-obk oli-obk requested changes

Assignees

@oli-obk oli-obk

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@estebank @rust-highfive @oli-obk @bors