Skip to content

Fixed type shadowing error#2967

Merged
garyb merged 4 commits intopurescript:masterfrom
LightAndLight:issue-2197
Sep 23, 2017
Merged

Fixed type shadowing error#2967
garyb merged 4 commits intopurescript:masterfrom
LightAndLight:issue-2197

Conversation

@LightAndLight
Copy link
Copy Markdown
Contributor

@LightAndLight LightAndLight commented Jul 4, 2017

Fixes #2197

Prim was being imported unqualified to every single module. I changed addDefaultImport to only add an unqualified import if the module-to-import has not already been imported qualified. I think this is in line with the behaviour that the person who raised the issue expected.

@LightAndLight
Copy link
Copy Markdown
Contributor Author

LightAndLight commented Jul 4, 2017

Note for later:

This hasn't accounted for a case like the following:

module Main where

import Prim (Number)
...

I think in this case only number should be imported from Prim. Meaning: a default module should only be imported unqualified if it is never imported, qualified or unqualified.

@paf31 paf31 requested a review from garyb July 4, 2017 16:43
@garyb
Copy link
Copy Markdown
Member

garyb commented Jul 4, 2017

Can you add a test to see what happens when you add type Blah = {} with a qualified import of Prim? Pretty sure it won't compile, we can't make this change until I do something else with the names that I have as a WIP. See #2951.

@LightAndLight
Copy link
Copy Markdown
Contributor Author

LightAndLight commented Jul 4, 2017

I will get back to you in a few hours, but if {} is represented as Prim.Record then it should compile fine. The behaviour of addDefaultImport for qualified imports hasn't changed.

@paf31
Copy link
Copy Markdown
Contributor

paf31 commented Sep 10, 2017

@garyb Could you please review this again?

@garyb
Copy link
Copy Markdown
Member

garyb commented Sep 22, 2017

Sorry about the delay on this, I'll definitely take a look this weekend. I'm sure it is fine, I just want to better understand for myself what's going on too 😄

@garyb garyb merged commit 48590ca into purescript:master Sep 23, 2017
paf31 pushed a commit that referenced this pull request Nov 8, 2017
@LightAndLight @paf31
Fixed type shadowing error (#2967)
c4577de 
* Fix and regression tests for #2197

* Updated Prim module documentation

* Added test using Record type

* Added some peace-of-mind tests
@joneshf joneshf mentioned this pull request Dec 20, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@garyb garyb garyb approved these changes

Assignees

No one assigned

Labels

status: ready for review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@LightAndLight @garyb @paf31