Skip to content

Give more helpful error when trying to set default values on an interface. #5736

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

Merged
merged 8 commits into from
Dec 1, 2015
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,7 @@ namespace ts {
}

// Because of module/namespace merging, a module's exports are in scope,
// yet we never want to treat an export specifier as putting a member in scope.
// yet we never want to treat an export specifier as putting a member in scope.
// Therefore, if the name we find is purely an export specifier, it is not actually considered in scope.
// Two things to note about this:
// 1. We have to check this without calling getSymbol. The problem with calling getSymbol
Expand Down Expand Up @@ -11398,7 +11398,7 @@ namespace ts {
// we can get here in two cases
// 1. mixed static and instance class members
// 2. something with the same name was defined before the set of overloads that prevents them from merging
// here we'll report error only for the first case since for second we should already report error in binder
// here we'll report error only for the first case since for second we should already report error in binder
if (reportError) {
const diagnostic = node.flags & NodeFlags.Static ? Diagnostics.Function_overload_must_be_static : Diagnostics.Function_overload_must_not_be_static;
error(errorNode, diagnostic);
Expand Down Expand Up @@ -12065,8 +12065,8 @@ namespace ts {
const symbol = getSymbolOfNode(node);
const localSymbol = node.localSymbol || symbol;

// Since the javascript won't do semantic analysis like typescript,
// if the javascript file comes before the typescript file and both contain same name functions,
// Since the javascript won't do semantic analysis like typescript,
// if the javascript file comes before the typescript file and both contain same name functions,
// checkFunctionOrConstructorSymbol wouldn't be called if we didnt ignore javascript function.
const firstDeclaration = forEach(localSymbol.declarations,
// Get first non javascript function declaration
Expand Down Expand Up @@ -16227,11 +16227,17 @@ namespace ts {
if (checkGrammarForNonSymbolComputedProperty(node.name, Diagnostics.A_computed_property_name_in_an_interface_must_directly_refer_to_a_built_in_symbol)) {
return true;
}
if (node.initializer) {
return grammarErrorOnNode(node.initializer, Diagnostics.An_interface_property_cannot_have_an_initializer);
}
}
else if (node.parent.kind === SyntaxKind.TypeLiteral) {
if (checkGrammarForNonSymbolComputedProperty(node.name, Diagnostics.A_computed_property_name_in_a_type_literal_must_directly_refer_to_a_built_in_symbol)) {
return true;
}
if (node.initializer) {
return grammarErrorOnNode(node.initializer, Diagnostics.An_object_type_literal_property_cannot_have_an_initializer);
}
}

if (isInAmbientContext(node) && node.initializer) {
Expand Down
10 changes: 9 additions & 1 deletion src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -783,6 +783,14 @@
"category": "Error",
"code": 1245
},
"An interface property cannot have an initializer.": {
"category": "Error",
"code": 1246
},
"An object type literal property cannot have an initializer.": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I didn't notice it earlier, but this should be "A type literal property"

"category": "Error",
"code": 1247
},

"'with' statements are not allowed in an async function block.": {
"category": "Error",
Expand Down Expand Up @@ -2418,7 +2426,7 @@
"Not all code paths return a value.": {
"category": "Error",
"code": 7030
},
},
"You cannot rename this element.": {
"category": "Error",
"code": 8000
Expand Down
6 changes: 6 additions & 0 deletions src/compiler/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2247,6 +2247,12 @@ namespace ts {
property.name = name;
property.questionToken = questionToken;
property.type = parseTypeAnnotation();

// Although object type properties cannot not have initializers, we attempt to parse an initializer
// so we can report in the checker that an interface property or object type literal property cannot
// have an initializer.
property.initializer = parseNonParameterInitializer();

parseTypeMemberSemicolon();
return finishNode(property);
}
Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,7 @@ namespace ts {
name: PropertyName; // Declared property name
questionToken?: Node; // Present on optional property
type?: TypeNode; // Optional type annotation
initializer?: Expression; // Optional initializer
}

// @kind(SyntaxKind.PropertyDeclaration)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
tests/cases/compiler/errorOnInitializerInInterfaceProperty.ts(2,19): error TS1246: An interface property cannot have an initializer.


==== tests/cases/compiler/errorOnInitializerInInterfaceProperty.ts (1 errors) ====
interface Foo {
bar: number = 5;
~
!!! error TS1246: An interface property cannot have an initializer.
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
//// [errorOnInitializerInInterfaceProperty.ts]
interface Foo {
bar: number = 5;
}


//// [errorOnInitializerInInterfaceProperty.js]
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
tests/cases/compiler/errorOnInitializerInObjectTypeLiteralProperty.ts(2,19): error TS1247: An object type literal property cannot have an initializer.
tests/cases/compiler/errorOnInitializerInObjectTypeLiteralProperty.ts(6,19): error TS1247: An object type literal property cannot have an initializer.


==== tests/cases/compiler/errorOnInitializerInObjectTypeLiteralProperty.ts (2 errors) ====
var Foo: {
bar: number = 5;
~
!!! error TS1247: An object type literal property cannot have an initializer.
};

let Bar: {
bar: number = 5;
~
!!! error TS1247: An object type literal property cannot have an initializer.
};

Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
//// [errorOnInitializerInObjectTypeLiteralProperty.ts]
var Foo: {
bar: number = 5;
};

let Bar: {
bar: number = 5;
};


//// [errorOnInitializerInObjectTypeLiteralProperty.js]
var Foo;
var Bar;
10 changes: 8 additions & 2 deletions tests/baselines/reference/objectTypeLiteralSyntax2.errors.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
tests/cases/conformance/types/objectTypeLiteral/objectTypeLiteralSyntax2.ts(12,22): error TS1005: ';' expected.
tests/cases/conformance/types/objectTypeLiteral/objectTypeLiteralSyntax2.ts(12,22): error TS1005: '=' expected.
tests/cases/conformance/types/objectTypeLiteral/objectTypeLiteralSyntax2.ts(12,22): error TS2304: Cannot find name 'bar'.
tests/cases/conformance/types/objectTypeLiteral/objectTypeLiteralSyntax2.ts(12,25): error TS1005: ';' expected.


==== tests/cases/conformance/types/objectTypeLiteral/objectTypeLiteralSyntax2.ts (1 errors) ====
==== tests/cases/conformance/types/objectTypeLiteral/objectTypeLiteralSyntax2.ts (3 errors) ====
var x: {
foo: string,
bar: string
Expand All @@ -15,4 +17,8 @@ tests/cases/conformance/types/objectTypeLiteral/objectTypeLiteralSyntax2.ts(12,2

var z: { foo: string bar: string }
~~~
!!! error TS1005: '=' expected.
~~~
!!! error TS2304: Cannot find name 'bar'.
~
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if we should accept these errors, as the '=' expected message implies that initializers are allowed in this case. Maybe we should spin this off into a new issue. @DanielRosenwasser @CyrusNajmabadi, what are your thoughts on this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be sufficient if you make it so that you only try to parse an initializer if the current token is a =.

!!! error TS1005: ';' expected.
3 changes: 3 additions & 0 deletions tests/cases/compiler/errorOnInitializerInInterfaceProperty.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
interface Foo {
bar: number = 5;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
var Foo: {
bar: number = 5;
};

let Bar: {
bar: number = 5;
};