Skip to content

Gracefully handle accessor declarations in ambient classes. #256

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.

Sign up for GitHub

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
DanielRosenwasser merged 1 commit into from
Jul 29, 2014
Merged

Gracefully handle accessor declarations in ambient classes. #256

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4948,7 +4948,7 @@ module ts {
}

checkSourceElement(node.body);
if (node.type) {
if (node.type && !isAccessor(node.kind)) {
Copy link
Copy Markdown
Contributor

@CyrusNajmabadi CyrusNajmabadi Jul 29, 2014

Choose a reason for hiding this comment

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

Don't you still need to do this for setters?

Copy link
Copy Markdown
Member Author

@DanielRosenwasser DanielRosenwasser Jul 29, 2014

Choose a reason for hiding this comment

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

Setters don't need to have their bodies checked - they don't return anything.

checkIfNonVoidFunctionHasReturnExpressionsOrSingleThrowStatment(node, getTypeFromTypeNode(node.type));
}

Expand Down
11 changes: 10 additions & 1 deletion src/compiler/parser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2902,7 +2902,16 @@ module ts {
node.typeParameters = sig.typeParameters;
node.parameters = sig.parameters;
node.type = sig.type;
node.body = parseBody(/* ignoreMissingOpenBrace */ false);

// A common error is to try to declare an accessor in an ambient class.
if (inAmbientContext && canParseSemicolon()) {
Copy link
Copy Markdown
Contributor

@CyrusNajmabadi CyrusNajmabadi Jul 29, 2014

Choose a reason for hiding this comment

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

when do you report the error for an accessor in an ambient context? That should be done in the parser yes (as it's not gramattically legal)?

Copy link
Copy Markdown
Member Author

@DanielRosenwasser DanielRosenwasser Jul 29, 2014

Choose a reason for hiding this comment

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

Yes, check parser.ts, line 2862 (just 45 lines above this).

parseSemicolon();
node.body = createMissingNode();
}
else {
node.body = parseBody(/* ignoreMissingOpenBrace */ false);
}

return finishNode(node);
}

Expand Down
8 changes: 3 additions & 5 deletions tests/baselines/reference/ambientGetters.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
==== tests/cases/compiler/ambientGetters.ts (3 errors) ====
==== tests/cases/compiler/ambientGetters.ts (2 errors) ====

declare class A {
get length() : number;
~
!!! '{' expected.
~~~~~~
!!! A function whose declared type is neither 'void' nor 'any' must return a value or consist of a single 'throw' statement.
~~~~~~
!!! An accessor cannot be declared in an ambient context.
}

declare class B {
Expand Down
4 changes: 1 addition & 3 deletions tests/baselines/reference/gettersAndSettersErrors.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
==== tests/cases/compiler/gettersAndSettersErrors.ts (10 errors) ====
==== tests/cases/compiler/gettersAndSettersErrors.ts (9 errors) ====
class C {
public get Foo() { return "foo";} // ok
~~~
Expand All @@ -16,8 +16,6 @@
public set Goo(v:string):string {} // error - setters must not specify a return type
~~~
!!! Accessors are only available when targeting ECMAScript 5 and higher.
~~~~~~
!!! A function whose declared type is neither 'void' nor 'any' must return a value or consist of a single 'throw' statement.
}

class E {
Expand Down
72 changes: 36 additions & 36 deletions tests/baselines/reference/giant.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,34 +374,34 @@
!!! A function implementation cannot be declared in an ambient context.
public get pgF()
~~~
!!! Accessors are only available when targeting ECMAScript 5 and higher.
~~~
!!! Duplicate identifier 'pgF'.
public psF(param:any) { }
~~~~~~
!!! '{' expected.
~
!!! A function implementation cannot be declared in an ambient context.
public set psF(param:any)
~~~
!!! Accessors are only available when targeting ECMAScript 5 and higher.
~~~
!!! Duplicate identifier 'psF'.
private rgF() { }
~~~~~~~
!!! '{' expected.
~
!!! A function implementation cannot be declared in an ambient context.
private get rgF()
~~~
!!! Accessors are only available when targeting ECMAScript 5 and higher.
~~~
!!! Duplicate identifier 'rgF'.
private rsF(param:any) { }
~~~~~~~
!!! '{' expected.
~
!!! A function implementation cannot be declared in an ambient context.
private set rsF(param:any)
~~~
!!! Accessors are only available when targeting ECMAScript 5 and higher.
~~~
!!! Duplicate identifier 'rsF'.
static tV;
~~~~~~
!!! '{' expected.
static tF() { }
~
!!! A function implementation cannot be declared in an ambient context.
Expand All @@ -410,18 +410,18 @@
!!! A function implementation cannot be declared in an ambient context.
static set tsF(param:any)
~~~
!!! Accessors are only available when targeting ECMAScript 5 and higher.
~~~
!!! Duplicate identifier 'tsF'.
static tgF() { }
~~~~~~
!!! '{' expected.
~
!!! A function implementation cannot be declared in an ambient context.
static get tgF()
~~~
!!! Accessors are only available when targeting ECMAScript 5 and higher.
~~~
!!! Duplicate identifier 'tgF'.
}
~
!!! '{' expected.
export declare module eaM {
var V;
function F() { };
Expand Down Expand Up @@ -804,34 +804,34 @@
!!! A function implementation cannot be declared in an ambient context.
public get pgF()
~~~
!!! Accessors are only available when targeting ECMAScript 5 and higher.
~~~
!!! Duplicate identifier 'pgF'.
public psF(param:any) { }
~~~~~~
!!! '{' expected.
~
!!! A function implementation cannot be declared in an ambient context.
public set psF(param:any)
~~~
!!! Accessors are only available when targeting ECMAScript 5 and higher.
~~~
!!! Duplicate identifier 'psF'.
private rgF() { }
~~~~~~~
!!! '{' expected.
~
!!! A function implementation cannot be declared in an ambient context.
private get rgF()
~~~
!!! Accessors are only available when targeting ECMAScript 5 and higher.
~~~
!!! Duplicate identifier 'rgF'.
private rsF(param:any) { }
~~~~~~~
!!! '{' expected.
~
!!! A function implementation cannot be declared in an ambient context.
private set rsF(param:any)
~~~
!!! Accessors are only available when targeting ECMAScript 5 and higher.
~~~
!!! Duplicate identifier 'rsF'.
static tV;
~~~~~~
!!! '{' expected.
static tF() { }
~
!!! A function implementation cannot be declared in an ambient context.
Expand All @@ -840,18 +840,18 @@
!!! A function implementation cannot be declared in an ambient context.
static set tsF(param:any)
~~~
!!! Accessors are only available when targeting ECMAScript 5 and higher.
~~~
!!! Duplicate identifier 'tsF'.
static tgF() { }
~~~~~~
!!! '{' expected.
~
!!! A function implementation cannot be declared in an ambient context.
static get tgF()
~~~
!!! Accessors are only available when targeting ECMAScript 5 and higher.
~~~
!!! Duplicate identifier 'tgF'.
}
~
!!! '{' expected.
export declare module eaM {
var V;
function F() { };
Expand Down Expand Up @@ -894,34 +894,34 @@
!!! A function implementation cannot be declared in an ambient context.
public get pgF()
~~~
!!! Accessors are only available when targeting ECMAScript 5 and higher.
~~~
!!! Duplicate identifier 'pgF'.
public psF(param:any) { }
~~~~~~
!!! '{' expected.
~
!!! A function implementation cannot be declared in an ambient context.
public set psF(param:any)
~~~
!!! Accessors are only available when targeting ECMAScript 5 and higher.
~~~
!!! Duplicate identifier 'psF'.
private rgF() { }
~~~~~~~
!!! '{' expected.
~
!!! A function implementation cannot be declared in an ambient context.
private get rgF()
~~~
!!! Accessors are only available when targeting ECMAScript 5 and higher.
~~~
!!! Duplicate identifier 'rgF'.
private rsF(param:any) { }
~~~~~~~
!!! '{' expected.
~
!!! A function implementation cannot be declared in an ambient context.
private set rsF(param:any)
~~~
!!! Accessors are only available when targeting ECMAScript 5 and higher.
~~~
!!! Duplicate identifier 'rsF'.
static tV;
~~~~~~
!!! '{' expected.
static tF() { }
~
!!! A function implementation cannot be declared in an ambient context.
Expand All @@ -930,18 +930,18 @@
!!! A function implementation cannot be declared in an ambient context.
static set tsF(param:any)
~~~
!!! Accessors are only available when targeting ECMAScript 5 and higher.
~~~
!!! Duplicate identifier 'tsF'.
static tgF() { }
~~~~~~
!!! '{' expected.
~
!!! A function implementation cannot be declared in an ambient context.
static get tgF()
~~~
!!! Accessors are only available when targeting ECMAScript 5 and higher.
~~~
!!! Duplicate identifier 'tgF'.
}
~
!!! '{' expected.
export declare module eaM {
var V;
function F() { };
Expand Down
4 changes: 1 addition & 3 deletions tests/baselines/reference/parserSetAccessorWithTypeAnnotation1.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
==== tests/cases/conformance/parser/ecmascript5/Accessors/parserSetAccessorWithTypeAnnotation1.ts (2 errors) ====
==== tests/cases/conformance/parser/ecmascript5/Accessors/parserSetAccessorWithTypeAnnotation1.ts (1 errors) ====
class C {
set foo(v): number {
~~~
!!! A 'set' accessor cannot have a return type annotation.
~~~~~~
!!! A function whose declared type is neither 'void' nor 'any' must return a value or consist of a single 'throw' statement.
}
}