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

[Errors] Empty object contextually typed by type with indexer #48

Closed
mhegazy opened this issue Jul 17, 2014 · 11 comments · Fixed by #589
Closed

[Errors] Empty object contextually typed by type with indexer #48

mhegazy opened this issue Jul 17, 2014 · 11 comments · Fixed by #589
Labels
Breaking Change Would introduce errors in existing code Bug A bug in TypeScript Fixed A PR has been merged for this issue Spec Issues related to the TypeScript language specification

Comments

@mhegazy
Copy link
Contributor

mhegazy commented Jul 17, 2014

interface Foo { a }
interface Bar { b }

interface Object {
    [n: number]: Foo;
}

interface Function {
    [n: number]: Bar;
}

var o = {};
var f = () => { };

var v1: {
    //!!! Cannot convert '{}' to '{ [x: number]: Foo; }':
    //!!!   Numeric index signature is missing in type '{}'.
    [n: number]: Foo
} = o;  
var v2: {
    //!!! Cannot convert '() => void' to '{ [x: number]: Bar; }':
    //!!!   Numeric index signature is missing in type '() => void'.
    [n: number]: Bar
} = f;  

Expected: no errors

Actual: errors in the comments above

Verify test case:
tests/cases/compiler/augmentedTypeBracketAccessIndexSignature.ts
tests/cases/conformance/types/members/objectTypeWithCallSignatureHidingMembersOfExtendedFunction.ts

@mhegazy mhegazy added this to the TypeScript 1.1 milestone Jul 17, 2014
@JsonFreeman
Copy link
Contributor

The actual behavior here matches the spec. It appears to be by design.

@JsonFreeman
Copy link
Contributor

Another interesting example where we don't error, but probably should:

var v: { [s: string]: string} = {
p: 3, // Expect an error here
q: null
}

This is a result of best common type losing information, and the nontransitivity of assignability.

@sophiajt
Copy link
Contributor

sophiajt commented Aug 1, 2014

What's the action here? If we're aligned to spec, shouldn't this just be closed as by design?

@JsonFreeman
Copy link
Contributor

As I recall from discussion, the spec needs to be updated to align with the current behavior. Specifically, I think section 3.8.1 needs to not pull in call/construct/index signatures for the augmented types.

And it is a breaking change.

@JsonFreeman JsonFreeman added Spec and removed Bug labels Aug 1, 2014
@sophiajt
Copy link
Contributor

sophiajt commented Aug 1, 2014

I'm confused. You mention earlier that the "actual behavior here matches the spec. It appears to be by design." If so, is there a spec change?

@JsonFreeman
Copy link
Contributor

Yeah my previous comments were wrong because I misunderstood what was going on. Ignore those. It needs a spec change per my recent comment.

@sophiajt
Copy link
Contributor

Before we make this change, please confirm that the change does not impact real world code before we make the spec change.

@JsonFreeman
Copy link
Contributor

Reopening this because the validation on real world code was not done yet.

@JsonFreeman JsonFreeman reopened this Sep 4, 2014
@ahejlsberg
Copy link
Member

Doesn't break any real world code we know of.

@RyanCavanaugh
Copy link
Member

Is this fixed, or by design?

@ahejlsberg
Copy link
Member

Fixed, in that it is a change from the old compiler. Spec has been updated to reflect behavior of the new compiler.

@ahejlsberg ahejlsberg added the Fixed A PR has been merged for this issue label Sep 8, 2014
@microsoft microsoft locked and limited conversation to collaborators Jun 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Breaking Change Would introduce errors in existing code Bug A bug in TypeScript Fixed A PR has been merged for this issue Spec Issues related to the TypeScript language specification
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants