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

Use enum for approximate suggestions #50204

Merged
merged 2 commits into from
Apr 30, 2018
Merged

Conversation

Manishearth
Copy link
Member

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 24, 2018
@@ -56,6 +56,14 @@ mod lock;

use syntax_pos::{BytePos, Loc, FileLinesResult, FileMap, FileName, MultiSpan, Span, NO_EXPANSION};

#[derive(Copy, Clone, Debug, PartialEq, Hash, RustcEncodable, RustcDecodable)]
pub enum SuggestionApproximate {
Copy link
Member

Choose a reason for hiding this comment

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

I feel like this should have a different name given that it seems to include non-approximate (MachineApplicable) suggestions. SuggestionDefiniteness?

Copy link
Member

Choose a reason for hiding this comment

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

I see what you mean. My first suggestion would've been SuggestionApplicability, though. (Let the bikeshedding begin)

Copy link
Member

Choose a reason for hiding this comment

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

I like Applicability

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated

@estebank
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Apr 29, 2018

📌 Commit 4e2cd41 has been approved by estebank

@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 Apr 29, 2018
@bors
Copy link
Contributor

bors commented Apr 30, 2018

⌛ Testing commit 4e2cd41 with merge 9ff8ec8...

bors added a commit that referenced this pull request Apr 30, 2018
@bors
Copy link
Contributor

bors commented Apr 30, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: estebank
Pushing 9ff8ec8 to master...

@kennytm-githubbot
Copy link

📣 Toolstate changed by #50204!

Tested on commit 9ff8ec8.
Direct link to PR: #50204

💔 clippy-driver on windows: test-pass → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk).
💔 clippy-driver on linux: test-pass → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk).

kennytm-githubbot added a commit to rust-lang-nursery/rust-toolstate that referenced this pull request Apr 30, 2018
Tested on commit rust-lang/rust@9ff8ec8.
Direct link to PR: <rust-lang/rust#50204>

💔 clippy-driver on windows: test-pass → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk).
💔 clippy-driver on linux: test-pass → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk).
@bors bors merged commit 4e2cd41 into rust-lang:master Apr 30, 2018
zackmdavis added a commit to zackmdavis/rust that referenced this pull request May 20, 2018
Consider this a down payment on rust-lang#50723. To recap, an `Applicability`
enum was recently (rust-lang#50204) added, to convey to Rustfix and other tools
whether we think it's OK for them to blindly apply the suggestion, or
whether to prompt a human for guidance (because the suggestion might
contain placeholders that we can't infer, or because we think it has a
sufficiently high probability of being wrong even though it's—
presumably—right often enough to be worth emitting in the first place).

When a suggestion is marked as `MaybeIncorrect`, we try to use comments
to indicate precisely why (although there are a few places where we just
say `// speculative` because the present author's subjective judgement
balked at the idea that the suggestion has no false positives).

The `run-rustfix` directive is opporunistically set on some relevant UI
tests (and a couple tests that were in the `test/ui/suggestions`
directory, even if the suggestions didn't originate in librustc or
libsyntax). This is less trivial than it sounds, because a surprising
number of test files aren't equipped to be tested as fixed even when
they contain successfully fixable errors, because, e.g., there are more,
not-directly-related errors after fixing. Some test files need an
attribute or underscore to avoid unused warnings tripping up the "fixed
code is still producing diagnostics" check despite the fixes being
correct; this is an interesting contrast-to/inconsistency-with the
behavior of UI tests (which secretly pass `-A unused`), a behavior which
we probably ought to resolve one way or the other (filed issue rust-lang#50926).

A few suggestion labels are reworded (e.g., to avoid phrasing it as a
question, which which is discouraged by the style guidelines listed in
`.span_suggestion`'s doc-comment).
zackmdavis added a commit to zackmdavis/rust that referenced this pull request Aug 27, 2018
Looks like we forgot about this analogue of the other
`_with_applicability` methods (first introduced in rust-lang#50204)?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants