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

rustc::lint-API: Distinguish directly applicable from colloquial code suggestions #33691

Closed
llogiq opened this issue May 17, 2016 · 11 comments
Closed
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-enhancement Category: An issue proposing an enhancement or a PR with one. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue.

Comments

@llogiq
Copy link
Contributor

llogiq commented May 17, 2016

There has been some inquiry into a "quick fix" IDE feature for Rust. Given that lints (including clippy) already generate suggestions based on code, a quick win could be made by allowing the lints to distinguish directly applicable suggestions and perhaps somehow flag those for tools using rustc's JSON output. Then IDEs could weave the suggestions into the code based on the reported span.

cc @jonathandturner @nrc @Manishearth @mcarton @birkenfeld

@Manishearth
Copy link
Member

IMO such a feature would probably be more involved than just span_suggestion, with hooks for replacing individual bits of code (instead of text substitution) in a typed fashion. But if not, a distinguishing span_help_code would be nice.

@Manishearth Manishearth added A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-tools labels May 18, 2016
@nrc
Copy link
Member

nrc commented May 18, 2016

span_suggestion probably needs an overhaul, but would be a good starting point. The suggestion JSON should be able to be nested in JSON errors or emitted individually (e.g., Clippy would do the latter, although perhaps it could just emit JSON errors when requested with the embedded suggestion).

@Manishearth what does "in a typed fashion" mean? I would expect that all the hard processing would be done by the tool generating the suggestion (rustc, clippy) and the suggestion is just a set of spans and replacement text (where the replacement text might be the empty string for deletions and the span to be replaced might be zero-length for insertions).

We might also want to let the replacing tool format the text, so the suggestion text could be some kind of reification of the AST. That has horrible problems. Alternatively the suggestion could be unformatted source code and if the replacement tool wants to reformat, it can re-parse and reformat.

@Manishearth
Copy link
Member

what does "in a typed fashion" mean?

Currently clippy takes span_to_snippets and splices them together to get suggestions. It would be nicer if we could take the typed AST objects and splice them together (e.g. moving an expression from an if block to the outside by appending it to the outer block) and print them, while retaining formatting and macros. There's no way to do that currently, and we have to do random hacks to get indentation to work.

But yeah, splice-and-feed-to-rustfmt sort of works too.

@oli-obk
Copy link
Contributor

oli-obk commented May 18, 2016

Related internals post: https://internals.rust-lang.org/t/rustcs-json-output-format/3446/11?u=ker

if things like unresolved name xyz, did you mean xyz1 are changed to use suggestions, we'd need a way to distinguish between guaranteed correct suggestions like changing !(a == b) into a != b against heuristic suggestions like changing xyz into xyz1 or like adding new use x::X; items when X is not found in the scope.

@Manishearth
Copy link
Member

Yeah, I think in that case we don't want to output code in the first place

@oli-obk
Copy link
Contributor

oli-obk commented May 18, 2016

Yeah, I think in that case we don't want to output code in the first place

Not sure I understand? If there's an ambiguity, the user can decide, but often it's still possible to provide sensible choices, so the user can click on a choice and the IDE inserts that one.

Are you suggesting to never provide snippets if there are ambiguities or heuristics involved?

@Manishearth
Copy link
Member

I guess? We'd need a clear distinction then, right.

@sanxiyn
Copy link
Member

sanxiyn commented May 18, 2016

I think it is enough to simply clarify that span_suggestion is only for directly applicable suggestions. In fact, have we ever used span_suggestion when it is not directly applicable?

@Manishearth
Copy link
Member

Yeah, that was my point -- we don't do it now and we can continue not doing it.

@llogiq
Copy link
Contributor Author

llogiq commented May 18, 2016

I think we have two interesting distinctions to make:

  1. If the suggestion can be directly applied to the code (that's what this issue is about)
  2. How many alternative suggestions we have and how to sort them (my vote is sort by likelyhood, though we may need to find good enough heuristics)

@alexcrichton alexcrichton added T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. and removed T-tools labels May 22, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jul 25, 2017
@oli-obk
Copy link
Contributor

oli-obk commented Aug 17, 2018

I think this has been resolved by the Applicability annotations

@oli-obk oli-obk closed this as completed Aug 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-enhancement Category: An issue proposing an enhancement or a PR with one. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants