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.

Sign up for GitHub

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

Empty import lists are too permissive #38012

Closed
jseyfried opened this issue Nov 26, 2016 · 1 comment · Fixed by #38154
Closed

Empty import lists are too permissive #38012

jseyfried opened this issue Nov 26, 2016 · 1 comment · Fixed by #38154
Assignees
@jseyfried
Labels
A-resolve Area: Name resolution

Comments

@jseyfried
Copy link
Contributor

For example,

struct S;
use S::{}; // this is allowed,
use S::*; // but this is an error.
@sanxiyn sanxiyn added the A-resolve Area: Name resolution label Nov 28, 2016
@jseyfried jseyfried self-assigned this Nov 29, 2016
@estebank estebank mentioned this issue Nov 30, 2016
Merged
@petrochenkov petrochenkov mentioned this issue Dec 8, 2016
Merged
@jseyfried jseyfried mentioned this issue Dec 12, 2016
Merged
@jseyfried
Copy link
Contributor Author

As @petrochenkov noted in #38313 (comment), we should also forbid

struct S;
use S::{self};

bors added a commit that referenced this issue Dec 26, 2016
@bors
Auto merge of #38154 - petrochenkov:altname, r=jseyfried
65c043f 
More systematic error reporting in path resolution

Path resolution for types, expressions and patterns used various heuristics to give more helpful messages on unresolved or incorrectly resolved paths.
This PR combines these heuristics and applies them to all non-import paths.

First a path is resolved in all namespaces, starting from its primary namespace (to give messages like "expected function, found macro, you probably forgot `!`").
If this resolution doesn't give a desired result we create a base error - either "path is not resolved" or "path is resolved, but the resolution is not acceptable in this context".
Other helps and notes are applied to this base error using heuristics.

Here's the list of heuristics for a path with a last segment `name` in order.
First we issue special messages for unresolved `Self` and `self`.
Second we try to find free items named `name` in other modules and suggest to import them.
Then we try to find fields and associated items named `name` and suggest `self.name` or `Self::name`.
After that we try several deterministic context dependent heuristics like "expected value, found struct, you probably forgot `{}`".
If nothing of the above works we try to find candidates with other names using Levenshtein distance.

---

Some alternatives/notes/unresolved questions:
- ~~I had a strong desire to migrate all affected tests to `test/ui`, diagnostics comparison becomes much more meaningful, but I did this only for few tests so far.~~ (Done)
- ~~Labels for "unresolved path" errors are mostly useless now, it may make sense to move some help/notes to these labels, help becomes closer to the error this way.~~ (Done)
- ~~Currently only the first successful heuristic results in additional message shown to the user, it may make sense to print them all, they are rarely compatible, so the diagnostics bloat is unlikely.~~ (Done)
- Now when #38014 landed `resolve_path` can potentially be replaced with `smart_resolve_path` in couple more places - e.g. ~~visibilities~~ (done), ~~import prefixes~~ (done), HIR paths.

---

Some additional fixes:
- Associated suggestions and typo suggestions are filtered with a context specific predicate to avoid inapplicable suggestions.
- `adjust_local_def` works properly in speculative resolution.
- I also fixed a recently introduced ICE in partially resolved UFCS paths (see test `ufcs-partially-resolved.rs`).     Minimal reproduction:
    ```
    enum E {}
    fn main() {
        <u8 as E>::A;
    }
    ```
    Fixes #38409, fixes #38504 (duplicates).
- Some bugs in resolution of visibilities are fixed - `pub(Enum)`, `pub(Trait)`, `pub(non::local::path)`.
- Fixes #38012.
---

r? @jseyfried for technical details + @jonathandturner  for diagnostics changes
How to read the patch: `smart_resolve_path(_fragment)/resolve_qpath_anywhere` are written anew and replace `resolve_trait_reference`/`resolve_type`/`resolve_pattern_path`/`resolve_struct_path`/`resolve_expr` for `ExprKind::Path`, everything else can be read as a diff.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jseyfried jseyfried

Labels
A-resolve Area: Name resolution
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

More systematic error reporting in path resolution petrochenkov/rust
2 participants
@sanxiyn @jseyfried