Suppose we use an external library that defines class C
Then we write a very simple extension function fun C.foo = println("extension")
Much later on, a new version of the library comes out which defines class C { fun foo = println("library") }
The evolution of the library clearly broke my code.
This is normal considering how extensions work,
Still, the compiler and IDE only show warnings, and let you compile something that most of the time won’t work as originally intended.
From my limited experience i can already foresee developpers being careless about warnings or tests and letting this mistake pass through in production with potentially dramatic consequences.
Is there any reason a shadowed extension is just a warning and not an error ?
I can’t see any situation where this would be an intended behaviour, is there any ?
I guess this could be problematic with upcoming libraries like android-ktx - if you would like to add your own method that shadows extension, your only real option would be to remove this library
I’m not sure I understood your point correctly, this seems to be a different situation, where the roles are “reversed” (user is the one adding a method and 3d-party lib is the one making use of an extension) ?
For example if you have a situation where you extend a class that have some extensions implemented somewhere : open class C fun C.foo() = println("base class extension")
If I add a method to a derived class C1 : class C1 : C() { fun foo() = println("member func of derived class") }
Then these calls : val c1 = C1() c1.foo() val c1asC : C = C1() c1asC.foo()
will print : member func of derived class base class extension
This seems legit to me because when you are calling foo on c1asC the method foo does not exists in C. Also you do not break the library that relied on C.foo() as an extension.
On the other hand, if you have the possibility to alter C and make something like : class C { fun foo() = println("new C method") }
Here, you will definitely break the library behaviour since the library rely on C not having a foo() method for its extension to work.
TL;DR : What I complain about is that the extension developper will only get a warning when the class he added his extension to has changed. I expect sloppy developpers not caring about warnings and letting this kind of otherwise difficult to detect errors go into production. Couldn’t we change the default behaviour of the compiler to an error to avoid this ?
I realize we won’t fix the problem of developpers ignoring warnings or not implementing proper testing but should we not try to limit the problem where we can ?
You can add in gradle:
compileKotlin {
kotlinOptions.allWarningsAsErrors = true
}
Please note that extensions are resolved statically, and require explicit import - so you can for example have situation like this (in one file):
class A
fun A.foo
class C : A { fun foo }
class B : A
val asA : A = C()
val asC : C = C()
val b : B = B()
asA.foo() ← will call extension as extensions are resolved statically and asA known type is A which doesn’t have member function foo
asC.foo() ← will call class function (member function always wins)
b.foo() ← will call extension
This is legit, although indeed asC.foo() can be slightly confusing. But if instead of a warning there was an error, you would have problem to call asA.foo or b.foo
I agree that since an extension function that has the same signature of another native function is completely ignored, it should be treated as an error.
What I do to minimize the risk, is starting all my extension functions with “ef”. In your example, it would look like this:
fun C.efFoo = println("extension")
If it becomes an error in the future, I will stop doing it as I really don’t like the aesthetics of my “solution”
asA.foo() or b.foo() calls should not give you any warning nor error since there are not ambiguous.
On the other hand, the asC.foo() call only gives you a warning, which I think should be an error.
This could avoid people being careless and letting wrong code go into production.
(Also I know you can treat all warning as error, the point is to make this specific warning an error by default)
Well, I’m still not sure that it should be an error. Yes, in some situations as mentioned someone may not notice that imported extension will not take effect as class already has function/property with the same name, but if it was error and in one file you use both class C and B (from my example) you will loose option to use extension on class B (as otherwise C overshadowing extension would cause error)
For example - I added extension visible to View (similar thing will be added to Kotlin Android extensions now). But what if I use some custom component (not created by myself) that also added property visible? I wouldn’t be able to use my extension in any file where I have this component, even for other views.
I think it’s similar case to unsafe cast etc - in general it’s recommended to avoid it, it’s good to be warned by compiler, but there are legitimate cases where you need to use it anyhow. In such case you add @SuppressWarning annotation to note that you’re aware of it but can’t find better way to deal at the moment.
I agree with the OP here. When extension call is unambiguous (like asA.foo()), then there shouldn’t be any error nor warning. But when there is more than one match, it should definitely be an error, instead of the “member always wins” rule.
Let me give you a not-so-abstract example. I recently wrote some code that is working with 16-bit words. I often need to check whether a word is even or odd, so I added an extension Int.isOdd(). Sounds harmless, except that in my domain, “odd” is defined as “lowest 10 bits have odd number of set bits” (Integer.bitCount(word & 0x3FF) % 2 != 0 in Java). Now imagine what would happen to my code if a later version of Kotlin standard library adds isOdd() function to Int that simply checks whether a number is odd or not! Fortunately, I quickly realized this potential problem and renamed it to isTmOdd to reduce the possibility of a name clash to virtually zero, but what if I didn’t?
Agree with @stachenov and @vb, that ideally an ambiguous extension should be an error.
However that isn’t so easy to do. Taking @stachenov 's example:
@stachenov writes his code including the custom isOdd.
Compiles and checks it runs fine - it does.
Kotlin is updated adding toOdd to the standard library.
@stachenov 's code is not recompiled but it is run on the new version of Kotlin. There will be no compilation error because the code isn’t recompiled. This would be usual not to recompile old code for every release (e.g. you are using a library and don’t have source code).
Code now fails at runtime - probably silently - producing strange results.
What is needed is a link time check that the call hasn’t become ambiguous. This is probably very hard to do.
It might be simpler to make all extensions final, so that the runtime check can simply be that there are two methods with the same signature.
If you don’t recompile the code, you will never stop using the extension function and start using the member. Members and extension functions don’t have the same signature, because extension functions are implemented using static functions. In fact
fun Int.foo() {}
compiles into
public final class TestFileKt {
public static final void foo(int $receiver) {
}
}
Therefor calls to this function will always call the extension function even if the standard library would extend Int with a new member foo. Even if a library would add an extension function the called function should never change as static function calls use the fully qualified name of the function (I would need to double check the generated byte-code or the jvm docs, but I’m like 95% sure).
So the question is only about how we want this to be handled when we recompile code.
I personally am not sure what I would prefer. I understand the problems outlined in here but then I have not ever seen a real time example where this is actually leading to problems. But then I treat warnings same as I do errors. No warnings in PRs unless there is a corresponding issue to fix it.
I am new to Kotlin and I’m reading the book ‘Kotlin in Action’. (What a great language, I love it, I have never seen such a wonderful thing before. Great work!)
When I read the section about extension functions I immediately had the error scenario described here in mind. I don’t understand why the Kotlin developers makes the decision, that the member function takes precedence if there are both?
I’m on a mixed Java/Kotlin project. Imagine given Scenario:
Java API:
public class Example {
public String doThat(String value) {
return "that was done internal " + value;
}
}
public class OtherJava {
public void foo(Example example) {
ExampleExtensionsKt.doThat(example, "2");
}
}
Kotlin:
fun Example.doThat(value: String) = "that was done external with $value"
fun useExample() {
val example = Example()
println(example.doThat("1"))
println(OtherJava().foo(example))
}
Output:
that was done internal 1
that was done external with 2
'doThat ’ was added to the used Java Library. Dependencies have been updated and the previously implemented extension function isn’t used anymore. The application works slightly different. Different in the the Kotlin part. The Java part is still using the extension function. I think the potential for error is enormous. The simple change in the java api has become a breaking change, but nobody of the api developers nor the own developers would expect that.
Wouldn’t it be better if the extension function takes precedence over the member function?
Everything would still work as expected, or am I seeing this wrong?
If I am wrong for a reason I can’t imagine yet, I’m with op and the others who think that this shouldn’t be a warning but a compile error. So it can not be overlooked and become a bug in production.
I think the best way to solve this would be to allow a specific syntax to enforce usage of extension functions e.g. a simple colon foo:doIt(). All extension function calls with current syntax should result in a warning, because of the risk of being shadowed.
