Skip to content

Unused opens false positive: extension method is preferred to instance method #5306

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

Open
auduchinok opened this issue Jul 7, 2018 · 4 comments
Labels
Area-LangService-UnusedOpens FCS and VS support for unused opens Bug Impact-Low (Internal MS Team use only) Describes an issue with limited impact on existing code.
Milestone

Comments

@auduchinok
Copy link
Member

auduchinok commented Jul 7, 2018

During checking open ... statements, extension method is chosen instead of instance one, which differs from actual name resolution during the type check phase.
A guess before looking into: It may be another getting declaring vs apparent entity bug.

using System;

namespace Ns1
{
    public class Extended
    {
        public void M(Action<string> a)
        {
        }
    }
}

namespace Ns2
{
    public static class Extension
    {
        public static void M(this  Ns1.Extended b, Action<string> a)
        {
        }
    }
}
namespace ClassLibrary13

open Ns1
open Ns2

type T() =
    member x.Foo(s: string) = ()

    member x.Bar() =
        Extended().M(x.Foo)
        Extended().M(fun s -> x.Foo(s))

screen shot 2018-07-08 at 00 30 22

screen shot 2018-07-08 at 00 30 27

@vasily-kirichenko
Copy link
Contributor

I cannot run Unused Open analyser tests in any way (VS, cmd, Rider), see #5308, hence no fix for this.

@vasily-kirichenko
Copy link
Contributor

vasily-kirichenko commented Jul 9, 2018

I think the bug is in the type checker, because the generated IL contains calls to the class method, not the extension one.

image

@vasily-kirichenko
Copy link
Contributor

If the method is cast to Action'1, Ns2 is not needed to be opened:

image

@cartermp cartermp added this to the 16.0 milestone Aug 29, 2018
@cartermp cartermp modified the milestones: 16.0, 16.1 Feb 21, 2019
@cartermp cartermp modified the milestones: 16.1, 16.2 Apr 23, 2019
@cartermp cartermp modified the milestones: 16.2, Backlog Apr 30, 2019
@TysonMN
Copy link

TysonMN commented Aug 12, 2020

My impression is that this issue is more serious than "just" an unused open. I will restate the example code and explain how I think about it.

This code compiles and the reference to M resolves to the instance method.

using System;
namespace Ns1 {
  public class Extended {
    public void M(Action<string> _) { }
  }
}
namespace Ns2 {
  public static class ExtensionMethods {
    public static void M(this Ns1.Extended a, Action<string> b) { }
  }
}
module Top
open Ns1
open Ns2 // marked as an unused open
type T() =
  member _.Foo(_: string) = ()
  member x.Bar() =
    Extended().M(x.Foo) // M resolves to the instance method with that name

I expect to be able to rename the instance method. I manually rename the definition and the one reference to it. However, the resulting code doesn't compile.

using System;
namespace Ns1 {
  public class Extended {
    public void M1(Action<string> _) { }
  }
}
namespace Ns2 {
  public static class ExtensionMethods {
    public static void M(this Ns1.Extended a, Action<string> b) { }
  }
}
module Top
open Ns1
open Ns2
type T() =
  member _.Foo(_: string) = ()
  member x.Bar() =
    Extended().M1(x.Foo) // FS0002 This function takes too many arguments, or is used in a context where a function is not expected

@dsyme dsyme added the Impact-Low (Internal MS Team use only) Describes an issue with limited impact on existing code. label Aug 26, 2020
@dsyme dsyme added Area-LangService-UnusedOpens FCS and VS support for unused opens and removed Area-LangService-API labels Apr 5, 2022
@vzarytovskii vzarytovskii moved this to Not Planned in F# Compiler and Tooling Jun 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-LangService-UnusedOpens FCS and VS support for unused opens Bug Impact-Low (Internal MS Team use only) Describes an issue with limited impact on existing code.
Projects
Status: New
Development

No branches or pull requests

6 participants