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

Implement lint plugins #15024

Merged
merged 20 commits into from
Jun 24, 2014
Merged

Implement lint plugins #15024

merged 20 commits into from
Jun 24, 2014

Conversation

kmcallister
Copy link
Contributor

This is a rebase of #14804 with two new commits on top to implement and test lint plugins.

r? @alexcrichton @huonw: Can you take a look at the new commits, and also weigh in about any issues from the old PR that you feel are still unresolved? I'm leaving the old branch alone to preserve discussion history.

@kmcallister
Copy link
Contributor Author

(Those new commits are "Implement lint plugins" and "Test lint plugins". The GitHub author-date-based view is very confusing.)

@alexcrichton
Copy link
Member

Sorry it took a little bit to get back to this. This is certainly a huge step forward in the right direction, and it's definitely susceptible to bitrot quickly. I think this is ready to merge as the design would only be getting tweaked here and there. The major bulk of work has been done and the design likely won't change radically.

Thanks again for working on this!

r=me with a rebase

Keegan McAllister added 20 commits June 24, 2014 10:22
We're going to have more modules under lint, and the paths get unwieldy. We
also plan to have lints run at multiple points in the compilation pipeline.
The immediate benefits are

* moving the state specific to a single lint out of Context, and
* replacing the soup of function calls in the Visitor impl with
  more structured control flow

But this is also a step towards loadable lints.
Also change some code formatting.

lint::builtin becomes a sibling of lint::context in order to ensure that lints
implemented there use the same public API as lint plugins.
It wasn't a very appropriate use of the trait. Instead, just enumerate
unit structs and those with a `fn new()` separately.
None of the builtin lints use this, and it's now available through the Context.
bors added a commit that referenced this pull request Jun 24, 2014
This is a rebase of #14804 with two new commits on top to implement and test lint plugins.

r? @alexcrichton @huonw: Can you take a look at the new commits, and also weigh in about any issues from the old PR that you feel are still unresolved? I'm leaving the old branch alone to preserve discussion history.
@bors bors closed this Jun 24, 2014
@bors bors merged commit 7e694e7 into rust-lang:master Jun 24, 2014
@alexcrichton
Copy link
Member

\o/ nice work!

@kmcallister
Copy link
Contributor Author

Yay! Thanks Alex and everyone else who helped get this in!

lnicola pushed a commit to lnicola/rust that referenced this pull request Jun 19, 2023
…Veykril

fix: deduplicate fields and types in completion

Fixes rust-lang#15024

- `hir_ty::autoderef()` (which is only meant to be used outside `hir-ty`) now deduplicates types and completely resolves inference variables within.
- field completion now deduplicates fields of the same name and only picks such field of the first type in the deref chain.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants