Skip to content
This repository was archived by the owner on Dec 29, 2022. It is now read-only.

Rename alias changes original import #818

Closed
5 tasks done
glmdgrielson opened this issue Apr 8, 2018 · 6 comments
Closed
5 tasks done

Rename alias changes original import #818

glmdgrielson opened this issue Apr 8, 2018 · 6 comments
Labels
Milestone

Comments

@glmdgrielson
Copy link

glmdgrielson commented Apr 8, 2018

Given

// blah.rs
pub struct Parser<P>(pub P);
// lib.rs
use blah::Parser as ParserTrait;

Renaming ParserTrait to say Combo gives

// blah.rs
pub struct Combo<P>(pub P);
// lib.rs
use blah::Combo as Combo;

Compare that to

// blah.ts
export const blah = () => true;
export const what = () => false;
// index.ts
import { what as why } from "./blah.ts";

which turns into

export const blah = () => true;
export const what = () => false;
// index.ts
import { what as how } from "./blah.ts";

This is kind of annoying. What gives? (Also, feel free to make the title more descriptive)

EDIT: Checklist!:

@nrc nrc added the bug label Apr 9, 2018
@nrc
Copy link
Member

nrc commented Apr 9, 2018

The Rust compiler loses knowledge of aliasing very early in compilation so by the later stages (and in the RLS) we only know that uses of ParserTrait refer to the definition Parser. A rename therefore ends up renaming both 'sides' of the alias.

We should not do this, we'll need to detect it in the RLS, but I'm not sure how to do that.

@nrc nrc added this to the 1.0 milestone Apr 9, 2018
@glmdgrielson
Copy link
Author

You know, I don't think relying on the compiler is such a good idea and this is a reason why. And what if I did this with an external crate? What actually requires the compiler and what could be done with just static analysis?

@nrc
Copy link
Member

nrc commented May 2, 2018

OK, so this is a bit trickier than expected. Tracking the bits of work:

@nrc
Copy link
Member

nrc commented May 16, 2018

Most of this is done now (with rust-lang/rust#50795 merging).

Outstanding work:

  • add a test to rls-analysis
  • check that this works in the RLS (waiting on a nightly Rust release before I can test)

@nrc
Copy link
Member

nrc commented May 17, 2018

This sort of works in the RLS: we block renaming the aliased import, but we seem to block renaming all functions too.

nrc added a commit to rust-dev-tools/rls-analysis that referenced this issue May 17, 2018
@nrc
Copy link
Member

nrc commented May 17, 2018

Tested and working and added a test to rls-analysis.

@nrc nrc closed this as completed May 17, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants