Incorrect error message when incorrectly implementing interface

[hachiko-8ko]

TypeScript Version: 2.7.2

Search Terms: error message, language, incorrectly implements interface, not assignable to type

Code

// The following code contains errors, by design.
class x {
    xx: string[] = [];
    xxx: string = "";
}
class y {
    yy: string[] = [];
}
interface IFoo {
    Bar: x;
}
class Foo implements IFoo {
    Bar: y = new y();
}
class Test {
    fb: IFoo = new Foo();
}

Expected behavior:
The following error should be returned: Class "Foo" incorrectly implements class 'IFoo'. ... etc

Actual behavior:
in class Test, the following error is returned: Type 'Foo' is not assignable to type 'IFoo'. Types of property 'Bar' are incompatible. Type 'y' is not assignable to type 'x'.

This message might be literally correct, but if you're thinking object-oriented, it doesn't fit. Consider the code. This message states that Foo, which implements IFoo, is not assignable to IFoo, which is nonsensical. It is normal to set a interface type to a concrete class that implements the interface, and there should never be an error returned for doing so.

The actual problem here is that Foo is not correctly implementing the interface. Bar has the wrong type. In this specific example, you do get helpful error on Bar, but still no message stating that Foo incorrect implements IFoo. Also, in a more complex case, where each definition is in a different file, the message in Test is the only one you see.

I noticed this when I had just modified an interface to return a different type, and needed to hunt down the implementers to change them. The only errors returned were on places where in constructors of classes that referenced the type, which I found confusing and unhelpful. In C#, the moment an interface is changed to something incompatible, all the implementing classes raise errors, and I can follow the errors to the classes I need to fix.

It seems to me as if the only time you see the "incorrectly implements" message is when a property is missing. If the type exists but is incompatible (wrong return type, wrong arguments, etc), I think it should also count as incorrectly implementing it.

[RyanCavanaugh]

By design, implements is only a "Is this right?" check on the declaring class and has no other effects. It doesn't affect the class shape itself; if a class does not correctly implement the interface, then you will certainly see the same error at a later site.

The presence of any really does make this behavior desirable; the argument that we should "believe" heritage clauses implies we should not error on the attempted incorrect assignment below because B and C both claim to be As, even though it's plainly wrong.

interface A {
  x: string;
}
interface B extends A {
  x: any;
}
interface C extends B {
  x: number;
}

var c: C = { x: 10 };
var b: A = c; // ??

[hachiko-8ko]

I see what you are trying to do. But there are a few things.

• In your example, there is no explicit class implementation. It is not the exact same case, only a related one. Though the syntax is similar, overriding is different from implementing.
• Since your example does not include an explicit class implementation, the point where A = c is the analogous place in the logic. It is the place where the JS code logic joins with the TS type definition. Using this analog, then if "b: A = c" should return an error, then this should also return an error for the same reason:

class D implements C {
  x: string
}

• Erroring on the assignment does not preclude also erroring on the implementation (though it does fill the log up with a bunch of errors). My issue is not so much the inclusion of the error on the assignment, but the error on the assignment instead of the error on the implementation. If the implementation is not right, which is the case for class D, it should fail.

[jcalz]

@hachiko-8ko I don't understand this report. When I enter your code, I get two error messages, and the first one is right inside the implementation of Foo as you expect:

class Foo implements IFoo {
    Bar: y = new y(); // error!
//  ^^^  
//  Property 'Bar' in type 'Foo' is not assignable to the same property in base type 'IFoo'.
//   Type 'y' is not assignable to type 'x'.
//     Property 'xx' is missing in type 'y'.
}

(Playground Link)

Those are not literally the words "Class Foo incorrectly implements IFoo" but it's the same idea and it's telling you exactly what you need to fix and where. What am I missing?

[hachiko-8ko]

Those are not literally the words "Class Foo incorrectly implements IFoo" but it's the same idea and it's telling you exactly what you need to fix and where. What am I missing?

Well, if the class incorrectly implements the interface, I'd like to expect to see an error saying so, especially because the compiler has set a precedent of showing just such a message in at least one case.

And it can be difficult to find the correct issue when your error log is filled with identical messages. Right now I changed one method to return a less derived type (a method that is used only once), and there are 10 error messages, all nearly identical, stating that something is not assignable to something. 9 of the 10 are unrelated, which makes it difficult to find the actual issue. As contrast, in the world of C#, the one error is in the relevant class and easy to find (as long as external references to the interface are consistent with its definition, they're happy).

I've been doing a lot of refactoring over the last month, and I've spent hours fighting with basically this issue. Anything that makes the error list more useful would be a godsend.

For now, I plan to add a line like the following after each class declaration:

function implementTest() {
    const test : IFoo = new Foo();
}

An error on that line is pretty obvious. It's not perfect, though. It's boilerplate for one, and not every class has a simple constructor.

[jcalz]

So, if the above error had actually been:

class Foo implements IFoo {
    Bar: y = new y(); // error!
//  ^^^  
//  Class 'Foo' incorrectly implements interface 'IFoo'.
//    Property 'Bar' in type 'Foo' is not assignable to the same property in base type 'IFoo'.
//      Type 'y' is not assignable to type 'x'.
//        Property 'xx' is missing in type 'y'.
}

you would be happy? That sounds reasonable to me, but not something I would glean from reading the issue as reported.

[hachiko-8ko]

Yes, I think that would be fine. That is what I was expecting in my "Expected Behavior" section.

In the ideal case, the error on "fb: IFoo = new Foo();" would be surpressed as well. It seems from Ryan's answer that that can't be done with the way that the check is implemented, but it's very unexpected to see an error casting a class as the interface it implements.

But even if the second isn't possible, the first should be easy to implement. Once people learn that the first error is responsible for many of the second, they can learn to attack it first.

[typescript-bot]

Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed.