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

Module and partially qualified namespace type shadowing interaction #13535

Open
NinoFloris opened this issue Jul 20, 2022 · 2 comments
Open

Module and partially qualified namespace type shadowing interaction #13535

NinoFloris opened this issue Jul 20, 2022 · 2 comments
Labels
Area-Compiler-Service Various compiler service issues which do not belong to other labels/areas. Feature Request Needs-design
Milestone

Comments

@NinoFloris
Copy link
Contributor

NinoFloris commented Jul 20, 2022

We shadow quite some things in FSharp.Core, either to poison them (compiler message to point to an alternative) or replace them with our own implementations. This is not really uncommon and a good open source example is the Prelude in Dark https://github.com/darklang/dark/blob/main/fsharp-backend/src/Prelude/Prelude.fs

In our case, probably less common, we're also aliasing Option to ValueOption, and precisely around shadowed types and aliases is where things break down a bit.

Reading the following example I would expect only the inner module 'Core' to be opened. Instead F# also opens the namespace Core (from Microsoft.FSharp.Core) and loads its types and aliases again :/

namespace Repro

module Common = 
    type Option<'T> = ValueOption<'T>

module SomeModule = 

    open Common

    module Core = 
        let maybeTest (x: Option<string>) = ()

    open Core
    // Have to re-open Common which Rider doesn't even see as a used import.
    //open Common

    // This expression was expected to have type 'Repro.Common.Option<string>' but here has type 'Microsoft.FSharp.Core.Option<string>'
    let maybeSomething (x: Option<string>) = maybeTest x

https://sharplab.io/#v2:DYLgZgzgNAJiDUAfAdgQwLYFMIAdUGNMACAJUxwCcB7AWACh70qYBXYYgYSvSeSIF4i9IiKIAXAJ45iAeRxiAllWQAeAOQAVAHwCiANVTAWmOYuXrt9RszbEAyt0wBZG+11W6oolWl8uPZQ8vJlY3LgpiQWEvUXYxInRUCQAjTA1seIAKAA8QIlMlVQgxCgVkAHMtAEpdTKqg0R9MPyoI6JEAeg6iAAlUADdiMSoiCIBaJpaAvgB3AAsFfDmiTBgFYYoIIhhlNXjMQb4ITGJULdQiFmOYIgV0HFaxADp2oi7Jon9eBs7ujQWtphspRsBBCkQZmcVsDMPgxKtxCM5gMhlJiGoyJQqE8vsongVzMVShUtGoiMkWPE5pgIkRkVtJNIiGonItqBAqGBngAxOzIig4HGtTD4+SFFREsqVNSvOIJJKpBxYMQLCpEHJ5AlFEpS6q6RIpNIZIjZIA===

Interestingly, this behavior only seems to be triggered without error if there is a shadowing module in scope:

namespace Sample

module SomeModule = 

    // error FS0893: This declaration opens the namespace or module 'Microsoft.FSharp.Core'
    // through a partially qualified path. Adjust this code to use the full path of the namespace.
    // This change will make your code more robust as new constructs are added to the F# and CLI libraries.
    open Core

    module Core  = 
        let test() = ()

    // Allowed because new module Core is in scope but additionally opens Microsoft.FSharp.Core through a partially qualified path without error.
    open Core

Given that partially qualified opening of namespaces:

  • Has produced an error since F# 1.0.
  • Can only be triggered via an edge case.
  • The implications of an open like this also doesn't flow through FCS or other tooling properly, given this re-import is seen as unused in Rider (can somebody try the repro in VS?).

Could we remove the behavior of re-opening namespaces via a partially qualified path entirely @dsyme?

@NinoFloris NinoFloris added the Bug label Jul 20, 2022
@vzarytovskii vzarytovskii modified the milestones: September-2022, Backlog Sep 9, 2022
@vzarytovskii vzarytovskii added Area-Compiler Compiler-related issues which don't belong to other categories Area-Compiler-Service Various compiler service issues which do not belong to other labels/areas. and removed Area-Compiler Compiler-related issues which don't belong to other categories labels Sep 21, 2022
@dsyme
Copy link
Contributor

dsyme commented Sep 21, 2022

Interesting problem, thanks. Yes we should at least emit a warning when this happens, and consider how we can allow explicit qualification so FSharp.Core doesn't come into scope.

@T-Gro
Copy link
Member

T-Gro commented Nov 14, 2022

(probably a broader issue about type direction, also related to tuples, collections,...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compiler-Service Various compiler service issues which do not belong to other labels/areas. Feature Request Needs-design
Projects
Status: New
Development

No branches or pull requests

4 participants