microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.77k stars 12.46k forks source link

Report error if code precedes /// reference #1920

Open danquirk opened 9 years ago

danquirk commented 9 years ago

a.ts:

'use strict'
/// <reference path='b.d.ts'/>

var x: Foo;

b.d.ts:

interface Foo { x: string; }
declare module "b" {
        export = Foo;
}
D:\test\refs\a> tsc a.ts
a.ts(4,8): error TS2304: Cannot find name 'Foo'.

There's no mention in section 11.1.1 of the spec that /// references must be the first thing in a file in order to work, although I do remember us deciding that was the case at one point. If that is the rule we should update the spec appropriately. We should also add an error to the compiler or else you may spend awhile trying to figure this out since we don't provide much (read: anything) in the way of diagnostics for debugging reference resolution.

nycdotnet commented 9 years ago

Will comments and whitespace be allowed? If not, I think this might break grunt-ts "references" transforms.

mhegazy commented 9 years ago

I would not support changing the semantics of what a reference is. top of the file is not too hard of a requirement, and that covers most of the user's existing code anyways. i am fine with making it an error to have a reference tag following a statement or a declaration.

nathggns commented 9 years ago

Had a quick look at doing this. The problem with it is, as far as I can tell, after any non-trivia (non-comments and non-whitespace) then the parser simply treats references as comments (it actually stops looking for them). This is a big optimisation actually, as it doesn't have to scan the rest of the comments in the file looking for more references. If we wanted to trigger an error on references after non-trivial code, then we would have to scan every comment in a file which is a huge speed decrease.

Without scanning every comment, I can compile TypeScript itself in about 22 seconds. With checking every comment in order to trigger an error, I was just running the build for about 10 minutes and it hadn't finished yet.

Is this speed decrease something you're happy with?

nathggns commented 9 years ago

Note, I am on an old machine - iMac mid-2007 4gb RAM.

nathggns commented 9 years ago

Actually, looks like I was being a bit of an idiot and not handling SyntaxKind.EndOfFileToken properly, causing an infinite loop.

nathggns commented 9 years ago

Right, I've managed to implement this without seemingly much slow down. References appearing after any non-trivia result in the following error.

error TS9003: References must appear before any other code.

Will submit a PR for it.

CyrusNajmabadi commented 9 years ago

@nathggns "Right, I've managed to implement this without seemingly much slow down." Can you give actual numbers for compiling before/after your change? Thanks!

danquirk commented 9 years ago

Isn't @CyrusNajmabadi's alternate approach in the linked PR a viable approach for a fix to this issue?

mhegazy commented 9 years ago

Sure. Let's keep it open then. This is going to be a bigger change though.