Static using makes extension method promotion a breaking change

[TylerBrinkley]

As an example, let's say Microsoft would like to promote the method Math.Abs to be an extension method so that users can use extension method syntax to make calls to this method like so.

int value = -42;
int absValue = value.Abs();

Extension methods are just syntactic sugar allowing another syntax for making calls to the method so promoting it shouldn't be a breaking change, right? Wrong, with the introduction of C#6's static using statements it is now a breaking change to promote a static method to an extension method. Consider the following perfectly valid code.

using static System.Math;

class Program
{
    static void Main()
    {
        int value = -42;
        Abs(value);
    }
}

If Math.Abs were then to be promoted to be an extension method the above code will no longer compile. The user will get a squiggly under Abs saying "The name 'Abs' does not exist in the current context". The problem is that the static using statement doesn't allow you to use extension methods like you can with standard static methods.

To stop making extension method promotion a breaking change I think C# should be updated to allow consuming extension methods with the same syntax as standard static methods when using a static using statement.

The sooner this is corrected the less impact it will have in the future. Perhaps even the C# language design team will consider it an implementation bug and enable this syntax in future Roslyn updates when targeting C# 6.0 - 7.0. That would just make it a breaking change for users of older Roslyn versions.

[lachbaer]

If Math.Abs then gets promoted to be an extension method

Why would one do that in the same namespace? If the method was already published a signature change, like the one necessary for an extension method, is a no go anyway. Also a new, seperate namespace (or static class) for the extension methods would probably be introduced. Nonetheless, I don't see a reason why extension methods shouldn't be called explicitly with an according instance as the first parameter.

[tannergooding]

@lachbaer I think this is more of an example.

Imagine you have a class which you created prior to extension methods and you called it MyTypeExtensions, in that class you have a method void Extension(MyType type, Blah blah). Now that extension methods exist, you want to go and change things so that users can do myType.Extension(blah) instead of having to do MyTypeExtensions.Extension(myType, blah). This just involves adding a this into the signature so you get void Extension(this MyType type, Blah blah).

However, some users have code where they had using static MyTypeExtensions. Due to the issue above, making a method an extension method causes code to break if you try to recompile.

[alrz]

I wonder what was the original rationale behind this (excluding extensions from using static).

[alrz]

It's not documented in the source either. http://source.roslyn.io/#Microsoft.CodeAnalysis.CSharp/Binder/Imports.cs,719

[jnm2]

@TylerBrinkley

To stop making extension method promotion a breaking change I think C# should be updated to allow consuming extension methods with the same syntax as standard static methods when using a static using statement.

Even if this was accomplished in C# 7.1, you still couldn't turn your methods into extension methods without breaking everyone exactly this way who is using C# 6.0 - 7.0. (There are plenty of OSS projects that use <LangVersion>6</LangVersion; I just ran into one with 5.)

It will forever be a breaking change unless you explicitly drop support of C# pre-7.1.

[TylerBrinkley]

@jnm2 you're right, but the sooner this is corrected the less impact it will have in the future. Perhaps even the C# language design team will consider it an implementation bug and enable this syntax in future Roslyn updates when targeting C# 6.0 - 7.0. That would just make it a breaking change for users of older Roslyn versions.

[jnm2]

Yes. I'm very interested to see the team's take on this.

[HaloFour]

Not including extension methods was an explicit design choice. I don't know the further details but it is mentioned here: New Features in C# 6

Extension methods will not be imported into top-level scope; instead they will show up as extension methods. This gives a way to import extension methods from just a single class, something which wasn’t possible before.

[ike86]

This gives a way to import extension methods from just a single class, something which wasn’t possible before.

I understand this part as the decision was deliberate
and the reason was to introduce a new feature - import extension methods from just a single class -
which would be broken by this change request,
as it would require to change the semantics of using static in case of a class containing extension methods.

But why would it break it?.. 🤔

[tannergooding]

I think this is worth modifying so that extension methods are both imported and made to show up as extensions.

Otherwise you have this issue (promoting to extension method is breaking), when it shouldn't be, since the only change in the emitted IL is an attribute (which isn't normally a breaking change since the method signature didn't change):

// Before
.method public hidebysig static void  Extension(class MyType myType) cil managed
{
}

// After
.method public hidebysig static void  Extension(class MyType myType) cil managed
{
  .custom instance void [System.Runtime]System.Runtime.CompilerServices.ExtensionAttribute::.ctor() = ( 01 00 00 00 ) 
}

[alrz]

@HaloFour

Sure, but it still doesnt explain why it's not permitted to have both.

You should be able add this modifier without breaking source​-compatibility, and maybe an analyzer could suggest to use extension method syntax afterwards.

[jnm2]

@tannergooding The part I'm interested in is whether this would be backported to 6.0 thru 7.0 language levels in a new Roslyn release or not. If not, then fixing this in 7.1 won't alleviate the fact that promoting a method to an extension method is still going to be breaking for any consumer who sets LangVersion 6 or 7 long term, which is a common practice in OSS projects.

[CyrusNajmabadi]

This is definitely an interesting issue. Tagging @jaredpar .

I'm not sure what our take will be on this given that we've already shipped the break. And, afaict, we haven't heard about any actual issues in practice. Will we just accept that making such an API change is breaking, but happens rarely enough to not matter? Or will we want to fix this, but accept that it will stay broken for C# 6.0-7.0 customers.

--

I'm leaning toward thinking that the breaking behavior has already shipped, and it hasn't been a problem, so we can just accept the new behavior and document the issue as such. But i'm amenable to other opinions here.

[HaloFour]

@alrz

Sure, but it still doesnt explain why it's not permitted to have both.

I agree. It only hints that it was deliberate and not an oversight.

@CyrusNajmabadi

As mentioned above this behavior seems like it was very intentional. I think it would help to understand what went into that decision. I couldn't find any discussions on Codeplex about it, only the cited blog post by Mads.

[tannergooding]

@CyrusNajmabadi, I might be wrong, but I don't think the compiler fixing it would be a breaking change.

Today, the compiler just doesn't make these functions available in the static using. With this fix, the functions would become available. Since promoting to an extension method doesn't actually change the method signature, there isn't risk of a FQN of the method conflicts with any existing FQN usage.

There is probably some overload resolution break I am missing though 😄

[CyrusNajmabadi]

I might be wrong, but I don't think the compiler fixing it would be a breaking change.

I don't think it would be breaking either. My point is that the ship has sailed. We've had two major releases with the new behavior. So 'fixing' anything here won't be helpful as the issue will still manifest for the huge number of people that will be on 6 or 7 for years to come.

[tannergooding]

@alrz: Figured it would be overload resolution, its always overload resolution 😈

[TylerBrinkley]

And, afaict, we haven't heard about any actual issues in practice.

@CyrusNajmabadi probably pretty insignificant but I reverted promoting a method in my library Enums.NET because I realized this would break users code.

More significantly though is that this issue may prevent any future potential extension method promotions in the BCL.

[TylerBrinkley]

I chose Math.Abs as an example because Math is a static class but consider if Permit declaring an extension method in a non-static class, which needs to be moved to this repo, gets implemented. Then it's not too hard to imagine that the BCL team might want to promote String.IsNullOrEmpty or Char.IsDigit to be extension methods.

[alrz]

String.IsNullOrEmpty or Char.IsDigit could be actually "extension properties" so once we have "extension everything" we could redeclare them as such.

[TylerBrinkley]

Okay, how about String.Format?

[alrz]

VB has a concept of type promotion for modules, if we apply that to nested extention classes, I think we could use those methods as extentions.

class String {
  extension String { // maybe even omit String a la "companion objects" in Kotlin
    public string Format(...) { ... }
  }
}
// both should be valid
String.Format(..)
str.Format(..)

This all depends on how "extension everything" is implemented to also retain binary-compatibility.

[HaloFour]

@alrz

It will break this code

That depends. If the compiler didn't consider extension methods until after exhausting other static methods that wouldn't be a breaking change. Although it could be confusing.

[alrz]

@HaloFour

Yes, but imagine you promote Y.M to an extension method. There is no way for the compiler to distinguish between the two.

[CyrusNajmabadi]

More significantly though is that this issue may prevent any future potential extension method promotions in the BCL.

Sounds like that ship has already sailed. We have already shipped C# 6.0 and 7.0. I don't think the BCL would do such a thing if it would mean that all those customers would/could break.

[HaloFour]

@alrz

True, it would have to be a compiler error. Just as it would be if they were both extension methods and called as such. Or if they both weren't extension methods but imported into scope via using static.

[alrz]

@HaloFour

The question is whether it is a breaking change to promote a static method to an extension method. If we permit using static to bring extension methods into the scope (with a precedence for regular methods as you're suggesting) and also allow extension methods in non-static classes (for String.Format example), it would be still a breaking change, because you don't know if client has an extension method with the same name in the scope.

[TylerBrinkley]

@CyrusNajmabadi are you then opposed to backporting support for this in C# 6 & 7 with a Roslyn update?

[HaloFour]

@alrz

Agreed, just throwing spaghetti at the wall. It doesn't sound like there are any good options for changing the behavior. Either way I'd love a comment or two regarding what went into the decision.

@TylerBrinkley

I'm going to assume that since this affects overload resolution and could cause compiler errors due to ambiguous static/extension collisions that it would be unlikely for the behavior to change at all, let alone backported to existing compilers.

[CyrusNajmabadi]

@CyrusNajmabadi are you then opposed to backporting support for this in C# 6 & 7 with a Roslyn update?

I don't see how that would help. As it would only work for customers that updated their compiler.

[yaakov-h]

Which, given enough time, would make this safe-enough for library authors, though probably not the BCL.